Skip to content

fix(core,cli): isolate teammate leader turns from agent context - #7576

Merged
wenshao merged 1 commit into
QwenLM:mainfrom
yiliang114:cx/7574-teammate-plan-als
Jul 23, 2026
Merged

fix(core,cli): isolate teammate leader turns from agent context#7576
wenshao merged 1 commit into
QwenLM:mainfrom
yiliang114:cx/7574-teammate-plan-als

Conversation

@yiliang114

Copy link
Copy Markdown
Collaborator

What this PR does

This change ensures that teammate-to-leader messages cross a clean async-local boundary before entering the leader conversation. It also isolates the React consumer that submits queued teammate messages, because React can restore an outer teammate frame after the producer callback returns. Focused regressions cover both boundaries independently.

Why it's needed

A plan-required teammate can request approval while running inside its own agent and model context. The leader callback and the later React drain could inherit that context, causing the approval turn to use the teammate model and appear to be a nested rather than top-level session.

Reviewer Test Plan

How to verify

Run the focused plan-approval tests and confirm that a request emitted from a teammate frame reaches the leader callback with no agent ID or teammate runtime. Run the CLI stream-hook tests and confirm that a React flush triggered inside a teammate runtime submits the resulting leader turn without that runtime.

Evidence (Before & After)

Before: the deterministic reproduction observed agentId='planner-agent', the teammate runtime/model, and isTopLevelSession=false in the leader drain. After: the producer callback observes no agent frame, and the consumer submission observes no teammate runtime. This is a non-visual runtime fix, so screenshots are N/A.

Tested on

OS Status
🍏 macOS
🪟 Windows N/A
🐧 Linux N/A

Environment (optional)

Node.js 22.22.0. Verified with 11 focused core tests, 160 CLI stream-hook tests, the repository lint, full build, and full typecheck.

Risk & Scope

  • Main risk or tradeoff: registered leader-message callbacks now intentionally execute outside every teammate agent frame; the consumer repeats this boundary because producer-only isolation is insufficient across React batching.
  • Not validated / out of scope: no live dual-model interactive TUI run; the real manager and React hook boundaries are covered deterministically without model calls. The separately tracked nested run_in_background fallback behavior is not changed.
  • Breaking changes / migration notes: none.

Linked Issues

Fixes #7574

Related to #7156 and #7194.

中文说明

本 PR 的改动

本改动确保 teammate 发往 leader 的消息在进入 leader 对话前跨过一个干净的异步本地上下文边界。同时也隔离了提交 teammate 队列消息的 React 消费端,因为 producer 回调返回后,React 仍可能恢复外层 teammate frame。两条聚焦回归测试分别独立覆盖这两个边界。

为什么需要

要求审批计划的 teammate 会在自己的 agent 和模型上下文中发起审批请求。leader 回调以及后续 React drain 可能继承该上下文,导致审批轮次使用 teammate 模型,并被误判为嵌套会话而不是顶层会话。

Reviewer Test Plan

如何验证

运行聚焦的计划审批测试,确认从 teammate frame 发出的请求到达 leader 回调时既没有 agent ID,也没有 teammate runtime。运行 CLI stream hook 测试,确认在 teammate runtime 内触发的 React flush 所提交的 leader 轮次不再携带该 runtime。

证据(修复前后)

修复前:确定性复现中,leader drain 观察到 agentId='planner-agent'、teammate runtime/model,以及 isTopLevelSession=false。修复后:producer 回调观察不到 agent frame,consumer 提交也观察不到 teammate runtime。该修复不涉及视觉变化,因此截图不适用。

测试平台

OS 状态
🍏 macOS
🪟 Windows N/A
🐧 Linux N/A

环境(可选)

Node.js 22.22.0。已通过 11 个聚焦 Core 测试、160 个 CLI stream hook 测试、仓库 lint、完整 build 和完整 typecheck。

风险与范围

  • 主要风险或取舍:注册的 leader message 回调现在会有意在所有 teammate agent frame 之外执行;由于仅隔离 producer 无法跨 React batching 保证安全,consumer 会再次建立该边界。
  • 未验证 / 范围外:未运行真实双模型交互式 TUI;真实 manager 与 React hook 边界已在无模型调用的情况下得到确定性覆盖。另行跟踪的嵌套 run_in_background fallback 行为不在本次修改范围内。
  • 破坏性变更 / 迁移说明:无。

