fix(core): preserve teammate identity when resuming a tool call after approval#4979
Merged
Conversation
…path
A teammate's send_message/task_update that requires confirmation resumes
from the UI's approval callback, on a different async chain than the
reasoning loop. runInAgentFrames already re-entered the agent's name,
runtime-view, and agent-id frames there, but not the in-process teammate
identity frame (teammateIdentityStore). So the resumed tool ran with
getAgentName() undefined and fell back to LEADER_NAME, which:
- forged a from="leader" envelope for the teammate's message (the exact
spoofing the envelopeNonce framing exists to prevent), mis-attributing
it in both the model-facing formatLeaderEnvelope and the UI's
formatLeaderDisplay ("leader reported back"); and
- let the teammate slip past the leader-only isTeammate() guard on
send_message(type=shutdown_request).
Capture the teammate identity at approval-emit time alongside the other
inherited frames and restore it in runInAgentFrames as the outermost
frame. No-op on the reasoning-loop path, where TeamManager already
establishes the frame. yolo/headless paths were unaffected (they don't
round-trip through UI approval).
Fires on every approved teammate send_message in default approval mode.
Collaborator
Author
E2E Test ReportInteractive run (tmux, real model glm-5.1, default approval mode) against the bundled build, before and after the fix. Scenario: leader creates a one-teammate team (
Regression reproduced on the pre-fix branch: step 4 showed Fix verified against the bundled artifact: step 4 shows Unit coverage: added a regression test for the post-approval resume path; mutation-checked (it fails with Windows/Linux not exercised locally — covered by CI. The fix is platform-agnostic (async-context propagation). |
Collaborator
Author
Background — this fixes Finding B from the #4844 maintainer verificationThis PR addresses Finding B from @wenshao's independent Linux maintainer-verification of #4844 (#4844 comment) — the one item flagged there as "worth fixing before (or right after) merge." We chose the fast-follow route, so this is that fix. Finding B diagnosed the mechanism precisely: in the default approval mode, a teammate's That is what this PR does, generalized to the shared post-approval resume path so it covers the message tool, the task tools, and the leader-only guard in one place: the teammate identity is captured while the loop frame is still live and restored across the resume boundary. The E2E in the comment above reproduces the exact symptom the verification reported ( For deeper context, this also closes the gap in two deferred #4844 hardening threads: the 中文说明本 PR 修复的是 @wenshao 在 #4844 独立维护者验证中提出的 Finding B（#4844 评论)——也就是那里标注为"建议在合并前或紧随其后修复"的唯一一项。我们选择了紧随其后的方式,这就是该修复。 Finding B 已准确定位机制:默认审批模式下,队友的 本 PR 正是这样做的,并将其推广到所有子代理共用的审批后恢复路径,从而在一处同时覆盖消息工具、任务工具和"仅 leader"防护:在循环帧仍存活时捕获队友身份,并在恢复边界上重建。上方评论中的 E2E 复现了验证所报告的确切症状( 作为更深的背景,这也补上了 #4844 两条延后加固讨论中的漏洞: |
Collaborator
|
Thanks for the PR! Template looks good ✓ — all required sections present, bilingual, before/after table, test plan, risk assessment. On direction: this is a clear-cut correctness bug in the experimental Agent Team feature. When a teammate's tool call goes through deferred approval, the AsyncLocalStorage frame for On approach: the scope is tight and the pattern is exactly right. The PR follows the same capture-at-emit / restore-at-respond pattern already used for Moving on to code review. 🔍 中文说明感谢 PR！ 模板完整 ✓ — 所有必需章节齐全，双语、前后对比表格、测试计划、风险评估。 方向：这是实验性 Agent Team 功能中的一个明确的正确性 bug。当队友的工具调用经过延迟审批时， 方案：范围紧凑，模式完全正确。PR 遵循与 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Collaborator
Code ReviewIndependent proposal (before seeing the diff): The bug is an AsyncLocalStorage frame loss at the deferred-approval boundary. When the reasoning loop emits Comparison: The PR's approach matches my independent proposal exactly. The diff is minimal and surgical:
No critical blockers. No AGENTS.md violations. The change is backward-compatible — the new parameter is optional, so all existing call sites (7 callers across Verification
Smoke Test (yolo mode)The bug only manifests in interactive default-approval mode, which requires real-time user interaction at the approval prompt. A yolo-mode smoke test verifies the Agent Team feature works end-to-end with the PR's changes — leader creates team, teammate sends message, leader receives it and reports DONE: Limitation: the deferred-approval identity restoration can only be tested interactively (default approval mode + approve prompt). The unit regression test covers the same code path programmatically and passes. 中文说明代码审查独立方案（在看到 diff 之前）： bug 是延迟审批边界的 AsyncLocalStorage 帧丢失。当推理循环发出 对比： PR 的方案与我的独立方案完全一致。diff 极简且精准：
无关键阻碍。无 AGENTS.md 违规。变更向后兼容——新参数是可选的。 验证
冒烟测试（yolo 模式）该 bug 仅在交互式默认审批模式下表现，需要在审批提示处进行实时用户交互。yolo 模式冒烟测试验证了 PR 变更后 Agent Team 功能的端到端可用性——leader 创建团队、队友发送消息、leader 接收并报告 DONE。 局限性： 延迟审批身份恢复只能在交互模式下测试（默认审批模式 + 批准提示）。单元回归测试以编程方式覆盖了相同代码路径并通过。 — Qwen Code · qwen3.7-max |
Collaborator
ReflectionStepping back: this is a clean, well-understood bug fix in a clearly defined area. The problem is specific (AsyncLocalStorage frame loss at the deferred-approval boundary), the impact is real (forged message attribution + security guard bypass), and the fix is minimal (one optional parameter, one capture, one pass-through). The PR's approach matches my independent proposal. The existing pattern — capture ambient context at emit time, restore it in the respond closure — was already established for What I'd maintain in six months: the code reads well. The The yolo smoke test confirms the feature works end-to-end with the change. The deferred-approval path can't be E2E tested in CI (requires interactive approval), but the unit regression test covers the same code path programmatically and passes. No concerns. Approving. 中文说明反思退一步看：这是一个干净、充分理解的 bug 修复，位于明确定义的领域中。问题具体（延迟审批边界的 AsyncLocalStorage 帧丢失），影响真实（伪造消息归属 + 安全保护绕过），修复最小化（一个可选参数、一次捕获、一次透传）。 PR 的方案与我的独立方案一致。现有模式——在发出时捕获环境上下文，在响应闭包中恢复——已为 六个月后维护时：代码可读性好。 yolo 冒烟测试确认了功能在变更后端到端正常工作。延迟审批路径无法在 CI 中进行 E2E 测试（需要交互式审批），但单元回归测试以编程方式覆盖了相同代码路径并通过。 无顾虑。批准。 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
approved these changes
Jun 11, 2026
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Contributor
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
approved these changes
Jun 11, 2026
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅ — qwen3.7-max via Qwen Code /review
This was referenced Jun 11, 2026
DragonnZhang
approved these changes
Jun 11, 2026
DragonnZhang
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed the full diff (+81/-7 across 2 files). No high-confidence issues found.
Summary of analysis:
The fix correctly captures the teammate identity frame (getTeammateContext()) at emit time while the loop's AsyncLocalStorage context is still live, and restores it via runWithTeammateIdentity() when the deferred-approval continuation runs from the UI's async chain. This follows the same capture-and-restore pattern already established for inheritedView and inheritedAgentId.
Key points verified:
- The new
inheritedTeammateIdentityparameter is optional, so all existing callers ofrunInAgentFrames()(e.g., the reasoning-loop path at line 547) are unaffected — they passundefinedimplicitly, and the identity wrapper is skipped (no-op for non-team scenarios). runWithTeammateIdentityusesAsyncLocalStorage.run(), which correctly scopes identity to the callback's async context without leaking to unrelated continuations.- The captured
TeammateIdentityis a plain value object (not a mutable ALS reference), so it is a correct snapshot. - The
respondclosure is already guarded by therespondedset against double-invocation. - The regression test faithfully simulates the scenario: identity frame live at capture, gone at resume, and includes a
setImmediatehop to ensure a fresh microtask chain. - CI: Lint and tests pass on Linux and Windows; macOS still in progress.
This fix also closes a security concern: a teammate's privileged control message (e.g., shutdown request) could previously bypass the leader-only isTeammate() guard after approval.
qwen-code-ci-bot
approved these changes
Jun 11, 2026
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
The capture-at-emit / restore-at-respond pattern is correctly applied, matching the existing inheritedView and inheritedAgentId conventions. The conditional wrapping ensures zero overhead for non-teammate agents. Test coverage is thorough, including the setImmediate microtask yield to verify ALS frame isolation. All 10,890 core tests pass.
— qwen3.7-max via Qwen Code /review
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
In the default approval mode, when a teammate sends a message to the leader and you approve it at the confirmation prompt, the message is now correctly attributed to that teammate. Previously it was mislabeled as coming from the leader.
Why it's needed
With a team running in the default approval mode, a teammate's message to the leader pauses for your confirmation. After you approved it, the system lost track of which teammate was acting and treated the message as if the leader had sent it. Two consequences:
This fired on every approved teammate message in the default approval mode. The
yoloand headless paths were unaffected, because they don't pause for a confirmation round-trip.Reviewer Test Plan
How to verify
QWEN_CODE_ENABLE_AGENT_TEAM=1 node dist/cli.js --approval-mode default● scribe reported back.● leader reported back, and the leader narrated the teammate as "appearing as 'leader' in the message routing".Evidence (Before & After)
● leader reported back● scribe reported backfrom="leader"(forged)from="scribe"Full E2E walkthrough posted as a comment below.
Tested on
Environment (optional)
Local bundled build (
node dist/cli.js), interactive tmux session, real model (glm-5.1 via OpenAI-compatible auth). New unit regression test added for the resume path.Risk & Scope
Linked Issues
Fast-follow on #4844 (Agent Team). Fixes Finding B from @wenshao's maintainer-verification of that PR (#4844 comment). Companion follow-up: #4981. Full background in the comment below.
中文说明
这个 PR 做了什么
在默认审批模式下,当队友给 leader 发消息、而你在确认提示处批准之后,该消息现在能正确归属到这名队友。此前它会被错误地标记为来自 leader。
为什么需要
当团队运行在默认审批模式下,队友发给 leader 的消息会暂停等待你确认。在你批准之后,系统丢失了"当前是哪名队友在执行"的上下文,于是把这条消息当作 leader 自己发的。由此带来两个后果:
在默认审批模式下,每一条被批准的队友消息都会触发该问题。
yolo和 headless 路径不受影响,因为它们不会经过确认往返。复核测试方案
如何验证
QWEN_CODE_ENABLE_AGENT_TEAM=1 node dist/cli.js --approval-mode default● scribe reported back。● leader reported back,且 leader 会叙述该队友"在消息路由中表现为 'leader'"。证据(前后对比)
● leader reported back● scribe reported backfrom="leader"(伪造)from="scribe"完整 E2E 走查见下方评论。
风险与范围