fix(core): surface nested sub-agent approvals under background parents - #9793
Conversation
A tool call needing confirmation inside a nested sub-agent (launched by a background agent or fork) was neither surfaced nor denied: the prompt- avoidance policy was stamped on an Object.create wrapper while the rebuilt tool registry binds to agentConfig, so nested schedulers resolved Config.prototype's false and believed they could prompt; and the nested invocation's emitter was never bridged, so TOOL_WAITING_APPROVAL fired with no listener and the call waited forever - the enclosing agent hung silently. - Stamp getShouldAvoidPermissionPrompts on agentConfig itself (launch and resume paths) so nested launches inherit the real policy through their config prototype chains: hang-forever becomes an explicit deny when bubbling is off. - Bridge nested foreground launches' approval events onto the nearest backgrounded running ancestor's Background-tasks entry (walking the registry's parentAgentId lineage), so they park where the ancestor's own approvals go and the user can answer them from the dialog. - Mark bridged approvals with the nested runtime's subagentId (declared by the bridge caller via nestedSource - runtime ids and registry ids use different suffixes so comparing them cannot work) and show the waiter in the Background tasks dialog. Fixes #9782
github-actions
Bot
added
the
review/self-reported
|
Re-run on the post-review head — the gate re-checked from scratch at
Moving on to code review. 🔍 中文说明本轮是在 review 反馈处理后的最新 head 上重跑——门禁在
进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewRe-reviewed the full diff at the current head — this pass covers the autofix rounds on top of the first review, not just the delta. My independent proposal for the stated problem (sink the prompt-avoidance policy onto the config the rebuilt registry binds to; bridge the nested invocation's emitter up to the nearest backgrounded ancestor; label nested waiters, since runtime and registry ids can't be compared) matches the PR's shape, and once several nested runtimes can park onto one shared ancestor entry, the composite The two Critical threads still open on the PR are both addressed at this head — I verified against the code at
The remaining open Suggestion threads are likewise covered at this head: the dialog UI branch has three tests (renders the nested line, omits it unstamped, routes the resolve with the key part); the second-parked, unstamped-own, and nested-dedup mutant cases each have discriminating tests; the fail reason and auto-reject error now carry the nested attribution and have tests pinning it. The ten outdated threads (hop cap → visited set, shared What I verified against the base code, not just the diff: the walk starts from No new blockers found this round. The Integration Tests (CLI, No Sandbox) job is skipped by this run's CI profile — same as the first pass, disclosed rather than papered over. sequenceDiagram
participant P1 as Nested agent runtime
participant P2 as Invocation emitter
participant P3 as Approval bridge (nestedSource)
participant P4 as Background task registry
participant P5 as Background tasks dialog
P1->>P2: TOOL_WAITING_APPROVAL
P2->>P3: forward event
P3->>P4: park on backgrounded ancestor entry, stamp subagentId
P5->>P4: user answers the parked approval
P4->>P1: respond resolves the waiting call
Files changed (18)
Test evidence — the PR's own CIAll checks are complete on
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The PR's Verification section (mutation probes per fix site, 445 + 178 regression tests) is the author's claim — I verified the new test code reads as described and the suite is green in CI, but this run does not execute PR code, so it is not independently re-run evidence. A sandboxed verification run ( 中文说明代码审查本轮在当前 head 上重新审查了完整 diff——覆盖首轮审查之上 autofix 各轮的改动，而不只是增量。我对该问题的独立方案（把提示回避策略下沉到重建注册表实际绑定的配置、把嵌套调用的 emitter 桥接到最近的 background 祖先、为嵌套等待者打标）与 PR 形态一致；一旦多个嵌套运行时可能停到同一个祖先条目，复合键 PR 上仍未关闭的两个 Critical 线程在当前 head 都已解决——我对照
其余未关闭的 Suggestion 线程同样已在当前 head 覆盖：对话框 UI 分支有 3 个测试；第二个停放者、未打标自身审批、嵌套去重的变异用例均有判别性测试；fail 原因与自动拒绝错误日志现带嵌套归因并有测试钉住。十个已过期线程（跳数上限 → 访问集合、共享 对照基线代码（而非仅凭 diff）核实：遍历从子代注册前的 本轮未发现新的阻塞问题。Integration Tests (CLI, No Sandbox) 被本次 CI 配置跳过——与首轮相同，如实披露，不掩饰。 测试证据
该 head 上已触发 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — every review finding answered with discriminating tests and verified fixed at this head; the one open item is the live end-to-end TUI behaviour, which the in-flight Stepping back: this re-run was the right call. The first pass reviewed a head that the If I'm maintaining this in six months, the comments still answer the only question that matters at each site — why the stamp, bridge, or key has to sit exactly there — and the stale doc pointers the review flagged are cleaned up rather than left as traps. No bloat, no drive-bys. The honest remainder: the suite is green but the integration leg is skipped by this CI profile, and the end-to-end claim (approval surfaces in the dialog, answer resumes the descendant) rests on wiring tests plus the in-flight Verdict: approve. All checks are complete and green on the reviewed commit, so no deferred approval is needed. 中文说明置信度：4/5 —— 所有 review 发现均已以判别性测试回应，并在当前 head 核实修复；唯一未落定的是端到端 TUI 实时行为，将由正在运行的 回顾：这次重跑是必要的。首轮审查的 head 随后被 六个月后维护这段代码时，每处注释仍回答着唯一重要的问题——为什么这个标记/桥接/键必须放在这里——review 指出的过期文档指针也已清理而非留成陷阱。没有膨胀，没有夹带。诚实的保留项：套件全绿，但集成测试被本次 CI 配置跳过；端到端声明（审批浮现在对话框、应答恢复后代）目前依托接线测试与在途的 结论：批准。审查 commit 上所有检查已完成且全绿，无需延迟批准。 — Qwen Code · qwen3.8-max 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.
LGTM, looks ready to ship — CI landed green after the review. ✅
|
@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. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
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.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
— qwen3.8-max via Qwen Code /review (v0.22.0)
| let id = startAgentId ?? undefined; | ||
| for (let hops = 0; id !== undefined && hops < 16; hops++) { |
There was a problem hiding this comment.
[Critical] The 16-hop cap on the lineage walk is below the supported nesting depth, reintroducing the hang this PR fixes.
maxSubagentDepth is user-configurable up to MAX_SUBAGENT_DEPTH_LIMIT = 100 (config.ts:1500-1522), but findBackgroundedAncestorAgentId stops after examining 16 entries. On a deep lineage the walk exhausts its hops before reaching the backgrounded ancestor and returns undefined, so no approval bridge is wired: the nested scheduler inherits the bubble-mode policy (getShouldAvoidPermissionPrompts() === false), emits TOOL_WAITING_APPROVAL on the invocation emitter nothing listens to, and the enclosing agent hangs silently forever with zero pending approvals shown in the Background-tasks dialog — the exact defect class this PR sets out to fix. The cap's comment says its purpose is only cycle protection, which a visited set serves without encoding a depth assumption.
Reproduced with a probe on the unmodified PR commit, sweeping a registry chain of k foreground entries under a backgrounded running root:
PR (unmodified): k=1..15 -> "bg-root"; k=16..20 -> undefined; firstMiss=16
FIXED (visited set): k=1..20 -> all "bg-root"; firstMiss: none; cyclic-lineage case still returns undefined in 0ms
Default depth (5) is unaffected, so this only bites under a supported non-default configuration — but it fails silently there.
| let id = startAgentId ?? undefined; | |
| for (let hops = 0; id !== undefined && hops < 16; hops++) { | |
| const seen = new Set<string>(); | |
| let id = startAgentId ?? undefined; | |
| while (id !== undefined && !seen.has(id)) { | |
| seen.add(id); |
— qwen3.8-max via Qwen Code /review (v0.22.0)
| confirmationDetails: event.confirmationDetails, | ||
| respond: event.respond, | ||
| at: event.timestamp, | ||
| ...(options?.nestedSource ? { subagentId: event.subagentId } : {}), |
There was a problem hiding this comment.
[Critical] The bridge auto-rejects a parked approval on duplicate TOOL_WAITING_APPROVAL re-emissions — any batch-mate status transition silently cancels the waiting call.
onWaiting treats every false from addPendingApproval as "entry gone/terminal" and answers with REJECTED_OUTCOME. But addPendingApproval also returns false for the intentional duplicate-callId case (background-tasks.ts:1065, pinned by the "ignores a duplicate callId" test). The scheduler re-notifies the whole batch on every status transition, and agent-core re-emits TOOL_WAITING_APPROVAL for every still-awaiting call without dedup (agent-core.ts:1894), so a sibling transition (e.g. an auto-allowed ReadFile moving to scheduled) re-emits for the parked call → duplicate → auto Cancel. Because the auto-reject is the first respond, the runtime's responded set then no-ops the user's actual dialog answer.
Concrete trigger: interactive session, bubble-mode background agent, nested foreground agent beneath it, and a batch [Shell, ReadFile] where Shell needs confirmation — Shell parks, ReadFile transitions, and Shell is silently cancelled while its prompt is visible in the dialog. Bubble-mode approvals survive only when the confirmation-requiring call is the sole call in its batch. The workflow bridge already handles this known re-emission correctly (seenSources + tri-state park result that drops duplicates, workflow-run-registry.ts:620-629).
Reproduced with a probe on the unmodified PR commit (real BackgroundTaskRegistry + bridgeApprovalEvents(..., { nestedSource: true }), re-emitting the parked callId):
PR (unmodified): AssertionError: expected "spy" to not be called at all, but actually been called 1 times — 1st spy call: Array ["cancel"]
PR (one-line fix dropping already-parked callIds silently): probe flips; all 124 existing background-tasks tests still pass
The reject branch predates this PR (it is also latent for the background agent's own emitter), but this diff newly routes nested approvals into it. A tri-state fix in onWaiting — e.g. addPendingApproval returns 'parked' | 'duplicate' | 'unparkable', or onWaiting checks entry.pendingApprovals?.some(a => a.callId === event.callId) before responding — would cover both.
— qwen3.8-max via Qwen Code /review (v0.22.0)
| /** | ||
| * Walks a foreground launch's registry lineage (parentAgentId links) to the | ||
| * nearest BACKGROUNDED, still-running ancestor entry. |
There was a problem hiding this comment.
[Suggestion] The new function was inserted between resolveSubagentApprovalMode's existing JSDoc and the function itself, orphaning that doc block: it now reads as a preamble to this unrelated function, and the exported resolveSubagentApprovalMode — whose resolution rules (permissive parent modes win; auto-edit fallback) are documented only there — silently loses its API documentation for readers and doc tooling. Move findBackgroundedAncestorAgentId (with its JSDoc) either above the /** Resolves the effective permission mode… */ block or below resolveSubagentApprovalMode's definition so each doc block stays adjacent to its function. No behavior impact — a pure reorder.
— qwen3.8-max via Qwen Code /review (v0.22.0)
| {selectedApproval?.kind === 'agent' && | ||
| selectedApproval.approval.subagentId !== undefined && ( |
There was a problem hiding this comment.
[Suggestion] This new UI branch — the sole read site of BackgroundApproval.subagentId — has no test: 'from nested agent' matches only the 9 locale files and this component, and the only subagentId fixtures in BackgroundTasksDialog.test.tsx are WorkflowApproval objects feeding the kind: 'workflow' path. One-line mutations survive with the entire suite green (flip the condition to === undefined, or read a renamed property), and the from nested agent: <id> line then silently stops rendering — so a user answering a bridged approval can no longer tell which descendant is blocked, losing exactly the UX this PR adds. Add a dialog test that enters detail mode on an entry whose pending approval carries subagentId: 'review-agent-abc123' and asserts the frame contains from nested agent: review-agent-abc123, plus the negative case (no subagentId → line absent).
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const bgConfig = activeAgentConfig; | ||
| bgConfig.getShouldAvoidPermissionPrompts = () => !shouldBubble; |
There was a problem hiding this comment.
[Suggestion] The new resume test only exercises the auto-deny outcome. The shouldBubble === true branch — a resumed bubble-mode agent in an interactive session — is untested on the resume path (grep bubble over background-agent-resume.test.tsx finds nothing; the harness pins isInteractive: () => false), while the launch path's equivalent is covered in agent.test.ts. As it stands, mutating () => !shouldBubble to () => true fails no test, and a resumed bubble-mode agent (and every nested launch under it) would silently auto-deny confirmations that should bubble to the Background-tasks dialog. Mirror the launch-path bubble test for resume: interactive harness config + subagentConfig.approvalMode = 'bubble', asserting bgConfig.getShouldAvoidPermissionPrompts() is false and inherited through Object.create(bgConfig).
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #9793Commit: Feedback dispositions[rc:3838430885] Critical — 16-hop cap on the lineage walk is below the supported nesting depth → Fixed. [rc:3838430888] Critical — bridge auto-rejects a parked approval on duplicate [rc:3838430890] Suggestion — orphaned JSDoc on [rc:3838430892] Suggestion — no test for the [rc:3838430894] Suggestion — resume path [rv:5002317158] Review state CHANGES_REQUESTED — "Partially reviewed — gaps disclosed. Not reviewed: build-and-test — Integration Tests" → Addressed by local verification; no code change applies. Mutation probes (each new guard witnessed)
Changed files
VerificationCommands actually run and their results:
中文说明Autofix 审查轮次 — PR #9793提交： 反馈处理结果[rc:3838430885] Critical — 血缘回溯的 16 跳上限低于受支持的嵌套深度 → 已修复。 [rc:3838430888] Critical — 桥接层在重复的 [rc:3838430890] Suggestion — [rc:3838430892] Suggestion — [rc:3838430894] Suggestion — resume 路径的 [rv:5002317158] 审查状态 CHANGES_REQUESTED — "Partially reviewed — gaps disclosed. Not reviewed: build-and-test — Integration Tests" → 以本地验证回应；无适用的代码修改。 变异探针（每个新守卫均有见证）
变更文件
验证（Verification）实际执行的命令及结果：
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/模型 |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Not reviewed: coverage — no plan was given, so this run cannot show that any of the diff was read.
— qwen-max via Qwen Code /review (v0.21.10)
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. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
— qwen3.8-max via Qwen Code /review (v0.22.0)
| if ( | ||
| this.getPendingApprovals(agentId).some( | ||
| (a) => a.callId === event.callId, | ||
| ) | ||
| ) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
[Suggestion] This guard re-runs the exact dedup predicate addPendingApproval already evaluated (~line 1065), because that method returns a single boolean conflating "duplicate callId" with "entry gone/terminal". The sibling registry already models the distinction — workflow-run-registry.ts's parkPendingApproval returns a discriminated result and its bridge switches on it. Today the two copies agree, but if addPendingApproval's dedup key ever widens — e.g. to include the source runtime, now that multiple nested agents can park onto one shared ancestor entry — this caller-side copy stays behind, and a re-emitted event for a still-parked call falls through to REJECTED_OUTCOME again: silently auto-rejecting a call whose prompt is visible in the dialog, the exact regression this PR fixes. Giving addPendingApproval a discriminated result ('parked' | 'duplicate' | 'unavailable', mirroring parkPendingApproval) lets this branch delete the second scan.
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // createApprovalModeOverride, so the stamp leaks nowhere. | ||
| const subagentRuntimeConfig = agentConfig; |
There was a problem hiding this comment.
[Suggestion] This stamp and its rationale comment are pasted nearly verbatim into two files — here and in background-agent-resume.ts — even though this seam already shares helpers: the resume path imports createApprovalModeOverride from this file, and rebuildToolRegistryOnOverride exists explicitly to avoid duplicated rebuild logic between these same two callers. The cost is demonstrated, not hypothetical: the earlier wrapper-only placement was wrong in both copies at once, and this PR had to fix both in lockstep. The next policy adjustment has to remember both sites again; missing one leaves one path with a nested scheduler resolving Config.prototype's default false — the unanswerable-approval hang this PR fixes, reintroduced on that path only. Consider extracting a shared stampBackgroundPromptPolicy(config, shouldBubble) next to createApprovalModeOverride, carrying the rationale comment once and leaving each caller only its shouldBubble computation.
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // Config.prototype's `false` and hanging on unanswerable approvals. | ||
| // activeAgentConfig is created per-resume by | ||
| // createApprovalModeOverride, so the stamp leaks nowhere. | ||
| const bgConfig = activeAgentConfig; |
There was a problem hiding this comment.
[Suggestion] With this line the last Object.create wrapper of the background path disappears, but two production JSDocs still cite bgConfig = Object.create(agentConfig) "from the background path" as their live example of wrapper-on-wrapper layering: hasRebuiltToolRegistry (agent.ts:459-462) and buildSubagentContextOverride (subagent-manager.ts:990-994). A reader reasoning about the skip-rebuild marker — or about where to stamp a per-agent config policy, exactly this PR's concern — is pointed at a construct this diff deleted, greps for it, finds nothing, and must reconstruct from git history whether a wrapper layer exists before trusting either comment. Update those examples to the current shape (the stamped createApprovalModeOverride override is passed directly), or drop the parenthetical.
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const cleanupNestedApprovalBridge = approvalAncestorId | ||
| ? registry.bridgeApprovalEvents(approvalAncestorId, this.eventEmitter, { | ||
| nestedSource: true, | ||
| }) | ||
| : undefined; |
There was a problem hiding this comment.
[Suggestion] Both existing bridgeApprovalEvents call sites gate the wiring on bubbling (shouldBubble ? ...) to keep the emitter free of dead listeners, matching the method's JSDoc ("only wire this up when the agent opted into permission bubbling"). This new nested bridge wires unconditionally whenever an ancestor exists. Under a non-bubble (auto-deny) backgrounded ancestor, the nested launch inherits prompt-avoidance through the very config chain this PR stamps, so no TOOL_WAITING_APPROVAL can fire there and the subscription is dead until the finally cleanup — and the call site now contradicts the documented wiring contract, so a future reader cannot tell whether the unconditionality is deliberate defense-in-depth or an oversight. Either gate like the siblings, or keep it unconditional and say so in one line, relaxing the method JSDoc to match.
| const cleanupNestedApprovalBridge = approvalAncestorId | |
| ? registry.bridgeApprovalEvents(approvalAncestorId, this.eventEmitter, { | |
| nestedSource: true, | |
| }) | |
| : undefined; | |
| const cleanupNestedApprovalBridge = | |
| approvalAncestorId && !this.config.getShouldAvoidPermissionPrompts() | |
| ? registry.bridgeApprovalEvents( | |
| approvalAncestorId, | |
| this.eventEmitter, | |
| { nestedSource: true }, | |
| ) | |
| : undefined; |
— qwen3.8-max via Qwen Code /review (v0.22.0)
| if ( | ||
| this.getPendingApprovals(agentId).some( | ||
| (a) => a.callId === event.callId, | ||
| ) | ||
| ) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
[Suggestion] This drop branch is the approval lifecycle's quietest leg: it writes no log, while the park leg logs at info and the unparkable-reject leg logs failures at error. Per its own comment, re-emissions of already-parked calls are the expected case (the scheduler re-notifies the whole batch on every status transition), so this branch runs constantly while any approval is parked. When the next debug session is "nested agent's approval never appeared / the user clicked Approve and nothing happened", the log shows one park line and zero errors — identical to the event never arriving, or being lost by a torn-down listener; nobody can distinguish "re-emitted and correctly dropped" from "never delivered" without reproducing the exact batch-transition timing. One debug-level line before the return closes the gap (debug, not info, because re-emissions are frequent):
| if ( | |
| this.getPendingApprovals(agentId).some( | |
| (a) => a.callId === event.callId, | |
| ) | |
| ) { | |
| return; | |
| } | |
| if ( | |
| this.getPendingApprovals(agentId).some( | |
| (a) => a.callId === event.callId, | |
| ) | |
| ) { | |
| debugLogger.debug( | |
| `Dropped re-emitted approval event for already-parked call ${agentId}/${event.callId}`, | |
| ); | |
| return; | |
| } |
— qwen3.8-max via Qwen Code /review (v0.22.0)
- addPendingApproval returns a discriminated result ('parked' |
'duplicate' | 'unavailable') mirroring the workflow registry's
parkPendingApproval, so the bridge drops re-emitted events without
re-running the dedup scan on the caller side
- log a debug line when a re-emitted approval event is dropped, so
"approval never appeared" sessions can tell a drop from a lost event
- extract stampBackgroundPromptPolicy next to createApprovalModeOverride;
both the launch and resume paths now share one stamp + rationale
- gate the nested approval bridge on the inherited prompt-avoidance
policy like the sibling bridges, instead of wiring a dead subscription
under auto-denying ancestors
- refresh two JSDoc examples that still cited the deleted
Object.create wrapper
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #9793 (round 2 feedback)Commit: Feedback points and dispositions[rv:5002663179] (COMMENTED, qwen-max via /review): review-process disclosure — "could not certify that any of this diff was reviewed" (no coverage plan supplied for that run). No code findings. → No action (informational; nothing checkable to reproduce or fix). [rv:5002734289] (COMMENTED, qwen3.8-max via /review): partial-review disclosure — build-and-test gap noted; its suggestions are the five inline findings below. → No action on the disclosure itself; the inline findings are addressed below. [rc:3838885111] R2-1 — discriminated result for [rc:3838885112] R2-2 — shared [rc:3838885116] R2-3 — stale [rc:3838885121] R2-4 — unconditional nested bridge wiring → Implemented (gate option). Verified the claim: the scheduler auto-denies before [rc:3838885127] R2-5 — silent drop branch → Implemented. The duplicate-drop branch now logs No findings were declined, deferred, or escalated this round. Changes
Conflict notes
Verification
中文说明Autofix 审查轮次 — PR #9793（第 2 轮反馈）提交： 反馈点及处理[rv:5002663179]（COMMENTED，qwen-max 经 /review）：审查流程披露 —— "无法证实本 diff 有任何部分被审查"（该次运行未提供覆盖计划）。无代码发现。→ 无需处理（纯信息性；没有可复现或可修复的检查项）。 [rv:5002734289]（COMMENTED，qwen3.8-max 经 /review）：部分审查披露 —— 指出 build-and-test 缺口；其建议即下方五条 inline 发现。→ 披露本身无需处理；inline 发现已在下方处理。 [rc:3838885111] R2-1 — [rc:3838885112] R2-2 — 共享的 [rc:3838885116] R2-3 — 过期的 [rc:3838885121] R2-4 — 嵌套桥接无条件接线 → 已实现（选择门控方案）。 已核实该论断：当 [rc:3838885127] R2-5 — 静默丢弃分支 → 已实现。 重复丢弃分支现在在提前返回前以 debug 级别输出 本轮没有拒绝、延后或升级任何发现。 变更
冲突说明
验证
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.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Convergence: round 3 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 5 (5 new). Findings keep coming back to the same files: packages/core/src/agents/background-tasks.ts (findings in round 2; 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.)
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // rejection (frames torn down post-termination) must not escape as | ||
| // an unhandledRejection. | ||
| if (!parked) { | ||
| if (parked === 'duplicate') { |
There was a problem hiding this comment.
[Critical] The duplicate branch keys on callId alone, but this PR's nested bridge now parks approvals from multiple runtimes (the entry's own plus every nested descendant) onto one Background-tasks entry, and generated callIds are only unique per conversation: nextGeneratedId hands every fresh conversation call_qwen_1 for its first id-less tool call, and normalizeModelToolCallIds runs upstream of the ${name}-${randomUUID()}-${index} fallback in processFunctionCalls, so that fallback never fires on this path. When a bubbling background agent runs two nested sub-agents whose first id-less tool calls both need confirmation, both events arrive as call_qwen_1 on the same ancestor entry; the first parks and the second is dropped here with a debug log and no respond, so the second nested call waits forever with nothing in the Background-tasks dialog to answer — the same silent-hang class this PR fixes. A probe against this commit (two fresh-conversation runtimes bridged onto one entry) confirms it:
PROBE-2 parked approvals: 1
PROBE-2 parked callIds/subagentIds: call_qwen_1/search-agent-aaa111
PROBE-2 sub-agent B respond() called: 0 time(s)
drop logs: ["Dropped re-emitted approval event for already-parked call bg-ancestor/call_qwen_1"]
Key identity on (subagentId, callId) as workflow-run-registry.ts already does: include subagentId in the duplicate predicate (own approvals stay unstamped, so same-call re-emissions still dedupe while different runtimes never collide), and extend the composite key to resolvePendingApproval and clearPendingApproval, which also match by callId alone today — a collision can otherwise let one resolution or TOOL_RESULT clear another runtime's parked prompt. With the composite key the probe flips to two parked approvals and no drops, and the existing 125-test suite still passes.
— qwen3.8-max via Qwen Code /review (v0.22.0)
Generated tool-call ids (`call_qwen_N`) are only unique per conversation, so two nested sub-agents whose first id-less call needs confirmation both arrive under the same callId on their shared background ancestor entry. The parked-approval queue keyed identity on callId alone: the second runtime's prompt was dropped as a "duplicate" with nothing in the dialog to answer, and a resolution or TOOL_RESULT for one runtime could clear another runtime's parked prompt. Key identity on (subagentId, callId) — the same composite the workflow run registry already uses — across addPendingApproval's duplicate predicate, resolvePendingApproval, and clearPendingApproval. Own approvals stay unstamped, so same-call re-emissions from the entry's own runtime still dedupe while different runtimes never collide.
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Review round summary — PR #9793Feedback points and decisions
ChangesKey identity on
No new state, maps, or guards were added — the round consolidates three callId-only match sites onto the existing composite-key shape, which also addresses the round-3 convergence observation that this file kept producing sibling findings. Mutation probes (each guard witnessed)
All four restored; focused suites re-ran to green on the committed tree. Conflict notes
Verification
Commit: 中文说明审查轮次总结 — PR #9793反馈点与处理决定
改动内容将身份键改为
没有新增任何状态、Map 或守卫 —— 本轮是将三处仅以 callId 匹配的位置整合到既有的复合键形态上，这也回应了第 3 轮关于该文件不断产生同类反馈的观察。 变异探针（每个守卫均有见证）
四处均已还原；聚焦测试套件在已提交的代码树上重新运行并全部通过。 冲突说明
验证
提交： 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.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Convergence: round 4 posted 4 inline comment(s), 4 of them reported for the first time; the previous round posted 1 (1 new). Findings keep coming back to the same files: packages/core/src/agents/background-tasks.ts (findings in round 3; 1 more now). The rate of new findings is not falling. 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. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. No Critical finding is open on this round, so merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending — a merged pull request cannot diverge further. (Observation only — nothing was withheld from this review because of this observation.)
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // approvals must stay unstamped — the runtime subagentId and the | ||
| // registry agentId use different suffixes, so the bridge caller | ||
| // declares the nested case rather than anyone comparing ids. | ||
| const makeWaiting = (callId: string, subagentId: string) => ({ |
There was a problem hiding this comment.
[Suggestion] This diff adds the same waiting-event factory twice in this one file: the module-level makeWaitingEvent(subagentId, callId, respond?) near line 79 (used by the three collision tests), and this test-local makeWaiting(callId, subagentId) whose body is identical except the two string parameters are swapped and respond is hardcoded; the re-emission test (~line 2838) also builds the same shape as an inline literal even though makeWaitingEvent already accepts a respond parameter. Both parameters are plain strings, so mixing up the (subagentId, callId) and (callId, subagentId) conventions compiles silently — in exactly the tests that pin the collision semantics this PR exists to protect, and any future field added to AgentApprovalRequestEvent must be added in two or three places. Consider deleting this local helper and calling makeWaitingEvent('fork-runtime1', 'own-1') / makeWaitingEvent('review-agent-abc123', 'nested-1'), and replacing the re-emission test's inline literal with makeWaitingEvent('bg-appr-reemit', 'c1', respond).
— qwen3.8-max via Qwen Code /review (v0.22.0)
| makeWaitingEvent('search-agent-bbb222', 'call_qwen_1', respondB), | ||
| ); | ||
|
|
||
| const ok = await registry.resolvePendingApproval( |
There was a problem hiding this comment.
[Suggestion] This collision-resolve test only resolves the FIRST-parked of the two colliding entries (search-agent-aaa111), so the subagentId conjunct in resolvePendingApproval's find is not pinned. Applying the mutant that finds by callId alone (keeping the composite-key removal filter) leaves the whole suite green (Tests 128 passed). Under that mutant, answering the second runtime's prompt in the dialog resumes the FIRST runtime instead, and the removal filter strips the second runtime's prompt without ever invoking its respond — the call then waits forever, the exact cross-runtime collision this PR fixes. Add a case (or a sibling test) that resolves the second-parked entry: park aaa111 then bbb222 under call_qwen_1, resolve with 'search-agent-bbb222', and assert respondB was called once, respondA not called, and the remaining parked entry is search-agent-aaa111. The discriminating test fails under the mutant (respondB … got 0 times) and passes on the real code (129 tests).
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // the runtime's responded set then no-ops the user's real answer. | ||
| const emitter = new AgentEventEmitter(); | ||
| registry.register(makeRegistration('bg-appr-reemit')); | ||
| registry.bridgeApprovalEvents('bg-appr-reemit', emitter); |
There was a problem hiding this comment.
[Suggestion] The silent re-emission drop ('duplicate') is only tested through the entry's OWN bridge, where the key part is subagentId === undefined; the stamped (nested) dedup case the composite key introduces has no test. A mutant that dedupes on approval.subagentId === undefined instead of comparing the parked and incoming subagentIds behaves identically in every existing test (verified: the mutant suite passes 128/128). The discriminating input is real: a nested runtime's scheduler re-emits TOOL_WAITING_APPROVAL on sibling status transitions too, so while a nested approval is parked its event can arrive again on the same nested bridge — and under that mutant the re-emission parks a second copy, double-listing the same nested call in the dialog and inflating the pending count. Consider adding a case that bridges with { nestedSource: true }, emits the same (subagentId, callId) event twice, and asserts the parked list stays at length 1 with respond untouched; that probe fails under the mutant (expected [ …(2) ] to have a length of 1 but got 2) and passes on the real code.
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // user's real answer. Debug level because re-emissions are | ||
| // frequent while any approval is parked. | ||
| debugLogger.debug( | ||
| `Dropped re-emitted approval event for already-parked call ${agentId}/${event.callId}`, |
There was a problem hiding this comment.
[Suggestion] None of the approval-lifecycle log lines carry the new subagentId key part — this drop line, the Parked approval … info line (~1086), the resolve-failure error (~1129), and the reject-failure error (~1222) all log only agentId/callId, even though the runtime id is in scope at every site. When a backgrounded ancestor hosts two nested runtimes that both park their first id-less call under the generated call_qwen_1 (the collision scenario this round's tests exercise), incident analysis of "the user answered call_qwen_1 and the wrong agent resumed" starts from these lines — and gets two byte-identical entries, with nothing attributing a parked prompt or a resolution to either runtime. Consider appending the runtime id when present, and likewise on the park/resolve/reject lines.
| `Dropped re-emitted approval event for already-parked call ${agentId}/${event.callId}`, | |
| `Dropped re-emitted approval event for already-parked call ${agentId}/${event.callId}` + | |
| (options?.nestedSource ? ` (nested ${event.subagentId})` : ''), |
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round — no action taken (PR #9793)Growth audit (required this window): The counting window is over its test-line budget (473 net test lines vs 400; source 62 vs 400). The audit verdict is sound — both axes pass (
Feedback: no actionable feedback this round — the Reviews, Inline comments, and Issue-level comments sections are empty, and there are no failed or still-red checks. The deferred non-Critical items (one review and four inline findings, all on test files or the duplicate-handling branch) are excluded from this round by the growth-only engagement and remain open for human follow-up. Result: no commits this round. The 中文说明Autofix 轮次 — 未采取任何行动（PR #9793）增长审计（本窗口必需）： 本计数窗口的测试行净增长已超出预算（测试净增 473 行，预算 400；源码 62 行，预算 400）。审计结论为 sound（合理） — 两个维度均通过（见
反馈： 本轮没有可处理的反馈 — Reviews、Inline comments、Issue-level comments 各区域均为空，也没有失败或持续失败的检查。被延后的非 Critical 条目（一条 review 和四条行内发现，均位于测试文件或 duplicate 处理分支）因纯增长触发模式被排除在本轮之外，保持开放留待人工跟进。 结果： 本轮无提交。 Deferred non-Critical feedbackCritical-only mode is active: the PR's diff grew src 62 / test 473 net lines beyond this counting window's baseline (budgets: 400/400). The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback continues to flow unaffected during a growth-only engagement (the per-author batch budget applies only after 5 change-producing rounds). (
中文说明已进入仅处理 Critical 的模式：本计数窗口内 diff 净增长已达 源码 62 / 测试 473 行（预算 400/400）。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。纯增长触发期间维护者反馈照常流动（按作者的批次预算仅在完成 5 个产生改动的轮次后生效）。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #9793Commit: Feedback dispositionsImplemented this round (round-4 findings against the current HEAD):
Already resolved in earlier commits of this PR (re-verified against current HEAD, no change needed):
Declined / Deferred / Escalated: none. Mutation probes (new tests must fail when the guarded behavior is broken)
Notes
Verification
中文说明Autofix 审查轮次 — PR #9793提交： 反馈处理本轮实现（针对当前 HEAD 的第 4 轮发现）：
已由本 PR 早前提交解决（对当前 HEAD 复核确认，无需改动）：
拒绝 / 延后 / 升级人工： 无。 变异探针（新测试必须在受保护行为被破坏时失败）
说明
验证
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.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
packages/core/src/agents/background-tasks.ts:1078 — [review] composite identity predicate inlined four times; extract one helperpackages/core/src/agents/background-tasks.ts:1059 — [review] stale JSDoc asserts callId uniqueness after composite-keying rewrite
Convergence: round 5 posted 2 inline comment(s), 2 of them reported for the first time; the previous round posted 4 (4 new). Findings keep coming back to the same files: packages/core/src/agents/background-tasks.test.ts (findings in round 4; 1 more now); packages/core/src/agents/background-tasks.ts (findings in round 4; 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. No Critical finding is open on this round, so merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending — a merged pull request cannot diverge further. (Observation only — nothing was withheld from this review because of this observation.)
— qwen3.8-max via Qwen Code /review (v0.22.0)
| `Failed to resolve background approval for ${agentId}/${callId}` + | ||
| (subagentId ? ` (nested ${subagentId})` : '') + | ||
| ':', |
There was a problem hiding this comment.
[Suggestion] The (nested …) attribution suffix this diff adds to the approval-lifecycle logs stops at this line: the fail() reason immediately below it (Failed to resolve background approval: ${callId}, ~line 1136) and the auto-reject error in rejectPendingApprovals (Failed to auto-reject parked approval …, ~line 1868) both name only the callId. Since generated callIds (call_qwen_1) now legitimately park several approvals on one entry, a failed resolve or auto-reject of one of a colliding pair can no longer be attributed to a runtime from the entry's failure reason or that error log — the same attribution gap these suffixes were added to close (the nested re-emit test's own comment cites exactly this incident-analysis need). Extending both is mechanical:
this.fail(
agentId,
`Failed to resolve background approval: ${callId}` +
(subagentId ? ` (nested ${subagentId})` : ''),
);
// and in rejectPendingApprovals:
`Failed to auto-reject parked approval ${entry.agentId}/${approval.callId}` +
(approval.subagentId ? ` (nested ${approval.subagentId})` : '') +
':',— qwen3.8-max via Qwen Code /review (v0.22.0)
| const ok = await registry.resolvePendingApproval( | ||
| 'bg-collide-resolve-2', | ||
| 'call_qwen_1', |
There was a problem hiding this comment.
[Suggestion] The collision tests never exercise the unstamped (own-entry, subagentId === undefined) side against a stamped collision on resolvePendingApproval/clearPendingApproval. The production resolver passes approval.subagentId, which is undefined for the entry's own approvals (BackgroundTasksDialog.tsx:1545), so a future regression relaxing the conjunct to (subagentId === undefined || a.subagentId === subagentId) ships green — and in production, with a nested approval parked before the ancestor's own same-callId approval, answering the ancestor's own prompt would resume the nested runtime and strip the nested prompt without invoking its respond: the ancestor's own call waits forever, re-manifesting exactly the silent hang this PR fixes. A mutant probe on this commit confirms the gap:
mutant + PR suite: Tests 130 passed (130)
mutant + mirror tests: Tests 2 failed | 130 passed (132)
correct code + mirror: Tests 132 passed (132)
Two mirror-image cases close it: (1) park an unstamped own approval plus a stamped nested one under the same callId (nested parked first), resolve with undefined, and assert the unstamped entry's respond ran while the stamped one stayed parked; (2) same setup, emit TOOL_RESULT on the own bridge and assert only the unstamped prompt clears.
— qwen3.8-max via Qwen Code /review (v0.22.0)
…d collision tests (#9793)
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Round summary (PR #9793, round 6)Two inline Suggestions from the automated reviewer; both addressed in code. No Critical findings open, no failed checks. Commit: Feedback dispositions
Changes
Mutation probes (witness verification)
Conflict notes
Verification
中文说明本轮总结（PR #9793，第 6 轮）自动化审阅者提出了两条内联 Suggestion，均已在代码中处理。本轮没有未解决的 Critical 发现，也没有失败的检查。提交： 反馈处理结果
改动内容
变异探针（见证验证）
冲突说明
验证
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.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round:
packages/core/src/agents/background-tasks.ts:1231 — [probe] No test covers the nestedSource:true case of bridgeApprovalEvents' unavailable branch — a one-line mutant survives the suite
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
@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: 701 passed · 0 failed · 701 total Flakiness gate: ✅ 4 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：701 通过 · 0 失败 · 701 总计 抖动门：✅ 4 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #9793 verification — fix(core): surface nested sub-agent approvals under background parentsVerdict: 中文摘要
Central claim + A/BCentral claim: a tool call needing confirmation inside a nested sub-agent under a backgrounded ancestor no longer hangs on a Control build: Cell 1 — policy stamp placement (
|
| cell | oracle | result |
|---|---|---|
| head | stamp on agentConfig (the createApprovalModeOverride receiver the rebuilt registry binds to) |
9/9: tool-bound config true, nested override true, derived Object.create true, no leak to parent, bubbling stays false |
| base | stamp on Object.create(agentConfig) wrapper (exact removed lines of HEAD^1) |
6/6 expected-bug: wrapper true but tool-bound config false, nested override false — the silent-hang precondition |
Both arms share the real createApprovalModeOverride + rebuildToolRegistryOnOverride (asserted hasRebuiltToolRegistry(agentConfig) === true on both), so the only delta is stamp placement.
Cell 2 — bridged-approval identity & collisions (h2-bridge-matrix.mjs, witness 02-bridge-matrix-ab-head-vs-base.png)
Real BackgroundTaskRegistry + real AgentEventEmitter; two nested runtimes whose first id-less calls share call_qwen_1, a scheduler re-emission, a TOOL_RESULT, a dialog answer, a late approval on a completed entry.
| cell | head | base |
|---|---|---|
| re-emission of a parked call | dropped, live prompt untouched (0 respond calls) | rejects the live prompt (cancel observed) |
| second runtime, colliding callId | parked as separate prompt (pending=2) | dropped + silently rejected |
| TOOL_RESULT for runtime a-1 | clears only a-1 (a-2 survives) | wipes all prompts sharing the callId |
| dialog answer for a-2 | resolves exactly a-2, queue empties | nothing left to resolve (already wiped) |
| approval after completion | rejected, not hung (both arms) | same |
| own (non-nested) bridge | unstamped, resolves with undefined identity | parks (pre-existing path intact) |
Head 13/13, base 8/8 (base cells assert the bugs reproduce). Observed respond() traffic: head = only legitimate user answers + the expected late rejection; base = two spurious cancels.
Cell 3 — lineage walker (h3-walker.mjs, witness 03-walker-head.png)
Head-only export; real registry entries via public register(). 10/10: two-hop and one-hop walks, self-resolution, terminal ancestor → undefined, broken lineage, unknown/null/undefined start, cyclic lineage terminates, 10-deep supported lineage.
Mutation matrix (witness 04-mutation-matrix.png)
Scratch worktree at the merge commit, vitest on TS source. Unmutated control: 458/458 green. Every revert landed in the same file as the mutants; positive controls included.
| # | mutation | suite | result |
|---|---|---|---|
| — | none (control) | agent+tasks+resume | 458/458 pass |
| M1 | launch stamp → detached wrapper (base bug shape) | agent.test.ts | killed, 4 red (incl. both policy tests; failure mode is a TypeError on the mock chain — see N1) |
| M2 | resume stamp → detached wrapper | resume.test.ts | killed, 2 red |
| M3 | nested bridge removed | agent.test.ts | killed, 1 red, clean expected spy called 1, got 0 |
| M4 | dedup identity → callId only | tasks.test.ts | killed, 5 red, clean expected length 3, got 1 |
| M4b | resolve identity → callId only | tasks.test.ts | killed, exactly the 2 resolve-collision tests |
| M7 | nestedSource subagentId stamp removed | tasks.test.ts | killed, 8 red (stamp + full collision cascade) |
| M8 | duplicate re-emission rejected (base behavior) | tasks.test.ts | killed, exactly the 2 re-emission tests |
| M9 | dialog drops subagentId key part | dialog.test.tsx | killed, exactly routes an agent approval response with the nested subagentId key part |
| M5 | control: 'unavailable' → 'parked' |
tasks.test.ts | caught, 2 red |
| M6 | control: walker ignores terminal status | agent.test.ts | caught, exactly 1 red |
8/8 mutants killed, 2/2 controls caught, 0 survivors. M4 vs M4b attribute the composite key precisely: the dedup predicate is pinned by the park/clear cascade, the resolve filter by the two resolve tests (one of which survives M4b because callId-only find happens to hit the matching runtime first).
Targeted gates
| gate | result |
|---|---|
core agent.test.ts + background-tasks.test.ts + background-agent-resume.test.ts |
458/458 pass |
cli background-view/ (7 suites incl. 86-test BackgroundTasksDialog.test.tsx) |
181/181 pass |
tsc --noEmit core / cli |
clean / clean |
check-i18n (9 locales got from nested agent) |
all checks passed |
| eslint on the 5 changed production files | clean; gate proven live (planted probe → 2 errors, then removed) |
Findings
No blocking findings. Non-blocking observations:
- N1 (test-harness note, not a code defect): M1/M2 fail with
getShouldAvoidPermissionPrompts is not a functionbecause the vi-mocked config objects carry no prototype getter, so removing the own stamp yieldsundefinedrather thanfalse. The intended assertion line is reached and discriminates; the real-Config value mismatch (falsewhere head givestrue) is independently proven by the base arm of cell 1. Repro: scratch worktree at head, replacestampBackgroundPromptPolicy(agentConfig, shouldBubble)with a stamp onObject.create(agentConfig), runnpx vitest run src/tools/agent/agent.test.tsinpackages/core. - N2 (semantics note):
findBackgroundedAncestorAgentIdstops at the first backgrounded entry even when not running (returnsundefinedinstead of walking past a paused ancestor). Sound — a non-running agent cannot launch code — and pinned by W4. Repro:CORE_DIST=<head dist> node h3-walker.mjs, cell W4. - N3 (design note): parked approvals (own or nested) are resolvable only from the Background-tasks dialog; the STREAM_JSON control channel and ACP session resolve only workflow approvals. Pre-existing shape for own approvals; in non-interactive sessions nested calls auto-deny via the inherited policy, so nothing parks there. Not a regression. Repro:
grep -rn "resolvePendingApproval" packages/cli/src --include=*.tsshows the background registry is only resolved fromBackgroundTasksDialog.tsx.
Sibling sweep: the only remaining getShouldAvoidPermissionPrompts = site is the shared stampBackgroundPromptPolicy (agent.ts:742), used by both fixed paths; the memory subsystem's Object.create(config) wrappers never stamp the policy and inherit it through the chain — the fix's mechanism.
Not covered
- Per-commit attribution: the checkout is depth-2;
git rev-list HEAD^1..HEAD^2yields only the squashed head while the metadata lists 7 commits. The aggregateHEAD^1..HEADdiff is what was verified. - Live-model E2E of the original hang (a real nested sub-agent waiting on approval): no API credentials in this environment. The hang precondition (policy resolving
falseon the tool-bound chain) and its removal are proven against the compiled builds; the bridge/collision behavior against the real registry and emitter. - TUI screenshot of the dialog: dialog logic verified by the 181 cli tests (3 new: nested line rendered, omitted when unstamped, subagentId routed on resolve) plus the M9 kill.
- Repo-wide lint/test suites beyond the affected workspaces (the PR's own CI covers those).
Methodology
Environment: CI verify container (node:22-bookworm), merge-ref checkout. Harnesses (h1-policy-chain.mjs, h2-bridge-matrix.mjs, h3-walker.mjs in this artifact dir) import the compiled dist/ of the head tree and of a base worktree rebuilt for packages/core only; they are mock-free with respect to the units under test (real Config/createApprovalModeOverride, real BackgroundTaskRegistry, real AgentEventEmitter). The base worktree was removed after the cells were captured and rendered (recreate with git worktree add tmp/base-tree HEAD^1 + node ../../scripts/build_package.js in packages/core with root node_modules/.bin on PATH). Mutation matrix ran in a scratch worktree via vitest on TS source, each mutant reverted after its run (tree verified clean). Raw per-cell logs in raw/, harness JSON in results/, evidence PNGs in evidence/ rendered with scripts/verify-capture.mjs.
Flakiness gate log
rounds=5 files=4 skipped=0
file packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx: (cd packages/cli) npx --no-install vitest run ./src/ui/components/background-view/BackgroundTasksDialog.test.tsx
file packages/core/src/agents/background-agent-resume.test.ts: (cd packages/core) npx --no-install vitest run ./src/agents/background-agent-resume.test.ts
file packages/core/src/agents/background-tasks.test.ts: (cd packages/core) npx --no-install vitest run ./src/agents/background-tasks.test.ts
file packages/core/src/tools/agent/agent.test.ts: (cd packages/core) npx --no-install vitest run ./src/tools/agent/agent.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx: PPPPP
packages/core/src/agents/background-agent-resume.test.ts: PPPPP
packages/core/src/agents/background-tasks.test.ts: PPPPP
packages/core/src/tools/agent/agent.test.ts: PPPPP
verdict: pass
summary: 4 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/background-view/BackgroundTasksDialog.test.tsx: P (exit 0)
round 1 · packages/core/src/agents/background-agent-resume.test.ts: P (exit 0)
round 1 · packages/core/src/agents/background-tasks.test.ts: P (exit 0)
round 1 · packages/core/src/tools/agent/agent.test.ts: P (exit 0)
round 2 · packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx: P (exit 0)
round 2 · packages/core/src/agents/background-agent-resume.test.ts: P (exit 0)
round 2 · packages/core/src/agents/background-tasks.test.ts: P (exit 0)
round 2 · packages/core/src/tools/agent/agent.test.ts: P (exit 0)
round 3 · packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx: P (exit 0)
round 3 · packages/core/src/agents/background-agent-resume.test.ts: P (exit 0)
round 3 · packages/core/src/agents/background-tasks.test.ts: P (exit 0)
round 3 · packages/core/src/tools/agent/agent.test.ts: P (exit 0)
round 4 · packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx: P (exit 0)
round 4 · packages/core/src/agents/background-agent-resume.test.ts: P (exit 0)
round 4 · packages/core/src/agents/background-tasks.test.ts: P (exit 0)
round 4 · packages/core/src/tools/agent/agent.test.ts: P (exit 0)
round 5 · packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx: P (exit 0)
round 5 · packages/core/src/agents/background-agent-resume.test.ts: P (exit 0)
round 5 · packages/core/src/agents/background-tasks.test.ts: P (exit 0)
round 5 · packages/core/src/tools/agent/agent.test.ts: P (exit 0)
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 action taken (PR #9793)Inputs this round. One new review ( Disposition of each point:
No source or test files were changed this round and nothing was committed. 中文说明Autofix 审查轮次 —— 未采取行动（PR #9793）本轮输入。 一条新审查（ 每一点的处置：
本轮没有修改任何源码或测试文件，也没有提交任何内容。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 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.
LGTM, looks ready to ship. ✅
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
LGTM. Verified the three open Criticals and both root-cause fixes at head c3715af:
- Lineage walk no longer has a hop budget:
findBackgroundedAncestorAgentIdterminates by a visited set, so any supported nesting depth (maxSubagentDepth is configurable to MAX_SUBAGENT_DEPTH_LIMIT) reaches the backgrounded ancestor; a non-running backgrounded entry still yields nothing. The 16-hop truncation that would have reintroduced the hang is gone. addPendingApprovalis three-state (parked | duplicate | unavailable) with composite identity(subagentId, callId): scheduler batch-mate re-emissions of an already-parked call hitduplicateand are dropped at debug level without touching the parked prompt — no more silent auto-reject of a waiting call whose dialog is still visible — while a genuinely gone/terminal entry still auto-rejects so the reasoning loop can't block. Generatedcall_qwen_Nids being unique only per conversation is handled by the subagentId leg, and own approvals stay unstamped so same-call re-emissions from the entry's own runtime still dedupe.- The policy-sink break point is fixed at the source:
stampBackgroundPromptPolicystamps the config the rebuilt tool registry binds (both launch and resume paths), with the Object.create wrapper removed and tests pinning the policy on the tool-bound config and its derived chains in both directions — the nested scheduler can no longer resolve Config.prototype'sfalseand wait on an unanswerable event. The stamp sits on a freshly created override config, so it leaks nowhere. - The bridge is gated consistently (no bridging under an auto-denying ancestor where no TOOL_WAITING_APPROVAL can fire), double answers dedupe via the runtime's responded guard, and the nested waiter is named in the dialog with locale entries across all nine languages.
CI at approval time: 15 checks passing, none failing. The bot threads still show unresolved because no automated round has re-probed this head.
|
Released in v0.22.2. |
…traint (QwenLM#10168) * feat(review): carry the fix's premises beside its claim — add fixConstraint `fixWitness` (QwenLM#9596) pins what a suggested fix claims: the test that reds when the fix is removed. Nothing pinned what a fix assumes — the premises it newly introduces — and those are a different defect class that passes a witnessed test cleanly. Two Criticals on the merged nested-approval fix (QwenLM#9793) were both of this class and both had a fixWitness-grade test: a hand-picked `hops < 16` lineage cap below the user-configurable `MAX_SUBAGENT_DEPTH_LIMIT = 100` reintroduced the very hang the fix was for, and parking several runtimes' approvals on one registry entry broke a `callId` uniqueness dedup and resolve relied on elsewhere, routing a user's answer to the wrong agent. The agent that found each defect had already run the greps that name the fact, so the marginal cost of carrying it is close to zero. The finding gains one optional field beside `fixWitness`, carried the same way: `fixConstraint` — an existing fact the fix must not violate, with its source. It reaches the finder brief (`FINDING_FORMAT`), the skill's Step 6 list and Step 4 aggregate slot, the artifact field list, and Step 7's posting rule, so the human fixer reading the comment — the loop with no other brake — sees it. Two properties differ from the sibling field, and both are deliberate. It is omitted when nothing was observed, never `N/A`: an absent constraint carries no information and would lengthen every posted comment (QwenLM#9177). `validateFindings` normalises a literal `N/A`/`none` to absence so the poster keys on presence alone, and a finder that copies the fixWitness habit cannot hand it a placeholder to post. And its evidence bar is the `witness` bar, not the `fixWitness` bar — quote the constant or give the `file:line`, or omit — because the costs are asymmetric: a wrong fixWitness is one test not written; a wrong constraint is confidently-stated misdirection the fixer will follow. "Be careful about concurrency" is forbidden in the field the way "this looks risky" is forbidden in the failure scenario. Like fixWitness, the field never gates reporting. Pinned at every site: the validator (round-trip, snake_case, placeholder drop, and the drop's narrowness), the built finder brief (format line, omission clause, evidence bar, forbidden prose, claim/premise split), and the skill corpus (Step 6 item, aggregate slot, field list, posting rule, and the no-`N/A` silence clause). DESIGN.md records the incident. Closes QwenLM#10153 * fix(review): drop the omission literals the pipeline names for fixConstraint (QwenLM#10168) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): keep fixConstraint through dedup merges and body-Critical relocation (QwenLM#10168) Both round-1 Criticals name sites where the new field can be dropped on its way to the fixer. Step 4's deduplication and the two pair-loop dedup bullets kept the most detailed description, the highest severity, and the source tags — never a fix-side field — so two agents reporting one root cause lost the constraint only the less detailed copy recorded, before canonicalization ever saw the record. The merge rule now keeps every sourced constraint: consistent ones combined, conflicting ones adjudicated explicitly instead of silently discarded. The posting rule also scoped the constraint sentence to inline comment bodies, but a Critical whose locations all fail anchor resolution moves to bodyCriticals — the finding's sole published copy. The posting rule and the compose-state field now both require that entry to append the constraint sentence, copied from the artifact. Each requirement is pinned by a static SKILL.test.ts assertion that reds when its clause is removed (mutation-probed), and both new tests red against the pre-fix prose. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): keep the whole fix side through dedup merges and body-Critical relocation (QwenLM#10168) * fix(review): pin the constraint sentence's mandate, order, and loss channels (QwenLM#10168) Three suggestions on the fixConstraint posting paragraph, one cluster. The carry-mandate clause — the operative half of the rule — had no assertion: the heading was pinned but the sentence was not, so an edit weakening "the posted body carries it" shipped green. The placement instruction, "beside the fix-witness sentence", underdetermined the position: the fix-witness rule closes the body with the witness, so immediately before it is the only consistent place, and a finding whose `fixWitness` is `N/A` had no referent to stand beside at all. The paragraph now fixes the order, names the N/A fallback (the constraint sentence takes the witness's place after the suggestion block), and states the same order for a `bodyCriticals` entry that carries both sentences. The paragraph also scoped the accepted loss to the Aone submit-time relocation, leaving two structurally identical channels unnamed: the deferral rendering (a `DeferredEntry` holds no fix-side field and the line caps at 240 characters) and the duplicate-drop account (name and location only, never the finding's own text). Both are now named beside the Aone residue; the loss stays a named acceptance, not a silent one. Each new clause is pinned by a toContain assertion in SKILL.test.ts; six mutation probes (weaken the mandate, delete the placement, the fallback, the order clause, or either channel naming) all red the tests, and the restored suite is green. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
5 participants
Fixes #9782
Problem
A tool call that requires user confirmation inside a nested sub-agent (an agent launched by a background agent or fork) was neither surfaced to any UI nor auto-denied — it waited forever on a
TOOL_WAITING_APPROVALevent nothing listened to, and the enclosing agent hung silently. Two independent break points:getShouldAvoidPermissionPromptson anObject.create(agentConfig)wrapper, but the rebuilt tool registry binds toagentConfigitself — a nestedAgentToolresolvedConfig.prototype'sfalsethrough its config chain, believed it could prompt, and emitted an approval event nobody could answer. Same shape on the resume path.bridgeApprovalEventscall sites (background launch, resume, workflow) are off that path.Trigger chain:
review-agentdeclares noapprovalMode→ trusted folder resolves to AutoEdit →run_shell_command(build/test) always needs confirmation → guaranteed silent hang for orchestrator-style skills fanning out under a fork. Permissive parent modes (YOLO/AUTO_EDIT/AUTO) are unaffected.Fix
agentConfig(launch path) /activeAgentConfig(resume path) — the configs the rebuilt tool registries bind to — so nested schedulers inherit the real policy through their config prototype chains. With bubbling off, hang-forever becomes an explicit deny. Both configs are per-launch/per-resume, so the stamp leaks nowhere.parentAgentIdlineage fromgetCurrentAgentId()to the nearest backgrounded running ancestor (findBackgroundedAncestorAgentId, exported, hop-capped) and bridge the invocation's emitter onto that ancestor's Background-tasks entry; unbridge in the foregroundfinally. Top-level foreground launches have no such ancestor and keep the inline confirmation path unchanged. Double answers are deduped by the runtime's existing per-call responded guard.bridgeApprovalEventsgains anestedSourceoption — declared by the caller because runtimesubagentIdand registryagentIduse different suffixes, so comparing ids cannot distinguish own from nested. Bridged approvals carry the nested runtime'ssubagentId, and the Background tasks dialog showsfrom nested agent: <id>under the approval header. Locale entries added for all nine languages.Verification
nestedSource+ unbridged on completion; top-level launches don't bridge), 2 for policy inheritance through derived config chains (auto-deny and bubbling), 1 for the resume path (asserts the runtime config IS thecreateToolRegistryreceiver — the assertion that distinguishes the fix from the wrapper), 1 fornestedSourcestamping.agent.test.ts,background-tasks.test.ts,background-agent-resume.test.ts— 445 tests pass; clibackground-viewsuites — 178 pass;tscclean on core and cli; eslint, prettier, andcheck-i18npass.