feat(web-shell): derive session issue bindings from the closing references of bound PRs - #10425
feat(web-shell): derive session issue bindings from the closing references of bound PRs#10425wenshao wants to merge 5 commits into
Conversation
…ences of bound PRs Sessions could be found by the PR they produced, but not by the issue they were working on — the maintainer flow starts from an issue and ends in a PR. GitHub already knows that link: the PR body's "Fixes #N" is its closing reference. Snapshot those references, with the issue state, onto each bound PR entry in the existing sidecar instead of adding a client-side binding, a second sidecar, or a prompt/branch heuristic that would misbind across the shared PR/issue number space. The refresh sweep gains a second, by-number GraphQL lookup: every non-merged binding (closing references change while a PR is open) and every merged binding that predates the snapshot (one catch-up) is queried in batches, and the result is written in place together with the PR state — order and createdAt untouched, url-mismatched (foreign repository) entries never touched, unchanged sidecars never rewritten. The slim PR list query stays as it was: nesting closing references under it measurably slows it and still carries no issue state, while a by-number lookup also reaches PRs outside the 500-entry window. gh exits non-zero over a NOT_FOUND alias yet still prints the other aliases, so the wrapper parses that payload rather than failing the batch. Every projection from the sidecar to the wire now goes through one shared helper, the bridge keeps the daemon-derived snapshot across a client re-bind, and the SDK guard validates issue entries with the same url rules as PRs. The Web Shell tooltip lists the issues under the PR rows (deduped across stacked PRs) with GitHub-style state icons, and sidebar search matches an issue number with or without the hash.
|
Thanks for the PR! Template looks good ✓ Problem: a real, observed workflow gap rather than theoretical hardening — the maintainer flow (bugfix / triage / autofix develop-issue) starts from an issue, and after #9729 and #10258 the sidebar could answer "which session produced PR #N" but not "which session is working on issue #N". Deriving issues from GitHub's own closing references is the high-precision source, since the Web Shell has no structured issue-binding write point. Direction: aligned. This is the natural next step of the session-binding feature line established by the two merged PRs it builds on. Claude Code's CHANGELOG has no direct counterpart, but the area is clearly established in this repo. No auth/sandbox/model-selection/telemetry surface is touched; the SDK type change is an additive optional field. Size: cross-package change spanning core, cli serve, acp-bridge, SDK, and web-shell. 1810 changed lines total: 784 production logic, 839 test, 187 docs/e2e-plan. The author is a maintainer, so the two-tier core gate does not apply — reported for information only. Approach: the scope feels right. Every edit serves the stated goal; the new shared Risk: no elevated risk signals — none of the changed files match the revert-correlated high-risk paths. One heads-up on the duplicate check: GitHub's closing-reference parser links this PR to #10270, which is closed as completed (by merged #10278). That link is accidental prose — the Evidence section describes test data as "(#10278, which closed #10270)" — and #10270 (modality auto-detection) is unrelated to this change, whose Linked Issues section correctly says "Builds on #9729 / #10258". No action taken on it. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题: 真实的工作流缺口,而非理论性加固——维护者的主力流程(bugfix / triage / autofix develop-issue)从 issue 出发,而 #9729、#10258 落地后侧栏只能回答"哪个会话产出了 PR #N",回答不了"哪个会话在处理 issue #N"。Web Shell 没有结构化的 issue 绑定写入点,从 GitHub 自己的 closing references 派生是唯一高精度来源。 方向: 对齐。这是 #9729、#10258 两个已合入 PR 确立的会话绑定功能线的自然延伸。Claude Code CHANGELOG 无直接对应,但该方向在本仓库已明确确立。不涉及 auth/沙箱/模型选择/遥测;SDK 类型变更为可选字段的增量添加。 规模: 跨包改动,涉及 core、cli serve、acp-bridge、SDK、web-shell。共 1810 行:784 行生产逻辑、839 行测试、187 行文档/e2e 计划。作者是维护者,两级核心门禁不适用——仅记录备查。 方案: 范围合理。每处改动都服务于既定目标;新增的共享投影 风险: 无升级风险信号——改动文件均未命中与 revert 相关的高风险路径。 重复检查提示: GitHub 的 closing-reference 解析器把本 PR 关联到了 #10270(已作为 completed 关闭,由已合入的 #10278 关闭)。该关联是行文误触——Evidence 一节描述测试数据时写了 "(#10278, which closed #10270)"——#10270(模态自动检测)与本改动无关,Linked Issues 一节也正确写明"基于 #9729 / #10258"。未据此采取任何动作。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewRead against What I verified rather than took on faith:
Two observations, neither blocking: the SDK guard accepts issue lists of any length while the sidecar caps at 10 — fine, since the cap is a storage policy and the daemon only ever produces ≤10 ( Files changed (29 of 29)
TestingThis is an unattended CI run, so nothing was built or executed here — the evidence below is the PR's own CI as seen through the API, quoted verbatim.
To be plain about the gap: as of this review the PR's own CI ( The test results quoted in the PR body (per-package unit counts, and a real-daemon sweep against this repository with the real Sandboxed verification would settle what neither static review nor (currently absent) CI can: 中文说明代码审查基于 我逐项核实而非照单全收:
两点非阻塞观察:SDK 守卫不限 issue 列表长度而 sidecar 上限 10——无碍,上限是存储策略且 daemon 只产出 ≤10( 测试本次为无人值守 CI 运行,未构建、未执行任何代码——以上证据是通过 API 读取的 PR 自身 CI,原样引用。 明确说明缺口:截至本次审查,PR 自己的 CI( PR 正文引用的测试结果(各包单测数量、macOS 上用真实 沙箱验证可以补上静态审查与(目前缺席的)CI 都补不了的缺口: — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean review across every stage; the one thing standing between this and a merge is CI evidence, which does not exist yet for this commit. Stepping back: the approach matches what I'd have proposed independently, and exceeds it on the edges — the NOT_FOUND handling, the legacy catch-up that costs one call and then zero, and the url guard surviving the merge of two queries are exactly the places where a lazier version of this feature would have leaked. The problem is real (the maintainer flow starts at an issue; the sidebar could find sessions by PR but not by issue), the direction is a continuation of two already-merged PRs, and every line in the diff earns its place — the projection consolidation included, since the new field would otherwise have touched those 8 sites individually. If I were maintaining this in six months, the three-layer validation and the The honest reservation: I could not verify a single runtime claim. The PR's own CI has not produced any run on this commit as of this review (detailed in the Stage 2 comment), and this run executes nothing by rule. The author's reported results look plausible and the mocked tests are thorough, but plausible is not evidence. That is what the deferred approval below is for — and the sandboxed lanes named in Stage 2 ( Approval is deferred until the PR's own CI lands green on this commit; the finalize step will post the commit-pinned approval then. 中文说明置信度:4/5 —— 各阶段审查均干净;挡在合入前的唯一事项是 CI 证据,而该 commit 目前还没有任何 CI 记录。 退一步看:方案与我独立设想的一致,且在边界处理上更胜一筹——NOT_FOUND 处理、存量补齐只花一次调用随后零调用、两条查询合并后 url 守卫仍然有效,正是这类功能偷懒实现会漏的地方。问题是真实的(维护者流程从 issue 出发,侧栏能按 PR 找会话却不能按 issue 找),方向是两个已合入 PR 的延续,diff 中每一行都必要——投影统一也包括在内,否则新字段要逐一改到那 8 处。若六个月后由我维护,三层校验与 诚实的保留意见:我无法核实任何运行时结论。截至本次审查,PR 自己的 CI 在该 commit 上没有任何运行记录(详见 Stage 2 评论),且本次运行按规则不执行任何代码。作者自述的结果看起来合理、mock 测试也周全,但合理不等于证据。这正是下方延迟审批的用途——Stage 2 提到的沙箱通道( 审批延迟到 PR 自己的 CI 在该 commit 上全绿之后;届时 finalize 步骤会发布绑定该 commit 的审批。 — Qwen Code · qwen3.8-max Reviewed at |
|
@qwen-code /resolve |
|
@qwen-code /takeover |
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。移除 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Test Plan (not a blocker): 78 passed — this review observed 1795, 21984, 1668, 4335, 605, 639 passed; 102 passed — this review observed 1795, 21984, 1668, 4335, 605, 639 passed; 1101 passed — this review observed 1795, 21984, 1668, 4335, 605, 639 passed; 805 passed — this review observed 1795, 21984, 1668, 4335, 605, 639 passed; 115 passed — this review observed 1795, 21984, 1668, 4335, 605, 639 passed; and 1 more.
中文说明
已审查——无阻断问题。 建议见行内评论。
Test Plan(非阻断):78 passed — this review observed 1795, 21984, 1668, 4335, 605, 639 passed; 102 passed — this review observed 1795, 21984, 1668, 4335, 605, 639 passed; 1101 passed — this review observed 1795, 21984, 1668, 4335, 605, 639 passed; 805 passed — this review observed 1795, 21984, 1668, 4335, 605, 639 passed; 115 passed — this review observed 1795, 21984, 1668, 4335, 605, 639 passed; and 1 more。
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
Qwen Code attempted to resolve merge conflicts but the run did not complete successfully. Check the workflow run for full logs. |
|
🤖 AutoFix ran out of time before finishing (timeout (3600000ms)) (attempt 1/100) — it will retry on the next scan.
See the Qwen Autofix agent step logs for model/tool output. 中文说明🤖 AutoFix 在完成前耗尽了时间(timeout (3600000ms))(第 1/100 次尝试)—— 将在下次扫描时重试。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/33211695484 🧠 Handled by Qwen Code · model/模型 |
…carry on the PR's url Review round 1 on the issue-snapshot sweep. A merged binding the repository cannot resolve — another repository's same-numbered PR — could never receive a snapshot, so it re-entered the by-number lookup on every sweep and broke the "all merged with snapshots costs no call" invariant; the sweep now writes it a converging empty snapshot once the lookup has succeeded, and only ever stores issues the lookup actually resolved for that url. The session-list merge of persisted and live bindings compared numbers alone, the one carry site that did not check the canonical url, so a cross-repository re-bind could briefly wear the previous repository's issues; it now applies the same url gate as the sidecar and bridge. The closing-references fetch bound and the sidecar's per-PR cap were two literals tied by a comment (raising one would void every sidecar on read); they are one constant now, declared in the utils layer the sidecar service imports. The GraphQL wrapper passes its own timeout to the error formatter, and the write-side binding types on the bridge and the SDK omit issues so a client-bound issue list is a compile error, matching the runtime drop. Tests pin each of these plus the gaps the review found: the killed-timeout message, the list-query failure still snapshotting issues, client-supplied issues dropped by the bridge, the cross-repository re-bind dropping the snapshot, a closed issue with no state reason mapping to completed, the open issue-state rendering, the full `-f query=` argument, and the SDK guard's number and non-object checks.
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterFull-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 12 scenario(s). — Qwen Code · serve A/B |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Test Plan (not a blocker): 78 passed — this review observed 1796, 25901, 22043, 1691, 4446, 605, 639 passed; 102 passed — this review observed 1796, 25901, 22043, 1691, 4446, 605, 639 passed; 1101 passed — this review observed 1796, 25901, 22043, 1691, 4446, 605, 639 passed; 805 passed — this review observed 1796, 25901, 22043, 1691, 4446, 605, 639 passed; 115 passed — this review observed 1796, 25901, 22043, 1691, 4446, 605, 639 passed; and 1 more.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/core/src/utils/github-pr-issues.ts:120 — [probe] query-level GraphQL failure surfaces a generic message, discarding gh's error textpackages/cli/src/serve/routes/session-pr-backfill.ts:719 — [probe] issues forwarding through backfill/bind projections pinned by no testpackages/cli/src/serve/server/session-list.ts:570 — [probe] mergeSummaryPrs issues overwrite pinned only for fill-in, not stale overwritepackages/core/src/utils/github-pr-issues.test.ts:206 — [probe] gitEnv denylist scrub at the new gh call site pinned by no test
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查:反向审计——在 5 轮的反审轮数上限内未收敛。
Test Plan(非阻断):78 passed — this review observed 1796, 25901, 22043, 1691, 4446, 605, 639 passed; 102 passed — this review observed 1796, 25901, 22043, 1691, 4446, 605, 639 passed; 1101 passed — this review observed 1796, 25901, 22043, 1691, 4446, 605, 639 passed; 805 passed — this review observed 1796, 25901, 22043, 1691, 4446, 605, 639 passed; 115 passed — this review observed 1796, 25901, 22043, 1691, 4446, 605, 639 passed; and 1 more。
收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 4 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
🤖 AutoFix ran out of time before finishing (timeout (3600000ms)) (attempt 2/100) — it will retry on the next scan.
See the Qwen Autofix agent step logs for model/tool output. 中文说明🤖 AutoFix 在完成前耗尽了时间(timeout (3600000ms))(第 2/100 次尝试)—— 将在下次扫描时重试。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/33233589564 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix hit a verification-gate error before reaching a verdict (attempt 3/100) — it will retry on the next scan. Round summary — PR #10425 (address-review, round 3)Two prior rounds exhausted the agent time budget, so this round implemented Base-conflict resolution (--conflict true)Merged Feedback points and dispositions
中文说明🤖 AutoFix 在得出结论之前遇到验证门错误(第 3/100 次尝试)—— 将在下次扫描时重试。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/33239705848 🧠 Handled by Qwen Code · model/模型 |
…ect non-safe PR numbers Review round 2 on the issue-snapshot sweep. A merged binding no lookup can ever snapshot now converges in every case where that is knowable — the lookup succeeded without resolving it, the lookup is structurally impossible (no gh binary, no git root), or the platform has no closing references at all (Aone) — while a transient failure still retries, since it says nothing about the PR's references. A non-merged binding to another repository stays out of the lookup altogether: the list query names this workspace's repository, and a binding outside that canonical prefix can never resolve here, the GitHub twin of the Aone refreshability filter. A workspace with only unresolvable bindings therefore costs nothing after one sweep, and a mixed workspace still refreshes its open bindings' state. The GraphQL wrapper accepts safe integers only: a positive integer-valued double at or beyond 1e21 stringifies in exponential notation, an invalid Int literal that failed the whole document instead of one alias — one such binding would have disabled issue refresh for the entire workspace. The session-list merge finds the persisted entry by url rather than a last-wins number map, so a hand-edited sidecar with two same-numbered entries cannot shadow the live binding's own snapshot. Tests cover each path (lookup resolving nothing, foreign open binding never queried, gh unavailable converging, transient failure retrying, mixed workspace running the list query, generation closing between the two queries, merged-only Aone workspace going quiet, the repository scope in the query, and the safe-integer bound), and the E2E plan's bridge filter now selects the client-supplied-issues test.
|
🤖 Could not produce a passing fix for this feedback (round 4/100) — the verification gate rejected the attempt. This item now needs a human; the loop stays engaged and still picks up new feedback and base conflicts, but will not retry this item on its own. Review round summary — PR #10425Same-run verification repair (the deterministic rejection)The previous commit was rejected because the gate's
Base-conflict resolutionMerged Findings addresseWhy it was not pushed: tests failed in packages/cli 中文说明🤖 未能为该反馈产生可通过验证的修复(第 4/100 轮) —— 验证门拒绝了该尝试。此项现在需要人工处理;循环保持在线,仍会拾取新反馈与 base 冲突,但不会自行重试此项。 验证门的拒绝原因与日志证据见上方英文部分(gate-rejection 不翻译)。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/33245186912 🧠 Handled by Qwen Code · model/模型 |
wenshao
left a comment
There was a problem hiding this comment.
Test Plan (not a blocker): 78 passed — this review observed 1796, 25884, 22138, 1691, 4446, 610, 639 passed; 102 passed — this review observed 1796, 25884, 22138, 1691, 4446, 610, 639 passed; 1101 passed — this review observed 1796, 25884, 22138, 1691, 4446, 610, 639 passed; 805 passed — this review observed 1796, 25884, 22138, 1691, 4446, 610, 639 passed; 115 passed — this review observed 1796, 25884, 22138, 1691, 4446, 610, 639 passed; and 1 more.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/server/session-pr-refresh.ts:356 — [review] second assertGenerationOpen() before the issue fetch is unwitnessed — deleting the assert keeps the suite green (both generation tests close the guard before/after this poin…packages/sdk-typescript/src/daemon/session-pr.ts:68 — [review] SDK runtime guard accepts unbounded issues arrays, diverging from the core sidecar validator's 10-cap the design doc claims synchronized
中文说明
Test Plan(非阻断):78 passed — this review observed 1796, 25884, 22138, 1691, 4446, 610, 639 passed; 102 passed — this review observed 1796, 25884, 22138, 1691, 4446, 610, 639 passed; 1101 passed — this review observed 1796, 25884, 22138, 1691, 4446, 610, 639 passed; 805 passed — this review observed 1796, 25884, 22138, 1691, 4446, 610, 639 passed; 115 passed — this review observed 1796, 25884, 22138, 1691, 4446, 610, 639 passed; and 1 more。
收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 2 条(原文未翻译,列表见上方英文部分)。
— glm-5.3 via Qwen Code /review (v0.22.2)
| const parsed: unknown = JSON.parse(stdout); | ||
| const repository = (parsed as { data?: { repository?: unknown } } | null) | ||
| ?.data?.repository; |
There was a problem hiding this comment.
[Critical] R2-6: [certifies-falsely] [new-surface] The response parser reads only data.repository and discards the top-level errors array, so a non-NOT_FOUND GraphQL partial error — an INTERNAL_SERVER_ERROR nulling one alias, or a sub-field error nulling closingIssuesReferences inside a resolved PR — is laundered into kind: 'ok'. The sweep then treats the silently-absent PR as authoritative: issuesFetched is set from the partially-errored response, and for a merged binding the convergence branch writes { url, issues: [] } — permanently recording "this PR closes no issues" from one transient platform error. Open bindings self-heal next sweep; merged ones are terminal and the false [] even survives same-PR re-binds (upsertSessionPr keeps known.issues, and [] is truthy). Only alias-level NOT_FOUND means genuine absence — the discriminator is in the discarded errors[].
Witness (probe, unmodified PR at this commit):
unit: gh non-zero, errors:[{type:'INTERNAL_SERVER_ERROR',path:['repository','p2']}], data.repository.p1 intact
=> {"kind":"ok"} keys [1] // p2 silently absent
sweep: merged binding 42, ok-with-empty-map => writes issues: [] (updated: 1)
second sweep, fetch mock now returns issue 7 => {scanned:1, updated:0}, fetch NOT called — [] persists forever
fix flip (fail on type!=='NOT_FOUND' or non-alias path): all three defect cases => 'failed'; pinned NOT_FOUND tolerance stays green
Suggested fix — inspect errors[] in fetchGitHubPullRequestIssues and return kind: 'failed' for any error whose type is not NOT_FOUND or whose path is not ['repository', '<alias>']:
const errors = (parsed as { errors?: Array<{ type?: string; path?: string[] }> }).errors;
for (const error of errors ?? []) {
const isAliasNotFound = error.type === 'NOT_FOUND'
&& Array.isArray(error.path) && error.path.length === 2 && error.path[0] === 'repository';
if (!isAliasNotFound) throw new Error(`gh api graphql partial error: ${JSON.stringify(error).slice(0, 200)}`);
}The guard must keep the pinned alias-level NOT_FOUND tolerance green ("keeps the resolved aliases when gh exits non-zero over a NOT_FOUND number"), and the foreign-merged convergence from review round 1 depends on NOT_FOUND absence remaining authoritative. The new tests this needs — a mocked INTERNAL_SERVER_ERROR expecting kind: 'failed', and a sweep test asserting a merged binding nulled by a non-NOT_FOUND error receives no issues write that sweep — must go red if the errors-type guard is removed.
中文说明
[Critical] 响应解析器只读 data.repository,丢弃了顶层的 errors 数组,于是非 NOT_FOUND 的 GraphQL 部分错误(某个别名被 INTERNAL_SERVER_ERROR 置空,或已解析 PR 内部的子字段错误把 closingIssuesReferences 置空)会被洗成 kind: 'ok'。sweep 随后把这个静默缺失的 PR 当成权威结果:从带错误的响应设置 issuesFetched,对已合入的绑定写入 { url, issues: [] } —— 一次瞬时的平台错误被永久记录为"该 PR 不关闭任何 issue"。开放绑定下一轮自愈;已合入的是终态,错误的 [] 甚至能挺过同 PR 重绑。只有别名级 NOT_FOUND 才代表真正的缺失——判别信息就在被丢弃的 errors[] 里。
证据(探针,未经修改的 PR 代码):单元层——gh 非零退出携带 INTERNAL_SERVER_ERROR 且 p1 完好 => ok,p2 静默缺失;sweep 层——已合入绑定 42、ok-空映射 => 写入 issues: [];第二轮 mock 返回真实 issue 7 => 不再调用查询,[] 永久保留。修复翻转(对 type 非 NOT_FOUND 或非别名路径判失败):三个缺陷场景全部翻为 failed,已钉住的 NOT_FOUND 容忍保持绿色。
建议修复:在 fetchGitHubPullRequestIssues 中检查 errors[],对任何 type 非 NOT_FOUND 或 path 非 ['repository','<alias>'] 的错误返回 kind: 'failed'(代码见英文部分)。该守卫必须保持已钉住的别名级 NOT_FOUND 容忍测试为绿,且第一轮评审的外仓合并收敛依赖 NOT_FOUND 缺失保持权威。新增的测试(mock INTERNAL_SERVER_ERROR 期望 failed;sweep 测试断言被非 NOT_FOUND 错误置空的已合入绑定当轮不写 issues)在移除守卫时必须变红。
— glm-5.3 via Qwen Code /review (v0.22.2)
| } else if (entry.merged) { | ||
| // The repository cannot resolve this binding (another | ||
| // repository's same-numbered PR), so no lookup can ever snapshot |
There was a problem hiding this comment.
[Suggestion] R1-6: (fix-induced) The round-1 convergence fix conflates "another repository's same-numbered PR" with "the same PR stored under a non-canonical URL spelling". Any merged binding whose stored URL is not canonically equal to the fetched URL — .../pull/42/files, http://…, www.github.com — is force-converged to issues: [], discarding the real closing issues this same sweep just fetched for that number. The bind write path accepts any http(s):// URL, so these spellings are storable today. The wrong [] is then permanent: the binding leaves issueEntries forever and the tooltip shows "closes no issues" for a PR whose issues were fetched and thrown away.
Witness (probe):
seed merged binding {42, url '.../pull/42/files'}; issue fetch mock returns [closingIssue(7)] for PR 42
sweep 1 => {scanned:1, updated:1}, persisted issues: [] // fetched issues discarded
sweep 2 => {scanned:1, updated:0}, fetchGitHubPullRequestIssues NOT called — converged on the false empty
fix flip (parse host/owner/repo/number triples, project issues onto the entry's own url)
=> persisted [{number:7,state:'open',url:'https://github.com/o/r/issues/7'}]; foreign-binding test stays green
Suggested fix — in the issueEntries loop distinguish "the repository could not resolve this number" (absent from the result → converge to []) from "resolved but URL-mismatched": compare the parsed host/owner/repo/number triple from both URLs and apply the fetched issues when the triple matches, converging to [] only when the repo path or number genuinely differs. Keep the change local to this gate. The fix must write the ENTRY's own url, not the fetched one — the core updateSessionPrStates canonical gate silently drops a write carrying the fetched url — and must not widen canonicalSessionPrUrl itself, which is shared by upsertSessionPr's same-PR matching and the session-list merge gate. The new test (merged binding with a variant URL, fetch returning its issues, assert the issues persist; the existing foreign-binding test must stay green) goes red without the fix.
中文说明
[Suggestion] 第一轮的收敛修复把"别的仓库的同号 PR"与"同一 PR 存了非规范 URL 拼写"混为一谈。任何存储 URL 与查询结果 URL 规范化后不相等的已合入绑定(.../pull/42/files、http://…、www.github.com)都会被强制收敛为 issues: [],把本轮刚刚取回的真实 closing issues 扔掉。绑定写入路径接受任意 http(s):// URL,这些拼写今天就存得进去。错误的 [] 随后是永久的:绑定永远离开 issueEntries,tooltip 对一个取到过 issues 的 PR 显示"不关闭任何 issue"。
证据(探针):种入变体 URL 的已合入绑定,查询 mock 返回 issue 7 → 第一轮 sweep 持久化 issues: [];第二轮不再调用查询——在错误的空快照上收敛。修复翻转(解析 host/owner/repo/number 四元组,把 issues 投影到条目自身的 url)→ 真实 issues 落盘,外仓绑定测试保持绿色。
建议修复:在 issueEntries 循环里区分"仓库解析不到该编号"(结果缺失 → 收敛为 [])与"已解析但 URL 不匹配":比较两个 URL 解析出的 host/owner/repo/number,四元组匹配时应用取回的 issues,仅当仓库路径或编号确实不同时才收敛为 []。修复必须写入条目自己的 url(写查询返回的 url 会被核心层的规范化门静默丢弃),也不要改 canonicalSessionPrUrl 本身。新测试(变体 URL 已合入绑定 + 查询返回其 issues + 断言 issues 落盘;既有外仓绑定测试保持绿)在无修复时变红。
— glm-5.3 via Qwen Code /review (v0.22.2)
| url, | ||
| merged: state === 'merged', | ||
| })); | ||
| if (pending.length > 0 || issueEntries.length > 0) { |
There was a problem hiding this comment.
[Suggestion] R2-2: An Aone workspace whose bindings are all merged now passes this gate every sweep forever and pays an uncached resolveAoneWorkspaceRepo (git remote get-url origin) spawn each round, because the issue-snapshot convergence this gate depends on can never fire on the Aone path — issuesFetched is set only inside the GitHub branch, so issueEntries never empties. The function's own doc comment ("all merged — with an issue snapshot in place — cost no platform call at all") is unreachable for Aone workspaces, and the all-merged Aone path has no test at all.
Witness (probe, PATH-shim git counting spawns):
sweep1: {"scanned":1,"updated":0,"aoneConsumed":0} origin spawns: 1
sweep2: {"scanned":1,"updated":0,"aoneConsumed":0} origin spawns (cumulative): 2
sidecar after two sweeps: no issues field, ever; gh lookups: 0
naive gate revert (pending.length > 0 only) => 0 spawns — but breaks GitHub merged convergence (pinned test fails)
Suggested fix: skip issueEntries collection for Aone workspaces (issues are GitHub-only work), or memoize resolveAoneWorkspaceRepo per runtime generation/env — the origin is stable, but a cwd-only cache would serve a stale origin across a trust/env replacement (per aone-mrs.ts's own sanitization comment), so the cache key must include the generation. The fix must not stop a merged GitHub binding without a snapshot from getting its one by-number lookup — pinned by "skips the list query when every binding is merged, and every query once their issues are snapshotted". The new test (an all-merged Aone workspace asserting the second sweep spawns nothing) is red today and green with the fix.
中文说明
[Suggestion] 全部绑定已合入的 Aone workspace 现在每轮都会通过这个 gate,并支付一次未缓存的 resolveAoneWorkspaceRepo(git remote get-url origin)spawn——因为这个 gate 依赖的 issue 快照收敛在 Aone 路径上永远不会发生(issuesFetched 只在 GitHub 分支里设置,issueEntries 永不清空)。函数自己的文档注释("全部合入且有快照的 workspace 完全不发调用")对 Aone 不可达,而且全部合入的 Aone 路径完全没有测试。
证据(探针,PATH 垫片统计 spawn):第一轮 1 次 origin spawn,第二轮累计 2 次;两轮后 sidecar 仍无 issues 字段;naive 的 gate 回退(只看 pending)=> 0 次 spawn,但会破坏 GitHub 合入收敛(被钉住的测试失败)。
建议修复:对 Aone workspace 跳过 issueEntries 收集(issue 是 GitHub 独有的工作),或按 runtime generation/env 记忆化 resolveAoneWorkspaceRepo——origin 是稳定的,但仅按 cwd 缓存会在信任/环境替换后提供过期的 origin(aone-mrs.ts 自己的净化注释),缓存键必须包含 generation。修复不得阻止无快照的已合入 GitHub 绑定获得它的一次按编号查询(被现有测试钉住)。新测试(全部合入的 Aone workspace 断言第二轮零 spawn)现在红、修复后绿。
— glm-5.3 via Qwen Code /review (v0.22.2)
| if ( | ||
| !persisted || | ||
| canonicalSessionPrUrl(persisted.url) !== canonicalSessionPrUrl(l.url) | ||
| ) { |
There was a problem hiding this comment.
[Suggestion] R2-3: The new canonical-URL merge gate's accept branch — canonical-equal but byte-different URL → inherit the sidecar state/issues — has no test. The only query-variant-URL test gives the sidecar entry no state/issues, and the same-url test uses byte-identical URLs, so a plausible tightening of the comparison ships silently: mutating the guard to byte equality keeps all 24 sidecar tests green (probe-proven), yet a live entry whose URL carries a query/trailing-slash/case variant would stop inheriting the refreshed state and issues until daemon restart — the exact staleness class the old number-only code was written to avoid. Core pins the same canonicalization for upsertSessionPr/updateSessionPrStates with ?v=2/uppercase/trailing-slash cases.
Witness (probe):
guard mutated to `persisted.url !== l.url` => 24/24 existing sidecar tests still pass (gap)
suggested test added (sidecar {9517,'.../pull/9517',state merged,issues[...]}, live {...pull/9517?v=2,state open}):
under mutation => RED (entry comes back open, no issues); intact code => green
Suggested fix: add the variant-URL inheritance test beside the existing merge tests — a sidecar entry with a canonical-equal-but-byte-different url plus state/issues, a live entry with the variant url, assert the merged entry inherits the sidecar state and issues. The guard must still reject the cross-repo case pinned by "does not attach the sidecar snapshot to a live re-bind of another repository". That added test is the fix witness: it goes red under the byte-equality mutation and green on intact code.
中文说明
[Suggestion] 新的规范化 URL 合并 gate 的接受分支——规范化相等但字节不同的 URL → 继承 sidecar 的 state/issues——没有测试。唯一的变体 URL 测试没给 sidecar 条目 state/issues,同 URL 测试用的是字节相同的 URL,于是对比较的一次合理收紧会静默上线:把守卫改成字节相等后现有 24 个 sidecar 测试全部保持绿色(探针证实),而携带 query/尾斜杠/大小写变体的 live 条目将不再继承刷新的 state 和 issues,直到 daemon 重启——这恰是旧代码要避免的陈旧类。核心层为 upsertSessionPr/updateSessionPrStates 用 ?v=2/大写/尾斜杠用例钉住了同样的规范化。
证据(探针):守卫突变为字节相等 => 24/24 测试仍绿(缺口);加入建议的判别测试后,突变下变红、完整代码下变绿。
建议修复:在现有合并测试旁加入变体 URL 继承测试——规范化相等但字节不同的 sidecar url 加 state/issues,变体 url 的 live 条目,断言合并后的条目继承 sidecar 的 state 和 issues。守卫仍须拒绝被"不把 sidecar 快照贴到另一仓库的 live 重绑"钉住的跨仓库用例。新增测试即修复见证:字节相等突变下变红,完整代码下绿。
— glm-5.3 via Qwen Code /review (v0.22.2)
| expect.objectContaining({ | ||
| cwd: dir, | ||
| timeout: 10_000, | ||
| env: expect.objectContaining({ GH_TOKEN: 'x' }), | ||
| }), |
There was a problem hiding this comment.
[Suggestion] R2-5: The only env assertion on the new gh api graphql spawn cannot distinguish gitEnv(env) sanitization from a raw { ...env } pass-through — it asserts only objectContaining({ GH_TOKEN: 'x' }), which a raw spread also satisfies — so the trust-boundary property this diff newly relies on is unwitnessed. Mutation-proven: replacing env: gitEnv(env) with a raw spread keeps the whole suite green (11/11), after which the spawn honors an inherited GH_REPO; gh resolves the {owner}/{repo} placeholders against the redirected repository, the returned PR urls fail the caller's url guard, and bindings silently never gain issues.
Witness (probe — mutation matrix):
HEAD + current assertion => 11/11 green
raw-spread mutant + current assertion => 11/11 green // mutation undetected
raw-spread mutant + strengthened assert => RED ("GH_REPO": "other/repo" observed reaching the spawn env)
HEAD + strengthened assert => 11/11 green
Suggested fix: in the "runs gh api graphql at the git root" test, call with { GH_TOKEN: 'x', GH_REPO: 'other/repo' } and assert env: expect.not.objectContaining({ GH_REPO: 'other/repo' }) (or add LC_ALL: 'C' to the objectContaining). The assertion must stay objectContaining/not.objectContaining — gitEnv legitimately adds LC_ALL/LANG and strips an unbounded denylist (git-branches.ts), so a whole-env toEqual would over-pin. The strengthened test is the fix witness: it goes red the moment runGhGraphql passes raw env.
中文说明
[Suggestion] 新的 gh api graphql spawn 上唯一的环境变量断言无法区分 gitEnv(env) 净化与裸 { ...env } 透传——它只断言 objectContaining({ GH_TOKEN: 'x' }),裸展开同样满足——于是这个 diff 新依赖的信任边界属性没有被钉住。突变证实:把 env: gitEnv(env) 换成裸展开后整套测试保持绿色(11/11),此后 spawn 会遵从继承的 GH_REPO;gh 把 {owner}/{repo} 占位符解析到被重定向的仓库,返回的 PR url 过不了调用方的 url 守卫,绑定静默地永远拿不到 issues。
证据(探针——突变矩阵):裸展开突变 + 现断言 => 11/11 绿(未检出);突变 + 强化断言 => 红;HEAD + 强化断言 => 绿。
建议修复:在"runs gh api graphql at the git root"测试里以 { GH_TOKEN: 'x', GH_REPO: 'other/repo' } 调用并断言 env: expect.not.objectContaining({ GH_REPO: 'other/repo' })(或给 objectContaining 加 LC_ALL: 'C')。断言必须保持 objectContaining/not.objectContaining——gitEnv 会合法地添加 LC_ALL/LANG 并剥除一个无界黑名单,整环境 toEqual 会过度钉死。强化后的测试即修复见证:runGhGraphql 一旦透传裸 env 就变红。
— glm-5.3 via Qwen Code /review (v0.22.2)
| const issue = { number: 7, url: 'https://github.com/o/r/issues/7' }; | ||
| expect(isDaemonSessionPrInfo({ ...valid, issues: [] })).toBe(true); | ||
| for (const state of ['open', 'completed', 'not_planned'] as const) { |
There was a problem hiding this comment.
[Suggestion] R2-7: The new issue-snapshot test pins every boundary of isDaemonSessionPrInfo except the state === undefined disjunct the guard explicitly whitelists — while the sibling PR-level test in the same file pins exactly that case for PRs. Mutation-proven: dropping the disjunct keeps the SDK suite green (4/4); a payload whose issues lack state then fails the guard, and DaemonClient's body.prs.filter(isDaemonSessionPrInfo) silently drops the whole binding — the tooltip loses the PR row and all its issues. Such payloads are servable today: the core reader whitelists state === undefined too, so a hand-edited or older-format sidecar passes the daemon gate and reaches SDK consumers.
Witness (probe — mutation matrix):
disjunct-drop mutant + current test => 4/4 green // narrowing undetected
disjunct-drop mutant + added assert => RED
HEAD + added assert => 4/4 green
Suggested fix: add one assertion — expect(isDaemonSessionPrInfo({ ...valid, issues: [{ number: 7, url: 'https://github.com/o/r/issues/7' }] })).toBe(true); — the fix is itself the pin. The SDK clause must keep accepting what the core reader serves (session-pr-service.ts whitelists state === undefined for issues too); tightening one side diverges the two validators the design doc claims synchronized.
中文说明
[Suggestion] 新的 issue 快照测试钉住了 isDaemonSessionPrInfo 的每个边界,唯独漏掉守卫显式放行的 state === undefined 析取分支——而同文件里 PR 层的兄弟测试恰好为 PR 钉住了这个情形。突变证实:删掉该分支后 SDK 测试套件保持绿色(4/4);缺少 state 的 issue 载荷将过不了守卫,DaemonClient 的 body.prs.filter(isDaemonSessionPrInfo) 会静默丢掉整个绑定——tooltip 同时失去 PR 行和它的全部 issues。这种载荷今天就能出现:核心读取器同样放行 state === undefined,手改的或旧格式的 sidecar 能通过 daemon 网关到达 SDK 消费者。
证据(探针——突变矩阵):删分支突变 + 现测试 => 4/4 绿(未检出);突变 + 新增断言 => 红;HEAD + 新增断言 => 绿。
建议修复:加一行断言——expect(isDaemonSessionPrInfo({ ...valid, issues: [{ number: 7, url: 'https://github.com/o/r/issues/7' }] })).toBe(true);——修复本身就是钉子。SDK 子句必须继续接受核心读取器所服务的内容(session-pr-service.ts 对 issue 同样放行 state === undefined);单边收紧会让设计文档声称同步的两个校验器产生分歧。
— glm-5.3 via Qwen Code /review (v0.22.2)
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — no test suite executed (per-call budget exhausted on clean builds; packages/web-shell never compiled); Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): chunk 1: executing the new/changed bridge tests — the review worktree (and the parent checkout) has no node_modules, and npm ci plus the workspace build the vitest glo….
Not reviewed: reverse audit — stopped before round 4 by the review time budget.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/web-shell/client/components/sidebar/SessionDetailsTooltip.test.tsx:259 — [probe] tooltip issue-dedup test never pins the newest-wins keep-direction (both duplicates of issue #7 carry identical state)packages/core/src/utils/github-pr-issues.test.ts:314 — [probe] chunk test's -3/2.5 exclusion unobservable — the mock responder regex cannot match a sign or a dotpackages/acp-bridge/src/bridge.ts:10449 — [probe] setSessionPrs issues pass-through pinned by no test — the 806-test bridge suite stays green under a full revertpackages/core/src/utils/github-pr-issues.test.ts:98 — [probe] mapIssue guard clauses 2-4 (number <= 0, non-integer number, missing url) pinned by no test
Convergence: round 3 posted 7 inline comment(s), 7 of them reported for the first time; the previous round posted 11 (11 new). Findings keep coming back to the same files: packages/core/src/utils/github-pr-issues.ts (findings in round 2; 2 more now); packages/cli/src/serve/server/session-pr-refresh.ts (findings in round 2; 1 more now); packages/core/src/utils/github-pr-issues.test.ts (findings in round 1; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — no test suite executed (per-call budget exhausted on clean builds; packages/web-shell never compiled); Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度(达到工具调用预算):chunk 1:executing the new/changed bridge tests — the review worktree (and the parent checkout) has no node_modules, and npm ci plus the workspace build the vitest glo…。
未审查:反向审计——评审时间预算不足,未能开始第 4 轮。
收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 4 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 3 轮发布了 7 条行内评论,其中 7 条是首次提出;上一轮发布了 11 条(其中 11 条首次提出)。发现反复回到同一批文件:packages/core/src/utils/github-pr-issues.ts(第 2 轮已出过发现,本轮又有 2 条);packages/cli/src/serve/server/session-pr-refresh.ts(第 2 轮已出过发现,本轮又有 1 条);packages/core/src/utils/github-pr-issues.test.ts(第 1 轮已出过发现,本轮又有 1 条)。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.22.3)
| const parsed: unknown = JSON.parse(stdout); | ||
| const repository = (parsed as { data?: { repository?: unknown } } | null) | ||
| ?.data?.repository; |
There was a problem hiding this comment.
[Critical] R3-6: [certifies-falsely] [new-surface] Re-checked the open blocker at this location (comment 3886632648, filed against 350c3ac9) against this head — it still stands. The response parser reads only data.repository and discards the top-level errors array, so a non-NOT_FOUND GraphQL partial error — an INTERNAL_SERVER_ERROR nulling one alias, or a sub-field error nulling closingIssuesReferences inside a resolved PR — is laundered into kind: 'ok'. The sweep then treats the silently-absent PR as authoritative: for a merged binding lacking a snapshot, the convergence branch writes { url, issues: [] }, permanently recording "this PR closes no issues" from one transient platform error. Open bindings self-heal next sweep; merged ones are terminal, and the false [] even survives same-PR re-binds (upsertSessionPr keeps known.issues; [] is truthy). Only alias-level NOT_FOUND means genuine absence — the discriminator is in the discarded errors[]. The parser is unchanged since the blocker was filed; the only edit to this file since is the safe-integer filter at :193-197, which does not touch this path.
Witness:
PROBE (this round, scratch tree, unmodified PR code at 75c5f88):
PROBE-A: INTERNAL_SERVER_ERROR payload nulling alias p42 (p1 intact, gh non-zero exit, non-empty stdout)
=> kind= ok, keys= [ 1 ] — alias 42 silently laundered away
PROBE-B: sub-field error nulling closingIssuesReferences inside a resolved PR
=> kind= ok, entry42= {"url":"https://github.com/o/r/pull/42","issues":[]} — certified as closing no issues
fix-flip (inspect errors[], fail on non-NOT_FOUND): PROBE-A kind= failed, PROBE-B kind= failed
all 12 existing github-pr-issues tests stay green, incl. the pinned NOT_FOUND tolerance
sweep half: 'converges a merged binding the lookup resolves to nothing' (session-pr-refresh.test.ts:1096) green at HEAD —
kind ok with the number absent writes issues: [] to the merged binding, second sweep issues zero lookups
Inspect errors[] in fetchGitHubPullRequestIssues and return kind: 'failed' for any error whose type is not NOT_FOUND or whose path is not ['repository', '<alias>'] — the full fix code and the two required tests are in the original blocker (comment 3886632648). The guard must classify only non-NOT_FOUND or non-alias-path errors as failures: the pinned alias-level NOT_FOUND tolerance and the round-1 foreign-merged convergence both depend on NOT_FOUND absence remaining authoritative. The mocked-INTERNAL_SERVER_ERROR test expecting kind: 'failed' and the sweep test asserting a non-NOT_FOUND-nulled merged binding receives no issues write must both go red if the errors-type guard is removed, while the NOT_FOUND tolerance test stays green.
中文说明
针对此处的未决阻断问题(comment 3886632648,提交于 350c3ac9)在当前 head 上复查——仍然成立。响应解析器只读取 data.repository,丢弃了顶层的 errors 数组,因此非 NOT_FOUND 的 GraphQL 部分错误(某个别名被 INTERNAL_SERVER_ERROR 置空,或已解析 PR 内部的子字段错误把 closingIssuesReferences 置空)会被洗成 kind: 'ok'。sweep 随后把静默缺失的 PR 当作权威结果:对缺少快照的已合入绑定,收敛分支写入 { url, issues: [] }——把一次瞬时的平台错误永久记录为"该 PR 不关闭任何 issue"。开放绑定下一轮自愈;已合入的是终态,错误的 [] 甚至能挺过同 PR 重绑(upsertSessionPr 保留 known.issues,且 [] 为真值)。只有关键字级 NOT_FOUND 才代表真正的缺失——判别信息就在被丢弃的 errors[] 里。解析器自该阻断提交后未变;此后本文件唯一的修改是 :193-197 的安全整数过滤,不涉及这条路径。证据(本轮探针,未经修改的 PR 代码):INTERNAL_SERVER_ERROR 置空别名 p42(p1 完好,gh 非零退出、stdout 非空)=> kind= ok, keys= [1],别名 42 被静默洗掉;子字段错误把已解析 PR 的 closingIssuesReferences 置空 => kind= ok,该 PR 被认证为"不关闭任何 issue";按建议修复(检查 errors[],对非 NOT_FOUND 判失败)翻转后两者均为 failed,既有 12 个测试保持绿色(含已钉住的 NOT_FOUND 容忍)。修复:在 fetchGitHubPullRequestIssues 中检查 errors[],对任何 type 非 NOT_FOUND 或 path 非 ['repository', '<alias>'] 的错误返回 kind: 'failed'——完整修复代码与两个必需测试见原阻断评论(comment 3886632648)。该守卫必须保持已钉住的别名级 NOT_FOUND 容忍为绿,且第一轮评审的外仓合并收敛依赖 NOT_FOUND 缺失保持权威。新增测试(mock INTERNAL_SERVER_ERROR 期望 failed;sweep 测试断言被非 NOT_FOUND 错误置空的已合入绑定当轮不写 issues)在移除守卫时必须变红,NOT_FOUND 容忍测试保持绿色。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| } else if ( | ||
| issuesResult.kind === 'cli_unavailable' || | ||
| issuesResult.kind === 'not_a_repo' | ||
| ) { | ||
| convergeMerged = true; |
There was a problem hiding this comment.
[Suggestion] R2-6: (fix-induced) The round-2 fix for this entry widened this convergence disjunction to cli_unavailable/not_a_repo and reported "Two new tests pin both directions" — but only cli_unavailable has a sweep-level witness: not_a_repo appears zero times in session-pr-refresh.test.ts. Dropping the disjunct keeps the whole suite green, while a workspace whose git root was deleted (the lookup returns { kind: 'not_a_repo' }) would lose its converging empty snapshot: the legacy merged binding re-enters the pending set forever, one origin resolution per sweep, violating the documented "all merged + snapshotted → zero calls" invariant.
Witness:
intact + mirrored not_a_repo sweep test => Tests 60 passed (60)
disjunct removed => FAIL > converges merged bindings when the cwd is not a git repository
AssertionError: expected { scanned: 1, updated: +0, …(1) } to deeply equal { scanned: 1, updated: 1 }
1 failed | 59 passed — every pre-existing sweep test stays green under the mutation
Add a sweep test mirroring converges merged bindings locally when the lookup is structurally impossible with fetchGitHubPullRequestIssuesMock.mockResolvedValue({ kind: 'not_a_repo' }): round one writes issues: [] (updated: 1), round two returns updated: 0 and makes no lookup call. That new test must go red if the not_a_repo disjunct is removed from this branch.
中文说明
(修复引入)针对本条目的第二轮修复把该收敛析取扩展为 cli_unavailable/not_a_repo,并回复"两个新测试钉住两个方向"——但只有 cli_unavailable 有 sweep 层见证:not_a_repo 在 session-pr-refresh.test.ts 中出现零次。删除该析取后整套测试保持绿色,而 git 根被删除的 workspace(查询返回 { kind: 'not_a_repo' })将失去它的收敛空快照:存量已合入绑定永久重新进入待处理集合,每轮一次 origin 解析,违反"全部合入且有快照 → 零调用"的文档不变量。修复:仿照 converges merged bindings locally when the lookup is structurally impossible 增加一个 sweep 测试,令 fetchGitHubPullRequestIssuesMock.mockResolvedValue({ kind: 'not_a_repo' }):第一轮写入 issues: [](updated: 1),第二轮返回 updated: 0 且不再调用查询。移除本分支中的 not_a_repo 析取时,该新测试必须变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| expect(query).not.toContain('1e+21'); | ||
| expect(query).not.toContain('9007199254740993'); |
There was a problem hiding this comment.
[Suggestion] R3-1: The safe-integer sibling test added with the R2-1 fix asserts not.toContain('9007199254740993') to pin that Number.MAX_SAFE_INTEGER + 2 never reaches the GraphQL document — but Number.MAX_SAFE_INTEGER + 2 === 9007199254740992 in IEEE-754, so the quoted literal can never appear in the query and the assertion can never fail. If the filter were weakened to any bound ≥ 2^53 (e.g. an exponential-notation-only exclusion), 9007199254740992 would enter the document as an out-of-Int32-range Int literal while this test still passes green — the exact document-poisoning mode the test's own comment says it guards against.
Witness:
PROBE: guard weakened to Number.isInteger(n) && n < 1e21 && n > 0
document contains p9007199254740992: pullRequest(number: 9007199254740992)
original test: Tests 12 passed (12)
with not.toContain(String(Number.MAX_SAFE_INTEGER + 2)):
AssertionError: … not to contain '9007199254740992' (1 failed)
intact code + fix: 12 passed
| expect(query).not.toContain('1e+21'); | |
| expect(query).not.toContain('9007199254740993'); | |
| expect(query).not.toContain('1e+21'); | |
| expect(query).not.toContain(String(Number.MAX_SAFE_INTEGER + 2)); |
After the fix, replacing the filter's Number.isSafeInteger(n) with Number.isInteger(n) && n < 1e21 must turn never puts a non-safe integer into the document red.
中文说明
随 R2-1 修复新增的安全整数兄弟测试断言 not.toContain('9007199254740993'),用来钉住 Number.MAX_SAFE_INTEGER + 2 永不进入 GraphQL 文档——但在 IEEE-754 里 Number.MAX_SAFE_INTEGER + 2 === 9007199254740992,所引用的字面量不可能出现在查询里,该断言永远不会失败。如果过滤被削弱为任何 ≥ 2^53 的界(例如只排除指数计数法),9007199254740992 会以超出 Int32 范围的 Int 字面量进入文档,而该测试仍然保持绿色——正是测试注释声称要防范的文档投毒模式。修复:把断言改为 expect(query).not.toContain(String(Number.MAX_SAFE_INTEGER + 2));。修复后,把过滤的 Number.isSafeInteger(n) 替换为 Number.isInteger(n) && n < 1e21 必须使 never puts a non-safe integer into the document 变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| if ((error as NodeJS.ErrnoException).code === 'ENOENT') { | ||
| return { kind: 'cli_unavailable' }; | ||
| } | ||
| return { | ||
| kind: 'failed', | ||
| message: ghErrorMessage(error, 'gh api graphql', GH_TIMEOUT_MS), | ||
| gitRoot, | ||
| }; |
There was a problem hiding this comment.
[Suggestion] R3-2: The convergence taxonomy misses a third structural failure shape: gh present + git root present, but gh cannot resolve the repository — a workspace whose git remote was deleted or renamed after binding (or any local-only repo with a manually bound merged GitHub PR). gh api graphql -F owner={owner} -F name={repo} exits 1 with empty stdout (error parsing "owner" value: no git remotes found), runGhGraphql rejects on the empty stdout, and this catch reports kind: 'failed' — indistinguishable from a transient 5xx. The sweep never converges on failed, so the legacy merged binding re-enters issueEntries every sweep and the workspace pays one gh api graphql spawn per 5-minute tick, forever. Pre-PR this shape cost zero gh calls (all-merged workspaces returned early).
Witness:
reproduction, fresh git repo with no remotes:
gh api graphql -F owner={owner} -F name={repo} … => EXIT=1, empty stdout
stderr: error parsing "owner" value: no git remotes found
trace: empty stdout => runGhGraphql rejects (github-pr-issues.ts:169) => kind 'failed' here => convergeMerged never set
Classify repository-resolution failures as a distinct structural kind in fetchGitHubPullRequestIssues (gh fails before the query runs — the stderr names the owner/name placeholder-resolution error) and add it to the sweep's convergence disjunction, so merged bindings converge exactly as they do for not_a_repo/cli_unavailable. The existing test keeps retrying a merged binding after a transient lookup failure (packages/cli/src/serve/server/session-pr-refresh.test.ts:1190) pins that { kind: 'failed', message: 'HTTP 502' } must NOT converge — the new classification must not absorb transient/API errors into the structural kind. A new sweep-level test (round one writes issues: [], round two updated: 0 with no lookup call) must go red if the new kind is removed from the disjunction.
中文说明
收敛分类漏掉了第三种结构性失败形态:gh 存在、git 根存在,但 gh 解析不到仓库——绑定后 git remote 被删除/改名的 workspace(或手工绑定了已合入 GitHub PR 的纯本地仓库)。gh api graphql -F owner={owner} -F name={repo} 以退出码 1 返回空 stdout(error parsing "owner" value: no git remotes found),runGhGraphql 因空 stdout 拒绝,此处的 catch 报告 kind: 'failed'——与瞬时 5xx 无法区分。sweep 从不对 failed 收敛,于是存量已合入绑定每轮重新进入 issueEntries,workspace 每 5 分钟支付一次 gh api graphql 派生,永无止境;本 PR 之前这种形态是零 gh 调用(全合入的 workspace 提前返回)。修复:在 fetchGitHubPullRequestIssues 中把仓库解析失败归为独立的结构性 kind(gh 在查询运行前失败——stderr 指明 owner/name 占位符解析错误),并加入 sweep 的收敛析取,使已合入绑定与 not_a_repo/cli_unavailable 一样收敛。既有测试 keeps retrying a merged binding after a transient lookup failure 钉住 { kind: 'failed', message: 'HTTP 502' } 不得收敛——新分类不得把瞬时/API 错误并入结构性 kind。新的 sweep 级测试(第一轮写入 issues: [],第二轮 updated: 0 且无查询调用)在移除新 kind 时必须变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| // No pending target at all: the sweep returns before it even | ||
| // resolves the workspace repository (no `aoneConsumed`). | ||
| expect(second).toEqual({ scanned: 1, updated: 0 }); |
There was a problem hiding this comment.
[Suggestion] R3-3: This test's second-sweep assertions cannot observe resolveAoneWorkspaceRepo, so the zero-cost steady state its own comment claims ("returns before it even resolves the workspace repository") is not pinned. resolveAoneWorkspaceRepo runs an uncached git remote get-url origin exec per call (packages/cli/src/serve/server/aone-mrs.ts:173-201); if a future restructure hoists that call above the pendingNumbers.length === 0 early return (session-pr-refresh.ts:229), every fully converged Aone workspace pays a git exec on each 5-minute sweep tick — precisely the per-sweep cost this convergence exists to eliminate — while these assertions still pass (the early return runs afterwards, the result shape is unchanged, backend.view is still uncalled).
Witness:
PROBE: clean hoist mutant vs committed test => Tests 1 passed (regression invisible)
with delegating spy asserting toHaveBeenCalledTimes(1):
mutant red — expected "resolveAoneWorkspaceRepo" to be called 1 times, but got 2 times
intact code + fix: green, exactly 1 call
Make origin resolution observable: a partial vi.mock('./aone-mrs.js', …) wrapping the real resolveAoneWorkspaceRepo in a spy that delegates to the original, then assert expect(resolveAoneWorkspaceRepoSpy).toHaveBeenCalledTimes(1) after both sweeps (first resolves once, second adds zero calls). The spy must wrap the original implementation, not stub it — the describe('on an Aone workspace') beforeEach seeds a real repo via git init + git remote add origin … (session-pr-refresh.test.ts:282-286) and the first sweep's convergence flow depends on that real resolution succeeding. The strengthened assertion must go red if the resolveAoneWorkspaceRepo call moves above the early return.
中文说明
该测试第二轮的断言无法观测 resolveAoneWorkspaceRepo,因此其注释声称的零成本稳态("在解析 workspace 仓库之前就返回")并未被钉住。resolveAoneWorkspaceRepo 每次调用都执行一次未缓存的 git remote get-url origin(aone-mrs.ts:173-201);若未来重构把该调用提升到 pendingNumbers.length === 0 早退之前(session-pr-refresh.ts:229),每个已完全收敛的 Aone workspace 都会在每个 5 分钟 tick 支付一次 git exec——正是本收敛要消除的成本——而这些断言仍然通过(早退随后执行、结果形状不变、backend.view 仍未被调用)。修复:用部分 vi.mock('./aone-mrs.js', …) 把真实 resolveAoneWorkspaceRepo 包进委托原实现的 spy,两轮后断言 expect(resolveAoneWorkspaceRepoSpy).toHaveBeenCalledTimes(1)(第一轮解析一次,第二轮零新增)。spy 必须包装原实现而非替换——Aone describe 的 beforeEach 通过 git init + git remote add origin … 种入真实仓库(session-pr-refresh.test.ts:282-286),第一轮收敛依赖该真实解析成功。强化后的断言在 resolveAoneWorkspaceRepo 调用移到早退之前时必须变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| it('keeps retrying a merged binding after a transient lookup failure', async () => { | ||
| // A 5xx or rate limit says nothing about the PR's references; an | ||
| // empty snapshot here would permanently record "fetched, none". |
There was a problem hiding this comment.
[Suggestion] R3-4: No sweep-level test exercises the isForeign(entry.url) disjunct of the merged-convergence branch (session-pr-refresh.ts:401) while the issue lookup is transiently failing (convergeMerged === false). Every failure-path test uses a local or an open binding; every foreign-binding test has either a successful lookup or no list query at all. The disjunct exists so a merged foreign binding — which no lookup from this workspace can ever resolve — converges to issues: [] during lookup outages instead of lingering; if it is dropped, all 59 existing sweep tests stay green and the foreign merged binding re-enters the very lookup the foreign filter exists to avoid.
Witness:
PROBE: (1) mutant dropping `|| isForeign(entry.url)` + untouched suite => Tests 59 passed (59)
(2) mutant + suggested test => red: expected { scanned: 2, updated: +0, …(1) } to deeply equal { scanned: 2, updated: 1 }
(3) intact + suggested test => Tests 60 passed (60)
Add a sweep test: seed an open local binding 43 (whose list query sets repoPrefix) and a merged foreign binding 42; list query { kind: 'ok', pullRequests: [pr(43, 'open')] }; lookup { kind: 'failed', message: 'HTTP 502', gitRoot: workspaceCwd }. Assert round 1 returns { scanned: 2, updated: 1 }, writes issues: [] on the foreign binding while the open binding's issues stays undefined, and the lookup was called only with [43]. isForeign can only return true once repoPrefix is derived from a successful list-query result (session-pr-refresh.ts:335-338), and the list query only runs when a non-merged binding exists — the test must seed the open binding alongside the foreign merged one. The new test must go red if || isForeign(entry.url) is removed from the convergence branch.
中文说明
没有任何 sweep 级测试在查询瞬时失败(convergeMerged === false)时覆盖已合入收敛分支(session-pr-refresh.ts:401)的 isForeign(entry.url) 析取。所有失败路径测试都用本地或开放绑定;所有外仓绑定测试要么查询成功、要么没有列表查询。该析取的作用是:外仓已合入绑定——本 workspace 的任何查询都永远解析不到它——在查询故障期间收敛为 issues: [] 而不是滞留;若删除它,现有 59 个 sweep 测试全部保持绿色,而外仓已合入绑定会重新进入外仓过滤本要避开的那个查询。修复:新增 sweep 测试——种入开放本地绑定 43(其列表查询设定 repoPrefix)与已合入外仓绑定 42;列表查询 { kind: 'ok', pullRequests: [pr(43, 'open')] };查询 { kind: 'failed', message: 'HTTP 502', gitRoot: workspaceCwd }。断言第一轮返回 { scanned: 2, updated: 1 },外仓绑定写入 issues: [] 而开放绑定的 issues 保持 undefined,且查询只以 [43] 被调用。isForeign 只有在 repoPrefix 从成功的列表查询结果派生后才返回 true(session-pr-refresh.ts:335-338),且列表查询只在存在未合入绑定时运行——测试必须在种入外仓已合入绑定的同时种入开放绑定。从收敛分支移除 || isForeign(entry.url) 时新测试必须变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| expect(first).toEqual({ scanned: 1, updated: 1, aoneConsumed: 0 }); | ||
| expect(backend.view).not.toHaveBeenCalled(); | ||
| expect((await readSessionPrs(prPath))?.[0]?.issues).toEqual([]); |
There was a problem hiding this comment.
[Suggestion] R3-5: This Aone convergence test cites "Closing references are GitHub-only" but never asserts fetchGitHubPullRequestIssuesMock was not called — no test anywhere pins that the GitHub issue lookup stays out of the Aone branch of refreshWorkspaceSessionPrStates. A refactor hoisting the issueNumbers/fetchGitHubPullRequestIssues block out of the else branch of if (aoneRepo) survives the entire sweep suite (the beforeEach default prIssues([]) resolves cleanly and every Aone assertion still holds), while in production any Aone workspace with one open binding spawns a gh api graphql subprocess with Aone MR global ids every 5-minute tick — permanently for the binding's open lifetime, and a failing gh exec each tick in a pure-Aone checkout with no GitHub remote.
Witness:
PROBE: mutant hoisting the issue-lookup block out of the else => Tests 59 passed (59), suite cannot see it
with the added assertion => mutant red (the lookup was called with the Aone number)
intact + assertion => 1 passed | 58 skipped (59)
| expect(first).toEqual({ scanned: 1, updated: 1, aoneConsumed: 0 }); | |
| expect(backend.view).not.toHaveBeenCalled(); | |
| expect((await readSessionPrs(prPath))?.[0]?.issues).toEqual([]); | |
| expect(first).toEqual({ scanned: 1, updated: 1, aoneConsumed: 0 }); | |
| expect(backend.view).not.toHaveBeenCalled(); | |
| expect(fetchGitHubPullRequestIssuesMock).not.toHaveBeenCalled(); | |
| expect((await readSessionPrs(prPath))?.[0]?.issues).toEqual([]); |
Moving the issue-lookup block out of the GitHub else branch makes the first sweep call the mock, turning this assertion red.
中文说明
这个 Aone 收敛测试引用了"Closing references 是 GitHub 独有的",但从未断言 fetchGitHubPullRequestIssuesMock 未被调用——没有任何测试钉住 GitHub issue 查询不进入 refreshWorkspaceSessionPrStates 的 Aone 分支。把 issueNumbers/fetchGitHubPullRequestIssues 块提出 if (aoneRepo) 的 else 分支的重构能挺过整个 sweep 套件(beforeEach 默认的 prIssues([]) 干净地解析,所有 Aone 断言仍成立),而生产中任何带一个开放绑定的 Aone workspace 会每 5 分钟 tick 派生一个携带 Aone MR 全局 id 的 gh api graphql 子进程——在绑定的整个开放期内永久如此;在没有 GitHub 远端的纯 Aone 检出里,每个 tick 都是一次失败的 gh 执行。修复:在该测试第一轮断言后加入 expect(fetchGitHubPullRequestIssuesMock).not.toHaveBeenCalled();。把 issue 查询块移出 GitHub else 分支会使第一轮调用该 mock,令此断言变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
🤖 AutoFix stopped after 5 consecutive rounds that pushed nothing (failed rounds, timeouts, gate rejections, or stops under instruction). Retrying at the same per-round budget is not converging — this usually means the PR is too large or conflicts with a fast-moving Round summary — Critical fixed, unwitnessed guards pinned, prior gate rejection diagnosedThis round landed the smallest blocking subset after two consecutive rounds exhausted Feedback dispositionsFixed (resolved in the code, commit ab46a81850):
中文说明🤖 AutoFix 已停止:连续 5 轮未能推送任何内容(失败轮次、超时、验证门拒绝或按指示停止)。以相同的单轮预算重试并不收敛 —— 这通常意味着 PR 过大,或与快速变动的 Run log: https://github.com/QwenLM/qwen-code/actions/runs/33265601025 🧠 Handled by Qwen Code · model/模型 |
|
⏸️ Takeover paused: this PR reached its round cap (100/100). Comment 中文说明⏸️ 托管已暂停:本 PR 达到轮次上限(100/100)。评论 |
What this PR does
A session bound to a GitHub PR now also carries the issues that PR closes. The daemon's existing PR-state refresh sweep looks up each bound PR's closing references (the
Fixes #Nlinks GitHub already tracks) together with the issue state — open, completed, or not planned — and snapshots them onto the PR entry in the same sidecar, in place, without touching binding order or timestamps. The Web Shell session tooltip lists those issues under the PR rows with GitHub-style state icons (deduped when stacked PRs close the same issue), and sidebar search finds a session by an issue number, with or without the leading hash. The session-row badge is unchanged and still shows only the PR.The lookup is a single by-number GraphQL call per workspace per sweep, batched at 100 PRs. It runs for every non-merged binding (closing references change while a PR is open) and once for merged bindings that predate the snapshot, so a workspace whose bindings are all merged and snapshotted costs no
ghcall at all. The slim PR list query is left as it was: nesting closing references under it measurably slows it and still carries no issue state, while a by-number lookup also reaches PRs outside the 500-entry list window.ghexits non-zero when one alias is NOT_FOUND (a binding to another repository's same-numbered PR) yet still prints every other alias, so the wrapper parses that payload instead of failing the batch; the existing url guard keeps the wrong repository's PR from ever matching a foreign binding.Every sidecar-to-wire projection now goes through one shared helper, the bridge keeps the daemon-derived snapshot across a client re-bind of the same PR, and the SDK's runtime guard validates issue entries with the same http(s)/length/control-character rules as PR urls. Clients cannot bind issues; the snapshot is daemon-derived only.
Why it's needed
The sidebar could answer "which session produced PR #N" (#9729, #10258) but not "which session is working on issue #N", and the maintainer flow — bugfix, triage, autofix's develop-issue mode — starts from an issue and ends in a PR. There is no structured place in the Web Shell where an issue is bound the way a PR is bound from the Git dialog, and PRs and issues share one number space, so inferring the issue from prompts or branch names would misbind. GitHub's own closing references are the one high-precision source, and they come for free from the PR the session already bound.
Reviewer Test Plan
How to verify
gh authconfigured, bind a session to an open PR whose body saysFixes #N(create it from the Git dialog, or run the backfill route). Wait for the sweep (60s after daemon start, then every 5 minutes). Expected: the session's.pr.jsonentry gains anissueslist with the issue's number, url, andstate: "open"; the tooltip showsIssue #Nunder the PR row with a green circle-dot icon; typingNor#Nin the sidebar search finds the session.Issue #N · Completedto screen readers; the next sweep issues noghcall for that workspace.Fixes #Nline. Expected: the next sweep updates the list; an unchanged list does not rewrite the sidecar.Unit coverage: the sidecar schema (invalid issue url/state or more than 10 issues voids the sidecar), same-PR re-bind keeping the snapshot, in-place writes with or without a state, the GraphQL wrapper's state mapping / NOT_FOUND handling / chunking / failure kinds, the sweep's catch-up and no-call paths, the session list preferring sidecar issues over the live entry, the bridge seed/re-bind paths, the SDK guard, and the tooltip/search rendering.
Evidence (Before & After)
Real sweep from the built daemon against this repository with the real
ghCLI, one session bound to a merged PR (#10278, the fix PR for issue #10270), a foreign-repository PR #42, and an open PR (#10303, the PR for issue #10293), run twice:Sidecar after round 1 (before: the same three entries with no
issuesfield):[ { "number": 10278, "url": "https://github.com/QwenLM/qwen-code/pull/10278", "state": "merged", "issues": [{ "number": 10270, "url": "https://github.com/QwenLM/qwen-code/issues/10270", "state": "completed" }] }, { "number": 42, "url": "https://github.com/other/repo/pull/42", "state": "open" }, { "number": 10303, "url": "https://github.com/QwenLM/qwen-code/pull/10303", "state": "open", "issues": [{ "number": 10293, "url": "https://github.com/QwenLM/qwen-code/issues/10293", "state": "open" }] } ]Query cost measured on this repository:
gh pr list --state all --limit 500withclosingIssuesReferencesnested takes 6.7s versus 4.9s without (theghtimeout is 10s), which is why the list query is untouched and the issues come from a separate by-number lookup (~1s for four PRs).Round 2 (
75c5f88), with aghshim logging every invocation — one session holding a foreign open PR #42, a foreign merged PR #43, the merged #10278 and the open #10303: the first sweep runs the list query and one GraphQL lookup for[10278, 10303]only (the foreign open binding is filtered out by the repository prefix, the foreign merged one converges locally without a lookup); the second sweep looks up[10303]only:Convergence for a binding the repository cannot resolve (review round 1): one session bound only to a merged foreign-repository PR, real
gh, three sweeps — round 1 makes one lookup and writes the converging empty snapshot, later rounds make noghcall at all:Tooltip rendering is covered by jsdom tests (icon classes, link hrefs, dedupe, sr-only state text); no browser screenshot yet.
Tested on
Environment (optional)
Unit tests per package, plus the built daemon run directly against this repository with the real
ghCLI (macOS, gh 2.96.0).Risk & Scope
gh api graphqlcall per workspace per sweep whenever a binding is open/closed or lacks a snapshot (measured ~1–3s); the first sweep after upgrade looks up every legacy binding once, batched at 100. An issue reopened after its PR merged is not tracked, the same "merged is terminal" tradeoff the PR state already makes./bugfix #N), inferring issues from prompts, branches, or commit trailers, showing issues in the session-row badge, and fetching issues immediately in the backfill route (the next sweep covers it). Windows/Linux not run locally. No browser screenshot of the tooltip.issuesis optional on the wire and in the sidecar; older sidecars stay valid and are enriched by the first sweep.Linked Issues
Builds on #9729 (PR bindings, backfill, state refresh) and #10258 (GitHub-style state icons).
中文说明
这个 PR 做了什么
绑定了 GitHub PR 的会话现在也会带上该 PR 关闭的 issue。daemon 已有的 PR 状态刷新定时器会顺带查询每个已绑 PR 的 closing references(GitHub 已经跟踪的
Fixes #N链接)及 issue 状态——open、completed 或 not planned——并原地快照到同一个 sidecar 的 PR 条目上,不改变绑定顺序和时间戳。Web Shell 会话 tooltip 在 PR 行下方用 GitHub 风格的状态图标列出这些 issue(stacked PR 关闭同一 issue 时去重),侧栏搜索可以按 issue 号找到会话,带不带#都行。会话行 badge 不变,仍只显示 PR。查询是每 workspace 每轮一条按编号的 GraphQL 调用,每批 100 个 PR。所有未合入的绑定都会查(PR 开放期间 closing references 会变),已合入但尚无快照的绑定只补一次,所以全部已合入且已有快照的 workspace 完全不发
gh调用。slim 的 PR 列表查询保持原样:把 closing references 嵌套进去会明显拖慢它且仍不带 issue 状态,而按编号查询还能覆盖 500 条列表窗口之外的 PR。当某个别名 NOT_FOUND(绑定指向别的仓库的同号 PR)时gh以非零退出码返回,但 stdout 仍带其它别名的完整数据,包装器照常解析而不是让整批失败;已有的 url 守卫保证别的仓库的 PR 永远不会匹配到外仓绑定。所有 sidecar 到线协议的投影统一走一个共享助手,bridge 在客户端重绑同一 PR 时保留 daemon 派生的快照,SDK 的运行时守卫用与 PR url 相同的 http(s)/长度/控制字符规则校验 issue 条目。客户端不能绑定 issue,快照只由 daemon 派生。
为什么需要
侧栏能回答"哪个会话产出了 PR #N"(#9729、#10258),但回答不了"哪个会话在处理 issue #N",而维护者的流程——bugfix、triage、autofix 的 develop-issue 模式——从 issue 出发、以 PR 收口。Web Shell 里没有像 Git 对话框绑定 PR 那样的结构化 issue 绑定入口,PR 与 issue 又共用一个编号空间,从提示词或分支名推断会误绑。GitHub 自己的 closing references 是唯一高精度的来源,而且会话已绑定的 PR 免费带来。
评审测试计划
如何验证
gh auth的 workspace 里,把会话绑定到一个正文写有Fixes #N的开放 PR(从 Git 对话框创建,或跑回填路由)。等待 sweep(daemon 启动后 60s,之后每 5 分钟)。预期:该会话.pr.json条目多出issues列表,含 issue 编号、url 和state: "open";tooltip 在 PR 行下显示Issue #N和绿色 circle-dot 图标;侧栏搜索输入N或#N能找到该会话。Issue #N · Completed;下一轮 sweep 对该 workspace 不再发gh调用。Fixes #N。预期:下一轮 sweep 更新列表;列表不变时不重写 sidecar。单测覆盖:sidecar schema(非法 issue url/状态或超过 10 个 issue 会使 sidecar 失效)、同 PR 重绑保留快照、带/不带状态的原地写入、GraphQL 包装器的状态映射 / NOT_FOUND 处理 / 分批 / 失败类型、sweep 的补齐与零调用路径、会话列表以 sidecar 的 issues 为准、bridge 的 seed/重绑路径、SDK 守卫,以及 tooltip/搜索渲染。
证据(前后对比)
用构建好的 daemon 配真实
ghCLI 对本仓库跑真实 sweep:一个会话绑定了已合入的 PR(#10278,关闭了 #10270)、外仓 PR #42 和开放 PR(#10303,引用 #10293),跑两轮:第一轮后的 sidecar(之前:同样三条、没有
issues字段):[ { "number": 10278, "url": "https://github.com/QwenLM/qwen-code/pull/10278", "state": "merged", "issues": [{ "number": 10270, "url": "https://github.com/QwenLM/qwen-code/issues/10270", "state": "completed" }] }, { "number": 42, "url": "https://github.com/other/repo/pull/42", "state": "open" }, { "number": 10303, "url": "https://github.com/QwenLM/qwen-code/pull/10303", "state": "open", "issues": [{ "number": 10293, "url": "https://github.com/QwenLM/qwen-code/issues/10293", "state": "open" }] } ]在本仓库实测的查询成本:
gh pr list --state all --limit 500嵌套closingIssuesReferences需 6.7s,不嵌套 4.9s(gh超时 10s),因此列表查询不动,issue 走单独的按编号查询(4 个 PR 约 1s)。仓库解析不到的绑定的收敛(评审第一轮):一个会话只绑定了一个已合入的外仓 PR,真实
gh,跑三轮——第一轮一次查询并写入收敛用的空快照,之后各轮完全不发gh调用:tooltip 渲染由 jsdom 测试覆盖(图标 class、链接 href、去重、sr-only 状态文本);暂无浏览器截图。
测试平台
环境(可选)
各包单测,加上构建好的 daemon 直接对本仓库用真实
ghCLI 运行(macOS,gh 2.96.0)。风险与范围
gh api graphql调用(实测约 1–3s);升级后首轮对每个存量绑定查一次,每批 100 个。PR 合入后 issue 被 reopen 不再跟踪,与 PR 状态"merged 是终态"的取舍一致。/bugfix #N)、从提示词/分支/commit trailer 推断 issue、会话行 badge 显示 issue、回填路由立即抓取 issue(下一轮 sweep 兜底)。Windows/Linux 未在本地运行。tooltip 无浏览器截图。issues在线协议和 sidecar 中均为可选;旧 sidecar 保持有效,首轮 sweep 补齐。关联 Issue
基于 #9729(PR 绑定、回填、状态刷新)与 #10258(GitHub 风格状态图标)。