fix(web-shell): defer assistant footer during background agent work - #8787
Conversation
|
Re-run on the current head — the earlier pass reviewed Template looks good ✓ — all required sections, bilingual body, before/after evidence, tested-on matrix. Problem: observed, not theoretical. The PR ships before/after screenshots of the footer churn, and the mechanism was real: final-answer classification looked only at Direction: aligned — Web Shell turn-completion correctness, consistent with how per-turn collapse already treats active agents and the summary wait as "turn still open". Size: this now spans two packages ( Approach: the core fix is still the minimal gate widening — reuse the two turn-completeness signals the component already computes instead of adding new state. What grew since the first pass is the defense against the failure mode the fix itself introduces (a stale "pending" agent card would hide the footer forever): the bounded reconciliation retry ladder (3s→60s cap, per-agent 8-round error budget, two-miss 404 grace) and the 5s unmatched-completion grace. That is a lot of machinery for a footer bug, and I would normally ask whether it could be cut — but earlier review rounds are exactly what forced these bounds, and each piece now has a fail-safe that releases the footer. One concrete contract problem found in code review — see the Stage 2 comment. Risk: no high-risk paths matched (Stage 1e clean). Moving on to code review. 🔍 中文说明本次为对当前 head 的重跑——上一轮审查的是 模板完整 ✓ —— 必填章节齐全，中英双语，含 before/after 证据与测试平台矩阵。 问题：已观测到的 bug，不是理论性加固。PR 自带 footer 跳动的前后截图，机制真实存在：最终回答判定只看 方向：对齐 —— Web Shell turn 完成判定正确性，与 per-turn 折叠已有的"活跃 agent/等待汇总视为 turn 未结束"语义一致。 规模：现跨两个包（ 方案：核心修复仍是最小门控扩展——复用组件已有的两个 turn 完成信号，不引入新状态。首轮之后增长的部分是对修复自身引入的失效模式的防御（卡在 pending 的 agent 卡片会让 footer 永远隐藏）：有界重试梯（3s→60s 上限、每 agent 8 轮错误预算、两次 404 宽限）与 5 秒 unmatched-completion 宽限。对一个 footer bug 来说这套机制不小，通常会先问能否砍掉——但正是此前评审轮次逼出了这些边界，且每一处现在都有释放 footer 的兜底。代码审查发现一个具体的契约问题——见 Stage 2 评论。 风险：未命中高风险路径（Stage 1e 干净）。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterℹ️ No screenshot changed against the PR base — but this PR edits 1 render-shaping file:
Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
Code reviewBefore reading the diff, my independent proposal for the stated problem was exactly the minimal version of this PR: widen the latest-turn completeness condition in the final-answer classification with the two signals turn collapse already computes (
One Critical finding — the new SDK 404 guards check a body field the daemon never emits.
The fix is mechanical: match on Everything else traced clean; no convention violations. sequenceDiagram
participant P1 as Turn narration
participant P2 as MessageList footer gate
participant P3 as useMessages reconciliation
participant P4 as Daemon subagent resolve
P1->>P2: narration arrives, agents still active
P2->>P2: hide footer, turn incomplete
P3->>P4: poll status, backoff 3s to 60s
P4-->>P3: terminal resolution, or 404 within two-miss grace
P1->>P2: completion notification, summary streams
P2->>P2: footer appears once, under final summary
Files changed (8 of 8)
Test evidenceFrom the PR's own CI on the reviewed commit (fetched via API; nothing here was re-run locally, and no PR code was executed). CI is fully settled and green — no pending checks on this head. The macOS/Windows unit legs are skipped by routing (merge-queue-only by design); the ubuntu leg ran and passed. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The central behavioural claim is already substantiated beyond what 2b can provide: @wenshao's independent real-stack A/B (comment in this thread) ran a real daemon + scripted provider against this exact head — base reproduced both flicker cycles, head showed zero footered intermediate messages across 243 samples, and the new suites fail on base under a mutation check (35 tests). A sponsored Real-scenario testing: not applicable on this CI-path run — no tmux session was driven; the live-behavior signal is the verification report and the in-flight 中文说明代码审查：读 diff 之前，我独立的解法正是本 PR 的最小版本——用 turn 折叠已有的两个信号扩展最终回答判定的"最新 turn 完成"条件，不引入新状态。PR 这样做了，并进一步增加了防止该门控在 agent 状态卡死时永远隐藏 footer 的机制。追踪当前 head 后，门控逻辑本身是正确的： 一个 Critical 发现——新增 SDK 404 守卫检查的 body 字段 daemon 从不发出。 其余追踪无问题；无规范违规。 测试证据：来自该 commit 的 CI（API 获取，本地未重跑、未执行任何 PR 代码）。CI 已完全落定且全绿——该 head 上无 pending 检查；macOS/Windows 单测腿按路由跳过（设计上仅 merge queue 运行），ubuntu 腿实跑通过。表格见上方 CI 区块。 核心行为主张已有超出 2b 能力的佐证：@wenshao 的独立真实栈 A/B（本贴评论）用真实 daemon + 脚本化 provider 在该 head 上运行——base 复现两轮闪烁，head 243 个采样中间消息带 footer 为 0，变异对照下新套件在 base 上失败 35 个。另有一个受助跑 真实场景测试：本次为 CI 路径运行，不适用——未驱动 tmux；实时行为信号即上述验证报告与进行中的 /verify 运行。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 3/5 — the fix itself is correct, bounded, and independently verified on this exact head; two things keep this from approving — a real SDK contract defect in the new 404 guards, and the cross-repo/size policy cap that wants a maintainer's sign-off on a fork PR of this footprint. Stepping back: going into the diff my independent proposal was the minimal gate widening, and the PR's core is exactly that — reusing the two turn-completeness signals instead of inventing state, which keeps footer classification consistent with turn collapse. The ~500 production lines added since the first review pass are the bounded reconciliation machinery, and while that is a lot for a footer bug, I can trace why each piece exists: every one of them is a fail-safe against the fix's own failure mode (footer pinned hidden by a stale agent card), and each was forced by a prior review round. The before/after here is not the author's word — @wenshao's real-stack A/B on this head reproduced the bug on base and measured it gone on the PR, and the new suites demonstrably fail on base. In six months this reads as a well-scarred but coherent fix. The reservation that blocks approval is concrete, not vibes: the two new SDK guards match ⏸️ Deferring to @wenshao — needs a human call on two points: (1) is the 中文说明冷静回顾：读 diff 之前我的独立解法就是最小门控扩展，PR 的核心正是如此——复用两个 turn 完成信号而非引入新状态，使 footer 判定与 turn 折叠语义一致。首轮评审之后新增的约 500 行生产代码是有界调和机制，虽然对一个 footer bug 来说不少，但每一块都能追踪到存在理由：全部是"修复自身失效模式"（陈旧 agent 卡片把 footer 永久隐藏）的兜底，且都是此前评审轮次逼出来的。这里的前后对比不是作者自述——@wenshao 在该 head 上的真实栈 A/B 在 base 复现了 bug、在 PR 上测得其消失，新套件在 base 上确实失败。半年后回看，这是一个伤痕累累但内部一致的修复。 阻止批准的保留意见是具体的：两个新 SDK 守卫匹配 ⏸️ 转交 @wenshao —— 需要人工拍板两点：(1) — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): This PR (web-shell) keeps the assistant footer actions hi...: verifying whether runtime.bridge.getSessionTasksStatus can report a dead background agent as 'running' indefinitely (couldn't trace the bridge implementation)…; This PR (web-shell) keeps the assistant footer actions hi...: actually running the new DOM tests.; This PR (web-shell) keeps the assistant footer actions hi...: did not trace runtime.bridge.getSessionTasksStatus implementation to confirm a dead background agent can be reported as running indefinitely (relevant to Fi…; This PR (web-shell) keeps the assistant footer actions hi...: did not execute the new DOM tests.; This PR (web-shell) keeps the assistant footer actions hi...: did not empirically confirm daemon background-notification ordering vs. a subsequent turn's response (verified turnAwaitsBackgroundSummary by code-trace only)….
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
中文说明
未探索到全部深度（达到工具调用预算）：This PR (web-shell) keeps the assistant footer actions hi...：verifying whether runtime.bridge.getSessionTasksStatus can report a dead background agent as 'running' indefinitely (couldn't trace the bridge implementation)…；This PR (web-shell) keeps the assistant footer actions hi...：actually running the new DOM tests.；This PR (web-shell) keeps the assistant footer actions hi...：did not trace runtime.bridge.getSessionTasksStatus implementation to confirm a dead background agent can be reported as running indefinitely (relevant to Fi…；This PR (web-shell) keeps the assistant footer actions hi...：did not execute the new DOM tests.；This PR (web-shell) keeps the assistant footer actions hi...：did not empirically confirm daemon background-notification ordering vs. a subsequent turn's response (verified turnAwaitsBackgroundSummary by code-trace only)…。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| const latestTurnIncomplete = | ||
| isResponding || latestTurnHasActiveAgent || latestTurnAwaitsAgentSummary; |
There was a problem hiding this comment.
[Critical] The new footer gate keys on latestTurnHasActiveAgent, but background-agent tool calls are force-mapped to status 'pending' on every transcript load (transcriptToMessages.ts: isBackgroundAgent ? 'pending' : statusMap[block.status]), and the only path that promotes them to a terminal status — reconciliation via resolveSubagentSession in useMessages — can fail silently: the daemon route 404s when no task/meta file is found, a non-terminal 'running' result is never retried (the effect re-runs only when the pending/notification keys change), rejections are swallowed by Promise.allSettled, and it requires a live connected session. An interrupted foreground agent persisted as 'in_progress' has no reconciliation path at all. A DOM probe reproduced the failure on this commit; reverting the gate to the pre-PR isResponding restored the footer. — Failure scenario: a turn whose last assistant message precedes such an agent launch goes idle → latestTurnIncomplete stays true, collectFinalAssistantTurnIds keeps skipping the latest turn, and copy/branch/timestamp/custom-footer actions never reappear until the user sends a new prompt; reloading re-pins 'pending' and re-queries the same failing route, so there is no recovery (pre-PR, the footer was shown for the same transcript).
Suggested fix: bound the suppression so an unrecoverable agent state cannot gate the footer forever — e.g. reconcile unresolvable (404) background agents to a terminal status, retry resolveSubagentSession on a timer/connection recovery, or apply the active-agent disjunct only while the session is live.
中文说明
新 footer 门控依赖 latestTurnHasActiveAgent，但后台 agent 工具调用在每次加载转录时都被强制映射为 'pending' 状态（transcriptToMessages.ts：isBackgroundAgent ? 'pending' : statusMap[block.status]），而将其提升为终态的唯一路径——useMessages 中通过 resolveSubagentSession 的对账——可能静默失败：无 task/meta 文件时 daemon 路由返回 404；非终态的 'running' 结果不会被重试（effect 仅在 pending/notification key 变化时重新运行）；拒绝被 Promise.allSettled 吞掉；且需要在线会话。以 'in_progress' 持久化的被中断前台 agent 则完全没有对账路径。DOM 探针在本次提交上复现了该故障；将门控还原为 PR 前的 isResponding 后 footer 恢复显示。— 失败场景：最后一条 assistant 消息之后启动了此类 agent 的 turn 进入空闲 → latestTurnIncomplete 恒为 true，collectFinalAssistantTurnIds 持续跳过最新 turn，复制/创建分支/时间戳/自定义 footer 操作在用户发送新消息之前永远不会再出现；重新加载会再次固定 'pending' 并再次查询同一条失败路由，因此无法恢复（PR 前同一转录可以显示 footer）。
建议修复：为抑制设置边界，使不可恢复的 agent 状态不能永久门控 footer——例如将无法解析（404）的后台 agent 对账为终态、在定时器/连接恢复时重试 resolveSubagentSession，或仅在会话活跃时应用 active-agent 分支。
— qwen3.8-max via Qwen Code /review (v0.21.8)
There was a problem hiding this comment.
Addressed in ca73a26. Footer suppression now considers only active background agents outside transcript-history baseline loading; foreground tools remain covered by the normal responding state. Background-agent reconciliation now retries non-terminal or recoverable results every 3 seconds, and a 404 that explicitly identifies the missing toolCallId is converted to terminal failed. Reconciled terminal siblings are removed from later retries. Added regression coverage for stale pending history, running-to-terminal retries, and missing-agent 404s; the two focused suites pass 104/104.
| const latestTurnIncomplete = | ||
| isResponding || latestTurnHasActiveAgent || latestTurnAwaitsAgentSummary; |
There was a problem hiding this comment.
[Critical] The diff propagates latestTurnAwaitsAgentSummary into the footer gate, but turnAwaitsBackgroundSummary accumulates every launched callId into unmatchedAgentCallIds and a completion notification that never arrives is never removed — the early return if (sawAgentCompletion && unmatchedAgentCallIds.size > 0) return true; (~line 1521) fires before the final-answer check, so the flag stays true even after the assistant summary exists (the code itself acknowledges lost notifications in the anonymous-fallback comment). A DOM probe observed the pin surviving the summary; recovery only via a new user turn or an anonymous agent notification, and reload merely masks it by resetting the grace baseline. — Failure scenario: a turn launches two background agents; one completion notification arrives and the assistant summarizes it, but the other agent's notification never lands → pre-PR this only deferred the agent group's auto-collapse, post-PR it additionally zeroes finalAssistantTurnIdByAssistantId for the latest turn, hiding the final answer's copy/branch/timestamp/custom-footer actions until the user happens to send another prompt.
Suggested fix: for the footer gate specifically, don't let latestTurnAwaitsAgentSummary hide actions once final assistant content exists after the latest notification (e.g. reuse the findFinalAnswerIndex(...) > lastNotificationIndex ordering check), or add a lost-sibling test and decide the intended behavior explicitly.
中文说明
本 diff 将 latestTurnAwaitsAgentSummary 引入 footer 门控，但 turnAwaitsBackgroundSummary 会把每个已启动的 callId 累积到 unmatchedAgentCallIds，而永远不到达的完成通知永远不会被移除——提前返回 if (sawAgentCompletion && unmatchedAgentCallIds.size > 0) return true;（约第 1521 行）在最终回答检查之前触发，因此即使 assistant 总结已经存在，该标志仍为 true（代码自身在匿名回退注释中也承认通知可能丢失）。DOM 探针观察到该固定状态在总结之后依然存在；仅当用户开启新 turn 或收到匿名 agent 通知时才能恢复，重新加载只是通过重置 grace 基线将其掩盖。— 失败场景：某 turn 启动两个后台 agent，其中一个完成通知到达且 assistant 对其总结，但另一个 agent 的通知始终未到达 → PR 前这只会推迟 agent 分组的自动折叠，PR 后还会将最新 turn 的 finalAssistantTurnIdByAssistantId 置空，导致最终回答的复制/创建分支/时间戳/自定义 footer 操作被隐藏，直到用户恰好再发一条消息。
建议修复：仅针对 footer 门控，当最新通知之后已存在最终 assistant 内容时不要让 latestTurnAwaitsAgentSummary 隐藏操作（例如复用 findFinalAnswerIndex(...) > lastNotificationIndex 的排序检查），或补充“丢失兄弟通知”测试并明确预期行为。
— qwen3.8-max via Qwen Code /review (v0.21.8)
There was a problem hiding this comment.
Addressed in ca73a26. I avoided treating every assistant message after a notification as final, because that would restore the original flicker for messages such as “agent A completed, waiting for B.” The footer now keeps unmatched completions blocking only while a background sibling is actually active. Once all siblings reconcile terminal, the normal notification-to-summary ordering check can release the footer even if one completion notification was lost. The regression test verifies waiting narration has no actions, then a final summary with one lost sibling notification does have actions.
| const turnIdByAssistantId = new Map<string, string>(); | ||
| for (let k = 0; k < userIdxs.length; k++) { | ||
| if (k === userIdxs.length - 1 && isResponding) continue; | ||
| if (k === userIdxs.length - 1 && latestTurnIncomplete) continue; |
There was a problem hiding this comment.
[Suggestion] No test exercises a completed earlier turn while the latest turn is incomplete: both new fixtures are single-turn and every existing multi-turn test renders a complete latest turn where the k === userIdxs.length - 1 skip never fires. As a result, the mutation if (latestTurnIncomplete) continue; (skip all turns) survives the whole suite — probe-demonstrated on this commit — and so does the turnHasActiveAgent scan-from-index-0 range mutant. — Concrete cost: a future regression that hides footer actions for every turn while background agents run would ship green, removing copy/branch/timestamp/custom-footer from all completed historical turns whenever a background agent is active or a summary is awaited — a state this PR makes far more common than the old isResponding-only trigger. The PR's own test plan names this guarantee ("completed historical turns retain their existing footer actions") only as a manual reviewer step.
Suggested test:
it('keeps completed turn actions while the latest turn awaits agents', () => {
const activeAgent = agentMsg('agent-1');
activeAgent.tools[0]!.status = 'pending';
const c = mount([
userMsg('u1'),
asstMsg('a1'),
userMsg('u2'),
activeAgent,
asstMsg('a2'),
]);
expect(assistantActions(c, 'a1')).toBe('true');
expect(assistantActions(c, 'a2')).toBe('false');
});中文说明
没有测试覆盖“最新 turn 未完成时更早的已完成 turn”这一情形：两个新 fixture 都是单 turn，而所有现有多 turn 测试渲染的最新 turn 都是完成态，k === userIdxs.length - 1 的跳过从不触发。因此变异 if (latestTurnIncomplete) continue;（跳过所有 turn）能在整个测试套件中存活（已在本次提交上用探针验证），turnHasActiveAgent 从索引 0 开始扫描的范围变异同样存活。— 具体代价：未来某个在后台 agent 运行时隐藏所有 turn footer 操作的回归将一路绿灯地合入，只要后台 agent 活跃或等待汇总，就会把所有已完成历史 turn 的复制/创建分支/时间戳/自定义 footer 移除——而本 PR 使这种状态比旧的仅 isResponding 触发条件常见得多。PR 自身的测试计划将这一保证（“已完成的历史 turn 保留原有 footer 操作”）仅列为人工验证步骤。
建议补充的测试：
it('keeps completed turn actions while the latest turn awaits agents', () => {
const activeAgent = agentMsg('agent-1');
activeAgent.tools[0]!.status = 'pending';
const c = mount([
userMsg('u1'),
asstMsg('a1'),
userMsg('u2'),
activeAgent,
asstMsg('a2'),
]);
expect(assistantActions(c, 'a1')).toBe('true');
expect(assistantActions(c, 'a2')).toBe('false');
});— qwen3.8-max via Qwen Code /review (v0.21.8)
There was a problem hiding this comment.
Added the requested multi-turn regression test in 756c3e4. It asserts that a completed earlier turn keeps its actions while the latest turn with a pending background agent does not expose actions. This is included in the 104/104 passing focused tests.
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): This PR fixes a web-shell bug: assistant footer actions (...: Could not execute the changed unit tests — node_modules in the worktree is empty (no install was ever run; @datafe-open/markdown-chart unresolvable at trans…; This PR fixes a web-shell bug: assistant footer actions (...: Did not verify the daemon-side guarantee that a launched sub-agent's tool-call status always reaches a terminal state (would require running the daemon / readin…; This PR fixes a web-shell bug: assistant footer actions a...: did not run the full web-shell test suite or the two-turn probe scenario against the built UI (the probe test present in the working tree mid-review was removed…; This PR fixes a web-shell bug: assistant footer actions (...: Could not execute the vitest suite — node_modules is not installed in this review worktree and the run fails at import resolution before collecting any tests;…; This PR fixes a web-shell bug: assistant footer actions (...: couldn't run the test suite (worktree has no node_modules; vite import-analysis failed on @datafe-open/markdown-chart)., and 1 more.
[Critical] (re-checked existing comment, still stands) MessageList.tsx:2595 — the new latestTurnHasActiveAgent gate has no escape hatch: background-agent tool calls are force-mapped to 'pending' on transcript load, and the only path to a terminal status (resolveSubagentSession reconciliation) can fail silently (404, no retry, swallowed rejections), so a stuck active status keeps latestTurnIncomplete true forever and the latest turn's final answer never shows copy/branch/custom-footer/timestamp actions.
[Critical] (re-checked existing comment, still stands) MessageList.tsx:2595 — turnAwaitsBackgroundSummary's unmatchedAgentCallIds early return fires before the final-answer check, so when a launched agent's completion notification is lost or diverges, latestTurnAwaitsAgentSummary stays true even after the assistant summary exists and the final answer's copy/branch/custom-footer/timestamp actions stay hidden.
中文说明
未探索到全部深度（达到工具调用预算）：This PR fixes a web-shell bug: assistant footer actions (...：Could not execute the changed unit tests — node_modules in the worktree is empty (no install was ever run; @datafe-open/markdown-chart unresolvable at trans…；This PR fixes a web-shell bug: assistant footer actions (...：Did not verify the daemon-side guarantee that a launched sub-agent's tool-call status always reaches a terminal state (would require running the daemon / readin…；This PR fixes a web-shell bug: assistant footer actions a...：did not run the full web-shell test suite or the two-turn probe scenario against the built UI (the probe test present in the working tree mid-review was removed…；This PR fixes a web-shell bug: assistant footer actions (...：Could not execute the vitest suite — node_modules is not installed in this review worktree and the run fails at import resolution before collecting any tests;…；This PR fixes a web-shell bug: assistant footer actions (...：couldn't run the test suite (worktree has no node_modules; vite import-analysis failed on @datafe-open/markdown-chart).，另有 1 条。
[Critical] (re-checked existing comment, still stands) MessageList.tsx:2595 — the new latestTurnHasActiveAgent gate has no escape hatch: background-agent tool calls are force-mapped to 'pending' on transcript load, and the only path to a terminal status (resolveSubagentSession reconciliation) can fail silently (404, no retry, swallowed rejections), so a stuck active status keeps latestTurnIncomplete true forever and the latest turn's final answer never shows copy/branch/custom-footer/timestamp actions.
[Critical] (re-checked existing comment, still stands) MessageList.tsx:2595 — turnAwaitsBackgroundSummary's unmatchedAgentCallIds early return fires before the final-answer check, so when a launched agent's completion notification is lost or diverges, latestTurnAwaitsAgentSummary stays true even after the assistant summary exists and the final answer's copy/branch/custom-footer/timestamp actions stay hidden.
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.8)
|
@wenshao Thanks for the review. Both Critical findings from the review on 118f6b4 are addressed in ca73a26:
Regression coverage now includes stale pending history, running-to-terminal retries, missing-agent 404s, a lost sibling notification, and preservation of completed historical-turn actions. The two focused suites pass 104/104; Web Shell TypeScript, ESLint, Prettier, application/library builds, and library type generation also pass locally. Could you please re-review the latest head when the current CI run completes? |
|
Qwen Code review timed out. Qwen review timed out after 21600 seconds (of the 360-minute budget). This run already used the maximum 360 minute timeout. See workflow logs. |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and Windows-specific behavior did not run locally.
中文说明
未审查：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and Windows-specific behavior did not run locally。
— gpt-5.6-sol via Qwen Code /review (v0.21.8)
| const latestTurnHasActiveBackgroundAgent = useMemo( | ||
| () => | ||
| !establishingBackgroundNotificationBaseline && |
There was a problem hiding this comment.
[Critical] Static transcript consumers can permanently hide footer actions for completed turns with stale pending agent cards. — Failure scenario: WebShellTranscript renders a completed saved transcript containing a pending background-agent card followed by a final assistant response; because it never runs useMessages reconciliation, the card remains active forever and copy, branch, custom-footer, and timestamp actions never appear. Separate live-session activity from static transcript status, or normalize stale pending statuses for every transcript entry point before MessageList classifies final answers.
中文说明
问题： 静态 transcript 消费方会因陈旧的 pending agent 卡片永久隐藏已完成 turn 的 footer 操作。
失败场景： WebShellTranscript 渲染一个已经完成的保存记录，其中包含 pending 的后台 agent 卡片以及后续最终 assistant 回复；它不会运行 useMessages 对账，因此该卡片会永久保持 active，复制、创建分支、自定义 footer 和时间戳操作永远不会出现。建议区分实时会话活动与静态 transcript 状态，或在 MessageList 判定最终回答前为所有 transcript 入口统一归一化陈旧 pending 状态。
— gpt-5.6-sol via Qwen Code /review (v0.21.8)
| !establishingBackgroundNotificationBaseline && | ||
| turnHasActiveBackgroundAgent( | ||
| displayItems, |
There was a problem hiding this comment.
[Critical] The catch-up baseline exemption can recreate the premature footer flicker during a live reconnect. — Failure scenario: a live session reconnects while a background agent is still running; the first post-catch-up render ignores the pending card and exposes footer actions, then reconciliation returns running and hides them again after request latency. Keep pending calls incomplete until reconciliation classifies them, while restoring historical footers only from a resolved terminal snapshot.
中文说明
问题： catch-up 基线豁免会在实时重连时重新产生过早 footer 闪烁。
失败场景： 后台 agent 仍在运行时实时会话发生重连；catch-up 后的首个 render 会忽略 pending 卡片并显示 footer 操作，随后对账返回 running，在请求延迟后又把操作隐藏。建议在对账完成分类前始终把 pending 调用视为未完成，并仅根据已解析的终态快照恢复历史 footer。
— gpt-5.6-sol via Qwen Code /review (v0.21.8)
| if (resolutions.size < callIds.length) { | ||
| retryTimer = setTimeout(() => { | ||
| if (reconciliationRequestRef.current?.request === request) { |
There was a problem hiding this comment.
[Critical] The reconciliation loop polls every unresolved, errored, running, or paused task forever at a fixed interval. — Failure scenario: a resolve call persistently returns a network/400/500 error, running, or paused; no terminal entry is added, so every open client sends another request for the pending set every three seconds indefinitely, while paused cards remain pending and keep footer actions suppressed. Track successful running and paused states separately from request failures; use event-driven wakeups or bounded exponential backoff, stop on permanent 4xx responses, and avoid fixed polling for stable paused tasks.
中文说明
问题： 对账循环会以固定间隔永久轮询所有未解析、报错、运行中或暂停的任务。
失败场景： resolve 调用持续返回网络/400/500 错误、running 或 paused 时，不会加入终态记录，因此每个打开的客户端都会每 3 秒无限发送一次 pending 集合请求；暂停卡片同时保持 pending 并持续隐藏 footer 操作。建议区分成功的 running/paused 状态与请求失败，使用事件驱动或有界指数退避，永久 4xx 停止重试，并避免对稳定 paused 任务固定轮询。
— gpt-5.6-sol via Qwen Code /review (v0.21.8)
| if (isMissingBackgroundAgent(error, callId)) { | ||
| return [callId, { status: 'failed' }] as const; | ||
| } |
There was a problem hiding this comment.
[Critical] A transient startup 404 can permanently mark a background agent failed before it registers. — Failure scenario: the transcript exposes the pending Agent tool call before BackgroundTaskRegistry registration finishes; the first resolve returns session_not_found, the client stores failed and removes the call from future polling, then footer actions can appear while the agent registers and runs moments later. Require a bounded grace period or repeated misses after the parent turn settles before converting session_not_found into a terminal failure.
中文说明
问题： 启动期间的瞬时 404 会在后台 agent 注册前将其永久标记为失败。
失败场景： transcript 在 BackgroundTaskRegistry 完成注册前就暴露 pending Agent 工具调用；第一次 resolve 返回 session_not_found，客户端存储 failed 并停止后续轮询，随后 agent 在片刻后注册并运行时 footer 操作却可能已经出现。建议在父 turn 稳定后经过有界宽限期或连续多次 miss，再把 session_not_found 转换为终态失败。
— gpt-5.6-sol via Qwen Code /review (v0.21.8)
| true, | ||
| latestTurnHasActiveBackgroundAgent, | ||
| ), |
There was a problem hiding this comment.
[Critical] Terminal reconciliation can release footer actions before a delayed sibling completion notification and final summary arrive. — Failure scenario: one agent notification arrives and the assistant says it is still waiting; a sibling then reconciles terminal before its notification arrives, making latestTurnHasActiveBackgroundAgent false, so the waiting narration receives footer actions until the delayed notification resumes the turn. Preserve the unmatched-completion hold for a bounded grace period after all siblings reconcile terminal, then apply the lost-notification fallback.
中文说明
问题： 终态对账可能在延迟的 sibling 完成通知和最终汇总到达前提前释放 footer 操作。
失败场景： 一个 agent 通知到达后 assistant 表示仍在等待；另一个 sibling 在自己的通知到达前被对账为终态，使 latestTurnHasActiveBackgroundAgent 变为 false，于是等待说明获得 footer 操作，直到延迟通知再次推进 turn。建议在所有 sibling 对账为终态后保留有界宽限期的 unmatched-completion 等待，再应用丢失通知回退。
— gpt-5.6-sol via Qwen Code /review (v0.21.8)
| const turnIdByAssistantId = new Map<string, string>(); | ||
| for (let k = 0; k < userIdxs.length; k++) { | ||
| if (k === userIdxs.length - 1 && isResponding) continue; | ||
| if (k === userIdxs.length - 1 && latestTurnIncomplete) continue; |
There was a problem hiding this comment.
[Critical] Older turns with active background agents are promoted to final answers when the user starts a new turn. — Failure scenario: Turn 1 launches a background agent and ends with waiting narration; before it completes, the user sends Turn 2, so Turn 1 is no longer the latest turn and its narration receives final footer actions even though completion and summary remain outstanding. A DOM probe reproduced the behavior and flipped when a per-turn active-agent guard was added. Compute incompleteness per turn and skip final-answer classification for every turn that still owns active background-agent work.
中文说明
问题： 用户开始新 turn 后，仍有后台 agent 活动的旧 turn 会被提升为最终回答。
失败场景： Turn 1 启动后台 agent 并以等待说明结束；agent 完成前用户发送 Turn 2，Turn 1 因不再是最新 turn 而获得最终 footer 操作，尽管完成通知和汇总仍未到达。DOM 探针复现了该行为，并在加入逐 turn active-agent guard 后由失败转为通过。建议逐 turn 计算未完成状态，并跳过仍拥有 active 后台 agent 工作的所有 turn 的最终回答判定。
— gpt-5.6-sol via Qwen Code /review (v0.21.8)
| if (!(error instanceof DaemonHttpError) || error.status !== 404) return false; | ||
| const body = getRecord(error.body); | ||
| return ( | ||
| body?.['code'] === 'session_not_found' && body?.['toolCallId'] === callId |
There was a problem hiding this comment.
[Suggestion] Add negative-branch tests for the missing-agent predicate. — Concrete cost: a future broadening to every 404 would keep the current positive-only test green while generic or mismatched 404 responses are incorrectly converted into terminal failures. Add parameterized cases for non-404 status, wrong or absent code, and missing or mismatched toolCallId.
中文说明
建议： 为 missing-agent 谓词补充负分支测试。
具体成本： 如果未来把判定放宽为所有 404，当前仅覆盖正例的测试仍会保持绿色，但普通或不匹配的 404 会被错误转换为终态失败。建议加入非 404、错误或缺失 code、缺失或不匹配 toolCallId 的参数化用例。
— gpt-5.6-sol via Qwen Code /review (v0.21.8)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. 2 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here. Not explored to full depth (tool budget reached): This PR (web-shell) keeps assistant footer actions hidden...: could not verify the agent-meta file status lifecycle in core (whether a crashed/killed background agent can leave meta.status = 'running' so resolve() an…. Not reviewed: reverse audit — stopped before round 3 by the review time budget.
中文说明
已审查。 建议见行内评论。 2 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。 未探索到全部深度（达到工具调用预算）：This PR (web-shell) keeps assistant footer actions hidden...：could not verify the agent-meta file status lifecycle in core (whether a crashed/killed background agent can leave meta.status = 'running' so resolve() an…。 未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| function isMissingBackgroundAgent(error: unknown, callId: string): boolean { | ||
| if (!(error instanceof DaemonHttpError) || error.status !== 404) return false; | ||
| const body = getRecord(error.body); |
There was a problem hiding this comment.
[Suggestion] The terminal-404 handling matches the daemon's error contract by string-literal body parsing inside the hook, at a shallower depth than the contract lives: nothing in the SDK ties resolveSubagentSession's "not found" result to this shape. Today's producer is verified correct (GET /session/:id/subagents/:toolCallId emits {code: 'session_not_found', sessionId, toolCallId}), but the coupling is invisible to both compilers, and the Java SDK already hand-rolls its own separate matcher (DaemonSessionClient.java:430-441) — this is the second client encoding the daemon's error body inline. — Concrete cost: if the route's 404 body ever changes (field renamed, toolCallId dropped, session-level shapes unified), isMissingBackgroundAgent silently returns false for every missing agent; the error is rethrown, the new retry loop re-queries every 3s indefinitely, the card never reaches a terminal state, and the footer gate this PR adds hides actions permanently — with no build or test failing on either side at the moment of drift.
Suggested fix: move the contract to the SDK depth — have resolveSubagentSession return a typed not-found result, or export a typed guard alongside DaemonHttpError in the daemon SDK, so the matcher is shared and compiler-checked.
中文说明
[建议] 终态 404 处理在 hook 层通过字符串字面量解析响应体来匹配 daemon 的错误契约，所处深度比契约本身更浅：SDK 中没有任何东西把 resolveSubagentSession 的“未找到”结果约束为这个形状。当前的生产方已核实无误（GET /session/:id/subagents/:toolCallId 会返回 {code: 'session_not_found', sessionId, toolCallId}），但这种耦合对两侧编译器都不可见，而且 Java SDK 已经自行手写了一个独立的匹配器（DaemonSessionClient.java:430-441）——本 PR 是第二个内联编码 daemon 错误体的客户端。— 具体代价：如果该路由的 404 响应体将来发生变化（字段改名、去掉 toolCallId、统一会话级形状），isMissingBackgroundAgent 会对所有缺失 agent 静默返回 false；错误被重新抛出，新的重试循环每 3 秒无限期重新查询，卡片永远无法到达终态，本 PR 新增的 footer 门控将永久隐藏操作——而在漂移发生的那一刻，两侧没有任何构建或测试会失败。
建议修复：把契约下沉到 SDK 深度——让 resolveSubagentSession 返回类型化的“未找到”结果，或在 daemon SDK 中 DaemonHttpError 旁边导出类型化守卫，使匹配器可共享且受编译器检查。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| turnHasActiveBackgroundAgent( | ||
| displayItems, | ||
| latestTurnStartIndex, |
There was a problem hiding this comment.
[Suggestion] No test pins the latest-turn scoping of this new gate: the mutation latestTurnStartIndex → 0 (scan the whole transcript for active background agents) survives all 220 tests in MessageList.test.ts + MessageList.dom.test.tsx (mutation-run-proven on this commit). All new fixtures place the pending agent in the latest turn, where both scan starts agree. — Concrete cost: a long-running background agent launched in turn 1 that reconciliation keeps reporting as 'running' (its card legitimately stays 'pending') while the user completes turn 2 → with scan-from-0, latestTurnIncomplete is true and the completed turn-2 answer's copy/branch/timestamp controls stay hidden indefinitely even though turn 2 awaits nothing.
Suggested test:
it('keeps turn-2 final actions while a turn-1 agent stays pending', () => {
const pendingAgent = agentMsg('agent-1');
pendingAgent.tools[0]!.status = 'pending';
const c = mount([
userMsg('u1'),
pendingAgent,
asstMsg('a1'),
userMsg('u2'),
asstMsg('a2'),
]);
expect(assistantActions(c, 'a2')).toBe('true');
});中文说明
[建议] 没有测试固定住这个新门控的“仅最新 turn”作用域：变异 latestTurnStartIndex → 0（扫描整个转录寻找活跃后台 agent）在本次提交上能在全套 220 个测试（MessageList.test.ts + MessageList.dom.test.tsx）中存活（已通过变异运行验证）。所有新 fixture 都把 pending agent 放在最新 turn 里，而在那里两种扫描起点结果一致。— 具体代价：turn 1 启动的长时运行后台 agent 一直被对账报告为 'running'（其卡片合理地保持 'pending'），同时用户完成了 turn 2 → 若从索引 0 开始扫描，latestTurnIncomplete 为 true，即使 turn 2 没有任何等待事项，其已完成回答的复制/创建分支/时间戳控件也会被无限期隐藏。
建议补充的测试见英文部分代码块。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| rerenderMessages(c, messages, { | ||
| catchingUp: false, | ||
| isResponding: false, | ||
| }); | ||
|
|
||
| expect(assistantActions(c, 'a1')).toBe('true'); |
There was a problem hiding this comment.
[Suggestion] This test pins only a single grace render: the assertion lands on the one render after catchingUp flips false, where wasLoadingBackgroundNotificationHistory.current is still true and forces latestTurnHasActiveBackgroundAgent false. The layout effect then clears that ref, so on any subsequent re-render while the agents stay pending (a reconciliation snapshot arriving, any messages prop change, isResponding toggling), data-assistant-actions flips back to 'false' — probe-verified on this commit (grace render 'true', next render 'false'; reverting the gate to the pre-PR isResponding keeps it 'true'). The test name claims a stable restoration the code does not provide. — Concrete cost: a user loading a session whose history contains stuck-pending background agents sees final actions appear and then disappear until reconciliation resolves every card (at least one daemon round-trip, and indefinitely while a card cannot be resolved), while this test reads as if the case were handled.
Suggested fix: either make the guarantee real (keep agents that were already pending when the transcript baseline was established out of latestTurnHasActiveBackgroundAgent until reconciliation speaks, or reconcile before gating), or change the test to assert the actual single-grace-render behavior and rename it accordingly.
中文说明
[建议] 该测试只固定了一次宽限渲染：断言恰好落在 catchingUp 翻转为 false 后的那一次渲染上，此时 wasLoadingBackgroundNotificationHistory.current 仍为 true，强制 latestTurnHasActiveBackgroundAgent 为 false。随后 layout effect 会清除该 ref，因此在 agent 仍为 pending 时的任何后续重新渲染（对账快照到达、messages prop 变化、isResponding 切换）都会让 data-assistant-actions 翻回 'false' ——已在本次提交上用探针验证（宽限渲染为 'true'，下一次渲染为 'false'；将门控还原为 PR 前的 isResponding 则保持 'true'）。测试名称声称了一种代码并未提供的稳定恢复。— 具体代价：用户加载含有卡死 pending 后台 agent 的历史会话时，会看到最终操作先出现、随后消失，直到对账解析每一张卡片（至少一个 daemon 往返；若卡片无法解析则无限期），而该测试读起来像是这个场景已被覆盖。
建议修复：要么让该保证真实成立（把转录基线建立时已处于 pending 的 agent 排除在 latestTurnHasActiveBackgroundAgent 之外，直到对账给出结论；或先对账再门控），要么把测试改为断言实际的单次宽限渲染行为并相应重命名。
— qwen3.8-max via Qwen Code /review (v0.21.8)
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/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. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。本 PR 来自 fork，首轮处理将由下一次定时扫描执行（通常几分钟内）。移除 |
|
🔀 Base updated: red check(s) [review-pr] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [review-pr] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
Separate the reconciliation backoff counter from the transient-error budget so healthy `running` responses back off without consuming the failure budget, keeping the completion notification's final query retryable. Reset the budget on reconnect and bound the unmatched-completion turn-collapse hold. Add regression coverage for multi-agent budget exhaustion, miss-counter re-arming, foreground-agent footer gating, and the reconnect budget reset.
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 4/100 轮）。改动内容与我反驳保留之处如下： Address-review summary — PR #8787 (round 4)Addressed the round-4 automated-review findings on background-agent reconciliation and the assistant-footer gate in one commit: Findings and dispositions[Critical] Healthy [Suggestion] Budget-exhausted agents are terminal forever and never re-queried — [Suggestion] Multi-agent budget exhaustion is untested — [Suggestion] Success-path reset of the missing-agent miss counter is untested — [Suggestion] Footer gate is not pinned to background agents — [Suggestion] Reconnect resumes the pre-disconnect retry budget — [Suggestion] The 5s unmatched-completion bound is not wired into turn collapse — Second-pass review points (doudouOUC)
VerificationCommands actually run (from the repo root unless noted) and their results:
中文说明处理评审总结 — PR #8787（第 4 轮）在一个提交中处理了第 4 轮自动评审关于后台 agent 对账与助手 footer 门控的发现： 发现与处理[严重] 健康的 [建议] 预算耗尽的 agent 被永久判为终态、之后再也无法被重新查询 — [建议] 多 agent 预算耗尽没有测试 — [建议] missing-agent miss 计数在成功路径上的重置没有测试 — [建议] footer 门控没有固定为仅针对后台 agent — [建议] 重连会沿用断开前的重试预算 — [建议] 5 秒未完成通知宽限没有接入 turn 折叠 — 二轮复审要点（doudouOUC）
验证实际运行的命令（未注明则在仓库根目录）及结果：
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget.; PR #8787 defers Web Shell assistant footer actions while ...: none — all planned checks completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks I started were completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks I started were completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all reads completed; no check left unfinished..
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
中文说明
未探索到全部深度（达到工具调用预算）：You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.；PR #8787 defers Web Shell assistant footer actions while ...：none — all planned checks completed within budget.；You are review agent reverse-audit — Reverse audit agen...：none — all checks I started were completed within budget.；You are review agent reverse-audit — Reverse audit agen...：none — all checks I started were completed within budget.；You are review agent reverse-audit — Reverse audit agen...：none — all reads completed; no check left unfinished.。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| useEffect(() => { | ||
| setUnmatchedCompletionGraceExpired(false); | ||
| }, [ | ||
| backgroundSummaryGraceActive, | ||
| latestAgentNotificationId, | ||
| latestTurnStartIndex, | ||
| ]); |
There was a problem hiding this comment.
[Critical] R5-2: The unmatchedCompletionGraceExpired latch is one-shot: a benign matched-completion hold can consume it mid-turn, and a later genuine lost-completion episode in the same turn starts with the grace already expired — none of the reset effect's deps (backgroundSummaryGraceActive, latestAgentNotificationId, latestTurnStartIndex) change when a new unmatched episode begins. — Failure scenario: agent A completes mid-turn with its (matched) notification while the model is still working (no final answer yet) → the hold memo goes true, the 5 s timer arms, the model keeps working past 5 s → the latch flips to expired invisibly (the release is inert while responding). The model then launches agent B in the same turn and emits the final answer, and B's completion notification is lost; B reconciles terminal. The genuine unmatched episode begins, but none of the reset deps changed → waitForUnmatchedAgentCompletions computes false immediately → copy/branch/timestamp actions appear on the final answer instead of holding the documented grace; if B's notification lands seconds later the turn flips back to awaiting — the user had a window to act on a turn that then receives new content. Probe-verified on this commit: the episode gets zero of its 5 s window; keying the reset effect on latestTurnHoldsUnmatchedAgentCompletion flips the probe, and the fix keeps all 101 existing DOM tests green.
Suggested fix: re-arm the latch when a hold episode begins — add latestTurnHoldsUnmatchedAgentCompletion to the reset effect's deps (declare the memo before the reset effect), or track the episode identity (notification id / unmatched set) instead of a bare boolean.
中文说明
[严重] unmatchedCompletionGraceExpired 门闩是一次性的：一次良性的"已完成且通知已匹配"等待会在 turn 中途消耗掉它，使同一 turn 中随后真正的"丢失完成通知"场景一开始就已超过宽限期——新场景开始时重置 effect 的三个依赖（backgroundSummaryGraceActive、latestAgentNotificationId、latestTurnStartIndex）都不会变化。— 失败场景：agent A 在模型仍在工作（尚无最终回答）时于 turn 中途完成且其通知已匹配 → 等待 memo 置真、5 秒计时器启动，模型继续工作超过 5 秒 → 门闩静默翻转为已过期（响应期间释放无效果）。随后模型在同一 turn 启动 agent B 并输出最终回答，B 的完成通知丢失，B 被对账为终态。真正的未匹配场景开始，但重置依赖均未变化 → waitForUnmatchedAgentCompletions 立即算出 false → 复制/创建分支/时间戳操作直接出现在最终回答上，而不是保持文档承诺的宽限等待；若 B 的通知几秒后到达，turn 又翻回等待状态——用户得到了一个对随后会收到新内容的 turn 执行操作的窗口。已在本提交上探针验证：该场景完全得不到 5 秒窗口；把重置 effect 的键改为包含 latestTurnHoldsUnmatchedAgentCompletion 后探针翻转，且修复后全部 101 个现有 DOM 测试仍通过。
建议修复：在等待场景开始时重置门闩——把 latestTurnHoldsUnmatchedAgentCompletion 加入重置 effect 的依赖（需把该 memo 声明移到重置 effect 之前），或改为记录场景标识（通知 id/未匹配集合）而非裸布尔值。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| const request = | ||
| existingRequest?.key === requestKey | ||
| ? existingRequest.request |
There was a problem hiding this comment.
[Critical] R5-6: A settled-but-unresolved reconciliation request reused from reconciliationRequestRef gets a second .then handler attached when the workspace.client identity changes while requestKey/connectionKey do not — one round's result is processed twice, doubling attempts/errorAttempts and skipping a backoff step. — Failure scenario: an erroring round settles → the handler increments the counters and schedules the backoff timer; a client swap (a transport/autoConnect change in the provider memo) re-runs the effect while connection.status stays 'connected' → cleanup clears the pending timer, connectionKey is unchanged so no budget reset, requestKey is unchanged so the settled promise is reused and a second .then attaches → it fires immediately, processes the same round again, and schedules the next ladder delay. Probe-verified on this commit: with one swap per backoff window the agent is marked failed after 4 real error rounds instead of the documented BACKGROUND_AGENT_RECONCILIATION_MAX_ATTEMPTS (8); a processed flag on the cached round (reuse only when unprocessed) flips the probe to 8 with all 27 tests still green. Reachability note: no shipped consumer currently swaps client identity while connected (baseUrl/token swaps self-heal via the connectionKey reset), so this violates the hook's documented budget contract rather than firing in the shipped configuration — an embedding host that swaps transport would hit it.
Suggested fix: store { key, request, processed } in reconciliationRequestRef and only reuse existingRequest.request when !processed; alternatively treat a workspace.client identity change like a connection transition and clear the ref.
中文说明
[严重] 从 reconciliationRequestRef 复用一个已 settle 但未消费的对账请求时，如果 workspace.client 标识变化而 requestKey/connectionKey 不变，会给它挂上第二个 .then 处理器——同一轮结果被处理两次，attempts/errorAttempts 重复计数，且跳过一个退避台阶。— 失败场景：某报错轮次 settle → 处理器递增计数并安排退避定时器；客户端替换（provider memo 中 transport/autoConnect 变化）在 connection.status 仍为 'connected' 时重跑 effect → cleanup 清掉待发定时器，connectionKey 未变故预算不重置，requestKey 未变故复用已 settle 的 promise 并挂上第二个 .then → 它立即触发、再次处理同一轮并安排下一级延迟。已在本提交上探针验证：每个退避窗口内替换一次客户端时，agent 在 4 个真实报错轮次后就被标记 failed，而非文档承诺的 BACKGROUND_AGENT_RECONCILIATION_MAX_ATTEMPTS（8）；给缓存轮次加 processed 标志（仅在未处理时复用）后探针翻转为 8，且 27 个测试全部保持通过。可达性说明：当前没有任何已发布的消费方在连接期间替换客户端标识（baseUrl/token 替换会经 connectionKey 重置自愈），因此这违反的是 hook 自身文档化的预算契约，而非在现有配置下必然触发——任何替换 transport 的宿主都会命中。
建议修复：在 reconciliationRequestRef 中存储 { key, request, processed }，仅当 !processed 时复用 existingRequest.request；或把 workspace.client 标识变化视同连接转换并清空该 ref。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| }, [ | ||
| latestTurnHoldsUnmatchedAgentCompletion, | ||
| latestAgentNotificationId, | ||
| latestTurnStartIndex, | ||
| ]); |
There was a problem hiding this comment.
[Suggestion] R5-1: No test pins the grace timer's restart when a new agent notification arrives mid-hold — dropping latestAgentNotificationId from the timer effect's deps keeps the whole suite green. — Concrete cost: three agents reconcile terminal with only bg-1's notification arrived → hold active, 5 s timer at T0; at T0+4 s bg-2 arrives (agent-3 still unmatched) → the deps entry restarts the bound to T0+9 s. If a future refactor drops the dep, the reset effect still runs but no timer is rescheduled: the hold releases at T0+5 s instead of T0+9 s, and footer actions render on the narration up to a grace-window early. None of the four existing grace tests catches this mutant (the late-notification test feeds only after expiry, the non-restart test uses a monitor notification, the re-arm test goes through catchingUp). Probe-verified: the proposed test flips under the mutation. — Suggested fix: add a DOM test — launch 3 agents, reconcile all terminal, deliver bg-1 (hold active), advance ~4 s, deliver bg-2, assert actions still suppressed at the old expiry (T0+5 s) and released after the restarted bound (~T0+9 s); place the final narration after the latest notification so the ordering rule does not mask the grace state.
中文说明
[建议] 宽限计时器在等待期间因新 agent 通知到来而重启的行为没有任何测试固定——从计时器 effect 的依赖中删掉 latestAgentNotificationId，整个测试套件仍全绿。— 具体代价：三个 agent 均已对账为终态且只有 bg-1 的通知到达 → 等待激活，5 秒计时器自 T0 启动；T0+4 秒时 bg-2 到达（agent-3 仍未匹配）→ 该依赖项把上界重启到 T0+9 秒。若未来重构删掉这个依赖，重置 effect 仍会运行但计时器不会被重新调度：等待会在 T0+5 秒而非 T0+9 秒释放，footer 操作最多提前一个宽限窗口出现在过渡说明上。现有四个宽限测试都抓不到这个变异（延迟通知测试只在过期后送通知、不重启测试用的是 monitor 通知、重新武装测试走 catchingUp）。已用探针验证：所提议的测试在该变异下翻转。— 建议修复：新增一个 DOM 测试——启动三个 agent、全部对账为终态、送达 bg-1（等待激活）、前进约 4 秒、送达 bg-2，断言在原过期点（T0+5 秒）操作仍被抑制、在重启后的上界（约 T0+9 秒）之后释放；让最终说明位于最新通知之后，避免排序规则掩盖宽限状态。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| await vi.waitFor(() => { | ||
| expect(container.textContent).toBe('failed,pending'); | ||
| }); |
There was a problem hiding this comment.
[Suggestion] R5-4: The multi-agent budget-exhaustion test ends at 'failed,pending' — nothing covers the healthy sibling continuing to poll after the erroring agent exhausts the shared budget. — Concrete cost: agent B's survival depends on a second-order interaction (A's failure shrinks the pending set → pendingBackgroundAgentKey changes → fresh retryScopeKey → new budget). A future refactor that stops the chain globally on exhaustion (a budget flag not keyed to the scope, or skipping the re-run when the set shrinks) pins B at pending forever — its completion is never reconciled — while all 27 tests stay green. Probe-verified: the global-budget mutant is invisible to the suite; the proposed continuation assertion flips. — Suggested fix: after the 'failed,pending' assertion, advance the fake timers one more backoff step and assert resolveSubagentSession is called again with 'agent-b'; then resolve B terminally and assert 'failed,completed'.
中文说明
[建议] 多 agent 预算耗尽测试在 'failed,pending' 处结束——没有覆盖报错 agent 耗尽共享预算后健康兄弟 agent 继续轮询的恢复路径。— 具体代价：agent B 的存活依赖二阶交互（A 被判失败使 pending 集合收缩 → pendingBackgroundAgentKey 变化 → 新的 retryScopeKey → 全新预算）。若未来重构在耗尽时全局停链（预算标志不按 scope 键控，或集合收缩时跳过重跑），B 会永远停在 pending——其完成永远不会被对账——而 27 个测试仍全绿。已用探针验证：全局预算变异对现有套件不可见；所提议的续跑断言能使其翻转。— 建议修复：在 'failed,pending' 断言之后，把假定时器再前进一个退避台阶，断言 resolveSubagentSession 再次以 'agent-b' 被调用；然后把 B 解析为终态并断言 'failed,completed'。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| setResolutionSnapshot((current) => ({ | ||
| sessionId, | ||
| resolutions: new Map([ | ||
| ...(current?.sessionId === sessionId ? current.resolutions : []), | ||
| ...resolutions, | ||
| ...exhaustedFailures, | ||
| ]), | ||
| })); |
There was a problem hiding this comment.
[Suggestion] R4-2: (still stands from round 4) Budget-exhausted agents are synthesized into the resolution snapshot as terminal 'failed'; since pendingBackgroundAgentKey derives from reconciledMessages, the agent is permanently removed from the pending set and never re-queried — even when its completion notification arrives later, the card stays failed for the whole browser session. — Concrete cost: the daemon returns 5xx/429/network errors for ~3.5 min while a card is pending → budget exhausted → card marked failed; the daemon recovers, the agent completes, the notification arrives, but the card still reads "failed" until the user reloads. reconcileBackgroundAgentResolutions applies resolutions only to active-status tools (failed is not active), so the real terminal status can never overwrite the synthesized one. Round 4's budget split (the R4-1 fix) narrowed the trigger — healthy running rounds no longer consume the budget — but genuinely erroring agents still hit it; recorded for a follow-up in round 4 and still reachable at this commit. — Suggested fix: distinguish exhaustion-synthesized failures from daemon-reported terminal statuses, and re-verify them on a matching completion notification or reconnect.
中文说明
[建议] R4-2：（第 4 轮起仍然存在）预算耗尽的 agent 被以终态 'failed' 合成进对账快照；由于 pendingBackgroundAgentKey 派生自 reconciledMessages，该 agent 被永久移出 pending 集合、再也不会被查询——即使之后它的完成通知到达，卡片也会在整个浏览器会话内保持 failed。— 具体代价：卡片 pending 期间 daemon 持续返回 5xx/429/网络错误约 3.5 分钟 → 预算耗尽 → 卡片被标记 failed；随后 daemon 恢复、agent 完成、完成通知到达，但卡片在用户刷新前一直显示"失败"。reconcileBackgroundAgentResolutions 只对 active 状态的工具应用对账结果（failed 不是 active），因此真实终态永远无法覆盖合成值。第 4 轮的预算拆分（R4-1 的修复）已收窄触发条件——健康的 running 轮次不再消耗预算——但真正持续报错的 agent 仍会命中；第 4 轮已记录为后续跟进，在本提交上仍可复现。— 建议修复：把耗尽合成的失败与 daemon 上报的终态区分开，并在匹配的完成通知到达（或重连）时重新验证。
— qwen3.8-max via Qwen Code /review (v0.21.10)
There was a problem hiding this comment.
Declined for this PR (deferred to the follow-up recorded in round 4). The finding is valid and still reachable, but the fix needs a new mechanism: distinguishing exhaustion-synthesized failed statuses from daemon-reported terminal statuses, re-admitting the agent into the pending set, and re-querying on a matching completion notification or reconnect. That is substantive new state/effect logic beyond this PR's minimal-fix scope; the round-4 budget split (healthy running rounds no longer consume the budget) already narrowed the trigger to genuinely erroring agents. Keeping this thread open so the deferral stays visible.
中文说明
本 PR 不予处理（维持第 4 轮已记录的后续跟进）。该发现属实且在本提交上仍可复现，但修复需要新机制：区分耗尽合成的 failed 状态与 daemon 上报的终态、把该 agent 重新纳入 pending 集合，并在匹配的完成通知到达或重连时重新查询。这是超出本 PR 最小修复范围的实质性新状态/effect 逻辑；第 4 轮的预算拆分（健康的 running 轮次不再消耗预算）已把触发条件收窄到真正持续报错的 agent。保持该线程打开，使延期处理保持可见。
Key the unmatched-completion grace reset on the hold itself so a benign matched-completion hold that expires mid-turn cannot consume the latch before a genuine lost-completion episode begins in the same turn. Track whether a cached reconciliation round was already processed so an effect re-run (for example a client identity swap) issues a fresh query instead of attaching a second handler to the settled promise and counting the same round twice against the retry budget. Add regression coverage for both fixes, for the grace timer restart on mid-hold agent notifications, and for the healthy sibling continuing to poll after budget exhaustion. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 5/100 轮）。改动内容与我反驳保留之处如下： Round summary — PR #8787 review feedbackFeedback dispositions
ConflictNone ( Changes
Verification
中文说明轮次总结 — PR #8787 评审反馈反馈处理
冲突无（ 变更
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): PR #8787 defers Web Shell assistant footer actions (copy/...: none — all planned checks completed (~45 tool calls).; You are review agent reverse-audit — Reverse audit agen...: none — all planned checks completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — I completed all checks I intended (diff range, full useMessages.ts , test harness, all five new tests, the SDK 404 guards, and the unmount refactor) wit….
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
中文说明
未探索到全部深度（达到工具调用预算）：PR #8787 defers Web Shell assistant footer actions (copy/...：none — all planned checks completed (~45 tool calls).；You are review agent reverse-audit — Reverse audit agen...：none — all planned checks completed within budget.；You are review agent reverse-audit — Reverse audit agen...：none — I completed all checks I intended (diff range, full useMessages.ts , test harness, all five new tests, the SDK 404 guards, and the unmount refactor) wit…。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| const misses = | ||
| (missingAgentMissesRef.current.get(callId) ?? 0) + 1; | ||
| missingAgentMissesRef.current.set(callId, misses); |
There was a problem hiding this comment.
[Critical] The grace-miss counter is mutated inside the per-call request closure regardless of the round's active flag, so an abandoned (superseded) round's 404 still counts. Two samples of the same unregistered daemon state arriving milliseconds apart exhaust MISSING_BACKGROUND_AGENT_GRACE_MISSES with zero elapsed time and fail a live agent. — Failure scenario: agent A launches; round R1 queries it during the registration race; a dependency change (agent B launches, a notification key changes, or the client swaps) supersedes R1, but R1's closure still records miss 1 when its 404 lands; replacement round R2 gets the same 404, computes misses = 2 ≥ 2, and resolves A as failed. A drops out of the pending key and is never re-queried; the card shows failed for the agent's entire remaining run unless a matching completion notification repairs it. Probe-verified on this commit: two concurrent 404s at t=0 stamp failed,pending; gating the miss increment on the round's active flag flips the probe to pending,pending and the agent is rescued at +3s.
Suggested fix: move miss accounting out of the request closure into the round's active handler — collect not-found callIds in the ReconciliationRound result (alongside errors) and increment missingAgentMissesRef only inside the .then handler that runs under if (!active) return;.
中文说明
grace-miss 计数器在 per-call 请求闭包内被修改，不受轮次 active 标志约束，因此被废弃（已被取代）轮次的 404 仍会计数。同一未注册 daemon 状态的两个样本相隔毫秒到达时，会在零时间消耗内耗尽 MISSING_BACKGROUND_AGENT_GRACE_MISSES，把一个运行中的 agent 标记为失败。— 失败场景：agent A 启动；R1 轮在注册竞态期间查询它；依赖变化（agent B 启动、通知 key 变化或 client 切换）取代了 R1，但 R1 的闭包在其 404 返回时仍记录 miss 1；替代轮 R2 得到同样的 404，计算出 misses = 2 ≥ 2，把 A 解析为 failed。A 从 pending key 中移除且不再被查询；除非有匹配的完成通知修复，卡片将在 agent 整个剩余运行期间显示失败。已在本提交上探针验证：t=0 时两个并发 404 直接打出 failed,pending；将 miss 递增置于轮次 active 标志之下后探针翻转为 pending,pending，agent 在 +3s 被救回。
建议修复：把 miss 计数从请求闭包移入轮次的 active 处理器——在 ReconciliationRound 结果中（与 errors 并列）收集 not-found 的 callId，仅在 if (!active) return; 下的 .then 处理器内递增 missingAgentMissesRef。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| waitForUnmatchedAgentCompletions: | ||
| latestTurnHasActiveBackgroundAgent || | ||
| !unmatchedCompletionGraceExpired, |
There was a problem hiding this comment.
[Suggestion] applyTurnCollapse's second consumer of this grace expression — the waitForUnmatchedAgentCompletions: false path — has no test. — Concrete cost: mutation run — deleting these three call-site lines keeps all 237 MessageList.test.ts + MessageList.dom.test.tsx tests green (the option defaults to true), yet after a truly lost sibling completion notification the final turn stays expanded forever (never auto-collapses) with a live elapsed-time indicator. Trigger: background agent reconciles terminal, its notification never arrives, 5s grace expires. A comparator mutation on the tested consumer fails 6 tests, so the harness is alive — coverage simply does not reach the collapse path.
Suggested fix: extend the collapseItems harness in MessageList.test.ts with the new option and add a direct case (waitForUnmatchedAgentCompletions: false → an unmatched completion no longer keeps the last turn open), or assert collapsibility in one of the grace DOM tests after vi.advanceTimersByTime(5_000).
中文说明
该 grace 表达式的第二个消费方——applyTurnCollapse 的 waitForUnmatchedAgentCompletions: false 路径——没有测试。— 具体代价：变异测试显示，删除这三行调用点代码后全部 237 个 MessageList.test.ts + MessageList.dom.test.tsx 测试仍然通过（选项默认为 true），但在 sibling 完成通知真正丢失后，最终 turn 会永远保持展开（永不自动折叠），且计时指示器持续跳动。触发条件：后台 agent 被对账为终态、其通知始终未到达、5 秒宽限期到期。对已测消费方施加对照变异会使 6 个测试失败，说明测试基建是活的——只是覆盖没有到达折叠路径。
建议修复：在 MessageList.test.ts 的 collapseItems 测试基建中加入该选项并补一个直接用例（waitForUnmatchedAgentCompletions: false → 未匹配完成不再让最后一个 turn 保持展开），或在某个 grace DOM 测试中于 vi.advanceTimersByTime(5_000) 之后断言 turn 可折叠。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| // The unmatched-completion hold only tracks agent notifications, so its | ||
| // grace reset/timer must key on those too; a monitor or shell-task | ||
| // notification must not restart the bound on a lost agent completion. | ||
| const latestAgentNotificationId = useMemo(() => { |
There was a problem hiding this comment.
[Suggestion] latestAgentNotificationId scans the whole transcript rather than the latest turn, while the hold it bounds is scoped to the latest turn — an unrelated earlier-turn agent's completion restarts the grace latch and re-hides an already-released final footer. — Failure scenario: T2 launched agents A and B; B's notification arrived, A's was lost, A reconciled terminal, the 5s grace expired and the footer was released. Then agent X, launched two turns earlier, completes: its notification is appended at the transcript tail, the memo returns X's id, the reset effect clears the latch and the timer restarts even though X's callId cannot match T2's unmatched set {A} (reconciliation updates card statuses, never the notification-based unmatched set). Probe-verified: the released footer flips back to hidden and — sustained by the unconditional ordering fallback in turnAwaitsBackgroundSummary — stays hidden until new assistant content lands. The memo's own comment says the reset must key only on notifications relevant to this hold; earlier-turn completions are exactly the irrelevant class it misses.
Suggested fix: scope the scan to the latest turn (walk mergedMessages only from latestTurnStartIndex, or derive the range from displayItems[latestTurnStartIndex..] like the hold does), so only notifications that can participate in the latest turn's unmatched set restart the grace window.
中文说明
latestAgentNotificationId 扫描整个转录而不是最新 turn，但它所约束的 hold 仅作用于最新 turn——无关的更早 turn 的 agent 完成会重启 grace 门闩，把已经释放的最终 footer 重新隐藏。— 失败场景：T2 启动了 agent A 和 B；B 的通知到达，A 的通知丢失，A 被对账为终态，5 秒宽限到期、footer 已释放。随后两个 turn 之前启动的 agent X 完成：其通知追加到转录末尾，memo 返回 X 的 id，重置 effect 清除门闩、计时器重启——尽管 X 的 callId 不可能匹配 T2 的未匹配集合 {A}（对账只更新卡片状态，从不更新基于通知的未匹配集合）。探针验证：已释放的 footer 翻回隐藏，并在 turnAwaitsBackgroundSummary 的无条件排序回退支撑下持续隐藏，直到新的 assistant 内容到达。memo 自身的注释写明重置只应键入与本 hold 相关的通知；更早 turn 的完成通知正是它漏掉的无关类别。
建议修复：把扫描限定到最新 turn（仅从 latestTurnStartIndex 开始遍历 mergedMessages，或像 hold 一样从 displayItems[latestTurnStartIndex..] 推导范围），使只有能参与最新 turn 未匹配集合的通知才会重启 grace 窗口。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| const exhaustedFailures = | ||
| unresolved && !scheduleRetry | ||
| ? errors.map( |
There was a problem hiding this comment.
[Suggestion] The transient-error budget is shared across all pending agents and the exhaustion sweep fails every agent that happens to error in the final round — one agent's persistent errors can permanently fail a healthy agent that had a single transient blip. — Failure scenario (probe-reproduced): agents A and B pending; A's resolveSubagentSession rejects with DaemonHttpError(500) every round (reachable in production — sendBridgeError maps session-restore timeout / channel quarantine to 503/504/500), B answers healthy 'running' in rounds 1–7 and has a single 500 blip in round 8. Observed final statuses: failed,failed — B permanently failed despite 7 healthy rounds; it drops out of the pending key and is never polled again until the real result lands in the transcript. Filtering exhaustedFailures by per-callId error-round counts flips the probe to failed,pending. The inline promise "neither may fail the agent" (429 branch) is voided through this shared path for 429s and 5xx alike.
Suggested fix: track errorAttempts per callId and only fail agents whose own consecutive-error count hit the cap; alternatively, at exhaustion only stop the timer chain and leave still-pending agents pending instead of synthesizing failed for the last-round errorers.
中文说明
瞬时错误预算由所有 pending agent 共享，且耗尽时会把恰好在最后一轮出错的所有 agent 标记失败——一个 agent 的持续出错可以让只出现一次瞬时抖动的健康 agent 被永久标记失败。— 失败场景（探针复现）：agent A 和 B 处于 pending；A 的 resolveSubagentSession 每轮都以 DaemonHttpError(500) 拒绝（生产中可达——sendBridgeError 将会话恢复超时/通道隔离映射为 503/504/500），B 在第 1–7 轮均健康返回 'running'，仅在第 8 轮出现一次 500 抖动。观察到最终状态：failed,failed——B 尽管有 7 轮健康响应仍被永久标记失败；它从 pending key 移除，在真实结果进入转录之前不再被轮询。按 callId 各自的出错轮数过滤 exhaustedFailures 后探针翻转为 failed,pending。429 分支中"两者都不得使 agent 失败"的内联承诺在这条共享路径上对 429 和 5xx 同样失效。
建议修复：按 callId 跟踪 errorAttempts，只把自身连续出错次数达到上限的 agent 标记失败；或者在耗尽时仅停止计时器链，让仍 pending 的 agent 保持 pending，而不是为最后一轮出错者合成 failed。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| rerenderMessages(c, [...settled, monitorNotificationMsg('monitor')], { | ||
| catchingUp: false, | ||
| }); | ||
| expect(assistantActions(c, 'waiting')).toBe('false'); |
There was a problem hiding this comment.
[Suggestion] This test blesses a path where a non-agent (monitor) notification re-hides an already-released final footer for a fresh 5s grace after a catch-up cycle — contradicting the invariant the production code states at MessageList.tsx:2591-2593 ("a monitor or shell-task notification must not restart the bound on a lost agent completion"). — Failure scenario: a turn's lost sibling keeps the hold; the 5s grace expires and the footer is released. An SSE blip runs a catch-up cycle, resetting backgroundNotificationBaselineId; then an unrelated background monitor completes: latestBackgroundNotificationId changes, reactivating backgroundSummaryGraceActive; latestTurnHoldsUnmatchedAgentCompletion flips false→true, the latch reset clears unmatchedCompletionGraceExpired, and the timer starts a new 5s window — the already-released final answer disappears again. backgroundAgentCompletionForMessage returns null for monitor notifications, so the monitor can never satisfy the unmatched set; it only re-opens the gate. The latestAgentNotificationId keying only guards restart during a continuously active hold, not reactivation after a baseline reset. Cost is bounded (5s per occurrence), but the test locks in behavior the adjacent comment promises is impossible.
Suggested fix: gate the unmatched-completion hold's grace activation on agent-kind notifications — e.g. require latestAgentNotificationId === latestBackgroundNotificationId inside latestTurnHoldsUnmatchedAgentCompletion, or compute the hold's grace flag from the latest agent notification. If the reactivation is deliberate, update the comment and this test to say any background notification reopens the window after a baseline reset.
中文说明
该测试为一个路径背书：catch-up 周期之后，非 agent（monitor）通知会把已释放的最终 footer 重新隐藏一个新的 5 秒宽限期——与生产代码在 MessageList.tsx:2591-2593 声明的不变量（"monitor 或 shell-task 通知不得重启丢失 agent 完成的约束"）相矛盾。— 失败场景：某 turn 丢失的 sibling 使 hold 持续；5 秒宽限到期、footer 已释放。一次 SSE 抖动触发 catch-up 周期并重置 backgroundNotificationBaselineId；随后一个无关的后台 monitor 完成：latestBackgroundNotificationId 变化，重新激活 backgroundSummaryGraceActive；latestTurnHoldsUnmatchedAgentCompletion 由 false→true，门闩重置清除 unmatchedCompletionGraceExpired，计时器开启新的 5 秒窗口——已释放的最终回答再次消失。backgroundAgentCompletionForMessage 对 monitor 通知返回 null，因此 monitor 永远无法满足未匹配集合；它只会重新打开门控。latestAgentNotificationId 的键入只防护持续 active 的 hold 期间的重启，不防护基线重置后的重新激活。代价有界（每次发生 5 秒），但测试把相邻注释承诺不可能出现的行为固定了下来。
建议修复：把未匹配完成 hold 的 grace 激活限定为 agent 类通知——例如在 latestTurnHoldsUnmatchedAgentCompletion 内要求 latestAgentNotificationId === latestBackgroundNotificationId，或从最新 agent 通知计算 hold 的 grace 标志。若重新激活是有意为之，请更新注释与本测试，说明基线重置后任何后台通知都会重开窗口。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| // Key the reset on the hold itself: an earlier benign hold in the same | ||
| // turn can consume the latch, and a later genuine lost-completion | ||
| // episode must still receive its full grace window. | ||
| useEffect(() => { | ||
| setUnmatchedCompletionGraceExpired(false); | ||
| }, [ |
There was a problem hiding this comment.
[Suggestion] The grace latch can be consumed while the hold is invisible (during isResponding), leaving zero grace for the moment the hold actually gates the final footer — none of the reset effect's deps change across that window, so the latch is never re-armed. — Failure scenario (probe-verified): a turn launches X and Y; X completes (N1 arrives), the model streams its reaction (isResponding=true); Y goes terminal while NY is delayed. The hold flips false→true and the 5s timer starts, but turnAwaitsBackgroundSummary checks the unmatched branch before the thinking/final-answer branches, so streaming cannot release the hold; if the reaction streams past 5s the latch expires while isResponding hides the turn anyway. Observed: PROBE-F9 footer after reaction ends: true — the turn is presented as final the moment streaming ends (footer actions shown, turn collapsed) with zero grace for NY; footer after NY lands: false — it re-hides when NY lands. The comment above this effect promises a later genuine lost-completion episode receives its full grace window; that only holds when the hold released between episodes.
Suggested fix: spend the grace budget only while the hold can actually gate the footer — gate the timer effect on !isResponding (if (!latestTurnHoldsUnmatchedAgentCompletion || isResponding) return;) and add isResponding to the deps of both the timer and the reset effect. Probe-verified: the flip holds the footer until NY lands, and all 103 existing DOM tests stay green.
中文说明
grace 门闩可能在 hold 不可见时（isResponding 期间）被消耗，等 hold 真正门控最终 footer 时宽限已为零——重置 effect 的依赖在该窗口内均不变化，门闩不会被重新装填。— 失败场景（探针验证）：某 turn 启动 X 和 Y；X 完成（N1 到达），模型开始流式输出其反应（isResponding=true）；NY 延迟期间 Y 进入终态。hold 由 false→true，5 秒计时器启动，但 turnAwaitsBackgroundSummary 在 thinking/最终回答分支之前检查未匹配分支，因此流式输出无法释放 hold；若反应流式输出超过 5 秒，门闩在 isResponding 仍隐藏该 turn 时就已到期。观察到：PROBE-F9 footer after reaction ends: true——流式结束的瞬间 turn 即被当作最终回答（显示 footer 操作、turn 折叠），对 NY 零宽限；footer after NY lands: false——NY 到达后又重新隐藏。本 effect 上方的注释承诺"之后真正的丢失完成场景仍会获得完整宽限窗口"；这只在两次场景之间 hold 曾释放时才成立。
建议修复：只在 hold 能实际门控 footer 时消耗宽限预算——给计时器 effect 加 !isResponding 门控（if (!latestTurnHoldsUnmatchedAgentCompletion || isResponding) return;），并把 isResponding 加入计时器与重置 effect 的依赖。探针验证：翻转后 footer 保持隐藏直到 NY 到达，且全部 103 个现有 DOM 测试保持通过。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| { code: 'session_not_found', toolCallId: 'other-call' }, | ||
| 'not found', | ||
| ), | ||
| ); | ||
| const { container, render, unmount } = mountStatusConsumer(); | ||
|
|
||
| await act(async () => render()); | ||
| await vi.waitFor(() => | ||
| expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(1), | ||
| ); | ||
| expect(container.textContent).toBe('pending'); | ||
|
|
||
| await act(async () => unmount()); |
There was a problem hiding this comment.
[Suggestion] This test asserts only that the first round leaves the card pending; it never asserts reconciliation keeps polling, so it cannot catch an "abandon without retrying" regression on the mismatched-toolCallId path. — Concrete cost: production currently does the right thing (the mismatched 404 falls past both not-found guards and the 4xx-permanent branch into roundErrors → retry scheduled; continuation probe: calls=2 status=pending after 3s). But the plausible follow-up fix for the grace-miss race on this review — early-return on a not-ours 404 instead of bucketing it as transient — would schedule no retry; the card stays pending forever with zero further queries, and this test still passes because it unmounts immediately after the first round without advancing the (fake) timers it never enabled. Probe-verified: under a simulated regression the shipped test stayed green while the continuation showed calls=1, status=pending.
Suggested fix:
vi.useFakeTimers();
// ... first round as today ...
await act(async () => vi.advanceTimersByTimeAsync(3_000));
expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(2);
expect(container.textContent).toBe('pending');中文说明
该测试只断言第一轮后卡片保持 pending；它从未断言对账会持续轮询，因此无法捕获 toolCallId 不匹配路径上的"放弃且不重试"回归。— 具体代价：当前生产代码行为正确（不匹配的 404 会越过两个 not-found 守卫和 4xx 永久分支，落入 roundErrors → 安排重试；延续探针：3 秒后 calls=2 status=pending）。但针对本次评审 grace-miss 竞态的一个合理后续修复——对不属于本 agent 的 404 提前返回而不是归入瞬时错误——将不再安排重试；卡片会永远停留在 pending 且不再有任何查询，而本测试仍会通过，因为它在第一轮之后立即卸载，且从未启用/推进假定时器。探针验证：在模拟回归下，现有测试保持绿色，而延续探针显示 calls=1, status=pending。
建议修复：启用假定时器，推进超过 3 秒基础重试间隔，断言 resolveSubagentSession 再次被调用且卡片仍为 pending（见上方代码）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| // same delay ladder but do not consume this budget, so a long-running agent's | ||
| // completion query keeps its retry tolerance. | ||
| const BACKGROUND_AGENT_RECONCILIATION_MAX_ATTEMPTS = 8; |
There was a problem hiding this comment.
[Suggestion] A successful response resets the grace-miss counter but never errorAttempts, while an in-grace sub-agent 404 (isSubagentSessionNotFound, misses < 2) still falls through to roundErrors.push and consumes one unit of the budget — intermittent misses accumulate across the agent's whole lifetime and the 8th cumulative one permanently fails a live agent. This undercuts the constant's own stated purpose ("a long-running agent's completion query keeps its retry tolerance"). — Failure scenario (probe-verified): one agent, odd calls reject with 404 + matching toolCallId (in-grace miss), even calls resolve 'running'. Rounds 1–14 stay pending; round 15 — the 8th cumulative erroring round — renders failed while the agent answered running in round 14. Each tolerated miss spends the budget; success only deletes the miss count.
Suggested fix: reset errorAttempts to 0 when a round has no errors (mirroring how a success already deletes the miss count), so intermittent transient errors don't accumulate across a long run. Probe-verified: the one-line reset kept the agent pending through all 16 probe rounds with all 28 existing tests still passing.
中文说明
成功响应会重置 grace-miss 计数器，但从不重置 errorAttempts；而宽限内的 sub-agent 404（isSubagentSessionNotFound，misses < 2）仍会落入 roundErrors.push 并消耗一格预算——间歇性丢失会在 agent 整个生命周期内累积，第 8 次累积出错就会永久失败一个运行中的 agent。这与该常量自身声明的目的（"长时间运行 agent 的完成查询保持其重试容忍度"）相悖。— 失败场景（探针验证）：单个 agent，奇数次调用以 404 + 匹配 toolCallId（宽限内 miss）拒绝，偶数次调用解析为 'running'。第 1–14 轮保持 pending；第 15 轮——第 8 次累积出错轮——在 agent 于第 14 轮还回答 running 的情况下渲染 failed。每次被容忍的 miss 都消耗预算；成功只删除 miss 计数。
建议修复：当一轮没有错误时把 errorAttempts 重置为 0（与成功时删除 miss 计数对称），使间歇性瞬时错误不会跨长时间运行累积。探针验证：这一行重置使 agent 在全部 16 轮探针中保持 pending，且现有 28 个测试全部仍通过。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| } else if ( | ||
| isSessionLevelNotFound(error) || | ||
| (error instanceof DaemonHttpError && |
There was a problem hiding this comment.
[Critical] The session-level 404 shape is consumed as "the parent session is permanently gone" — immediate terminal {status:'failed'} synthesis on the first occurrence, zero grace — but the daemon also emits exactly this shape whenever the owning workspace runtime entry is merely not 'active' (draining/transitioning), a transient state the daemon itself treats as reversible. — Failure scenario: multi-workspace daemon; operator removes the agent's workspace → beginDrain (workspace-registry.ts:469-474) only flips entry.state='draining' (cancelDrain can restore 'active'; the established SSE stream keeps flowing), but activeRuntime returns runtimes only for state==='active', so resolveLiveSessionOwner/scanLiveOwners skip the draining entry → requireSessionRuntime answers not_found → 404 {code:'session_not_found'} without toolCallId (session-runtime.ts:84-90) even though the session and agent are alive. The next reconciliation round hits isSessionLevelNotFound → immediate terminal 'failed' for a live agent, unlike the sibling agent-level 404 on the same endpoint, which gets 2 grace misses. The synthesized failure drops the agent from the pending key (never re-queried) and survives reconnects, so the card shows failed for the rest of a run that is actually succeeding — and is simply wrong when the drain is cancelled. force-removal bypasses the busy check, widening the window. Single-workspace daemons are unaffected (requirePrimarySessionRuntime answers 503, transient). Verified against workspace-registry.ts, session-runtime.ts, routes/session.ts:2395-2441, and the workspace-management removal flow.
Suggested fix: do not treat isSessionLevelNotFound(error) as immediately terminal — route it through the same grace-miss counter as the agent-level branch (or keep it transient like unrecognized 404s while the SSE connection is alive), and correct the guard's doc comment, which omits that transient workspace states also produce this shape.
中文说明
session 级 404 形状被当作"父会话已永久消失"处理——首次出现即合成终态 {status:'failed'，零宽限——但 daemon 在所属 workspace 运行时条目仅为非 'active'（draining/transitioning）时也会发出完全相同的形状，而这是 daemon 自身视为可逆的瞬时状态。— 失败场景：多 workspace daemon；操作者移除 agent 所在 workspace → beginDrain（workspace-registry.ts:469-474）只翻转 entry.state='draining'（cancelDrain 可恢复 'active'；已建立的 SSE 流继续工作），但 activeRuntime 只对 state==='active' 返回运行时，因此 resolveLiveSessionOwner/scanLiveOwners 跳过 draining 条目 → requireSessionRuntime 返回 not_found → 404 {code:'session_not_found'} 且无 toolCallId（session-runtime.ts:84-90）——尽管会话和 agent 都还活着。下一个对账轮命中 isSessionLevelNotFound → 对运行中的 agent 立即终态 'failed'，而同一条端点上的 sibling agent 级 404 却有 2 次 grace miss。合成的失败把 agent 从 pending key 移除（永不再查询）且跨重连保留，于是卡片在实际上正在成功的整个剩余运行期间显示失败——在 drain 被取消时则完全是错误的。force 移除会绕过 busy 检查，扩大该窗口。单 workspace daemon 不受影响（requirePrimarySessionRuntime 返回 503，按瞬时处理）。已对照 workspace-registry.ts、session-runtime.ts、routes/session.ts:2395-2441 与 workspace-management 移除流程验证。
建议修复：不要把 isSessionLevelNotFound(error) 当作立即终态——将其路由到与 agent 级分支相同的 grace-miss 计数器（或在 SSE 连接存活期间像未识别的 404 一样保持瞬时），并修正守卫的文档注释（它遗漏了瞬时 workspace 状态也会产生该形状）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| if ( | ||
| waitForUnmatchedAgentCompletions && | ||
| sawAgentCompletion && |
There was a problem hiding this comment.
[Suggestion] The grace bound only gates the unmatched-launch branch; this function's final ordering return (findFinalAnswerIndex(items, start, end, false) < lastNotificationIndex, line ~1586) is unconditional, so after grace expiry a lost sibling notification still holds the final turn whenever the turn's final narration precedes the notification — the ordinary placement, since background notifications land after the turn ends. This defeats the diff's own stated guarantee (UNMATCHED_AGENT_COMPLETION_GRACE_MS: "a truly lost notification cannot hide the final footer forever"). — Failure scenario (probe-verified): turn = [u1, agent-1(pending), agent-2(pending), asst('launched')]; bg-1 lands after the narration; agent-2 reconciles terminal with its notification lost; fake timers advance past the 5s grace — observed: at grace expiry actions=false aria-expanded=true; +60s actions=false. Pinned forever: latestTurnAwaitsAgentSummary stays true → collectFinalAssistantTurnIds skips the last turn (no copy/branch footer) and applyTurnCollapse keeps shouldStayOpen + liveStartedAt (permanently ticking elapsed indicator), until a new prompt or catch-up reset. All 11 new grace tests append narration after the notification, so none can catch this.
Suggested fix (probe-verified, all 103 pre-existing tests stay green): when grace has expired, end the summary wait entirely — e.g.
if (sawAgentCompletion && unmatchedAgentCallIds.size > 0) {
return waitForUnmatchedAgentCompletions;
}and add a DOM grace test whose final narration precedes the matched notification.
中文说明
grace 边界只门控未匹配启动分支；该函数最后的排序返回（findFinalAnswerIndex(items, start, end, false) < lastNotificationIndex，约第 1586 行）是无条件的，因此宽限到期后，只要 turn 的最终叙述位于通知之前（这是通常情况，因为后台通知在 turn 结束之后才到达），丢失的 sibling 通知仍会持续 hold 最终 turn。这违背了 diff 自身声明的保证（UNMATCHED_AGENT_COMPLETION_GRACE_MS："真正丢失的通知不能永远隐藏最终 footer"）。— 失败场景（探针验证）：turn = [u1, agent-1(pending), agent-2(pending), asst('launched')]；bg-1 在叙述之后到达；agent-2 被对账为终态但其通知丢失；假定时器推进超过 5 秒宽限——观察到：宽限到期时 actions=false aria-expanded=true；+60s 时 actions=false。被永久固定：latestTurnAwaitsAgentSummary 恒为 true → collectFinalAssistantTurnIds 跳过最后一个 turn（没有复制/创建分支 footer），且 applyTurnCollapse 保持 shouldStayOpen + liveStartedAt（计时指示器永久跳动），直到用户发新消息或 catch-up 重置。全部 11 个新 grace 测试都把叙述放在通知之后，因此没有一个能捕获此问题。
建议修复（已探针验证，全部 103 个既有测试保持通过）：宽限到期时彻底结束 summary 等待——例如把该分支改为 if (sawAgentCompletion && unmatchedAgentCallIds.size > 0) { return waitForUnmatchedAgentCompletions; }（见上方代码），并补一个最终叙述先于匹配通知的 DOM grace 测试。
— qwen3.8-max via Qwen Code /review (v0.21.10)
… budgets Route session-level 404s through the missing-agent grace instead of failing the card on first occurrence: multi-workspace daemons answer that shape while the owning workspace runtime is merely transiently inactive. Count grace misses only in the active round's handler so a superseded round's late 404 cannot consume the live round's grace, and track the transient-error budget per callId so one agent's persistent errors can no longer fail healthy siblings. Release the unmatched-completion hold entirely once its grace expires regardless of notification ordering, key the grace latch on the unmatched set so monitor and earlier-turn notifications cannot restart or re-arm it, and pause the grace timer while streaming hides the turn. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 6/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 6/100 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #8787 (address-review)All 10 findings from the round-6 review were verified against the code and resolved in one commit ( Critical findings
Suggestions
Files changed
VerificationCommands actually run this round (repo root unless noted):
中文说明Autofix 轮次总结 — PR #8787（address-review）第 6 轮评审的全部 10 条发现均已对照代码核实，并在一次提交中解决（ Critical 发现
Suggestion 发现
变更文件
验证本轮实际执行的命令（未注明者均在仓库根目录）：
Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not reviewed: reverse audit — stopped at the 5-round cap without two consecutive dry rounds (rounds 2-5 each reported probe-verified findings, so the audit never went dry).
Not explored to full depth (tool budget reached): PR #8787 keeps Web Shell assistant footer actions hidden ...: none — the chunk was fully read (lines 1–255, no truncated reads) and all follow-up verifications completed within budget.; PR #8787 keeps Web Shell assistant footer actions hidden ...: none — finished under the tool ceiling with no checks left open.; PR #8787 keeps Web Shell assistant footer actions hidden ...: none — all checks completed within budget.; PR #8787 keeps Web Shell assistant footer actions hidden ...: none — all checks above completed within budget.; PR #8787 keeps Web Shell assistant footer actions hidden ...: none — all planned checks completed, including the empirical probe., and 3 more.
中文说明
已审查。 建议见行内评论。
未审查：reverse audit — stopped at the 5-round cap without two consecutive dry rounds (rounds 2-5 each reported probe-verified findings, so the audit never went dry)。
未探索到全部深度（达到工具调用预算）：PR #8787 keeps Web Shell assistant footer actions hidden ...：none — the chunk was fully read (lines 1–255, no truncated reads) and all follow-up verifications completed within budget.；PR #8787 keeps Web Shell assistant footer actions hidden ...：none — finished under the tool ceiling with no checks left open.；PR #8787 keeps Web Shell assistant footer actions hidden ...：none — all checks completed within budget.；PR #8787 keeps Web Shell assistant footer actions hidden ...：none — all checks above completed within budget.；PR #8787 keeps Web Shell assistant footer actions hidden ...：none — all planned checks completed, including the empirical probe.，另有 3 条。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| ? `HTTP ${error.status} ${code}` | ||
| : `HTTP ${error.status}`; | ||
| } | ||
| return error instanceof Error ? error.message : String(error); |
There was a problem hiding this comment.
[Suggestion] The non-DaemonHttpError branch of describeReconciliationError is executed by an existing test but never asserted. 'applies successful resolutions when another pending Agent fails' rejects agent-b with new Error('not found') through this branch but installs no console.warn spy; the only warn-payload assertions in the file pin the DaemonHttpError branch ('HTTP 429 rate_limit_exceeded', 'HTTP 500 internal_error'). Probe-verified: mutating this branch (e.g. to return String(error);) leaves all 32 tests green. — Failure scenario: a plain-Error rejection fires the retry-scheduled warn through this branch; a mutation here (logging 'Error: not found', or undefined for a non-Error throw) ships green, so the only operator diagnostic for why a background-agent card is stuck retrying can silently degrade.
Suggested fix — pin the payload in that test:
expect(warnSpy).toHaveBeenCalledWith(
'[web-shell] background agent reconciliation retry scheduled',
expect.objectContaining({ errors: ['not found'] }),
);中文说明
describeReconciliationError 的非 DaemonHttpError 分支会被现有测试执行，但从未被断言。'applies successful resolutions when another pending Agent fails' 中 agent-b 以 new Error('not found') 拒绝并走到该分支，但该测试没有安装 console.warn 探针；文件中仅有的 warn 载荷断言都固定在 DaemonHttpError 分支上（'HTTP 429 rate_limit_exceeded'、'HTTP 500 internal_error'）。已用探针验证：变异该分支（例如改为 return String(error);）后全部 32 个测试仍然通过。— 失败场景：普通 Error 拒绝会经由该分支触发 "retry scheduled" 警告；此处的变异（记录成 'Error: not found'，或非标 Error 抛出时记录 undefined）可以静默合入，导致"后台 agent 卡片为何卡在重试中"的唯一运维诊断信息悄然退化。
建议修复：在该测试中固定 warn 载荷（见英文部分代码）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| expect(assistantActions(c, 'waiting')).toBe('false'); | ||
| expect(assistantActions(c, 'summary')).toBe('true'); |
There was a problem hiding this comment.
[Suggestion] This phase does not exercise the property its comment claims ("A late notification still re-hides the narration until the summary"). bg-2 and summary arrive in one render, so summary becomes the turn's final assistant message and waiting === 'false' holds by finality alone — independent of any hold/grace/ordering state. Probe-verified: a mutation breaking the matched-notification re-hide path (turnAwaitsBackgroundSummary's ordering rule) leaves this phase green while 9 other tests catch it; no test anywhere renders a late matched notification alone after a grace release, so the post-release re-hide is asserted-but-not-exercised. — Failure scenario: a regression breaking the ordering-rule re-hide of an already-released footer ships green through this phase, and the re-hide stays pinned nowhere.
Suggested fix — split the phase into two renders: first append backgroundNotificationMsg('bg-2', 'call-agent-2') alone and assert assistantActions(c, 'waiting') flips back to 'false' (discriminating the ordering-rule re-hide), then append asstMsg('summary') and assert the footer moves to it.
中文说明
该阶段没有真正验证其注释所声称的属性（"延迟通知会在总结到来前重新隐藏叙述"）。bg-2 和 summary 在同一次渲染中到达，summary 成为该 turn 的最终 assistant 消息，因此 waiting === 'false' 仅凭最终性就成立——与任何 hold/grace/排序规则状态无关。已用探针验证：破坏 matched-notification 重新隐藏路径（turnAwaitsBackgroundSummary 的排序规则）的变异不会让该阶段失败（另外 9 个测试能捕获）；整个测试套件中没有任何测试在 grace 释放之后单独渲染一条延迟的 matched 通知，因此"释放后重新隐藏"这一行为只被断言、未被真正执行。— 失败场景：破坏"已释放 footer 被排序规则重新隐藏"的回归可以在该阶段静默合入，该重新隐藏行为将没有任何测试固定。
建议修复：把该阶段拆成两次渲染——先单独追加 backgroundNotificationMsg('bg-2', 'call-agent-2') 并断言 assistantActions(c, 'waiting') 翻回 'false'（以区分排序规则的重新隐藏），再追加 asstMsg('summary') 并断言 footer 移到其上。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| const messages = [userMsg('u1'), firstAgent, secondAgent, asstMsg('a1')]; | ||
| const c = mount(messages, undefined, { | ||
| catchingUp: true, | ||
| isResponding: false, | ||
| }); |
There was a problem hiding this comment.
[Suggestion] This test mounts with catchingUp: true but asserts nothing until after it flips to false, so the PR's core footer suppression is unpinned during the catch-up window itself. Probe-verified: the mutant gateBackgroundAgentStatus = transcriptRenderMode === 'interactive' && !catchingUp (footer suppression disabled for the whole session-restore replay) leaves all 105 DOM tests green; the one-line assertion below fails under the mutant and passes on correct code. — Failure scenario: a regression disabling suppression during catch-up ships green — reconnecting to / restoring a session whose background subagent is still running would show clickable copy/branch/timestamp actions on a non-final turn during the replay, self-healing only when catch-up ends.
Suggested fix:
expect(assistantActions(c, 'a1')).toBe('false'); // before the first rerenderMessages中文说明
该测试以 catchingUp: true 挂载，但在其翻转为 false 之前没有做任何断言，因此 PR 的核心 footer 抑制在 catch-up 窗口本身内完全没有被固定。已用探针验证：变异 gateBackgroundAgentStatus = transcriptRenderMode === 'interactive' && !catchingUp（在整个会话恢复重放期间关闭 footer 抑制）下全部 105 个 DOM 测试仍然通过；下面这行断言在该变异下失败、在正确代码下通过。— 失败场景：关闭 catch-up 期间抑制的回归可以静默合入——重连/恢复一个后台 subagent 仍在运行的会话时，重放期间会在非最终 turn 上显示可点击的复制/创建分支/时间戳操作，直到 catch-up 结束才自行恢复。
建议修复：在第一次 rerenderMessages 之前断言 assistantActions(c, 'a1') 为 'false'（见英文部分代码）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| await act(async () => vi.advanceTimersByTimeAsync(3_000)); | ||
| expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(3); | ||
| await act(async () => vi.advanceTimersByTimeAsync(9_000)); |
There was a problem hiding this comment.
[Suggestion] This window cannot pin what its test names: any round-3 delay in (3s, 12s] satisfies both assertions, so backoff continuity across a superseding notification is unpinned — and a stale-timer mutant (removing the cleanup clearTimeout(retryTimer), leaving the superseded round's t=9000 timer armed) passes the entire 32-test suite. The ladder itself is pinned globally by the sibling ladder test; what is pinned nowhere is continuity across a notification supersession and superseded-round timer hygiene. — Failure scenario: a regression computing the notification round's delay from the previous attempts value (6s instead of 12s), or a refactor that stops clearing the superseded round's timer, ships green — the backoff ladder silently degrades into more aggressive polling after every foreign notification, the exact regression this test exists to catch.
Suggested fix — split the window at the expected boundary:
await act(async () => vi.advanceTimersByTimeAsync(6_000));
expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(3); // still 3 at t=9000
await act(async () => vi.advanceTimersByTimeAsync(6_000));
await vi.waitFor(() => expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(4));中文说明
该时间窗口无法固定测试名所声称的属性：任何 (3s, 12s] 区间内的第 3 轮延迟都能同时满足两个断言，因此"通知取代当前轮后退回延迟保持增长"没有被固定——而且一个陈旧定时器变异（删除 cleanup 中的 clearTimeout(retryTimer)，让被取代轮次 t=9000 的定时器仍然挂起）能通过整个 32 个测试的套件。退避阶梯本身由同文件的阶梯测试在全局固定；完全没有被固定的是跨通知取代的延迟连续性和被取代轮次的定时器清理。— 失败场景：通知触发轮次按上一轮 attempts 计算延迟（6s 而非 12s）的回归、或不再清理被取代轮次定时器的重构，都可以静默合入——每次外来通知后退避阶梯都会悄然退化为更激进的轮询，正是本测试要捕获的回归。
建议修复：在预期边界处切分窗口（见英文部分代码）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| await vi.waitFor(() => | ||
| expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(8), | ||
| ); | ||
| expect(container.textContent).toBe('pending'); |
There was a problem hiding this comment.
[Suggestion] This test pins the error-budget reset but not the documented retry-ladder (attempts) reset its comment claims ("the next error opens a fresh ladder"): it ends immediately after the first post-reconnect error (call 8) without advancing a timer. Probe-verified: a mutant preserving the backoff scope key + attempts across the connection transition schedules the retry at min(3000 * 2^7, 60000) = 60s instead of restarting at 3s — the probe flips (9 calls at +3s on correct code vs only at +60s under the mutant) while all 32 tests stay green. — Failure scenario: a "clear just the error budget" refactor ships green, leaving pending background-agent cards up to 57s slower per retry after every reconnect.
Suggested fix — pin the fresh ladder at exact boundaries after call 8:
await act(async () => vi.advanceTimersByTimeAsync(2_999));
expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(8);
await act(async () => vi.advanceTimersByTimeAsync(1));
await vi.waitFor(() => expect(hookState.resolveSubagentSession).toHaveBeenCalledTimes(9));中文说明
该测试固定了错误预算的重置，但没有固定其注释所声称的（"下一次错误开启全新阶梯"）重试阶梯（attempts）重置：它在重连后第一个错误（第 8 次调用）之后立即结束，没有推进任何定时器。已用探针验证：跨连接转换保留退避作用域 key + attempts 的变异会把重试安排在 min(3000 * 2^7, 60000) = 60s 而不是重新从 3s 开始——探针翻转（正确代码 +3s 时第 9 次调用，变异下要等到 +60s），而全部 32 个测试保持绿色。— 失败场景：一个"只清错误预算"的重构可以静默合入，使每次重连后 pending 的后台 agent 卡片每次重试最多慢 57 秒。
建议修复：在第 8 次调用后用精确边界固定新阶梯（见英文部分代码）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| // B's own consecutive-error count is one, so it keeps polling on a | ||
| // fresh scope and still reconciles terminal. |
There was a problem hiding this comment.
[Suggestion] The fresh-scope reset of the sibling's per-callId errorAttempts when the pending set shrinks is asserted in this comment but exercised by no assertion: the post-shrink phase swaps the mock so agent-b immediately resolves completed and never errors again. Probe-verified: a mutant seeding the new scope's errorAttempts from the previous one (the "carry counts across the shrink" refactor) fails agent-b after 7 fresh-scope errors instead of 8 — the probe flips while all 32 tests stay green. — Failure scenario: that refactor ships green, and a sibling that blipped once before the shrink enters the new scope with count 1 and is marked failed one round early — the exact regression class (leaked error budget failing a healthy sibling) these tests were written to pin.
Suggested fix — extend the post-shrink phase before the completed swap: keep agent-b rejecting for seven post-shrink rounds and assert it stays pending (surviving 7 fresh errors despite the pre-shrink blip), failing on the eighth.
中文说明
当 pending 集合收缩时，sibling 的 per-callId errorAttempts 在新作用域下重置——这一行为由本注释断言，但没有任何断言真正执行它：收缩后的阶段直接换 mock 使 agent-b 立即解析为 completed，之后不再出错。已用探针验证：用上一个作用域的 errorAttempts 初始化新作用域的变异（"跨收缩保留计数"的重构）会让 agent-b 在新作用域第 7 次错误（而非 8 次）后就失败——探针翻转，而全部 32 个测试保持绿色。— 失败场景：该重构可以静默合入，一个在收缩前闪断过一次的 sibling 会带着计数 1 进入新作用域，并提前一轮被标记 failed——正是这些测试要固定的回归类型（泄漏的错误预算使健康 sibling 失败）。
建议修复：在换成 completed 之前扩展收缩后阶段——让 agent-b 在收缩后连续拒绝七轮并断言其保持 pending（尽管收缩前闪断过一次仍要扛住 7 次新错误），第 8 次才失败。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| // The live round keeps polling and reconciles on the next attempt. | ||
| await act(async () => vi.advanceTimersByTimeAsync(3_000)); |
There was a problem hiding this comment.
[Suggestion] This test pins only that a stale round's 404 does not consume grace; nothing in the suite pins the dual direction — that the live round's miss count survives a foreign notification between two misses. missingAgentMissesRef is reset only on success, pending-set pruning, or connection transition, so today a live miss correctly persists; but probe-verified: a plausible mutant clearing the miss map when backgroundAgentNotificationKey changes ships all 32 tests green (here the map is empty at supersede time and the live round misses only once before success; no other test interposes a notification between two 404 misses). — Failure scenario: that mutant shipped, every completion notification from another agent would reset a lost agent's grace window, deferring its terminal failed mark by a further query cycle per interfering notification — precisely the mark this PR relies on to stop hiding footer actions.
Suggested fix — after the first live miss asserts pending: append a second foreign notification block, re-render, let the next round 404 (the map now holds miss 1), and assert container.textContent becomes 'failed'.
中文说明
该测试只固定了"陈旧轮次的 404 不消耗 grace"；整个套件没有固定其对偶方向——活跃轮次的 miss 计数在两次 miss 之间经历一次外来通知后仍然保留。missingAgentMissesRef 只在成功、pending 集合修剪或连接转换时重置，因此当前实现中活跃 miss 确实会保留；但已用探针验证：一个在外来通知（backgroundAgentNotificationKey 变化）时清空 miss map 的合理变异能通过全部 32 个测试（本测试在取代发生时 map 为空，且活跃轮次在成功前只 miss 一次；其他测试也没有在两次 404 miss 之间插入通知）。— 失败场景：该变异合入后，其他 agent 的每一条完成通知都会重置丢失 agent 的 grace 窗口，每条干扰通知都把其终态 failed 标记再推迟一个查询周期——而这个标记正是本 PR 用来停止隐藏 footer 操作的依据。
建议修复：在第一次活跃 miss 断言 pending 之后——追加第二条外来通知块、重新渲染、让下一轮再返回 404（此时 map 已有 miss 1），并断言 container.textContent 变为 'failed'。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| hookState.resolveSubagentSession.mockRejectedValue( | ||
| new DaemonHttpError(400, { code: 'invalid_tool_call_id' }, 'bad request'), | ||
| ); |
There was a problem hiding this comment.
[Suggestion] The permanent-client-error class (status >= 400 && status < 500 && !== 404 && !== 429) is exercised with a single status code (400), so the class condition itself is unpinned. Probe-verified: narrowing it to error.status === 400 ships the whole suite green; a probe in the suggested-fix shape fails under the mutant for 401/403/422 (routed into the retry machinery) and passes on the PR code. The SDK documents 401 bad token as a live daemon semantic. — Failure scenario: with a stale/revoked daemon token, every completion query 401s but rides the transient path — the card stays pending through the full 8-round ladder (~213s of doomed queries) and ends in a misleading retry budget exhausted warn naming HTTP 401, instead of going terminal on the first query as the code comment intends.
Suggested fix — parameterize the class:
it.each([400, 401, 403, 422])(
'treats permanent client error %i as terminal without retrying',
async (status) => { /* same shape: immediate 'failed', exactly one call */ },
);中文说明
永久客户端错误类（status >= 400 && status < 500 && !== 404 && !== 429）只用了一个状态码（400）来测试，因此类的条件本身没有被固定。已用探针验证：把它收窄为 error.status === 400 后整个套件仍然通过；按建议修复构造的探针在该变异下对 401/403/422 失败（被路由进重试机制），在 PR 代码下通过。SDK 文档把 401 bad token 列为 daemon 的活跃语义。— 失败场景：daemon token 过期/被吊销时，每次完成查询都返回 401 但走瞬时路径——卡片保持 pending 走完整个 8 轮阶梯（约 213 秒注定失败的查询），最后以一条点名 HTTP 401 的误导性 retry budget exhausted 警告结束，而不是按代码注释的意图在第一次查询就终态化。
建议修复：参数化该类（见英文部分代码）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| it('does not consume grace misses for a superseded round', async () => { | ||
| vi.useFakeTimers(); |
There was a problem hiding this comment.
[Suggestion] This test (like ~15 sibling fake-timer tests added by this PR) restores real timers and unmounts only on the success path; the file has no afterEach, so any mid-test assertion failure leaks installed fake timers plus the hook's live 3s retry timer into subsequent tests. The author's own 'keeps the full retry budget when the client identity changes between rounds' already wraps its body in try/finally for exactly this cleanup. Probe-observed cascade: a failing mid-body test leaked fake timers into a later exact-call-count test — expected "spy" to be called 8 times, but got 14 times — converting one clear failure into a cascading failure blaming the wrong test. — Failure scenario: if e.g. the toBe('pending') assertion below fails during a future refactor, unmount() and vi.useRealTimers() never run; the leaked retry timer fires inside the next real-timer test's vi.waitFor (which auto-advances detected fake timers), driving call counts past exact assertions.
Suggested fix — file-level afterEach(() => { vi.useRealTimers(); }) (plus a shared unmount registry), or try/finally bodies matching the existing precedent.
中文说明
该测试（与本 PR 新增的约 15 个同类假定时器测试一样）只在成功路径上恢复真实定时器并卸载；文件没有 afterEach，因此任何测试中途断言失败都会把已安装的假定时器和 hook 挂着的 3s 重试定时器泄漏进后续测试。作者自己的 'keeps the full retry budget when the client identity changes between rounds' 已经用 try/finally 做了同样的清理。探针观察到的级联：一个中途失败的测试把假定时器泄漏进后面一个精确调用计数测试——expected "spy" to be called 8 times, but got 14 times——把一个清晰的失败变成归咎于错误测试的级联失败。— 失败场景：如果未来重构中例如下方的 toBe('pending') 断言失败，unmount() 和 vi.useRealTimers() 永远不会执行；泄漏的重试定时器会在下一个真实定时器测试的 vi.waitFor（会自动推进检测到的假定时器）中触发，使调用计数越过精确断言。
建议修复：文件级 afterEach(() => { vi.useRealTimers(); })（外加共享卸载注册表），或按现有先例使用 try/finally 包裹测试体。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| // Permanent client errors never recover on retry; make the | ||
| // card terminal so it can stop gating the UI. A 429 is the | ||
| // daemon's rate-limit signal and unrecognized 404 shapes | ||
| // stay transient, so neither may fail the agent. | ||
| return [callId, { status: 'failed' }] as const; |
There was a problem hiding this comment.
[Suggestion] The permanent-4xx synthesized-failure path terminally fails an agent card with zero console diagnostics — a third silent terminal path beside the budget warn and the grace path (which logs via the retry-scheduled warn). Probe-verified: DaemonHttpError(400, {code:'invalid_tool_call_id'}) flips the card to failed after exactly 1 call with zero console.warn calls — the closure returns a fulfilled [callId, {status:'failed'}], the callId never enters roundErrors, and with a single pending agent unresolved is false, so the handler returns before any log. The route emits this shape in practice (400 invalid_tool_call_id); with multiple pending agents the retry warn lists only roundErrors entries, so the 4xx-failed callId is still unmentioned. — Failure scenario: an operator sees a failed background-agent card with no log line distinguishing a daemon-rejected resolution query from a daemon-reported failure or grace exhaustion.
Suggested fix — emit one structured console.warn when synthesizing a failure from a permanent 4xx, mirroring the budget-exhaustion warn (sessionId, callId, describeReconciliationError(error)).
中文说明
永久 4xx 合成失败路径会把 agent 卡片终态化为失败，但没有任何控制台诊断——这是继预算警告、grace 路径（经由 retry-scheduled 警告输出日志）之后的第三条静默终态路径。已用探针验证：DaemonHttpError(400, {code:'invalid_tool_call_id'}) 在恰好 1 次调用后把卡片翻转为 failed，且 console.warn 一次都没有——闭包返回的是已履行的 [callId, {status:'failed'}]，该 callId 从不进入 roundErrors，且单个 pending agent 时 unresolved 为 false，处理器在任何日志之前就返回了。该路由在实际中会发出这种形状（400 invalid_tool_call_id）；多个 pending agent 时 retry 警告只列出 roundErrors 条目，4xx 失败的 callId 依然不会被提及。— 失败场景：运维看到一张 failed 的后台 agent 卡片，却没有任何日志可以区分"daemon 拒绝了解析查询"与"daemon 上报的失败"或 grace 耗尽。
建议修复：在永久 4xx 合成失败时发出一条结构化 console.warn，对齐预算耗尽警告（sessionId、callId、describeReconciliationError(error)）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No action taken (PR #8787)This round has no actionable feedback, so no code changes were made and no commit was created.
The branch head remains at 中文说明未采取任何操作（PR #8787）本轮没有可执行的反馈，因此未做任何代码改动，也未创建提交。
分支头仍为 Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
Independent verification report (local real stack)Verdict: the fix works as described. Base reproduces both flicker cycles of the bug; the PR head keeps every intermediate message footer-free and shows the footer exactly once, under the final summary. All new tests pass on head and genuinely fail on base. Recommended for merge. Environment
Real-stack A/B result
Head had zero footered samples on any intermediate message across the whole run (243 samples); base spent ~21 s of the 27 s pre-summary window incorrectly showing final-answer controls. Results were reproduced across two independent runs per arm.
Full-size single screenshots: base phase 1 (bug) · phase 2 (bug) · phase 3 — head phase 1 · phase 2 · phase 3 Tests
Scope notes
Evidence assets are on the 中文版本（Chinese version）独立验证报告（本地真实环境）结论：修复符合描述。base 完整复现了两轮 footer 闪烁 bug；PR head 全程不给中间消息 footer，最终总结完成后 footer 恰好出现一次。新增测试在 head 全绿、在 base 上确实失败。建议合并。 环境
真实栈 A/B 结果
head 全程 243 个采样中，中间消息带 footer 的采样为 0；base 在总结前 27 秒窗口里约 21 秒错误地显示最终回答控件。两臂各独立跑了两轮，结果一致。 测试
范围说明
证据图在 |
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 4151 passed · 2 failed · 4153 total 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：4151 通过 · 2 失败 · 4153 总计 Verification reportPR #8787 deep verification — fix(web-shell): defer assistant footer during background agent workVerdict: 中文摘要
Central claim + A/BCentral claim: while the latest turn owns active background agents or awaits their summary, the assistant footer actions (copy, branch, custom footer, timestamp) stay hidden; they appear once, under the completed final answer; completed historical turns keep their actions. A/B cells (identical scenario = the PR's focused suites, 3 files / 281 tests; control = merge-commit tree with only
The 35 flips are exactly the claimed behavior: 13 DOM tests where Mutation matrix (one point-mutant per introduced guard, focused suites; unmutated head control 281/281 green — positive control):
5/5 killed, 0 survivors — every guard the PR introduces decides at least one test outcome, and each kill set matches the guard's stated responsibility. Witness: Reviewer Test Plan, per step: step 1 (live Web Shell conversation with two background subagents) not executable here — no model credentials/browser in this container (see Not covered). Steps 2–4 (no controls while agents active; controls stay hidden between completion and summary; controls appear once under the final summary; historical turns keep actions) are each pinned by DOM tests that are green at head and red at base, so the plan's observable criteria are verified at the component level, not in a live session. FindingsF1 (medium) — SDK 404 guards' identifying field does not match the daemon wire; identifying branch is dead against the real daemonThe daemon's
The new guards in
Blast radius (bounded): the only production consumer is Reproducing command: Suggested fix (measured in a scratch copy, not applied)In both guards, accept the daemon's real identifying field: Not covered
MethodologyEnvironment: CI verify container ( Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round — no changes neededPR #8787 · head Feedback triage
DecisionNo code changes this round. The only new feedback is a positive independent verification recommending merge, so the PR stays as-is at head 中文说明Autofix 审阅轮次 — 无需任何改动PR #8787 · head 反馈分类
决定本轮不做任何代码改动。唯一的新反馈是一份建议合并的正面独立验证，因此 PR 保持在 head Deferred non-Critical feedbackCritical-only mode is active: 5 change-producing rounds are complete. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明已进入仅处理 Critical 的模式：已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No action this round. The actionable sections of the prepared feedback are all empty: no new reviews, no inline comments, no issue-level comments, no failed checks, and no still-red checks. The only feedback newer than the last evaluation sits in the deferred non-Critical section (critical-only mode engaged after 5 change-producing rounds), which is an audit record rather than work for this round, so no code changes, thread resolutions, or comment replies were made for it. There was also no base conflict to resolve. Nothing was changed, and there was nothing to verify or commit. 中文说明本轮无可执行操作。 工作流准备的反馈中所有可执行区域均为空：没有新的评审（reviews）、没有行内评论、没有 issue 级评论、没有失败的检查、也没有持续失败的检查。自上次评估以来唯一更新的反馈位于已延后的非 Critical 区域（在 5 个产生改动的轮次后已启用仅处理 Critical 的模式），该区域是审计记录而非本轮的工作项，因此未对其做任何代码改动、未解决任何讨论串、也未回复任何评论。同时也没有需要解决的 base 冲突。本轮未做任何改动，也没有需要验证或提交的内容。 Deferred non-Critical feedbackCritical-only mode is active: 5 change-producing rounds are complete. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明已进入仅处理 Critical 的模式：已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
|
Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 3901 passed · 2 failed · 3903 total 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：3901 通过 · 2 失败 · 3903 总计 Verification reportPR #8787 deep verification (follow-up round) — fix(web-shell): defer assistant footer during background agent workVerdict: 中文摘要
Previous-finding status table (follow-up round)
No declined or deferred rows existed in the prior round; nothing to re-measure beyond F1. Note: the prior round and this round verified the same head commit ( Central claim + A/BCentral claim: while the latest turn owns active background agents or awaits their summary, the assistant footer actions (copy, branch, custom footer, timestamp) stay hidden; they appear once, under the completed final answer; completed historical turns keep their actions. A/B cells (identical scenario = the PR's focused suites, 3 files / 281 tests; control = merge-commit tree with only
The 35 flips are exactly the claimed behavior: 13 DOM tests where Mutation matrix (one point-mutant per introduced guard, focused suites; unmutated positive control 281/281 green; each mutant verified as a single-line change and restored via
5/5 killed, 0 survivors — every guard the PR introduces decides at least one test outcome, matching the prior round's counts exactly. Witness: Vacuity: the base control is the revert-vacuity check for the web-shell tests — the 35 reds fail the intended behavioral assertion ( Reviewer Test Plan, per step: step 1 (live Web Shell conversation with two background subagents) not executable here — no model credentials/browser in this container (see Not covered). Steps 2–4 (no controls while agents active; controls stay hidden between completion and summary; controls appear once under the final summary; historical turns keep actions) are each pinned by DOM tests that are green at head and red at base, verified at the component level this round. FindingsF1 (medium, stands) — SDK 404 guards' identifying field does not match the daemon wire; identifying branch is dead against the real daemonRe-measured at the same head; the code is unchanged and the finding stands. The daemon's
Against that wire, the guards in
Blast radius (bounded, re-verified): repo-wide census shows exactly one production consumer — Reproducing command: Suggested fix (re-measured in a scratch build, not applied)In both guards, accept the daemon's real identifying field: Not covered
MethodologyEnvironment: CI verify container ( Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round summary: no action takenPR: #8787 · Branch: Why no changes were made this roundCritical-only mode is active: the PR has completed 5 change-producing review rounds, so the workflow's deterministic brake excluded all non-Critical feedback from this round's actionable sections. This round's actionable sections were all empty:
The only feedback present is one deferred non-Critical PR comment from the automated reviewer ( Check stateAll 54 checks on the PR are completed: 14 SUCCESS, 40 SKIPPED, 0 failing. Nothing requires a fix. OutcomeNo code was modified and no commit was created. The working tree remains clean at 中文说明Autofix 轮次总结：本轮未采取任何操作PR： #8787 · 分支： 本轮为何没有改动当前处于仅处理 Critical 的模式：该 PR 已完成 5 个产生改动的评审轮次，工作流的确定性刹车机制已将全部非 Critical 反馈排除在本轮可执行区域之外。 本轮所有可执行区域均为空：
本轮唯一存在的反馈是一条被延后的非 Critical PR 评论，来自自动评审机器人（ 检查状态PR 上全部 54 项检查均已完成：14 项 SUCCESS，40 项 SKIPPED，0 项失败。无需任何修复。 结果未修改任何代码，也未创建任何提交。工作区保持干净，停留在 Deferred non-Critical feedbackCritical-only mode is active: 5 change-producing rounds are complete. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明已进入仅处理 Critical 的模式：已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Deep verification pass — approving.
Verification performed. This fork PR has no real CI (133/134 check-runs skipped, only the trivial route check completed), so I checked out the head locally and ran: full web-shell suite (181 files / 3303 tests pass, incl. the 272 tests in the three changed files), full sdk-typescript suite (33 files / 1520 tests pass, incl. the new guard tests), plus typecheck / eslint / prettier on all changed files — all clean.
State machine (MessageList.tsx). Enumerated the signal space (agent started/completed incl. anonymous completions, turn_error, prompt_cancelled, isResponding, catch-up baseline, transcriptRenderMode):
- No early-show path found:
turnHasActiveBackgroundAgentsuppresses the footer of the turn that owns the agent and is applied to every turn, so an earlier turn with a live agent stays held even after the user moves on, while the newer turn's footer shows normally. Matched completions hold via the ordering rule; unmatched/lost completions hold only under the bounded 5s grace;turn_error/prompt_cancelledafter the last notification release the hold; grace is keyed on the sorted unmatched-callId set so monitor / earlier-turn notifications can neither restart nor re-arm it. - No hidden-footer regression in the common no-subagent path:
backgroundAgentSummaryStatereturns null without agent notifications, readonly transcripts bypass the gate entirely, and restore/reconnect are covered by the notification-baseline logic plus the connection-key reset inuseMessages. Timer cleanup and StrictMode round reuse (processedflag) are correct.
Reconciliation (useMessages.ts). Backoff 3s→60s keyed by session+pending-set (foreign notifications can't pin the delay at base); per-callId error budget (8 consecutive transient-error rounds ≈ 3.5 min) fails only the erroring agent and resets implicitly on any healthy round; 404 grace is 2 consecutive misses, re-armed by any success, cleared on connection transitions. No livelock: a flapping daemon converges to the 60s cap, and the pending set only changes on terminal resolution. Maps/refs are bounded by the pending-agent count, pruned each round, snapshot holds at most one session's resolutions — no leaks found.
SDK guards. Cross-checked against the daemon contract in packages/cli/src/serve/routes/session.ts (GET /session/:id/subagents/:toolCallId) and session-runtime.ts: both 404 shapes match exactly (agent-level with toolCallId, session-level for disabled subagent registry / inactive multi-workspace runtime); mismatched-toolCallId and non-session_not_found 404s stay transient; a daemon restart resets miss/budget state via the connection transition. No misclassification that breaks recovery.
Findings (none blocking):
- (P2, acceptable) A matched completion whose summary narration never arrives (e.g. the turn ends right after the notification) holds the latest turn's footer until new assistant/user content arrives. It self-heals on the user's next turn since
latestTurnAwaitsAgentSummarygates only the latest turn; the scenario is rare and fails in the conservative direction. - (P3) An agent the daemon keeps reporting as healthy
runningkeeps its owning turn's footer hidden indefinitely — faithful state reporting, consistent with the feature's intent; the transient-error path is bounded (~3.5 min to budget exhaustion). - (P3, nit) In the per-call catch, the not-found branch falls through into
roundErrors.push(...)+throw, so a grace miss also counts toward the error budget and shows up in the retry warning. Harmless (the 2-miss grace dominates any pure-404 sequence), but it reads like a missing early throw — a short comment or restructure would help future readers. - (P3, nit) 429 consumes the error budget like other transient errors and a body
retryAfterMsis ignored in favor of the ladder; after a budget-exhaustedfailedmark, a late real completion only reconciles on transcript reload.
Residual risk: upstream CI never ran on this fork PR — the local verification above stands in for it, but maintainers may want to re-run workflows before/at merge.
|
Released in v0.21.12. |
6 participants
What this PR does
This PR keeps assistant footer actions hidden while the latest turn still has background agents running or is waiting for the main agent to summarize their results. Copy, branch, custom footer content, and the message timestamp are shown only after the actual final response is available.
It also adds a regression test covering the full transition from active agents, through the post-completion summary wait, to the final summarized response.
Why it's needed
When the main agent temporarily became idle after launching background agents, the Web Shell treated its intermediate narration as the final answer. That caused copy, branch, and timestamp controls to appear under text such as "I’ll summarize when the results return," then disappear as soon as a subagent update resumed the turn. The transient footer incorrectly suggested that the turn had finished and produced visible UI churn.
The root cause was that final-answer classification considered only whether the main agent was currently responding. Background agent activity and the summary-wait state were not included in the latest-turn completion decision.
Reviewer Test Plan
How to verify
Start a Web Shell conversation that launches at least two background subagents and emits an intermediate assistant message before their results return. While any subagent is active, confirm that the intermediate message has no copy, branch, custom footer, or timestamp controls. After all subagents complete but before the main-agent summary arrives, confirm those controls remain hidden. When the final summary completes, confirm the controls appear beneath that final response and that completed historical turns retain their existing footer actions.
Evidence (Before & After)
Before: the intermediate waiting message briefly displayed its footer timestamp and actions, then lost them when a subagent update resumed the turn.
After: the intermediate message never receives final-answer controls; they appear once, beneath the completed final summary.
Tested on
Environment (optional)
Verified with the focused Web Shell message-list suites (217 tests), Web Shell TypeScript type checking, ESLint, Prettier, and diff checks.
Risk & Scope
usage_updaterendering issue are unchanged.Linked Issues
N/A
中文说明
本 PR 做了什么
当最新一轮仍有后台 subagent 在运行，或者正在等待主 agent 汇总其结果时，本 PR 会保持隐藏 assistant footer 操作。复制、创建分支、自定义 footer 内容和消息时间只会在真正的最终回答完成后显示。
同时新增回归测试，覆盖 subagent 运行中、全部完成后等待汇总、最终总结完成这三个连续阶段。
为什么需要
主 agent 启动后台 subagent 后会暂时进入空闲状态，Web Shell 此时会把类似“结果回来我汇总给你”的中间说明误判为最终回答。这会让复制、创建分支和时间控件短暂出现，并在 subagent 更新重新推进会话时消失，既错误暗示 turn 已结束，也造成明显的界面跳动。
根因是最终回答判定只考虑主 agent 当前是否正在响应，没有把后台 subagent 活动和等待汇总状态纳入最新 turn 的完成条件。
Reviewer 测试计划
如何验证
在 Web Shell 中发起一个至少启动两个后台 subagent、并在结果返回前输出中间说明的会话。任一 subagent 运行期间，确认中间消息下方没有复制、创建分支、自定义 footer 或时间控件。全部 subagent 完成但主 agent 汇总尚未到来时，确认这些控件仍保持隐藏。最终总结完成后，确认控件出现在最终回答下方，并且历史已完成 turn 的 footer 操作不受影响。
前后证据
修改前：等待结果的中间消息会短暂显示 footer 时间和操作，subagent 更新恢复 turn 后又消失。
修改后：中间消息始终不会获得最终回答控件；控件只在最终总结完成后出现一次。
测试平台
环境
已验证 Web Shell 消息列表定向测试（217 个测试）、Web Shell TypeScript 类型检查、ESLint、Prettier 和 diff 检查。
风险与范围
usage_update渲染问题均未修改。关联 Issue
无