fix(cli): deliver Agent View queued follow-ups from the provider - #10315
Conversation
Queue delivery lived in an AgentComposer effect, but DefaultAppLayout keys the composer by the active view. An agent that settled to idle (or a terminal status) while its tab was unfocused never flushed: queued follow-ups stayed accepted-but-undelivered until the user revisited the tab, or were shown as "queued" forever after a terminal status. Move delivery to the always-mounted AgentViewProvider: one AgentQueueFlusher child per registered agent joins and enqueues the queue when the agent settles to a non-terminal idle, and drops the queue when the agent becomes terminal. The composer now only displays the queue. Delivery stays exactly-once via the queue-identity dedupe, re-anchored from the composer mount scope to the flusher. Fixes #10148 Co-authored-by: Qwen-Coder <qwen-coder @alibabacloud.com>
github-actions
Bot
added
the
review/self-reported
|
Re-run at
One process note for later: the author is a committer (admin) and the branch is same-repo, so neither the fork-refactor approval guardrail nor the maintainer-only core gate is in play. Moving on to code review. 🔍 中文说明本次是在 round 9 头部
一条留给后文的流程说明：作者是 committer（admin），分支来自同仓库，因此 fork refactor 批准护栏与核心模块维护者门禁均不适用。 进入代码审查。🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewIndependent baseline, written before opening the diff: the flush has to move to something that is (a) always mounted and (b) owns both the queues and the agent registry — that is the provider, with one small child per agent reusing The contracts the gate rests on, re-verified at this head (the merge pulled 42 commits of main in since the last pass, so these are fresh reads, not carry-over):
So "drop at every terminal status" is the correct semantic rather than a conservative guess — three of those five facts are what make a delivered follow-up to a FAILED agent actively harmful. The one new failure mode I went looking for, and it is clean. The flusher subscribes at registration time, where the composer used to subscribe at focus time. If the event emitter did not exist yet at registration, the hook would subscribe to nothing and never re-subscribe (its deps are the agent identity, not the emitter) — which would have silently reproduced the exact bug this PR fixes. It cannot happen: Rest of the read:
sequenceDiagram
participant P1 as User
participant P2 as AgentComposer (keyed by activeView)
participant P3 as AgentViewProvider (always mounted)
participant P4 as AgentQueueFlusher (one per agent)
participant P5 as AgentInteractive in core
P1->>P2: submit follow-up while agent A is busy
P2->>P3: appendToAgentMessageQueue(A, text)
P1->>P3: switch to tab B, unmounting the composer for A
P5-->>P4: STATUS_CHANGE for A
alt A settles to IDLE with a non-empty queue
P4->>P3: setAgentMessageQueue(A, empty)
P4->>P5: enqueueMessage(joined text)
else A reaches COMPLETED, CANCELLED or FAILED
P4->>P3: setAgentMessageQueue(A, empty) and drop
end
Not verified, and I would rather say so than imply otherwise: no test anywhere exercises the real Two more standing items, both already reported and resolved as Suggestions in earlier rounds. This is round 9 — well past the ~5-round point in AGENTS.md — so they belong in a follow-up, not another round here: the flusher subscribes via the full hook, so a running agent's flusher holds its own 500 ms Testing — unattended CI run; PR code not executedFetched once at review time, no polling. Every check on this head has completed and none failed:
Orchestration jobs (assign, label, route, authorize, review-pr, delay-automatic-review, Remind on force-push) are green too; conditional lanes (macOS/Windows unit jobs, sandboxed integration, fork signalling) report skipped, which is normal for a same-repo branch. Two things worth calling out about that signal:
Sandboxed verification would settle the one gap I could not close by reading: Real-scenario (tmux) testing: not driven on this run. This is an unattended CI re-run ( 中文说明代码审查独立基线（读 diff 之前写下）：flush 必须移到一个既常驻挂载、又同时拥有队列与智能体注册表的地方——即 provider，每个智能体一个小组件，复用 门控所依赖的契约，本轮在该头部重新核实（自上一轮以来合并从 main 带进 42 个提交，因此以下是重新阅读的结果，而非沿用）：
因此"任何终态都丢弃队列"是正确的语义，而不是保守猜测——上述五条中有三条正是"向 FAILED 智能体投递 follow-up 有害"的原因。 我主动去找的那一个新失效模式，结论是干净的。 投递器在注册时订阅，而 composer 过去是在聚焦时订阅。若注册时事件发射器尚不存在，hook 会订阅到空并且永不重订阅（其依赖是智能体标识，而非发射器）——那将静默地重现本 PR 要修的那个 bug。这不会发生： 其余阅读结论：
未验证的部分，我宁可直说也不含糊：没有任何测试走通真实的 另有两条长期项，此前各轮已作为 Suggestion 报告并解决。本轮是 round 9——远超 AGENTS.md 的约 5 轮界限——因此它们应进入后续 issue/PR，而不是在这里再开一轮：投递器经由完整 hook 订阅，因此运行中智能体的投递器会各自持有一个 500 ms 的 测试 —— 无人值守 CI 运行；未执行 PR 代码审查时一次性获取，不轮询。该头部上的所有检查均已完成，且无失败项（表格见上）。 关于该信号有两点需要点明：
沙箱验证可以补足我仅靠阅读无法闭合的那一处缺口： 真实场景（tmux）测试：本次未执行。这是无人值守 CI 重跑（ — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 4/5 — the change is right, minimal, and I verified its core-side premises myself at this head; the one thing I could not settle by reading is that no test drives the real streaming-event chain, and that is a follow-up, not a blocker. Stepping back: this is the ninth round on a PR whose scope never grew. What started as "move the flush out of the keyed composer" is still exactly that — 136 production lines, two files, the rest tests. The interesting part of the history is that three Criticals were found in the terminal-status branch (a FAILED agent that has already been released by the backend, an arena record that discards My own baseline before reading the diff was the same design, so there is no simpler path I am quietly conceding. What I did go looking for was the failure the move could plausibly introduce — a flusher that subscribes before the agent's event emitter exists would never re-subscribe and would silently reproduce the original bug — and it does not exist here, because the emitter is built in If I had to maintain this in six months I would thank the author: one effect, terminal branch first, and a comment that names the exact core locations the reasoning depends on instead of asserting a conclusion. The honest reservation is the one in Stage 2 — the delivery suite mocks the streaming hook, so "a real Per AGENTS.md's round-balloon rule, the two standing Suggestions (full-hook subscription overhead per running agent; no trace left by a terminal drop) should go to a follow-up issue rather than another round here — both are already resolved threads on this PR. Verdict: approve, pinned to 中文说明置信度：4/5 —— 改动正确、范围最小，且我在该头部亲自核实了它依赖的 core 侧前提；唯一无法靠阅读确定的是没有测试驱动真实的 streaming 事件链，那属于后续项，不是阻断项。 退一步看：这是一个范围从未膨胀的 PR 的第九轮。最初那句"把 flush 移出按 key 挂载的 composer"，到现在依然就是这件事——136 行生产代码、两个文件，其余都是测试。这段历史里值得注意的是：终态分支上先后发现三个 Critical（已被 backend 释放的 FAILED 智能体、丢弃 我在读 diff 之前的基线方案与之相同，因此不存在我正悄悄让步的更简路径。我主动去找的是这次迁移可能引入的失效：一个在智能体事件发射器存在之前就订阅的投递器将永不重订阅，从而静默重现原始 bug——这里不存在该问题，因为发射器在 若六个月后由我维护，我会感谢作者：一个 effect、终态分支在前，注释精确点名推理所依赖的 core 位置，而不是直接断言结论。诚实的保留意见就是 Stage 2 中那一条——投递套件 mock 了 streaming hook，因此"真实的 按 AGENTS.md 的轮次控制规则，两条长期 Suggestion（每个运行中智能体的完整 hook 订阅开销；终态丢弃不留痕迹）应转入后续 issue，而不是在此再开一轮——两者在本 PR 上都已是 resolved 线程。 结论：批准，钉在 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
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 explored to full depth (tool budget reached): "agent 3b": none — no check was cut short..
Test Plan (not a blocker): src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx — no such file or directory; src/ui/contexts/AgentViewContext.test.tsx — no such file or directory; src/ui/components/QueuedMessageDisplay.test.tsx — no such file or directory; src/ui/components/Composer.test.tsx — no such file or directory; src/ui/components/InputPrompt.test.tsx — no such file or directory.
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："agent 3b"：none — no check was cut short.。
Test Plan（非阻断）：src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx — no such file or directory; src/ui/contexts/AgentViewContext.test.tsx — no such file or directory; src/ui/components/QueuedMessageDisplay.test.tsx — no such file or directory; src/ui/components/Composer.test.tsx — no such file or directory; src/ui/components/InputPrompt.test.tsx — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.2)
Pick up the classify-release-notes helper-test fix (#10402) so CI runs green. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed.
4 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-1 #10148 delivery/drop tests live in the composer test file — still stands, already reported (comment 3874185235)
- R1-2 no test exercises the real useAgentStreamingState event chain — still stands, already reported (comment 3874185244)
- R1-3 flushers subscribe via the full streaming-state hook — still stands, already reported (comment 3874185251)
- R1-4 terminal queue drop leaves zero trace — still stands, already reported (comment 3874185257)
Test Plan (not a blocker): src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx — no such file or directory; src/ui/contexts/AgentViewContext.test.tsx — no such file or directory; src/ui/components/QueuedMessageDisplay.test.tsx — no such file or directory; src/ui/components/Composer.test.tsx — no such file or directory; src/ui/components/InputPrompt.test.tsx — no such file or directory; and 1 more.
中文说明
已审查。
本轮确认的 4 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
Test Plan（非阻断）：src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx — no such file or directory; src/ui/contexts/AgentViewContext.test.tsx — no such file or directory; src/ui/components/QueuedMessageDisplay.test.tsx — no such file or directory; src/ui/components/Composer.test.tsx — no such file or directory; src/ui/components/InputPrompt.test.tsx — no such file or directory; and 1 more。
— qwen3.8-max via Qwen Code /review (v0.22.2)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- Flushers subscribing via the full useAgentStreamingState hook (500ms timer + usage-tracking overhead) — already reported as R1-3 (comment 3874185251), author declined
- Terminal queue drop leaving no trace (no log/notice for discarded queued follow-ups) — already reported as R1-4 (comment 3874185257), author declined
Test Plan (not a blocker): src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx — no such file or directory; src/ui/contexts/AgentViewContext.test.tsx — no such file or directory; src/ui/components/QueuedMessageDisplay.test.tsx — no such file or directory; src/ui/components/Composer.test.tsx — no such file or directory; src/ui/components/InputPrompt.test.tsx — no such file or directory.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/contexts/AgentViewContext.tsx:456 — [review] no test queues follow-ups for more than one agent — per-agent flusher mount only delivery-tested for the first-registered agent (code-age: anchored on code unchanged since the…
中文说明
已审查。
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
Test Plan（非阻断）：src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx — no such file or directory; src/ui/contexts/AgentViewContext.test.tsx — no such file or directory; src/ui/components/QueuedMessageDisplay.test.tsx — no such file or directory; src/ui/components/Composer.test.tsx — no such file or directory; src/ui/components/InputPrompt.test.tsx — no such file or directory。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-2 no test exercises the real useAgentStreamingState event chain — still stands, already reported (comment 3874185244), author declined
- R1-3 flushers subscribe via the full streaming-state hook (500ms timer + usage-tracking overhead) — still stands, already reported (comment 3874185251), author declined
Not explored to full depth (tool budget reached): "agent 6a": running AgentComposer.queuedMessages.test.tsx and AgentViewContext.test.tsx — the review worktree's node_modules is incomplete ( @opentelemetry/* packages ….
Not reviewed: reverse audit — stopped before round 4 by the review time budget.
Test Plan (not a blocker): src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx — no such file or directory; src/ui/contexts/AgentViewContext.test.tsx — no such file or directory; src/ui/components/QueuedMessageDisplay.test.tsx — no such file or directory; src/ui/components/Composer.test.tsx — no such file or directory; src/ui/components/InputPrompt.test.tsx — no such file or directory.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未探索到全部深度（达到工具调用预算）："agent 6a"：running AgentComposer.queuedMessages.test.tsx and AgentViewContext.test.tsx — the review worktree's node_modules is incomplete ( @opentelemetry/* packages …。
未审查：反向审计——评审时间预算不足，未能开始第 4 轮。
Test Plan（非阻断）：src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx — no such file or directory; src/ui/contexts/AgentViewContext.test.tsx — no such file or directory; src/ui/components/QueuedMessageDisplay.test.tsx — no such file or directory; src/ui/components/Composer.test.tsx — no such file or directory; src/ui/components/InputPrompt.test.tsx — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
3 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-2 no test exercises the real useAgentStreamingState event chain — already reported (comment 3874185244), author declined
- R1-3 flushers subscribe via the full streaming-state hook — already reported (comment 3874185251), author declined
- R1-4 terminal queue drop leaves zero trace — already reported (comment 3874185257), author declined
Not explored to full depth (tool budget reached): "agent 6c": executing AgentViewContext.test.tsx and AgentComposer.queuedMessages.test.tsx — vitest's globalSetup stopped the run because workspace dist/ builds and p….
Test Plan (not a blocker): src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx — no such file or directory; src/ui/contexts/AgentViewContext.test.tsx — no such file or directory; src/ui/components/QueuedMessageDisplay.test.tsx — no such file or directory; src/ui/components/Composer.test.tsx — no such file or directory; src/ui/components/InputPrompt.test.tsx — no such file or directory.
中文说明
本轮确认的 3 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未探索到全部深度（达到工具调用预算）："agent 6c"：executing AgentViewContext.test.tsx and AgentComposer.queuedMessages.test.tsx — vitest's globalSetup stopped the run because workspace dist/ builds and p…。
Test Plan（非阻断）：src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx — no such file or directory; src/ui/contexts/AgentViewContext.test.tsx — no such file or directory; src/ui/components/QueuedMessageDisplay.test.tsx — no such file or directory; src/ui/components/Composer.test.tsx — no such file or directory; src/ui/components/InputPrompt.test.tsx — no such file or directory。
— qwen3.8-max-2026-09-02 via Qwen Code /review (v0.22.3)
…MPLETED/CANCELLED The Agent View queue flusher dropped pending follow-ups at any terminal status, but FAILED is not terminal for delivery: core's enqueueMessage has no terminal guard and restarts the run loop (agent-interactive.ts, "should survive round errors and recover"), so a failed teammate still processes the queued text. Dropping it silently lost the only copy. Narrow the drop branch to COMPLETED/CANCELLED, where the master abort is tripped or the agent is shut down and delivery is genuinely impossible, and extend the regression test to cover all three terminal statuses. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtk2jph6cv
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (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: 434 passed · 0 failed · 434 total Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：434 通过 · 0 失败 · 434 总计 抖动门：✅ 2 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #10315 Deep Verification —
|
| Cell | Source under test | Oracle | Result |
|---|---|---|---|
| head | merge 47b13562 (= base + PR) |
2 test files, expect all green | 16/16 green |
| base | HEAD^1 c3bf0fea + same test files |
expect exactly the #10148 tests red | 4 red / 12 green |
The four red cells on base are exactly the new-behavior tests, with the PR's claimed
failure messages verbatim:
delivers queued follow-ups while the user is on another tab (#10148)—
expected "spy" to be called 1 times, but got 0 timeshandles the queue when the agent reaches 'completed'and'cancelled'—
frame still containsqueued at terminal(the permanent-queued display the PR removes)handles the queue when the agent reaches 'failed'—
expected "spy" to be called 1 times, but got 0 times(base's!isTerminalStatus
guard blocked FAILED delivery too)
The 12 base-green cells are the pre-existing exactly-once / join / same-batch /
idle-submit / storage tests — base behavior for the focused flows was correct; only the
unfocused-delivery and terminal-drop behaviors flip. Cells as printed:
01-ab-head-two-test-files-green.png, 02-ab-base-four-tests-red.png.
Note: the PR body speaks of "the two #10148 tests"; the shipped file expresses them as
4 cells (one it + a 3-row it.each) after the final FAILED-narrowing commit. All 4
were red before, green after — the claim holds, it is just finer-grained than the prose.
Mutation matrix (scratch worktree at head state; same two test files per mutant)
| # | Mutation | Predicted | Observed | Verdict |
|---|---|---|---|---|
| control | none | 16 green | 16 green | harness live |
| M1 | flusher mounting removed from provider | delivery tests red | 7 red / 9 green | killed — flusher load-bearing |
| M2 | FAILED added back to the drop branch |
only the FAILED row red | 1 red / 15 green (exactly that row) | killed — final commit pinned |
| M3 | flushedQueueRef dedupe check+assignment removed |
StrictMode might catch it | 16 green — survived | adjudicated by probe P4 below: redundant defence |
| M4 | composer flush effect restored (reverse of the composer hunk), flusher kept | double delivery when focused | 3 red / 13 green, all called 2 times |
killed — composer removal is the other half of exactly-once |
M1's 7 reds = the 4 #10148 cells + the 3 focused-delivery cells (with the composer no
longer flushing, only the flusher delivers anywhere). M4 proves the two hunks are a set:
either one alone double-delivers or under-delivers. Matrix as printed:
03-mutant-failed-drop-one-test-red.png.
Boundary probes (real hook, real emitter, no mocks of the unit under test)
The shipped tests mock useAgentStreamingState; a separate probe harness
(queue-flusher-boundary.probe.test.tsx in this artifact dir) drives the real hook
with a real AgentEventEmitter:
| Probe | Fact under test | Head result |
|---|---|---|
| P1 | FAILED status ⇒ hook derives Idle ⇒ flusher delivers to an already-FAILED agent |
✅ 1 call |
| P2 | STATUS_CHANGE RUNNING→FAILED re-renders the flusher and flushes with no tab visit |
✅ 1 call |
| P3 | FAILED + pending approvals ⇒ WaitingForConfirmation ⇒ delivery deferred; after approvals clear the held queue is delivered, not lost |
✅ deferred then 1 call |
| P4 | StrictMode mount with a pre-seeded queue delivers exactly once (dedupe under double effect invocation) | ✅ 1 call |
| P5 | unregister clears the queue; re-registration under the same id never delivers the stale queue, and a fresh queue delivers once | ✅ 0 stale / 1 fresh |
Probes as printed: 04-boundary-probes-real-hook.png. All 5 also run on base-irrelevant
head state only; they describe the shipped code's boundary, not a base comparison.
Core semantics behind the COMPLETED/CANCELLED/FAILED split (code census)
The final commit's premise was verified against packages/core source, not taken on faith:
enqueueMessage(agent-interactive.ts:321) has no terminal guard: it enqueues and
restarts the run loop when not processing.runOneRoundsets RUNNING again, so a
FAILED agent genuinely processes further messages (the FAILED→recovery test at
agent-interactive.test.ts:408-425 covers the same mechanism).CANCELLED⇒abort()callsqueue.drain();AsyncMessageQueue.enqueueis silently
dropped after drain — delivery is impossible at the core level, so the UI dropping the
queue matches reality (and avoids the message being dequeued-and-discarded by a dead
loop).COMPLETEDis reachable in this tree only viashutdown()(which also drains): the
census forcompleteOnIdlefinds exactly two assignment sites — TeamManager sets
falsefor teammates, InProcessBackend passes the config through — and no caller in
the tree setstrue. AcompleteOnIdleagent would reach COMPLETED with a live
chat and could technically accept enqueues; no such agent exists behind the agent view
today.- TeamManager's own
flushNextMessagegates onstatus === IDLE, so held peer/leader
messages behave differently from UI follow-ups at FAILED (they wait). The two policies
coexist benignly: a successfully processed UI follow-up settles the agent back to IDLE,
which then unblocks the team-layer flush.
Reviewer Test Plan, walked step by step
- Run the two named test files — done on head (16/16 green) and on base (4 red with the
exact claimed messages). ✅ - Broader regression surface (
agent-view/,AgentViewContext.test.tsx,
QueuedMessageDisplay.test.tsx,layouts/,Composer.test.tsx,InputPrompt.test.tsx)
— 11 files, 327/327 green at head (PR quoted 325; the merged head picked up 2 tests
from main since, all green). ✅ tsc --noEmitfor the changed workspace — clean at head; gate proven live by planting
const x: number = "not a number"in a scratch copy (TS2322 reported, then clean again). ✅
Findings
F1 — Suggestion (completeness): the StrictMode dedupe guard is not pinned by any shipped test.
Deleting flushedQueueRef's check and assignment (mutant M3) leaves all 16 shipped tests
green. Classification — not dead code, redundant defence: probe P4 constructs the
shape the guard exists for (flusher mounts with a non-empty queue under StrictMode's
double effect invocation) and shows the mutant double-delivers there (called 2 times on
P1 and P4) while head delivers exactly once. The guard is unreachable through today's
registration flow only because queues are always empty at flusher mount time; the PR's
own comment names the hazard correctly. If the maintainers want it pinned, probe P4 in
this artifact dir is a drop-in fixture (its construct: registerAgent +
appendToAgentMessageQueue in one batched effect, agent IDLE, assert
toHaveBeenCalledTimes(1)). No code change requested.
F2 — Nit (pre-existing core behavior, surfaced by this PR): delivery to a FAILED agent
whose chat session was never created consumes the text without processing it.
startInner sets FAILED when createChat returns falsy; for such an agent
enqueueMessage restarts the loop, runOneRound early-returns on !this.chat, the
message lands in visible history but never reaches the model, and the agent settles to
IDLE (agent-interactive.ts runLoopInner/settleRoundStatus). Base behavior queued the
message forever instead; neither is correct, but the defect is core-side and rare
(chat-creation failure). Flagging for the core owner, not blocking this PR.
F3 — Note: FAILED with pending approvals defers delivery. While approvals are pending
the hook derives WaitingForConfirmation, so the flusher waits; probe P3 shows the queue
survives and delivers once approvals clear. Identical gating to pre-PR (same hook), and
cancelCurrentRound clears approvals on the user's behalf — no action.
Not covered
- Per-commit attribution. The checkout is depth 2: only the merge commit, base tip,
and PR head exist locally, while the metadata snapshot lists 7 commits (1 feature
commit + main-sync merges).git rev-list HEAD^1..HEAD^2returns 1 at the shallow
boundary, so the aggregateHEAD^1..HEADdiff is what was verified. The final
commit's behavior (FAILED narrowing) was nonetheless exercised in isolation via
mutant M2 and probes P1/P2. - Real two-teammate TUI session. The PR explicitly scopes this out (cannot be
constructed stably; Agent Team: a queued Agent View message disappears after switching teammate tabs #10069 precedent). Component-level assertions reproduce the
issue's observable sequence, not a live teammate round-trip; theenqueueMessage
seam is asserted against a spy, with core-side semantics taken from source census
above rather than an end-to-end run. - Repo-wide test suite / other workspaces — not run; affected workspace only.
- macOS/Windows — Linux container only.
- Tab-bar pending-queue indication — out of scope in Agent View: deliver queued follow-ups from the provider, not the keyed composer #10148 per the PR; not examined.
Methodology
Environment: the CI verify container (node:22-bookworm), npm ci + npm run build
pre-completed at the merge commit; PR metadata from the read-only snapshot. A/B and
mutants ran in scratch git worktrees under tmp/ (removed after capture), wired to
the root node_modules; that is a clean control because the diff touches no
package.json/package-lock.json and nothing outside packages/cli — sibling
workspaces' built dist/ outputs and nested node_modules were symlinked from the head
build and are byte-equivalent for the base/mutant trees. Tests drove the real
AgentViewProvider through ink-testing-library under StrictMode (matching
production's startInteractiveUI.tsx); the probe harness additionally used the real
useAgentStreamingState hook and core's AgentEventEmitter with no mocks of the unit
under test. Raw per-cell logs, the two probe test files, and four terminal captures live
in this directory (logs/, evidence/, *.probe.test.tsx).
Flakiness gate log
rounds=5 files=2 skipped=0
file packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx: (cd packages/cli) npx --no-install vitest run ./src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx
file packages/cli/src/ui/contexts/AgentViewContext.test.tsx: (cd packages/cli) npx --no-install vitest run ./src/ui/contexts/AgentViewContext.test.tsx
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx: PPPPP
packages/cli/src/ui/contexts/AgentViewContext.test.tsx: PPPPP
verdict: pass
summary: 2 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx: P (exit 0)
round 1 · packages/cli/src/ui/contexts/AgentViewContext.test.tsx: P (exit 0)
round 2 · packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx: P (exit 0)
round 2 · packages/cli/src/ui/contexts/AgentViewContext.test.tsx: P (exit 0)
round 3 · packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx: P (exit 0)
round 3 · packages/cli/src/ui/contexts/AgentViewContext.test.tsx: P (exit 0)
round 4 · packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx: P (exit 0)
round 4 · packages/cli/src/ui/contexts/AgentViewContext.test.tsx: P (exit 0)
round 5 · packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx: P (exit 0)
round 5 · packages/cli/src/ui/contexts/AgentViewContext.test.tsx: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
4 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-3 flusher subscribes via the full streaming-state hook (500ms timer + usage-tracking overhead) — already reported (comment 3874185251), author declined
- R1-2 no test exercises the real useAgentStreamingState event chain — already reported (comment 3874185244), author declined
- R1-4 terminal queue drop leaves zero trace — already reported (comment 3874185257), author declined
- per-agent flusher isolation test (no test queues follow-ups for more than one agent) — already recorded as deferred in round 3 (review 5061425066)
Not explored to full depth (tool budget reached): "agent 3c": none — no check was cut short..
Test Plan (not a blocker): src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx — no such file or directory; src/ui/contexts/AgentViewContext.test.tsx — no such file or directory; src/ui/components/QueuedMessageDisplay.test.tsx — no such file or directory; src/ui/components/Composer.test.tsx — no such file or directory; src/ui/components/InputPrompt.test.tsx — no such file or directory; and 1 more.
Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/contexts/AgentViewContext.tsx:172 — [probe] D6-1 COMPLETED drop rests on an unenforced premise (completeOnIdle)
Convergence: round 6 posted 2 inline comment(s), 2 of them reported for the first time; the previous round posted 2 (2 new). Findings keep coming back to the same files: packages/cli/src/ui/contexts/AgentViewContext.tsx (findings in round 5; 2 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
本轮确认的 4 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未探索到全部深度（达到工具调用预算）："agent 3c"：none — no check was cut short.。
Test Plan（非阻断）：src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx — no such file or directory; src/ui/contexts/AgentViewContext.test.tsx — no such file or directory; src/ui/components/QueuedMessageDisplay.test.tsx — no such file or directory; src/ui/components/Composer.test.tsx — no such file or directory; src/ui/components/InputPrompt.test.tsx — no such file or directory; and 1 more。
收敛姿态下延后（第 6 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 6 轮发布了 2 条行内评论，其中 2 条是首次提出；上一轮发布了 2 条（其中 2 条首次提出）。发现反复回到同一批文件：packages/cli/src/ui/contexts/AgentViewContext.tsx（第 5 轮已出过发现，本轮又有 2 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.3)
…ship Two FAILED flavors cannot accept queued follow-ups, and delivering to them certifies falsely: - Fatal failure (core sets error, not lastRoundError): the chat was never created or the run loop threw, so enqueueMessage restarts a loop whose runOneRound early-returns on !this.chat — silently consuming the message while settleRoundStatus flips FAILED to IDLE, erasing the failure state (R5-1). - Team-managed teammate: TeamManager tears the agent down synchronously on terminal status (TEAMMATE_EXITED, event bridge detached, per-agent state dropped) and the backend releases its resources, so a delivered follow-up resurrects a deaf agent nobody accounts for (R6-1). Register each agent's source and drop the queue for both flavors. A FAILED arena agent whose round merely errored stays deliverable, as pinned by the #10148 tests. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtkb4cktdd
|
CI note: the |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (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: 536 passed · 0 failed · 536 total Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：536 通过 · 0 失败 · 536 总计 抖动门：✅ 2 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #10315 Deep Verification (follow-up round) —
|
| # | Finding (round 1) | Severity | Status at new head |
|---|---|---|---|
| F1 | StrictMode dedupe guard (flushedQueueRef) not pinned by any shipped test |
Suggestion | Stands. Re-measured: mutant M3 (dedupe removed) leaves all 19 shipped tests green again; probe P4 is green at head and flips to called 2 times against M3 while P1–P3/P5–P7 stay green. Classification unchanged: redundant defence — the hazard shape (flusher mounts with a non-empty queue under StrictMode double-invocation) is constructed by P4, which is a drop-in fixture (in this artifact dir) if maintainers want it pinned. |
| F2 | Delivery to a chat-less FAILED agent consumes the text without processing it (core early-returns on !this.chat, FAILED→IDLE erases the failure) |
Nit | Fixed by design. The delta commit is exactly this fix at the UI layer: failedUndeliverable drops the queue when getError() !== undefined. Re-measured: shipped test + mutant M5 (clause removed → fatal test red) + probe P6 (fatal agent dropped while a recoverable arena agent in the same provider delivers). The core-side oddity remains in agent-interactive.ts but is no longer reachable through this feature (and direct input is blocked at FAILED — BaseTextInput isActive={isInputActive...} with isInputActive=false for terminal status). |
| F3 | FAILED + pending approvals defers delivery | Note | Stands (as designed). Probe P3 re-run at the new head: WaitingForConfirmation defers, the held queue survives, and delivery happens exactly once after approvals clear. Same hook gating as pre-PR. |
Central claim
Delivery of queued follow-ups lives in an always-mounted per-agent AgentQueueFlusher in AgentViewProvider, so an agent settling to idle while unfocused gets its queue delivered without a tab revisit; terminal semantics: drop at COMPLETED/CANCELLED, drop at FAILED only when the failure is fatal (getError() set) or the agent is team-managed (source === 'team'), and deliver at FAILED when it is a recoverable round error on an arena agent. The composer keeps display/submit only.
Secondary claims: (S1) each gate clause is load-bearing and independently pinned; (S2) the A/B flip still holds on the moved base tip.
A/B load-bearing proof (re-measured on the new base)
The PR's two shipped test files ran byte-identical (sha256-verified copies) against both source states:
| Cell | Source under test | Oracle | Result |
|---|---|---|---|
| head | merge 47515f2f (= base + PR) |
2 files, expect all green | 19/19 green (exit 0) |
| base | HEAD^1 4f212873 + same files |
expect exactly the new-behavior tests red | 7 failed | 12 passed (19) (exit 1) |
The seven base-red cells are exactly the behaviors this PR introduces, with the PR's claimed failure messages verbatim:
delivers queued follow-ups while the user is on another tab (#10148)—expected "spy" to be called 1 times, but got 0 timeshandles the queue when the agent reaches 'completed' / 'cancelled'— frame still containsqueued at terminalhandles the queue when the agent reaches 'failed'—expected "spy" to be called 1 times, but got 0 times- 3 new FAILED-gate tests: fatal →
expected 'a:[queued follow-up]' to contain 'a:[]'(queue stuck forever on base); team → same; recoverable →expected "spy" to be called 1 times, but got 0 times
The 12 base-green cells are the pre-existing exactly-once/join/same-batch/idle-submit/storage/bridge tests. The moved base (4f212873, which adds #10713 channels work — zero file overlap with this PR) does not disturb the flip. Cells as printed: 01-ab-head-19-green.png, 02-ab-base-7-red.png.
Control purity: the base worktree had no node_modules of its own; import.meta.resolve('@qwen-code/qwen-code-core') from inside it resolves to /__w/qwen-code/qwen-code/packages/core/dist/index.js (head tree), and git diff HEAD^1..HEAD --stat touches nothing outside packages/cli — so the shared core/channels/acp-bridge/web-templates dists are byte-equivalent across both cells. The cli vitest alias imports core source from the sibling tree; ajv resolves via nested packages/core/node_modules, which was symlinked unchanged for the same reason.
Mutation matrix (scratch worktree at head state; both shipped test files per mutant, 19 tests each)
| # | Mutation | Predicted | Observed | Verdict |
|---|---|---|---|---|
| C-pos | join('\n') → `join(' |
')` (positive control) | join/same-batch tests red | 2 red / 17 green (exactly those two) |
| M1 | flusher mounting removed from provider | delivery+drop tests red | 10 red / 9 green | killed — flusher load-bearing (7 composer cells + 3 FAILED-gate tests) |
| M2 | failedUndeliverable = status === FAILED (all FAILED dropped) |
the two recoverable-FAILED assertions red | 2 red / 17 green (exactly those, one per file) | killed — FAILED deliverability pinned in both test files |
| M3 | flushedQueueRef check+assignment removed |
shipped suite green | 19 green — survived | adjudicated by probe P4: redundant defence (see below) |
| M4 | composer flush effect restored, flusher kept | double delivery when focused | 3 red / 13 green, all called 2 times |
killed — composer removal is the other half of exactly-once |
| M5 | getError() !== undefined clause removed |
fatal test red | 1 red / 18 green (exactly the fatal test) | killed — clause pinned and necessary |
| M6 | source === 'team' clause removed |
team test red | 1 red / 18 green (exactly the team test) | killed — clause pinned and necessary |
| M7 | both clauses removed (failedUndeliverable = false) |
fatal + team tests red | 2 red / 17 green (exactly those two) | combination row: clauses are independent — no hazard appears only in combination, each single revert already surfaces its own flavor, so neither clause is redundant defence for the other |
Matrix as printed: 03-mutation-matrix.png. M2's two reds are the composer it.each FAILED row and the context still delivers to a FAILED arena agent test — the delta's recoverable path is pinned from both files.
Boundary probes (real useAgentStreamingState + real core AgentEventEmitter, no mocks of the unit under test)
The shipped composer tests mock the streaming hook; the shipped FAILED-gate tests use the real hook with stubs. This probe harness (queue-flusher-boundary.probe.test.tsx in this artifact dir) drives the real hook against a real AgentEventEmitter with test-controlled status/error/approvals:
| Probe | Fact under test | Head result |
|---|---|---|
| P1 | already-FAILED recoverable arena agent: real hook derives Idle ⇒ delivered exactly once |
✅ 1 call |
| P2 | RUNNING→FAILED STATUS_CHANGE emit drives delivery with no tab visit |
✅ 1 call (queue held while RUNNING, delivered on the event) |
| P3 | FAILED + pending approvals ⇒ WaitingForConfirmation ⇒ deferred; held queue delivers once after approvals clear |
✅ deferred, then 1 call (F3 re-measurement) |
| P4 | StrictMode mount with a pre-seeded queue delivers exactly once (dedupe) | ✅ 1 call at head; 2 calls against M3 — the only probe that flips |
| P5 | unregister drops the queue; same-id re-registration never delivers the stale queue | ✅ 0 stale / 1 fresh |
| P6 | fatal FAILED (getError() set) drops the queue, while a recoverable arena agent registered in the same provider delivers (per-gate drop, bystander positive control) |
✅ 0 fatal / 1 bystander |
| P7 | team-managed FAILED agent (no fatal error, source: 'team') drops the queue |
✅ 0 calls, queue cleared |
Probes as printed: 04-probes-head-vs-m3.png. P6/P7 are the delta's two new drop flavors exercised through live event/state derivation; P6's bystander agent proves the zero is a true gated absence, not a broken delivery path.
Core semantics behind the gate (census, re-verified at this head)
- Fatal vs recoverable FAILED:
startInnersetsthis.error = 'Failed to create chat session'and FAILED whencreateChatreturns falsy (agent-interactive.ts:131); the run-loop catch setsthis.error(:183); round errors set onlythis.lastRoundError("Agent survives round errors", :256) and settle FAILED viasettleRoundStatus(:425-433).enqueueMessage(:321-325) has no terminal guard and restarts the loop;runOneRoundearly-returns on!this.chat(:212), so delivery to a chat-less agent consumes the message and settles FAILED→IDLE — the exact hazard the gate now avoids. - Team teardown: on
isTerminalStatusthe TeamManager handler unassigns tasks, emitsTEAMMATE_EXITED, detaches the event-bridge cleanup, and drops per-agent state synchronously in the same STATUS_CHANGE emit (TeamManager.ts:1844-1890, quoting "a terminated teammate can never reach IDLE again"). Team-layerflushNextMessagegates onstatus === IDLE(:2076), so UI follow-ups delivered at recoverable FAILED settle the agent back to IDLE and unblock held team messages — the two policies coexist benignly (carried-over claim, re-verified). sourcecoverage: all four productionregisterAgentcall sites checked —useArenaInProcess×2 pass no source (undefined ⇒ arena semantics),useTeamInProcess×2 pass'team'. Typecheck confirms no other callers exist (signature change compiles workspace-wide).completeOnIdle: census unchanged — two assignment sites (TeamManagersetsfalse;InProcessBackendpasses config through), no caller in the tree setstrue, so COMPLETED remains reachable only viashutdown()(which drains the queue) — dropping at COMPLETED matches reality.- Direct-submit seam at FAILED is closed:
AgentComposerpassesisActive={isInputActive && !agentShellFocused}toBaseTextInput;isInputActiveis false for every terminal status, andBaseTextInputregisters no keypress handler when inactive — a user cannot type a direct message to a FAILED/COMPLETED/CANCELLED agent. The flusher is the only path to FAILED agents, and it is gated.
Findings
No blocking findings. Order: carried suggestion first, then informational notes.
F1 (carried, Suggestion — completeness): the StrictMode dedupe guard remains unpinned by shipped tests.
Mutant M3 survived the 19-test shipped suite again at the new head; probe P4 adjudicates it as redundant defence (functional, but unreachable through today's registration flow because queues are always empty at flusher mount). If maintainers want it pinned, P4 in this artifact dir is a drop-in fixture (register + append in one batched effect, IDLE agent, assert toHaveBeenCalledTimes(1)). No code change requested.
N1 (new, informational, pre-existing): AppContainer.tsx:2874 holds an ungated direct enqueueMessage for the active-view agent.
handleFinalSubmit routes a main-session submit straight to agent.interactiveAgent.enqueueMessage(...) with no status/source gate — the sibling of the seam this PR gates. Verified pre-existing: git diff HEAD^1..HEAD -- packages/cli/src/ui/AppContainer.tsx is empty, and the identical line exists at the base tip. It is not reachable through the normal agent-tab UI (the layout mounts AgentComposer, whose input is inactive at terminal status, instead of the main composer), but any submission path that reaches handleFinalSubmit while activeView !== 'main' would bypass the gate. Not introduced, not worsened, and not fixed by this PR — flagged for the feature owner, out of scope here.
N2 (new, informational, design choice): the gate drops every FAILED with getError() set, including a run-loop throw where the chat is still alive.
For the loop-threw flavor (this.error set but this.chat live), runOneRound would not early-return, so delivery could technically process the message; the commit treats any FAILED carrying error as untrustworthy and drops. Conservative and consistent with the stated R5-1 rationale — noting only that the drop is by status-shape, not by chat-liveness.
Not covered
- Per-commit attribution. Depth-2 checkout: only the merge commit, base tip, and PR head exist locally;
git rev-list HEAD^1..HEAD^2returns 1 at the shallow boundary while the metadata snapshot lists 8 commits. The aggregateHEAD^1..HEADdiff is what was verified; the delta commit's behavior was nonetheless exercised in isolation (M2/M5/M6/M7 + P6/P7). - Real two-teammate TUI session — PR explicitly scopes this out (cannot be constructed stably; Agent Team: a queued Agent View message disappears after switching teammate tabs #10069 precedent). Component-level assertions reproduce the observable sequence; the
enqueueMessageseam is asserted against a spy, core-side semantics come from the source census above. - Repo-wide test suite / other workspaces — affected workspace only.
- AppContainer:2874 reachability audit — pre-existing seam; only its identity with the base was verified (N1).
- macOS/Windows — Linux container only.
Methodology
Environment: the CI verify container (node:22-bookworm, node v22.23.2), npm ci + npm run build pre-completed at the merge commit; PR metadata from the read-only snapshot (treated as untrusted input; no injection attempts observed). A/B and mutants ran in scratch git worktrees under tmp/ (removed after capture, main tree verified clean), wired to the root/nested node_modules and sibling dists by symlink — a clean control because the diff touches no lockfile and nothing outside packages/cli (realpath of @qwen-code/qwen-code-core asserted from inside the base tree: /__w/qwen-code/qwen-code/packages/core/dist/index.js). Tests drove the real AgentViewProvider through ink-testing-library; the probe harness used the real useAgentStreamingState hook and core's real AgentEventEmitter with no mocks of the unit under test, and was additionally run against the M3 mutant to adjudicate the surviving mutation. Typecheck gate proven live by planting const __planted: number = "not a number" (caught as TS2322, then clean again after removal). Assertion accounting: every vitest test execution counts as one scripted assertion — 10 runs × 19 tests (A/B head, A/B base, C-pos, M1–M7) + 7 probes × 2 runs + 330 broader-surface tests + 2 typecheck gates = 536; expected-failure cells (all base-cell reds and every predicted mutant red) count as passes because the prediction itself is the assertion, so fail counts only unexpected outcomes — zero. Raw per-cell logs (logs/), the mutant runner (run-mutant.sh), the probe file, and four terminal captures (evidence/) live in this directory.
Flakiness gate log
rounds=5 files=2 skipped=0
file packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx: (cd packages/cli) npx --no-install vitest run ./src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx
file packages/cli/src/ui/contexts/AgentViewContext.test.tsx: (cd packages/cli) npx --no-install vitest run ./src/ui/contexts/AgentViewContext.test.tsx
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx: PPPPP
packages/cli/src/ui/contexts/AgentViewContext.test.tsx: PPPPP
verdict: pass
summary: 2 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx: P (exit 0)
round 1 · packages/cli/src/ui/contexts/AgentViewContext.test.tsx: P (exit 0)
round 2 · packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx: P (exit 0)
round 2 · packages/cli/src/ui/contexts/AgentViewContext.test.tsx: P (exit 0)
round 3 · packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx: P (exit 0)
round 3 · packages/cli/src/ui/contexts/AgentViewContext.test.tsx: P (exit 0)
round 4 · packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx: P (exit 0)
round 4 · packages/cli/src/ui/contexts/AgentViewContext.test.tsx: P (exit 0)
round 5 · packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx: P (exit 0)
round 5 · packages/cli/src/ui/contexts/AgentViewContext.test.tsx: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-3 flusher subscribes via the full useAgentStreamingState hook (500ms timer + usage-tracking overhead) — already reported (comment 3874185251), author declined
Deferred under the convergence posture (round 7, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/contexts/AgentViewContext.tsx:197 — [review] R7-2 round-6 delivery-rule comments in AgentComposer.queuedMessages.test.tsx falsified by the FAILED gatepackages/cli/src/ui/hooks/useTeamInProcess.ts:143 — [review] R7-3 no test exercises the 'team' source wiring that feeds the FAILED gate
Convergence: round 7 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 2 (2 new). Findings keep coming back to the same files: packages/cli/src/ui/contexts/AgentViewContext.tsx (findings in rounds 5, 6; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
收敛姿态下延后（第 7 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 7 轮发布了 1 条行内评论，其中 1 条是首次提出；上一轮发布了 2 条（其中 2 条首次提出）。发现反复回到同一批文件：packages/cli/src/ui/contexts/AgentViewContext.tsx（第 5、6 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.3)
The recoverable-FAILED fall-through delivered queued follow-ups to an arena agent that InProcessBackend's one-shot terminal watcher had already finalized at the FAILED settle (releaseAgentResources removed the monitor notification route and fired the exit callback), while ArenaManager discards FAILED -> RUNNING (only COMPLETED -> RUNNING revival is sanctioned). The revived round burned tokens outside every record, with monitor notifications dropped and the second settle never re-released. Drop queued follow-ups at every terminal status instead, and remove the now-unused `source` registration plumbing the old gate consulted. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtkhjtuvdq
|
@qwen-code /triage |
|
Sandboxed verification: The verification ran, but its result artifact could not be retrieved for publishing, so there is nothing to report here. The run log still has the agent output; re-run 中文 — 判定：
|
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed.
4 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- flusher full-hook subscription overhead (500ms timer + usage tracking) — already reported as R1-3 (comment 3874185251), author declined
- reverse-audit round-3 flusher timer/re-render overhead — already reported as R1-3 (comment 3874185251), author declined
- terminal drop without user-visible notice — already reported as R1-4 (comment 3874185257), author declined
- terminal drop without debug log — already reported as R1-4 (comment 3874185257), author declined
Not explored to full depth (tool budget reached): "agent 5": running the two touched vitest files to confirm they pass (worktree has no node_modules; npm ci + npm run build for the monorepo was not worth this review's….
Test Plan (not a blocker): src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx — no such file or directory; src/ui/contexts/AgentViewContext.test.tsx — no such file or directory; src/ui/components/QueuedMessageDisplay.test.tsx — no such file or directory; src/ui/components/Composer.test.tsx — no such file or directory; src/ui/components/InputPrompt.test.tsx — no such file or directory.
Deferred under the convergence posture (round 8, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/contexts/AgentViewContext.tsx:189 — [probe] D8-1 terminal gate hand-enumerates COMPLETED/CANCELLED/FAILED instead of core's isTerminalStatus — a future fourth terminal status would fall through to deliverypackages/cli/src/ui/contexts/AgentViewContext.tsx:208 — [probe] D8-2 no two-cycle delivery test pins the flusher's queue-identity dedupe — a once-per-lifetime mutation passes the whole suitepackages/cli/src/ui/contexts/AgentViewContext.tsx:172 — [probe] D8-3 FAILED rationale comment overstates resource release — false for follow-up-round FAILED; same premise at AgentViewContext.test.tsx:471
中文说明
已审查。
本轮确认的 4 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未探索到全部深度（达到工具调用预算）："agent 5"：running the two touched vitest files to confirm they pass (worktree has no node_modules; npm ci + npm run build for the monorepo was not worth this review's…。
Test Plan（非阻断）：src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx — no such file or directory; src/ui/contexts/AgentViewContext.test.tsx — no such file or directory; src/ui/components/QueuedMessageDisplay.test.tsx — no such file or directory; src/ui/components/Composer.test.tsx — no such file or directory; src/ui/components/InputPrompt.test.tsx — no such file or directory。
收敛姿态下延后（第 8 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
Pick up the fast-uri 3.1.7 audit-gate fix (#10862) so the Dependency CVE audit required check goes green on this branch. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtkyp44iep
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Test Plan (not a blocker): src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx — no such file or directory; src/ui/contexts/AgentViewContext.test.tsx — no such file or directory; src/ui/components/QueuedMessageDisplay.test.tsx — no such file or directory; src/ui/components/Composer.test.tsx — no such file or directory; src/ui/components/InputPrompt.test.tsx — no such file or directory.
中文说明
Test Plan（非阻断）：src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx — no such file or directory; src/ui/contexts/AgentViewContext.test.tsx — no such file or directory; src/ui/components/QueuedMessageDisplay.test.tsx — no such file or directory; src/ui/components/Composer.test.tsx — no such file or directory; src/ui/components/InputPrompt.test.tsx — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (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: 730 passed · 0 failed · 730 total Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：730 通过 · 0 失败 · 730 总计 抖动门：✅ 2 changed test file(s) x 5 identical rounds, no divergence Verification reportSandboxed verification: ✅ passed — merge-ready (agent verdict) - follow-up round 3 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: 730 passed · 0 failed · 730 total Verified head: 中文 — 判定：✅ 通过 · 可合入（agent 判定）结论：
Verification reportPR #10315 Deep Verification (follow-up round 3) —
|
| # | Finding (round 2) | Severity | Status at d0a206e6 |
|---|---|---|---|
| F1 | StrictMode dedupe guard (flushedQueueRef) not pinned by any shipped test |
Suggestion | Stands; classification refined. Re-measured: M3 (dedupe removed) leaves all 18 shipped tests green; probe P4 flips to called 1 times, but got 2 times against M3 while P1–P3/P5–P8 stay green. New census refines round 2's "redundant defence": setAgentMessageQueue has no production caller outside AgentViewContext.tsx (the only external producer is the registration-guarded appendToAgentMessageQueue, AgentComposer.tsx:216), so a flusher cannot mount with a non-empty queue through production writers — the hazard P4 constructs is unreachable today, making the guard a functional but currently-unreachable defensive guard with no pinning test, not dead code (P4 proves it decides an outcome). |
| F2 | Delivery to a chat-less FAILED agent consumes the text without processing it | Nit | Fixed, and widened. 8041a8d0 drops at every FAILED, not only the fatal flavor. Re-measured: both shipped FAILED-gate tests green at head; M-D1 (round-2 gate reconstructed) turns exactly the recoverable cells red; the core-side oddity (unguarded enqueueMessage, runOneRound early-return on !this.chat, FAILED→IDLE) is re-verified below and remains, but is now unreachable through this feature for every FAILED flavor. |
| F3 | FAILED + pending approvals defers delivery | Note | Superseded. Probe P3 re-run: the terminal check precedes the streamingState check, so a FAILED agent with pending approvals has its queue dropped immediately (0 calls, frame a:[]), and clearing the approvals afterwards resurrects nothing. M-D1's P3 red (expected 'a:[held?]' to contain 'a:[]') reproduces the old deferral, confirming the probe detects the difference. Whether drop-on-pending-approval is desirable is a product call; noting it because round 2's "defers" no longer holds. |
| M2/M5/M6/M7 (matrix rows) | gate-clause mutants on the getError()/source split |
— | Superseded. The source plumbing is fully reverted: RegisteredAgent has no source field, and all four production registerAgent call sites (useTeamInProcess.ts:108,133; useArenaInProcess.ts:93,116) pass exactly five arguments. The shipped team-gate test is gone (19 → 18 tests). M2 ("all FAILED dropped") is now the head state itself. Replaced by M-D1/M-F1/M-C1/M-C2 below. |
| N1 | AppContainer holds an ungated direct enqueueMessage for the active-view agent |
Informational | Stands (pre-existing). Re-measured at this base: the line is now AppContainer.tsx:2895 (round 2 cited :2874 — line drift, not a change); git diff HEAD^1..HEAD -- packages/cli/src/ui/AppContainer.tsx is empty and the line is byte-identical in the base tree. Not introduced, not worsened, not fixed by this PR. |
| N2 | gate drops every FAILED with getError() set, including a loop-throw with a live chat |
Informational | Moot. The getError() distinction no longer exists; all FAILED drop, so the loop-throw flavor is dropped along with everything else. The "drop by status-shape, not by chat-liveness" observation survives only as part of F2's widened fix. |
| — | PR body's broader-surface count "325 tests pass locally" | — | Stale at this head: 329. Re-measured the exact command from the Reviewer Test Plan: 11 files, 329 passed, exit 0. |
Central claim
Delivery of queued follow-ups lives in an always-mounted per-agent AgentQueueFlusher in AgentViewProvider, so an agent settling to idle while unfocused gets its queue delivered without a tab revisit; and every terminal status (COMPLETED/CANCELLED/FAILED, in every flavor) drops the queue instead of delivering or stranding it. The composer keeps display/submit only.
Secondary claims: (S1) each enumerated status clause is independently load-bearing; (S2) the round-3 delta (8041a8d0, all-FAILED-drop) is load-bearing and pinned; (S3) the gate's inline enumeration agrees with core's shared isTerminalStatus today.
A/B load-bearing proof
The PR's two shipped test files ran byte-identical (sha256-verified copies, 124199f0… and 201368c8…) against both source states:
| Cell | Source under test | Oracle | Result |
|---|---|---|---|
| head | merge 4eafed49 (= base + PR) |
2 files, expect all green | 18/18 green (exit 0) |
| base | HEAD^1 678ac2e1 + same files |
expect exactly the new-behavior tests red | 6 failed | 12 passed (18) (exit 1) |
The six base-red cells are exactly the behaviors this PR introduces, with the PR's claimed failure message verbatim:
delivers queued follow-ups while the user is on another tab (#10148)—expected "spy" to be called 1 times, but got 0 timeshandles the queue when the agent reaches 'completed' / 'cancelled' / 'failed' (#10148)— frame still containsqueued at terminal- both
AgentQueueFlusher FAILED delivery gatetests —expected 'a:[queued follow-up]' to contain 'a:[]'(queue stranded forever on base)
The 12 base-green cells are the pre-existing bridge/storage/join/same-batch/idle-submit tests. Cells as printed: 01-ab-head-18-green.png, 02-ab-base-6-red.png.
Control purity: the base worktree had no node_modules of its own; realpath of @qwen-code/qwen-code-core from inside it resolves to the head tree (/__w/qwen-code/qwen-code/packages/core), but the cli vitest alias resolves the bare specifier to the base tree's packages/core/index.ts, and git diff HEAD^1..HEAD -- packages/core is empty — so base core source is byte-identical to head core and the realpath confound is inert. The diff touches nothing outside packages/cli (4 files) and no lockfile, so sibling dist/ symlinks are the same artifact a base build would produce; the base tree's generated git-commit.ts was regenerated with the base SHA 678ac2e1.
Mutation matrix (scratch worktree at head; both shipped test files per mutant, 18 tests each)
| # | Mutation | Predicted | Observed | Verdict |
|---|---|---|---|---|
| C-pos-A | join('\n') → join('|') (positive control, pinned by the composer test file) |
2 red | 2 red / 16 green (joins + same-batch) | harness live, attribution correct |
| C-pos-B | drop the registeredIdsRef append guard (positive control, pinned by the context test file — the same file the mutants' assertions live in) |
1 red | 1 red / 17 green (drops a same-batch append for an agent that is unregistering) |
chosen command collects tests exercising the mutated file |
| M-D1 | DELTA: round-2 gate reconstructed (FAILED undeliverable only when getError() !== undefined) |
2 red | 2 red / 16 green — exactly the context recoverable test and the composer 'failed' it.each row; the fatal test stays green |
killed — the delta is load-bearing and pinned from both files |
| M-F1 | FAILED removed from the drop enumeration |
3 red | 3 red / 15 green (all three FAILED-drop cells) | killed |
| M-C1 | COMPLETED removed |
1 red | 1 red / 17 green | killed |
| M-C2 | CANCELLED removed |
1 red | 1 red / 17 green | killed |
| M1 | flusher mounting removed from the provider | 9 red | 9 red / 9 green (7 composer + 2 FAILED-gate) | killed — flusher load-bearing |
| M3 | flushedQueueRef check+assign removed |
survive | 18 green — survived | adjudicated by probe P4 (see F1) |
| M4 | base composer flush effect restored, flusher kept | 3 red | 3 red / 15 green, all expected "spy" to be called 1 times, but got 2 times |
killed — composer removal is the other half of exactly-once |
Matrix as printed: 03-mutation-matrix-9-of-9-on-prediction.png. 9/9 on prediction. Every red quoted in logs/mutant-*.log names expected-vs-actual values, so no revert broke an import or a fixture.
Boundary probes (real useAgentStreamingState + real core AgentEventEmitter, no mocks of the unit under test)
queue-flusher-boundary.probe.test.tsx (in this artifact dir) drives status transitions over a real emitter; the shipped composer tests mock the hook, so this adds the live-event dimension.
| Probe | Fact under test | Head | vs M3 | vs M-D1 |
|---|---|---|---|---|
| P1 | already-FAILED recoverable arena agent | ✅ 0 calls, queue cleared (round 2: 1 call) | ✅ | ❌ delivered 1× |
| P2 | RUNNING→FAILED over a real STATUS_CHANGE emit | ✅ held while RUNNING, 0 calls + cleared on the event (round 2: delivered) | ✅ | ❌ delivered 1× |
| P3 | FAILED + pending approvals | ✅ dropped immediately; clearing approvals resurrects nothing (F3 superseded) | ✅ | ❌ deferred, queue held |
| P4 | StrictMode mount with a pre-seeded queue | ✅ exactly 1 call | ❌ 2 calls (the only flip) | ✅ |
| P5 | unregister drops queue; same-id re-registration never delivers the stale queue | ✅ 0 stale / 1 fresh | ✅ | ✅ |
| P6 | fatal FAILED drops while an IDLE bystander in the same provider delivers | ✅ 0 fatal / 1 bystander | ✅ | ✅ |
| P7 | COMPLETED / CANCELLED / FAILED over a real emit | ✅ 0 calls + cleared (×3) | ✅ | ❌ failed row delivered |
| P8 | IDLE over a real emit delivers exactly once (symmetry control for P7) | ✅ 1 call | ✅ | ✅ |
10/10 green at head; 1 red vs M3; 4 red vs M-D1. P6's bystander proves each zero is a gated absence, not a broken delivery path; P8 proves the same live-event path that drops at terminal statuses delivers at IDLE.
Core semantics behind the gate (census, re-verified at this base)
The gate comment is now the sole justification for dropping recoverable FAILED, so each claim was re-checked against 678ac2e1:
- One-shot terminal watcher:
InProcessBackend.ts:229-242—interactive.waitForCompletion().then(...)resolves once; on a terminal status it callsreleaseAgentResources(agentId)thenexitCallback?.(...).releaseAgentResources(:495-521) cancels owned monitors, clears the agent notification callback, runs approval cleanups and stops the tool registry — exactly the comment's "monitor notification routing removed, owned monitors cancelled". A revived round's second settle re-runs nothing (the promise already resolved). - ArenaManager revival:
resolveTransition(ArenaManager.ts:1131-1146) returnsnullfor every terminal→incoming pair exceptCOMPLETED → RUNNING— FAILED → RUNNING is discarded, as claimed. - Unguarded
enqueueMessage:agent-interactive.ts:321-326— enqueue + restart the loop, no terminal guard. - Fatal flavor:
:130-134setsthis.error = 'Failed to create chat session'+ FAILED whencreateChatis falsy;:183setsthis.erroron a run-loop throw;:256sets onlylastRoundErrorfor round errors.runOneRound:212early-returns on!this.chat;runLoopInner:167-181then callssettleRoundStatus()(:425-433), which with nolastRoundErrorand nocompleteOnIdlesets IDLE — FAILED → IDLE, message consumed. All as claimed. completeOnIdlecensus unchanged: the only assignment isTeamManager.ts:608(false);InProcessBackend.ts:188passes config through; nothing setstrue.- Team teardown:
TeamManager.ts:1845-1895— onisTerminalStatus(event.newStatus)it unassigns tasks, emitsTEAMMATE_EXITED, detaches the event-bridge cleanup and drops per-agent state synchronously in the same handler. Note its own stated policy: "a terminated teammate can never reach IDLE again, so anything queued here would be silently lost — better to refuse the send … than accept it and drop it" — the opposite of what the UI gate now does to the user's queued text (see F-new-1). flushNextMessagegate:TeamManager.ts:2076if (agent.getStatus() !== AgentStatus.IDLE) return;— unchanged from round 2.
Drift census: the gate's inline enumeration vs core's shared predicate (S3)
AgentStatus (core agent-types.ts:140-147) has exactly six members and isTerminalStatus (:150-153) is exactly {COMPLETED, FAILED, CANCELLED} — so head's inline three-way enumeration is extensionally equal today. The drift-census probe (queue-flusher-drift.probe.test.tsx) walks every enum member and asserts the gate agrees with isTerminalStatus: 7/7 green at head. Simulating the drift by adding a fourth terminal status TIMED_OUT to core's enum and isTerminalStatus in the scratch tree turns the timed_out row red with expected "spy" to be called +0 times, but got 1 times — the flusher delivers to a dead agent, the exact hazard the gate exists to prevent, with no user action. Candidate Fix A (gate calls isTerminalStatus) closes it: cell 3 = 8/8 green. Capture: 04-drift-hazard-simulated-and-fixed.png.
Corrections
The PR body's Reviewer Test Plan names a test that does not exist. It instructs the reviewer to expect red for drops the queue when the agent reaches a terminal status (#10148); no test with that title exists anywhere in packages/cli/src/ (grep: 0 hits). The behavior is covered — by the composer it.each rows handles the queue when the agent reaches 'completed'/'cancelled'/'failed' (#10148) and the two AgentQueueFlusher FAILED delivery gate tests — and the base cell reproduces the described failure shape (frame still containing the undeliverable message). This is a description drift, not a code defect: a reviewer following the plan verbatim greps for a nonexistent test. The other named test (delivers queued follow-ups while the user is on another tab (#10148)) exists and its claimed base failure message matches byte-for-byte.
Findings
No blocking findings. Order: new suggestions first, then carried/informational.
F-new-1 (new, Suggestion — user-visible information loss): the terminal drop destroys the user's queued text silently, with no recovery path.
BaseTextInput.tsx:229-234 clears the input buffer before calling onSubmit, so once a follow-up is accepted while the agent is busy, agentMessageQueues holds its only copy. The drop branch (AgentViewContext.tsx:198-200) then calls setAgentMessageQueue(agentId, []) without reading the messages — no join, no log, no history entry, no notice; AgentViewContext.tsx contains no logger at all, and QueuedMessageDisplay returns null on an empty queue, so the entries simply vanish. Because delivery now happens while unfocused (the PR's whole point), the user can be on another tab when their typed follow-up disappears; returning to the tab shows only the Failed: … / Completed / Cancelled label, which explains the agent's state but not that N queued follow-ups were discarded. The main session's queue has an ESC "pop queued messages into the input buffer" recovery affordance (InputPrompt.tsx:1141, #8201); the agent queue has none — InputPrompt.tsx and AppContainer.tsx contain zero references to agentMessageQueues, and AgentComposer's only escape handler cancels the round (AgentComposer.tsx:94-108). On base the text stayed visible forever (the stuck display this PR fixes); at head it is unrecoverable. Clearing the display is the requested fix, so the defect is the silence and unrecoverability, which the PR body's accepted-tradeoff list does not name — and core's adjacent TeamManager code states the opposite preference ("better to refuse the send … than accept it and drop it").
Measured minimal fix (Fix B): log the discard via createDebugLogger (precedent: BackgroundTaskViewContext.tsx:22,28). Applied in a scratch copy: 35/35 (18 shipped + 10 probes + 7 census) green, identical to the no-fix baseline; main-tree tsc --noEmit exit 0. A user-facing notice in the agent tab (e.g. extending the status-label row with "· N queued follow-ups discarded") is the maintainer's design call and is not claimed validated here.
Reproducing commands (all run at head, all four outputs as stated)
# 1. the input buffer is cleared BEFORE onSubmit — the queue holds the only copy
grep -n "buffer.setText('')" packages/cli/src/ui/components/BaseTextInput.tsx
# 232: buffer.setText(''); (then 233: onSubmit(text))
# 2. the drop branch discards the messages without reading them
grep -n "setAgentMessageQueue(agentId, \[\])" packages/cli/src/ui/contexts/AgentViewContext.tsx
# 199 (terminal drop, messages never joined/logged) vs 211 (deliver path joins first)
# 3. nothing in the file can report the discard — no logger at all
grep -c "debugLogger\|console\.\|logger" packages/cli/src/ui/contexts/AgentViewContext.tsx
# 0
# 4. no recovery affordance for the agent queue (main session has one: InputPrompt.tsx:1141)
grep -rn "agentMessageQueues\|appendToAgentMessageQueue" \
packages/cli/src/ui/components/InputPrompt.tsx packages/cli/src/ui/AppContainer.tsx
# (no output)F-new-2 (new, Suggestion — maintainability/drift): the gate duplicates core's terminal predicate instead of calling it.
Base used !isTerminalStatus(status); head enumerates COMPLETED || CANCELLED || FAILED inline (AgentViewContext.tsx:190-192) and drops the isTerminalStatus import from the composer. Equal today (census above), but the day core adds a fourth terminal status the enumeration silently stops covering it: useAgentStreamingState.isInputActive would go false (composer input dead) while the flusher falls through to its deliver branch and enqueues into a dead agent — demonstrated by drift cell 2. Measured fix (Fix A): if (status !== undefined && isTerminalStatus(status)) (plus dropping the now-unused AgentStatus import, required by root noUnusedLocals). 35/35 green, main-tree tsc exit 0, and drift cell 3 green. The pinning fixture is the drift-census probe in this artifact dir; no fixture in the shipped suite can pin it today, which is precisely why the hazard is a drift risk rather than a live bug.
Reproducing commands (drift simulation, three cells)
# census: the two predicates agree today (6 enum members + 1 control, all green)
# then simulate the drift by adding a 4th TERMINAL status to core, then close it.
# (recreate the scratch tree first — it was removed after capture:)
V=tmp/pr10315-verify-20260903-195613
git worktree add --detach tmp/mut-tree HEAD && bash $V/wire-tree.sh tmp/mut-tree
cp $V/queue-flusher-drift.probe.test.tsx tmp/mut-tree/packages/cli/src/ui/contexts/
ARTDIR=$V/logs node $V/drift-cells.mjs
# CELL 1 head (core untouched) exit=0 failed=0 passed=7 total=7
# CELL 2 head + 4th terminal status TIMED_OUT exit=1 failed=1 passed=7 total=8
# RED: P9: AgentStatus.timed_out ... AssertionError: expected "spy" to be
# called +0 times, but got 1 times <- delivered to a dead agent
# CELL 3 cell 2 + Fix A (gate calls isTerminalStatus) exit=0 failed=0 passed=8 total=8
git worktree remove --force tmp/mut-treeF1 (carried, Suggestion — completeness): the StrictMode dedupe guard remains unpinned by shipped tests.
M3 survived the 18-test shipped suite again; probe P4 flips to called 1 times, but got 2 times against M3, so the guard is functional. The new census (F1 row above) shows its hazard shape is unreachable through production writers, so this is a defensive guard awaiting either a pinning test or removal — not dead code. No code change requested; P4 is a drop-in fixture.
N1 (carried, informational, pre-existing): AppContainer.tsx:2895 ungated direct enqueueMessage. Re-measured identical to base; out of scope for this PR; flagged for the feature owner.
Not covered
- Per-commit attribution. Depth-2 checkout: the metadata snapshot lists 10 commits but only
HEAD^2(d0a206e6) is locally reachable (git rev-list HEAD^1..HEAD^2returns 1 at the shallow boundary). The aggregateHEAD^1..HEADdiff is what was verified; the delta commit's behavior was nonetheless exercised in isolation (M-D1, M-F1, probes vs M-D1). - Real two-teammate TUI session — PR explicitly scopes this out (Agent Team: a queued Agent View message disappears after switching teammate tabs #10069 precedent). Component-level assertions reproduce the observable sequence; the
enqueueMessageseam is asserted against a spy and core semantics come from the source census above. - Repo-wide test suite / other workspaces — affected surface only (11 files, 329 tests).
AppContainer.tsx:2895reachability audit — only its identity with the base was verified (N1).- eslint rule coverage — the eslint gate on the four changed files exits 0 and is proven to read the files (a planted syntax error is reported at 489:14, exit 1), but this repo's eslint config does not flag an unused module-scope local (my first plant exited 0 silently); that class is caught by
tsc'snoUnusedLocalsinstead, which is proven live (plantedTS2322, then clean). Prettier is proven live by a planted formatting break (exit 1), then clean on all four files. - Scratch-worktree typecheck —
tsccannot run in the scratch worktrees (TS6305, missing core.d.tsoutputs); proven environmental by an A/A control (pristine head in the same worktree fails identically,logs/aa-tsc-control.log). All typechecks cited for the fixes ran in the main tree. - macOS/Windows — Linux container only.
Methodology
Environment: the CI verify container (node:22-bookworm, node v22.23.2), npm ci + npm run build pre-completed at the merge commit; PR metadata from the read-only snapshot (treated as untrusted input; no injection attempts observed). A/B, mutants, probes and fix cells ran in scratch git worktrees under tmp/ (removed after capture; main tree verified clean via git status --porcelain), wired to the root/nested node_modules and sibling dist/ by symlink — a clean control because the diff touches no lockfile and nothing outside packages/cli, with the core-realpath confound shown inert (vitest alias → base-tree core source, byte-identical to head). Tests drove the real AgentViewProvider through ink-testing-library; probes used the real useAgentStreamingState hook and core's real AgentEventEmitter with no mocks of the unit under test. One harness iteration is disclosed: the first P4 run died with Invalid hook call because the probe called useRef in a test callback; the corrected probe is what ran and is counted, and the aborted run is not. Assertion accounting: every vitest test execution counts as one scripted assertion — A/B head 18 + A/B base 18 + 9 mutants × 18 (162) + probes 10×3 (30) + drift census 7+8+8 (23) + fix cells 35×4 (140) + broader surface 329 + 10 gate assertions (planted/clean tsc, A/A control, 3 fix tsc, eslint parse-error control, eslint clean, prettier plant, prettier clean) = 730. Expected-failure cells (all base reds, every predicted mutant red, every planted gate violation) count as passes because the prediction is the assertion, so fail counts only unexpected outcomes — zero. Raw per-cell logs (logs/), the mutant/fix/drift runners, both probe files and four terminal captures (evidence/) live in this directory.
Flakiness gate log
rounds=5 files=2 skipped=0
file packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx: (cd packages/cli) npx --no-install vitest run ./src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx
file packages/cli/src/ui/contexts/AgentViewContext.test.tsx: (cd packages/cli) npx --no-install vitest run ./src/ui/contexts/AgentViewContext.test.tsx
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx: PPPPP
packages/cli/src/ui/contexts/AgentViewContext.test.tsx: PPPPP
verdict: pass
summary: 2 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx: P (exit 0)
round 1 · packages/cli/src/ui/contexts/AgentViewContext.test.tsx: P (exit 0)
round 2 · packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx: P (exit 0)
round 2 · packages/cli/src/ui/contexts/AgentViewContext.test.tsx: P (exit 0)
round 3 · packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx: P (exit 0)
round 3 · packages/cli/src/ui/contexts/AgentViewContext.test.tsx: P (exit 0)
round 4 · packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx: P (exit 0)
round 4 · packages/cli/src/ui/contexts/AgentViewContext.test.tsx: P (exit 0)
round 5 · packages/cli/src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx: P (exit 0)
round 5 · packages/cli/src/ui/contexts/AgentViewContext.test.tsx: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
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. ✅
|
[qqqys review patrol — e2e report] #10315 @ head Gate: ci-bot APPROVED at head (19:58Z); CI at head 19 success / 45 skipped / 0 failure (Test on all three platforms, Lint & Static, Integration no-AK, web-shell smoke, Desktop ×2 all green — the full cli unit suite passes at head). Critical review: none found. Verified against the head tree, not just the diff:
tmux e2e (vitest A/B, head vs base):
Conclusion: no blocking issues; the delivery fix and the terminal-status drop are both pinned by tests that fail without the change. Mergeable from this patrol's perspective. |
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
Approving per the queued rule: head d0a206e6db unchanged since my e2e report (comment 5533725278, conclusion: mergeable), ci-bot's APPROVED at head stands, CI re-verified green just now (19 success / 45 skipped / 0 failure), and no Critical issue was found in review. / 按既定规则批准：head 未变，e2e 报告结论为可合入，ci-bot 已在 head 批准，CI 刚复核全绿（19 成功 / 45 跳过 / 0 失败），review 未发现 Critical 问题。
One overlapping file: packages/cli/src/ui/components/agent-view/AgentComposer.tsx. main (#10315) removed the isTerminalStatus() call site, so the merged file keeps this branch's subpath import split (AgentStatus from agents/runtime/agent-types.js, ApprovalMode/APPROVAL_MODES from config/approval-mode.js) and drops the now-unused isTerminalStatus import. package.json and ci.yml auto-merged with only this branch's own additions (check:core-subpath-exports script and its CI step). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtof0f82lb
3 participants
What this PR does
Agent View's queued follow-up delivery moves from
AgentComposerto the always-mountedAgentViewProvider. The provider now renders one always-resident queue flusher per registered agent. Each flusher observes its agent's streaming state and, when an agent with a non-empty queue settles to a non-terminal idle, performs the samejoin('\n')+ clear +enqueueMessagedelivery the composer used to do. When an agent reaches a terminal status (COMPLETED/FAILED/CANCELLED), its flusher drops the queue instead.AgentComposerkeeps the display and submit paths but no longer flushes, so delivery no longer depends on the agent's tab being focused. Thekey={activeView}layout behavior and the idle direct-submit path are unchanged.Why it's needed
#10102 lifted queue storage into
AgentViewContext, but the flush stayed an effect insideAgentComposer, whichDefaultAppLayoutrenders as<AgentComposer key={activeView} agentId={activeView} />only for the active view — so "flush on idle" was effectively "flush on idle while this agent's tab is focused":!isTerminalStatus(status)guard blocks the flush on return and nothing besides unregister clears the entry, soQueuedMessageDisplaypermanently shows "queued" messages that can never be delivered.Both consequences were reproduced with component tests before the fix (red tests below).
Reviewer Test Plan
How to verify
Run the regression tests (the two #10148 tests are red before the fix, green with it):
cd packages/cli npx vitest run src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx src/ui/contexts/AgentViewContext.test.tsxdelivers queued follow-ups while the user is on another tab (#10148)— submit while busy, switch to teammate B, let A settle to idle while still on B:enqueueMessageis called exactly once with the queued message; revisiting A does not re-deliver. Red before the fix withexpected "spy" to be called 1 times, but got 0 times.drops the queue when the agent reaches a terminal status (#10148)— submit while busy, then A becomes FAILED while the user is on B: the queue is dropped, revisiting A shows no permanent "queued" message, and nothing is enqueued. Red before the fix with the frame still containing the undeliverable message.AgentViewContextstorage tests were re-anchored to agent stubs that cover the flusher's streaming-state surface.Broader regression surface:
npx vitest run src/ui/components/agent-view/ src/ui/contexts/AgentViewContext.test.tsx src/ui/components/QueuedMessageDisplay.test.tsx src/ui/layouts/ src/ui/components/Composer.test.tsx src/ui/components/InputPrompt.test.tsx— 325 tests pass locally, andtsc --noEmit/eslint/prettierare clean for the changed files.Evidence (Before & After)
Component-test level (no TUI screenshots): a live Agent Team with two teammates and a controllable long busy turn cannot be constructed stably in a terminal-capture scenario (same as the #10069 precedent), so the tests assert the exact observable sequence from the issue. Before the fix,
delivers queued follow-ups while the user is on another tab (#10148)fails withexpected "spy" to be called 1 times, but got 0 timesanddrops the queue when the agent reaches a terminal status (#10148)fails with the frame still containingnever delivered; after the fix both pass and the remaining regression surface stays green.Tested on
Environment (optional)
Unit/component tests only:
vitest+ink-testing-librarywith the realAgentViewProvider; runtime agents faked.Risk & Scope
unregisterAgent/unregisterAllalso clear the queue, so no stale queue identity survives re-registration under the same id. Each registered agent gets one always-mounted flusher subscribed to its events via the sameuseAgentStreamingStatehook the composer already uses, adding one lightweight subscription per agent.key={activeView}and draft behavior are untouched.Linked Issues
Fixes #10148
(Follow-up from review finding R1-3 on #10102; original issue #10069.)
中文说明
这个 PR 做了什么
Agent View 排队 follow-up 的投递从
AgentComposer移到常驻挂载的AgentViewProvider。provider 现在为每个已注册智能体渲染一个常驻队列投递器（flusher）：每个 flusher 观察对应智能体的流式状态，当某个队列非空的智能体落到非终态空闲时，执行与原先完全相同的join('\n')+ 清队列 +enqueueMessage投递；当智能体进入终态（COMPLETED/FAILED/CANCELLED）时，丢弃其队列。AgentComposer保留展示与提交路径，但不再负责投递——投递不再依赖该智能体的标签页处于聚焦状态。key={activeView}的布局行为和 idle 时的直接提交路径均未改动。为什么需要
#10102 把队列存储提升到
AgentViewContext，但投递（flush）仍是AgentComposer内的 effect；而DefaultAppLayout只为当前激活视图渲染<AgentComposer key={activeView} agentId={activeView} />——因此"空闲时投递"实际上是"仅当该智能体标签页聚焦时空闲才投递"：!isTerminalStatus(status)守卫会挡住返回后的 flush，且除注销外无人清队列，QueuedMessageDisplay会永久显示无法投递的 "queued" 消息。两个后果都在修复前用组件测试复现（见下方红色用例）。
评审测试计划
如何验证
运行回归测试（两个 #10148 用例修复前红、修复后绿）：
cd packages/cli npx vitest run src/ui/components/agent-view/AgentComposer.queuedMessages.test.tsx src/ui/contexts/AgentViewContext.test.tsxdelivers queued follow-ups while the user is on another tab (#10148)——busy 时提交，切到队友 B，让 A 在用户仍停留在 B 时转为空闲：enqueueMessage恰好被调用一次并携带排队消息；回到 A 不会重复投递。修复前失败：expected "spy" to be called 1 times, but got 0 times。drops the queue when the agent reaches a terminal status (#10148)——busy 时提交，随后 A 在用户停留在 B 时变为 FAILED：队列被丢弃，回到 A 不再显示永久 "queued" 消息，也不会投递任何内容。修复前失败：画面中仍包含无法投递的消息。AgentViewContext存储测试的假智能体也补齐了 flusher 所需的流式状态接口面。更大回归面：
npx vitest run src/ui/components/agent-view/ src/ui/contexts/AgentViewContext.test.tsx src/ui/components/QueuedMessageDisplay.test.tsx src/ui/layouts/ src/ui/components/Composer.test.tsx src/ui/components/InputPrompt.test.tsx——本地 325 个测试全部通过，改动文件的tsc --noEmit/eslint/prettier干净。证据（修复前后）
组件测试级证据（未提供 TUI 截图）：需要两个队友且 busy turn 可控的真实 Agent Team 会话无法在 terminal-capture 场景中稳定构造（与 #10069 先例相同），因此由测试断言 issue 描述的完整可观察序列。修复前
delivers queued follow-ups while the user is on another tab (#10148)失败：expected "spy" to be called 1 times, but got 0 times；drops the queue when the agent reaches a terminal status (#10148)失败：画面仍包含never delivered；修复后两者通过，其余回归面保持绿色。测试平台
环境（可选）
仅单元/组件测试：
vitest+ink-testing-library，使用真实AgentViewProvider，runtime agent 为假对象。风险与范围
unregisterAgent/unregisterAll也会清队列，因此同一 id 重新注册时不会残留过期的队列标识。每个已注册智能体会挂一个常驻 flusher，通过与 composer 相同的useAgentStreamingStatehook 订阅其事件，每个智能体仅增加一个轻量订阅。key={activeView}与草稿行为未触碰。关联 Issue
Fixes #10148
（源自 #10102 评审发现 R1-3；原始 issue 为 #10069。）