关联 Issue

Fixes #7574

关联 #7156#7194

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: observed bug with a deterministic reproduction on main (dc74279103). Issue #7574 documents agentId='planner-agent' and the teammate runtime view leaking into the leader callback and drain. This is the same class of AsyncLocalStorage carrier that #7194 fixed for background notifications — a distinct, uncovered path.

Direction: aligned. Multi-agent context isolation is core to the teammate feature, and this follows the exact pattern established by #7194 (runOutsideAgentContext at the producer callback and the React consumer drain). CHANGELOG: no direct reference, but the area is clearly relevant — #7156 and #7194 are prior art in the same release cycle.

Size: 36 production lines (TeamManager.ts +5/-1, useGeminiStream.ts +17/-14) and 89 test lines. Well under any threshold. Not applicable for escalation.

Approach: the scope feels right — two wrapping points (producer callback registration, consumer effect drain), each independently tested. No unrelated changes, no drive-by refactors. The fix is the minimal application of an established pattern to the remaining uncovered path.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题:已观测到的 bug,有确定性复现(main 分支 dc74279103)。Issue #7574 记录了 agentId='planner-agent' 和 teammate runtime view 泄漏到 leader 回调和 drain 中。这与 #7194 修复的后台通知 AsyncLocalStorage 载体属于同一类问题——是一条尚未覆盖的路径。

方向:对齐。多智能体上下文隔离是 teammate 功能的核心,本 PR 完全遵循 #7194 建立的模式(在 producer 回调和 React consumer drain 处使用 runOutsideAgentContext)。CHANGELOG 无直接引用,但该领域明显相关——#7156#7194 是同一发布周期的先例。

规模:36 行生产代码(TeamManager.ts +5/-1,useGeminiStream.ts +17/-14),89 行测试代码。远低于任何阈值,无需升级。

方案:范围合理——两个包裹点(producer 回调注册、consumer effect drain),各自独立测试。无无关改动,无顺手重构。修复是将已建立的模式应用于剩余未覆盖路径的最小改动。

进入代码审查 🔍

Qwen Code · qwen3.8-max-preview

Reviewed at 28504e2cd13684cd086c3475400de8fa9c45cbef · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code Review

Independent proposal: given the issue (teammate plan approval leaks AsyncLocalStorage context into the leader turn), I would wrap the leaderMessageCallback invocation in runOutsideAgentContext() at registration time in TeamManager, and wrap the teammate queue drain effect in runOutsideAgentContext() in useGeminiStream — the same two-boundary approach #7194 used for background notifications.

