fix(core,cli): drain background notifications outside the subagent's ALS frame - #7194
Conversation
|
Thanks for the PR! Template looks good ✓ — all required sections filled with substantive content, including the Chinese translation. Problem: this is a real, observed bug. Issue #7156 (P1) documents the model leak with detailed evidence — status line flickering, debug log timestamps showing contextLimit dropping from 1M to 300K, and a deterministic 400 error killing the session. The before/after table in the PR body shows exactly which requests are affected. No question about whether the problem exists. Direction: clearly aligned. A background subagent's AsyncLocalStorage frame leaking into the main session's model resolution is a correctness bug in the agent runtime — squarely within core scope. The mechanism (React batching a subagent's progress setState with the notification trigger, causing the drain effect to execute on the wrong ALS stack) is well-explained and matches the observed symptoms. Size: 119 production lines (useGeminiStream.ts +87/-36 indented wrap, background-tasks.ts +13/-1, agent-context.ts +17, index.ts +1), 56 test lines. Well within bounds for a focused fix. Approach: the two-layer defense is the right call. The consumer-side guard ( Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ — 所有必填章节都有实质内容,包括中文翻译。 问题:这是一个真实、已观测到的 bug。Issue #7156(P1)用详细证据记录了模型泄漏——状态栏闪烁、debug 日志时间戳显示 contextLimit 从 1M 降至 300K、确定性 400 错误导致会话死亡。PR 正文中的 before/after 表格精确标注了哪些请求受影响。问题存在性无疑。 方向:完全对齐。后台 subagent 的 AsyncLocalStorage frame 泄漏到主会话的模型解析,是 agent 运行时的正确性 bug——属于核心模块范畴。机制解释(React 将 subagent 的进度 setState 与通知触发合并到同一次 commit,导致 drain effect 在错误的 ALS 栈上执行)与观测到的症状吻合。 规模:119 行生产代码(useGeminiStream.ts +87/-36 缩进包裹、background-tasks.ts +13/-1、agent-context.ts +17、index.ts +1),56 行测试。在聚焦修复的合理范围内。 方案:两层防线的选择正确。消费端防线( 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Code ReviewThe implementation is clean and well-targeted. A few observations: The new The consumer-side guard in The The regression test ( No critical issues found. No AGENTS.md violations. Unit Tests
Real-Scenario TestingBefore (installed build)After (this PR)Background agent notification path (this PR)The background agent launched, completed, the notification drained successfully, and the main session responded with the correct result. No model confusion, no errors. Note: full reproduction of #7156 requires two different model endpoints (large-context main + small-context subagent), which isn't available in this CI environment — the smoke test exercises the notification drain path but cannot verify the model-switching aspect directly. The unit regression test covers that deterministically. 中文说明代码审查实现简洁、目标明确。几点观察: 新的
回归测试设计良好:在 agent frame 内设置、调用 未发现关键问题。无 AGENTS.md 违规。 单元测试
真实场景测试基础 CLI 功能和后台 agent 通知路径均正常工作。后台 agent 启动、完成、通知汇入成功、主会话正确返回结果,无报错。注意:完整复现 #7156 需要两个不同的模型端点(大上下文主会话 + 小上下文 subagent),当前 CI 环境不具备此条件。单测回归测试已确定性覆盖该场景。 — Qwen Code · qwen3.7-max Reviewed at |
|
Confidence: 5/5 — Clean across every stage; a well-diagnosed bug with a minimal, correct fix. This is a textbook example of how to fix an AsyncLocalStorage scoping bug. The problem is real (P1 issue #7156 with detailed debug log evidence), the root cause analysis is precise (React batching a subagent's progress setState with the notification trigger, causing the drain effect to inherit the wrong ALS frame), and the fix is exactly what the problem calls for — The two-layer defense is the right architectural choice. The consumer guard closes the entire class (any producer's setState triggering the commit), and the producer guard adds defense-in-depth for the notification callback specifically. Neither layer is redundant — they protect against different failure modes. 119 production lines, 56 test lines, 5 files, zero scope creep. The regression test is deterministic and would catch the bug on the unpatched code. Everything builds, typechecks, and tests clean. The background agent smoke test confirms the notification drain path works without errors. Ship it. ✅ 中文说明置信度: 5/5 — 各阶段均通过;诊断准确的 bug,修复简洁正确。 这是 AsyncLocalStorage 作用域 bug 修复的范例。问题真实存在(P1 issue #7156,附详细 debug 日志证据),根因分析精确(React 将 subagent 的进度 setState 与通知触发合并到同一次 commit,导致 drain effect 继承了错误的 ALS frame),修复恰好对应问题所需—— 两层防线的架构选择正确。消费端防线关闭整类问题(任何 producer 的 setState 触发 commit),生产端防线为通知回调增加纵深防御。两层各有用途,保护不同的失败模式。 119 行生产代码、56 行测试、5 个文件、零范围蔓延。回归测试具有确定性,能在未修复代码上捕获该 bug。构建、类型检查、测试全部通过。后台 agent 冒烟测试确认通知路径无报错。 可以合入。✅ — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
| runOutsideAgentContext(() => { | ||
| const queue = notificationQueueRef.current; | ||
| const targetType = queue[0]!.sendMessageType; |
There was a problem hiding this comment.
[Suggestion] The consumer-side runOutsideAgentContext guard — described in the PR as the "decisive" fix for #7156 — has no targeted unit regression test. The existing notification test in useGeminiStream.test.tsx (line 6388, regression for #7114) invokes the drain without any agent ALS frame, so storage.exit(fn) is a transparent no-op in that test. Removing this wrapping would not change that test's outcome.
Failure scenario: a future refactor that removes or restructures the runOutsideAgentContext wrapping around the drain effect would silently re-introduce the model leak — notification turns resolve to the subagent's model, causing 400 errors on smaller-context models. No automated test would catch this regression.
The producer-side guard in background-tasks.ts has a well-constructed regression test that proves the mechanism. A similar test here — invoking the drain from inside runWithAgentContext/runWithRuntimeContentGenerator and asserting the drained submitQuery uses the main session's model — would close the gap.
— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Added in 5a1a6aa: drains a notification outside a background agent ALS frame (useGeminiStream.test.tsx). It drives the notification callback with the whole act() flush inside runWithRuntimeContentGenerator — mirroring the contaminated React commit — bypassing the producer-side guard, and asserts the drained sendMessageStream call observes no runtime view. Verified it fails when the runOutsideAgentContext wrapping around the drain effect is removed (captured view = the subagent's). The branch is also rebased onto latest main: the earlier Test-job failure was the NOTICES.txt drift guard from #7161 tripping on this branch's pre-#7161 base, unrelated to the changed files.
中文:已在 5a1a6aa 补上该回归测试——整个 act() flush 在 runWithRuntimeContentGenerator 内执行以复刻被污染的 React commit(绕过生产端防线),断言汇入的 sendMessageStream 观察不到 runtime view;移除 drain effect 的 runOutsideAgentContext 包裹后该测试确实失败。分支已 rebase 到最新 main:此前 Test job 失败是 #7161 引入的 NOTICES.txt 漂移守卫在旧基线上触发,与本 PR 改动无关。
…ALS frame A background subagent running on its own model leaked that model into the main session: the notification turn (and every turn after it) went out on the subagent's model, overflowing its smaller context window with a 400 — the same symptom QwenLM#7119 fixed on a different path (QwenLM#7156). Mechanism, confirmed with a deterministic E2E reproducer: progress setState calls issued from inside the subagent's AsyncLocalStorage frame can be batched by React/Ink into the same commit as the notification trigger. The drain effect then executes on that batch's synchronous stack, and every async continuation submitQuery starts — including Config.getModel() at send time — inherits the subagent's runtime view. The "persistent" model switch reported in the issue is each subsequent turn deriving from the contaminated drain chain. Two layers: - Consumer guard (the decisive fix): the notification drain effect in useGeminiStream runs its body inside runOutsideAgentContext(), a new agent-context helper wrapping AsyncLocalStorage.exit — the drained turn always runs on the main session's configuration regardless of which producer's setState triggered the commit. The E2E reproducer (session on big-context, custom agent on small-default, run_in_background) flips from the notification turn and all subsequent calls going to small-default, to everything staying on big-context. - Producer defense: BackgroundTaskRegistry.emitNotification invokes the notification callback outside any agent frame, with a unit regression test that fails on the unpatched registry. Thanks to @Aleks-0 for the instrumented traces on QwenLM#7156 that ruled out persistent ModelsConfig mutation and pinned the ALS mechanism. Fixes QwenLM#7156 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review follow-up on QwenLM#7194: the consumer-side guard had no targeted unit regression test — the existing notification drain test runs with no agent ALS frame active, so removing the wrapping would not change its outcome. The new test drives the notification callback with the whole act() flush inside runWithRuntimeContentGenerator, mirroring the contaminated React commit from QwenLM#7156, and asserts the drained sendMessageStream call observes no runtime view. Verified to fail when the runOutsideAgentContext wrapping around the drain effect is removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
cc0e9c4 to
5a1a6aa
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push,因为这会使已有的评审评论失效。另外,供日后参考:作为集成流程的一部分,机器人始终会自动将所有改动压缩(squash)为单个提交。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
Local build + real-run verification — ✅ recommend mergeI built this PR from source and verified it locally on Linux with three layers of evidence: the full touched unit suites, a real compiled harness (no mocks) driving the actual 1. Touched unit suites — green
2. Real compiled harness (
|
|
Released in v0.20.1. |


What this PR does
Stops a background subagent's model from leaking into the main session when its completion notification drains. Two layers: the decisive consumer guard — the notification drain effect in
useGeminiStreamnow runs inside a newrunOutsideAgentContext()helper (AsyncLocalStorage.exit), so the drained turn and every async continuation it starts resolveConfig.getModel()to the main session's configuration regardless of which producer's setState triggered the React commit; and a producer defense —BackgroundTaskRegistry.emitNotificationinvokes the notification callback with no agent frame on the stack, pinned by a unit regression test. The helper is exported from core for reuse by other main-session-owned paths that can be triggered from inside an agent frame.Why it's needed
#7156: a session on a large-context model launching a background subagent on a smaller model got a 400 on the notification turn — the accumulated history was sent to the subagent's model. #7119 fixed a different path to the same symptom (override clearing); here the model resolution itself was mis-scoped. The mechanism, confirmed with a deterministic reproducer: progress setState calls issued from inside the subagent's AsyncLocalStorage frame can be batched by React/Ink into the same commit as the notification trigger; the drain effect then executes on that batch's synchronous stack, and everything
submitQuerystarts inherits the subagent's runtime view. The "persistent" model switch users observed (all turns after the notification wrong, status line flickering) is each subsequent turn deriving from the contaminated drain chain — which is also why guarding a single producer is a whack-a-mole; the consumer guard closes the class. Credit to @Aleks-0's instrumented traces on the issue for ruling out persistentModelsConfigmutation and pinning the ALS mechanism.Reviewer Test Plan
How to verify
.qwen/agents/worker.md) withmodel:pointing at a smaller-context model.run_in_background: true; wait for completion and the notification to drain.Automated coverage: a unit regression test proves the notification callback previously ran inside the subagent's frame (
getCurrentAgentId()/runtimeViewset) and now runs with none — it fails on the unpatched registry.useGeminiStreamsuite 169/169,background-taskssuite 111/111,npm run typecheck, eslint, prettier all clean.Evidence (Before & After)
Deterministic E2E: PTY harness drives the real bundle against a local fake OpenAI-compatible server logging the
modelof every request. Session modelbig-context; project-level custom agentworkerwithmodel: small-default; scripted turn launches it in background:big-contextbig-contextbig-contextbig-contextsmall-default(its model ✓)small-default(unchanged ✓)big-contextbig-contextsmall-default❌big-context✅small-default❌ (persistent leak)big-context✅Row 6 shows the fix also ends the "session permanently switched" aspect — the contamination chain is severed at the drain.
Tested on
Environment (optional)
macOS (Darwin 24.6), Node v22.23.1; PTY harness + local fake OpenAI-compatible SSE server against the esbuild bundle, plus vitest unit tests.
Risk & Scope
AsyncLocalStorage.exit— a no-op when no frame is active (the common case). Notification semantics are unchanged; only the async-context scoping of the drained turn changes.Linked Issues
Fixes #7156
中文说明
本 PR 做了什么
阻止后台子 agent 的模型在完成通知汇入时泄漏进主会话。两层防护:决定性的消费端防线——
useGeminiStream的通知 drain effect 整体运行在新的runOutsideAgentContext()(AsyncLocalStorage.exit)内,无论哪个 producer 的 setState 触发了这次 React commit,汇入轮次及其启动的所有异步延续都以主会话配置解析Config.getModel();以及生产端防御——BackgroundTaskRegistry.emitNotification在无 agent frame 下调用通知回调,由单测回归固定。helper 从 core 导出,供其它可能在 agent frame 内被触发的主会话路径复用。为什么需要
#7156:大上下文会话启动小上下文后台子 agent,通知轮次 400——累积历史被发给了子 agent 的模型。#7119 修复的是同一症状的另一条路径(override 清除);这里是模型解析本身作用域错误。经确定性复现器证实的机制:子 agent ALS frame 内发出的进度 setState 会被 React/Ink 与通知触发合并进同一次 commit;drain effect 在该批次的同步栈上执行,
submitQuery启动的一切都继承子 agent 的 runtime view。用户观察到的「持久」切换(通知后所有轮次都错、状态栏闪烁)是每个后续轮次都从被污染的 drain 链派生——这也是只防单个 producer 属于打地鼠的原因;消费端防线关闭整类问题。感谢 @Aleks-0 在 issue 中的插桩记录排除了持久ModelsConfigmutation 并锁定 ALS 机制。审阅测试计划
如何验证
.qwen/agents/worker.md)的model:指向小上下文模型;run_in_background: true启动该 agent,等完成与通知汇入;自动化覆盖:单测证明通知回调此前运行在子 agent frame 内、现在无 frame(在未修复的 registry 上失败);
useGeminiStream169/169、background-tasks111/111、typecheck / eslint / prettier 全绿。证据(Before & After)
确定性 E2E(PTY + 伪 OpenAI 服务器记录每请求 model;会话
big-context、自定义 agentmodel: small-default、后台启动):修复前 #5 通知轮次与 #6 后续系统调用均为small-default(❌ 持久泄漏);修复后全部保持big-context(✅)——第 6 行同时证明「会话被永久切换」的表象随 drain 链被切断而消失。测试平台
macOS 已本地验证(✅);Windows / Linux 依赖 CI(⚠️ )。
环境
macOS(Darwin 24.6)、Node v22.23.1;PTY harness + 本地伪 OpenAI SSE 服务器 + vitest 单测。
风险与范围
AsyncLocalStorage.exit执行——无 frame 时是 no-op(常态)。通知语义不变,仅改变汇入轮次的异步上下文作用域。关联 Issue
Fixes #7156
🤖 Generated with Claude Code