Comparison with the diff: the PR does exactly this. No simpler path missed.

  • TeamManager.setLeaderMessageCallback now wraps the callback at registration: cb ? (message, display) => runOutsideAgentContext(() => cb(message, display)) : null. This ensures every invocation — plan approval, teammate reports — exits the agent frame before reaching the CLI layer. Correct.
  • The teammate queue drain effect in useGeminiStream.ts wraps the entire splice-and-submit block in runOutsideAgentContext(). This mirrors the existing notification drain at line 3954 (the fix(core,cli): drain background notifications outside the subagent's ALS frame #7194 fix). The comment explains why: React can flush the effect after restoring the teammate frame. Correct.
  • Both tests verify the boundary independently: the core test asserts callbackAgentId === null and callbackRuntimeView === undefined inside the leader callback; the CLI test asserts capturedRuntimeView === undefined inside sendMessageStream. Good coverage.

No critical blockers. No AGENTS.md violations. The change reuses the existing runOutsideAgentContext helper (already exported from core's index) — no new abstractions.

Real-Scenario Testing

This is a non-visual runtime fix (AsyncLocalStorage context isolation). The observable effect — correct model selection in leader turns — requires a multi-agent team with different models per agent, which is not feasible to drive headlessly. The deterministic tests provide the before/after evidence (see issue #7574's reproduction).

tmux is not available on this CI runner. Headless smoke test confirming the CLI starts and responds correctly with the PR code:

$ npm run dev -- -p 'say hello in one word'

> @qwen-code/qwen-code@0.20.1 dev
> node scripts/dev.js -p say hello in one word

Hello!

Unit tests (all pass):

✓ src/agents/team/TeamManager.plan-approval.test.ts (11 tests) 39ms
✓ src/ui/hooks/useGeminiStream.test.tsx (160 tests) 1900ms

Build and typecheck: pass.

中文说明

代码审查

独立方案: 根据 issue(teammate 计划审批将 AsyncLocalStorage 上下文泄漏到 leader 轮次),我会在 TeamManager 注册时用 runOutsideAgentContext() 包裹 leaderMessageCallback,并在 useGeminiStream 的 teammate 队列 drain effect 中包裹 runOutsideAgentContext()——与 #7194 对后台通知使用的双边界方法相同。

与 diff 对比: PR 完全这样做了。没有遗漏更简路径。

  • TeamManager.setLeaderMessageCallback 在注册时包裹回调:确保每次调用(计划审批、teammate 报告)在到达 CLI 层之前退出 agent frame。正确。
  • useGeminiStream.ts 中的 teammate 队列 drain effect 包裹了整个 splice-and-submit 块。这与第 3954 行的现有通知 drain(fix(core,cli): drain background notifications outside the subagent's ALS frame #7194 修复)一致。注释解释了原因:React 可能在恢复 teammate frame 后 flush 此 effect。正确。
  • 两个测试独立验证边界:core 测试断言 leader 回调中 callbackAgentId === nullcallbackRuntimeView === undefined;CLI 测试断言 sendMessageStreamcapturedRuntimeView === undefined。覆盖良好。

无关键阻塞。无 AGENTS.md 违规。改动复用现有 runOutsideAgentContext 辅助函数——无新抽象。

真实场景测试

这是一个非视觉运行时修复(AsyncLocalStorage 上下文隔离)。可观测效果——leader 轮次的正确模型选择——需要具有不同模型的多智能体团队,无法在无头模式下驱动。确定性测试提供了 before/after 证据(见 issue #7574 的复现)。

此 CI runner 上 tmux 不可用。无头冒烟测试确认 CLI 使用 PR 代码正常启动和响应(见上方输出)。

单元测试全部通过(11 + 160 = 171 个测试)。Build 和 typecheck 通过。

Qwen Code · qwen3.8-max-preview

Reviewed at 28504e2cd13684cd086c3475400de8fa9c45cbef · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 5/5 — clean across every stage; would merge without hesitation.

This is a textbook application of an established pattern to a remaining uncovered path. The problem is real and deterministically reproduced (#7574), the fix is the same runOutsideAgentContext wrapping that #7194 introduced for background notifications, and the two boundaries (producer callback, consumer drain) are independently tested. 36 production lines, no new abstractions, no unrelated changes. If I had to maintain this in six months, I'd thank the author — the comment on the drain effect explains exactly why the second boundary is needed across React batching.

The only thing I couldn't do is a live dual-model TUI run (tmux unavailable on this runner, and the scenario requires a multi-agent team with distinct models). The deterministic tests cover both boundaries thoroughly, and the headless smoke test confirms the CLI starts and responds correctly.

中文说明

置信度:5/5 — 每个阶段都干净;毫不犹豫地合并。

这是将已建立的模式应用于剩余未覆盖路径的教科书式修复。问题真实且有确定性复现(#7574),修复与 #7194 为后台通知引入的 runOutsideAgentContext 包裹完全一致,两个边界(producer 回调、consumer drain)独立测试。36 行生产代码,无新抽象,无无关改动。如果六个月后维护这段代码,我会感谢作者——drain effect 上的注释精确解释了为什么需要第二个边界来跨 React batching。

唯一无法执行的是实时双模型 TUI 运行(此 runner 上 tmux 不可用,且场景需要具有不同模型的多智能体团队)。确定性测试充分覆盖了两个边界,无头冒烟测试确认 CLI 正常启动和响应。

Qwen Code · qwen3.8-max-preview

Reviewed at 28504e2cd13684cd086c3475400de8fa9c45cbef · re-run with @qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship. ✅

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found. LGTM! ✅

— qwen3.7-max via Qwen Code /review

@yiliang114

Copy link
Copy Markdown
Collaborator Author

I ran an additional live tmux check with Qwen Code 0.20.1:

  • A completed regular background agent was revived through send_message(task_id=...) and continued with the same task ID.

Background subagent continued with the same task ID

  • A named teammate remained addressable after becoming idle and handled a second instruction through send_message(to="worker-e2e") without another Agent call.

Named teammate handled a second instruction without respawning

  • The shared task board successfully auto-assigned a task to worker-e2e, although the model misinterpreted the instruction and missed the bounded expected-response check.

Shared task board auto-assignment and bounded response result

This is useful supplemental lifecycle coverage, but it does not directly verify the plan-approval AsyncLocalStorage fix in this PR. A live verification of that fix would still require a plan_mode_required teammate using a different model from the leader. The deterministic producer/consumer tests remain the relevant before/after evidence for #7574.

@gwinthis gwinthis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: APPROVE (C=0)

Summary

Focused context-isolation fix (+110/-15) that prevents teammate agent/model context from leaking into leader conversation turns. Two boundaries are isolated: the producer callback (TeamManager) and the consumer drain (React effect), because React batching can restore an outer teammate frame after the producer returns.

Findings

None blocking. Correct and minimal:

  1. Producer boundary (TeamManager.setLeaderMessageCallback): wraps callback in runOutsideAgentContext() — clears AsyncLocalStorage before the leader callback executes.
  2. Consumer boundary (useGeminiStream effect): wraps the queue drain in runOutsideAgentContext() — necessary because React can flush the effect after restoring a teammate frame.
  3. Both boundaries are independently tested — core test verifies getCurrentAgentId() === null and getRuntimeContentGenerator() === undefined in the callback; CLI test verifies the runtime view is undefined when sendMessageStream is called.
  4. No API changes, no breaking changes. The runOutsideAgentContext utility is the established mechanism for clearing agent context.

Architecture Insight

AsyncLocalStorage context leakage across callback boundaries: When a callback is registered in one async context (teammate) but invoked in another (leader), the invocation context can leak through AsyncLocalStorage. The fix pattern is: wrap the callback registration to clear the context at invocation time. When the consumer is a React effect, a second boundary is needed because React's batching can restore the outer frame between producer and consumer.

中文说明

评审:APPROVE (C=0)

概要

聚焦的上下文隔离修复(+110/-15):防止 teammate agent/model 上下文泄漏到 leader 对话轮次。隔离两条边界:生产者回调(TeamManager)和消费者 drain(React effect),因为 React batching 可在生产者返回后恢复外层 teammate frame。

架构洞察

AsyncLocalStorage 跨回调边界的上下文泄漏: 回调在一个异步上下文(teammate)中注册但在另一个(leader)中调用时,调用上下文可通过 AsyncLocalStorage 泄漏。修复模式:在回调注册时包裹上下文清除。当消费者是 React effect 时,需要第二条边界。

— qwen3.7-max via Qwen Code /review

@doudouOUC doudouOUC left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Clean, well-targeted fix for the AsyncLocalStorage context leak in teammate→leader message paths.

What it does

Two-layer isolation ensuring leader turns never inherit a teammate's agent frame:

  1. Producer (TeamManager.setLeaderMessageCallback): wraps the registered callback in runOutsideAgentContext at registration time, so invocations from within a teammate frame execute cleanly.
  2. Consumer (useGeminiStream teammate queue drain): wraps the entire batch submission in runOutsideAgentContext, guarding against React flushing the effect while still inside a teammate's ALS frame.

Why both layers are needed

Producer-only isolation is insufficient because React batching can restore an outer teammate frame after the producer callback returns. The consumer repeats the boundary independently — a sound defensive pattern already validated by the notification drain fix for #7156.

Verification

  • Two focused regression tests independently cover each boundary with deterministic reproduction (no model calls needed).
  • Ubuntu CI passes. The runOutsideAgentContextstorage.exit() path is synchronous with negligible overhead.

Minor observations (non-blocking)

  • The as never type casts in tests are a pragmatic hack; a shared mock factory could unify these later.
  • setLeaderMessageCallback creates a new closure wrapper on each call — acceptable given it's only called once per manager lifecycle.

LGTM — minimal, focused, and well-tested.

@wenshao

wenshao commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Maintainer Local Verification Report

Verified PR #7576 locally on macOS (darwin, Node.js v22.22.2) against commit 28504e2.

1. Focused Unit Tests

Core — TeamManager.plan-approval.test.ts: 11/11 passed ✅

Includes the new test delivers plan approval requests outside the teammate agent context, which verifies that getCurrentAgentId() returns null and getRuntimeContentGenerator() returns undefined inside the leader callback — confirming the teammate's AsyncLocalStorage frame is properly stripped.

Core plan-approval tests

CLI — useGeminiStream.test.tsx: 160/160 passed ✅

Includes the new test drains teammate reports outside the teammate runtime context, which verifies that the React effect drain submits the leader turn without inheriting the teammate runtime view.

CLI teammate drain tests

2. Build / Typecheck / Lint

All three checks passed with zero errors:

Build, typecheck, lint

3. CI Status

All 10 CI checks passed (Ubuntu test suite, lint, build, typecheck, triage, etc.). macOS and Windows test jobs were skipped per CI configuration.

4. Code Review Summary

The change is minimal and well-scoped (+110/−15 across 4 files):

  • TeamManager.ts (+4/−1): Wraps setLeaderMessageCallback with runOutsideAgentContext() — the producer boundary.
  • useGeminiStream.ts (+17/−14): Wraps the teammate queue drain effect with runOutsideAgentContext() — the consumer boundary (needed because React can restore the teammate frame after the producer callback returns).
  • Two test files: Focused regression tests for both boundaries independently.

The approach is consistent with the existing fix in #7194 for background-agent notifications.

5. Notes

  • Local macOS vitest requires a @xterm/headless CJS interop workaround (alias shim) to run tests that transitively import shellExecutionService.ts. This is a pre-existing environment issue unrelated to this PR — CI (Ubuntu) is unaffected.
  • Integration test failures observed locally (ACP/daemon/MCP) are pre-existing and unrelated to this PR's changes.

Verdict

Ready to merge. The fix correctly isolates both the producer and consumer boundaries, the tests are deterministic and focused, and all standard checks pass.

中文版本

维护者本地验证报告

在 macOS (darwin, Node.js v22.22.2) 上针对 commit 28504e2 本地验证 PR #7576

1. 聚焦单元测试

Core — TeamManager.plan-approval.test.ts:11/11 通过 ✅

包含新增测试 delivers plan approval requests outside the teammate agent context,验证 leader 回调中 getCurrentAgentId() 返回 nullgetRuntimeContentGenerator() 返回 undefined——确认 teammate 的 AsyncLocalStorage frame 被正确剥离。

CLI — useGeminiStream.test.tsx:160/160 通过 ✅

包含新增测试 drains teammate reports outside the teammate runtime context,验证 React effect drain 提交 leader 轮次时不继承 teammate runtime view。

2. 构建 / 类型检查 / Lint

三项检查均零错误通过。

3. CI 状态

全部 10 项 CI 检查通过(Ubuntu 测试套件、lint、build、typecheck、triage 等)。macOS 和 Windows 测试任务按 CI 配置跳过。

4. 代码审查摘要

改动精简且范围明确(+110/−15,4 个文件):

  • TeamManager.ts(+4/−1):用 runOutsideAgentContext() 包装 setLeaderMessageCallback——producer 边界。
  • useGeminiStream.ts(+17/−14):用 runOutsideAgentContext() 包装 teammate 队列 drain effect——consumer 边界(因为 React 可能在 producer 回调返回后恢复 teammate frame)。
  • 两个测试文件:分别独立覆盖两条边界的聚焦回归测试。

方案与 #7194 中对 background-agent 通知的已有修复一致。

5. 备注

  • 本地 macOS vitest 需要 @xterm/headless CJS interop 变通(alias shim)才能运行间接导入 shellExecutionService.ts 的测试。这是与 PR 无关的既有环境问题——CI(Ubuntu)不受影响。
  • 本地观察到的集成测试失败(ACP/daemon/MCP)为既有问题,与本 PR 改动无关。

结论

可以合入。 修复正确隔离了 producer 和 consumer 两条边界,测试确定性且聚焦,所有标准检查通过。

@wenshao
wenshao added this pull request to the merge queue Jul 23, 2026
Merged via the queue into QwenLM:main with commit 4f7429a Jul 23, 2026
86 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Teammate plan approval leaks its agent context into the leader turn

5 participants