Repository navigation
feat: project agent identity and Agent Team delivery onto the ACP/daemon wire (refs #13785) - #13902
yiliang114 wants to merge 86 commits into
Conversation
…ames
Every output item in OpenAI's Responses multi-agent contract carries
`agent: { agent_name }` as a path (`/root`, `/root/security`), so a client can
attribute output and rebuild the tree without cross-frame association state.
Our frames carried only `parentToolCallId` plus a `subagentType` name, so two
parallel subagents of the same type were indistinguishable except by walking
parent chains.
Stamp a derived identity through the existing `SubagentMeta` seam rather than
adding a relay hop: both emitters already spread `...subagentMeta` into
`extra`, which `buildUpdateMeta` folds into the wire `_meta`, so message and
tool frames carry it from one field. The client parses it into typed fields and
the reducer carries it onto transcript blocks.
A half-populated identity projects to nothing instead of to a plausible name --
missing depth, missing kind, unknown kind, empty name and negative depth are
all rejected, because a client keys rendering and auto-approval policy on
`agentName` and must never see one whose depth/kind disagree with it.
Refs QwenLM#13785
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Team tool registration is gated on `Config.isAgentTeamEnabled()`, which knows nothing about the host, while the leader callback and the approval listener were attached by the TUI and the stream-json CLI only. On every other host the failure was silent: a model created a team, teammates ran, and `TeamManager.pollLeaderInbox()` returned before consuming because no callback was attached -- so reports were never delivered and the leader was never woken, while `TEAMMATE_APPROVAL_REQUEST` was emitted with no listener, leaving a blocked teammate neither approved nor cancelled. `attachTeamSink` closes both, reusing the channel core already models for cross-session messages instead of inventing a wake mechanism: reports are enqueued as `kind: 'peer'` background notifications, and `#enqueueBackgroundNotification` already ends by draining, so an idle leader wakes with no ACP protocol change. Approvals route through the existing `#requestPermissionQueued`; one the host cannot answer fails closed and tells the leader why, rather than letting the teammate proceed unreviewed. `peer` is the right kind, not merely a convenient one: core already carries it through `GROUP_ORDER`, `droppedNoun`, the `hasPeerLoss` accounting and `background-turn-context`, and like `cron` it names no task the model could go and read, so a dropped report is recorded as a real loss. Refs QwenLM#13785 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
The wire and the reducer already carried a path-style producer identity, but nothing read it, so a panel could only name its subagent by type. Two parallel subagents of the same type were therefore indistinguishable in the UI even though the frames said which was which. Carry the transcript block's `agent` through to the tool presentation as `agentPath`, and show it only where it adds information: at depth 1 the path's last segment *is* the localized type name, so rendering both would be noise in the common case. The full path goes on the name's `title` for hover precision, and only a nested subagent gets a visible path chip. `appendSubContent` adopts the first identity and never rewrites it, so a later frame cannot relabel a card the user is already reading. Refs QwenLM#13785 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
The producer-side derivation and the client-side type disagreed about what `agentName` guarantees: the client doc called it stable and invited keying policy on it, while the path's last segment is the subagent type -- so renaming an agent definition renames the path, and a policy keyed on the old string silently stops matching. State the real contract in both places: deterministic and safe to group or key in-session policy on, but not a durable id; a consumer that must survive renames keys on `parentToolCallId` instead. Minting a durable authority-owned agent id stays open in QwenLM#13785 rather than being faked here. Refs QwenLM#13785 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
The peer notification that carries a teammate report named its sender only inside the formatted display text, so a client had to parse the sender out of a string -- the exact escape-hatch failure the identity work exists to prevent. The subagent column already carried `_meta.agent`; the team column did not. `TeamManager` already knows the sender: the leader inbox entry carries `from`, and `MESSAGE_SENT` emits it. Thread it through the leader callback as an optional third argument (the two-argument callers in the TUI and stream-json are unaffected), and mint `/root/<member>` with `kind: 'teammate'` on the peer frame. A mixed-sender batch stays unattributed rather than naming one teammate for several reports. Refs QwenLM#13785 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
A client could see a teammate's report arrive but never learn who is on the team: roster membership lived only in TeamManager's in-memory state and the team file, with no wire surface. Join and exit events already exist on the team event emitter with structured names, so the sink now forwards them and the session projects each as a `peer` notification carrying the member's identity, letting a client reconstruct the roster from joins and exits without a roster route. Status changes and idle pulses are deliberately not projected: they fire on every transition, and each would wake the leader into an automatic turn for noise. Membership changes are the roster-defining subset. Refs QwenLM#13785 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterℹ️ No screenshot changed against the PR base — but this PR edits 4 render-shaping files:
Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
The live wire stamps _meta.agent on AgentTool frames from SubAgentTracker, but persistence keeps only subagentName inside the task_execution display, so a reloaded session replayed with no identity and the Web Shell lost the agent path on subagent panels. Replay now mints the root-parented identity from the task_execution display on the closing result frame; the SDK reducer adopts it on an existing tool block with the same first-wins policy as subagentType; the web-shell adapter maps the block agent onto agentPath so the panel tooltip survives reload.
A teammate report or roster change reached the Web Shell as a generic system row: the daemon projects `_meta.agent.kind === 'teammate'` on those frames, but the transcript adapter dropped the identity, so a multi-agent session read as a stack of anonymous notices instead of a conversation between named agents. The adapter now carries the minted teammate name onto the system message and SystemMessage renders it as an `@name` chip, while task and subagent notifications stay unchipped. Refs QwenLM#13785 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
…k_completed Real peer frames carry `_meta.source: 'background_task_completed'`, not 'background_notification', so the teammate chip added in the previous commit never rendered on a live session: the attribution lived on the wrong push site. Compute it on the task-completed push site as well and cover that path with a regression test. Refs QwenLM#13785 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
The Lint & Static job failed on three errors: teamSink.ts and teamSink.test.ts violate the KEBAB_CASE filename convention, and teamSink.ts imports values from the core package root, which the no-restricted-imports rule sends to the module defining each symbol (QwenLM#10908). Rename both files to team-sink.ts / team-sink.test.ts (updating the Session.ts import) and split the core import into the defining modules: config/approval-mode, agents/team/team-events, agents/team/TeamManager, tools/tools, utils/debugLogger and config/config. No allowlist entries added; the legacy lists only shrink.
The Agent Team column stamped teammate identity only on the live wire: `_meta.agent` rode the SSE frames, but `recordNotification` / `recordBackgroundTaskCompleted` dropped `item.agent` on the floor, so a reloaded session replayed peer notices with no attribution — the Web Shell turn-start row fell back to the generic 'peer' and the notice bubble named no one. Persist the identity inside the `backgroundTask` payload (replay passes it through wholesale), label peer turns with the sender's name so the turn-start row reads 'Cross-session message · <name>' both live and after reload, and render the persisted identity as a teammate chip on the notification bubble.
…-code into pr13902-lint-fix
In a live session the leader consumes each peer notification, so the `background_task_completed` content row is suppressed by the awaiting-processing guard and the only peer row the user sees is the turn-started marker — which carries no identity of its own. Correlate it by background task id against the display frames (which do carry `_meta.agent.kind === 'teammate'`) and render the same `@name` chip there, so a multi-agent session reads as named members talking instead of anonymous system rows. Refs QwenLM#13785 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
A replayed peer notice arrives as a flat user_message_chunk (no update wrapper), which the normalizer routes through the content-part path — the earlier agent extraction only covered the wrapped plain-text branch. Extract `_meta.agent` there too, so the block reducer's `block.agent` and the Web Shell's teammate attribution work after a reload, matching the live wire.
…' into iterate/pr-13902
Teammates ran as a black box: the leader's UI showed roster joins and final reports, but nothing of the work between. The event bridge already observed every teammate tool call, tool result, and team message — and discarded them as activity heartbeats. Forward that data as display-only frames: a new team activity event carries tool name, phase, and one-line description; the team sink also subscribes to the existing message-sent event. The session publishes them as peer frames stamped with the teammate's identity, without entering the background notification queue — activity is observational and must never wake the leader into an automatic turn, which is why these frames are also not persisted: the report a teammate eventually sends is what the transcript must keep. The web shell renders them as attributed notice bubbles: tool calls and results show the teammate chip and the tool line, inter-agent messages render the sender-to-recipient thread, so a multi-agent team now reads as named members collaborating instead of silent workers punctuated by system rows. Refs QwenLM#13785 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
The inter-agent frames rendered direction only in their display text, so a client that wanted the sender had to parse prose. OpenAI's Responses multi-agent contract models this natively: an agent_message item names the recipient through its agent attribute and the sender through a separate author field. Carry the same semantics on this wire, following the existing borrow-shape-not-spelling rule the agent identity type documents: the peer frame's agent now names the message recipient (the leader's teammates map to /root when addressed), and the structured payload gains an author identity with path, depth, and kind. Tool activity frames gain toolName, phase, and success alongside, so the whole activity surface is machine-readable rather than display-only. The web shell renders the sender as a second chip on inter-agent message rows — recipient chip from the teammate attribution, sender chip from the structured author — so a message thread reads at a glance who told whom. Refs QwenLM#13785 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
The direction already reads from the row text (writer → researcher: …); a second from-chip restated it, and the recipient chip alone carries the structured attribution. The wire keeps the author field — OpenAI's agent_message semantics stay machine-readable for clients that want the sender programmatically. Refs QwenLM#13785 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
There is a same-instance manager notification inverse in the new ACP team sink, independent of the normal manager-replacement path. At this head, Minimal deterministic control (using this PR's const manager = makeFakeTeamManager();
const detach = attachTeamSink(deps);
config.setTeamManager(manager);
config.setTeamManager(manager); // same identity, legal API call
expect(manager.emitter.listenerCount(TeamEventType.TEAMMATE_APPROVAL_REQUEST))
.toBe(1); // predicted: 2An emitted Bounded repair: in Source-path review only; I have not run the full ACP test suite. Isolated runtime reproduction: With Node's actual |
Config.setTeamManager fires its change callbacks even when the manager object is identical, and the sink re-registered all five emitter listeners on every notification: one teammate approval would open two host permission prompts and race two respond() calls, roster and activity events would double, and detach() would leave one copy of every listener leaked on the manager. Return early when the notified manager is the one already bound, and pin the single-listener invariant plus clean detach with a same-instance re-notification regression test. Refs QwenLM#13785 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Bilingual design doc for the projection this PR lands: the single AgentIdentity shape across the subagent and teammate columns, the display-only teammate activity surface (no queue, no persistence, no leader wake), OpenAI-shaped message direction (agent = recipient, structured author = sender), and the decisions behind ephemerality. Records the CJK teammate-name collapse and the follow-up list (persisted per-teammate summary, colors, collapsed grouping). Refs QwenLM#13785 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
The same-instance re-notification bug is fixed in b1eeae5: Pinned by a regression test that re-fires the change callback with the already-bound manager and asserts every listener count stays 1 (and reaches 0 after detach) — alongside the existing A→B swap coverage. The fail-closed approval path is untouched. Also reproduced your exact fixture shape: |
E2E report: teammate activity & inter-agent message projectionAll scenarios ran against a local daemon ( S1 — Multi-teammate parallel run, distinct attribution. A three-teammate team (researcher / writer / checker) with sentinel-terminated replies. 15 peer frames on the wire, each stamped with the correct identity; no taskId collisions across teammates. After a full page reload every cross-session row renders its own chip ( S2 — Teammate tool activity without waking the leader. A two-teammate relay produced 20 activity frames in one turn ( S3 — Inter-agent messages with structured direction. A coder→critic verification relay produced 8 message frames. Direction follows the agent_message semantics this PR adopted: the frame's agent attribute names the recipient ( S4 — Chinese content end to end. Teammates with pinyin names exchanged Chinese-message content over the wire: 5 of 6 message frames carried Chinese text (3 containing the expected 天空 marker sentence, plus the ZH-OK sentinel). Non-ASCII paths extract correctly. One pre-existing issue surfaced outside this PR: CJK teammate names collapse in the team name sanitizer on main — filed separately as #13920, not introduced here. S5 — Reload attribution. Live activity frames are ephemeral by design (documented in the design doc); the persisted reports and roster changes reload with frame-level identity, so post-reload the transcript still attributes every peer row (12/12 rows chipped in the DOM probe). Unit coverage for the same contracts runs in CI: activity direction + no-wake + listener retraction (Session suite), tool/message forwarding + same-instance listener counts (team-sink suite), activity-frame classification and chip rendering (web-shell suites). Design doc: |
Teammate activity notices — attributed bubbles for teammate tool calls and inter-agent messages — grow the transcript entry by 3,487 UTF-16 code units, measuring 1,323,853 locally and identically in CI, past the 1,323,000 ceiling the bundle-size guard pinned. Raise the ceiling to 1,325,000 and record the measurement next to the previous ones, per the guard's own re-measure-and-raise convention. Refs QwenLM#13785 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
main's QwenLM#13760 (managed cwd slice) and this branch's teammate activity notices both grew the transcript entry and both edited the same measurement comment. Keep both narratives — the W2 i18n table growth and the 3,487-unit teammate notice addition — so the ceiling comment still explains every contributor to the 1,325,000 bound.
The export document's block author came only from the workspace-agent message meta, so a peer notice block carried its text but lost the producer identity the live Web Shell attributes — the exported transcript read as anonymous system rows for everything an Agent Team teammate said. Fall back to the block's wire agent field when no workspace-agent author exists: the path-style name travels as the author name and the identity kind alongside it, so an exporter can tell a teammate from a delegated subagent. The agent-message meta author still wins when both exist — it names the workspace agent that actually wrote the block, while the wire field names the projected identity. Refs QwenLM#13785 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
…' into iterate/pr-13902
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Code Review Summary (QwenCode Reviewer)
Overall Assessment: Changes Requested
This PR projects agent identity and Agent Team coordination onto the ACP/daemon wire. The design is solid, test coverage is extensive (2106+ focused cases), and security posture is strong (fail-closed approvals, binding epoch recycling, XML escaping). However, CI gate is not fully green.
Blocking Issues
-
CI gate not fully green — Two checks are in
failstate:Several additional checks remain
pending(TUI parity snapshots, SDK Java full suites, authorize). Before merge, please confirm whether these failures are introduced by this PR or are pre-existing/flaky. Suggest re-running or providing exclusion evidence.
Suggestions (non-blocking)
-
packages/cli/src/acp-integration/session/team-sink.ts:42—let nextBindingId = 0is a module-level mutable global. If multiple ACP hosts exist in the same process (e.g., testing or multi-tenant scenarios), binding IDs increment across sessions without reset. Functionally correct under high-watermark semantics, but consider per-instance scoping if isolation is needed in the future. -
packages/cli/src/acp-integration/session/Session.ts(#emitTeammateActivity) — Activity text fields (name,description,message) are concatenated into plain-text display strings without XML/HTML escaping. This is safe under the current architecture (display-only, non-persisted), but if downstream consumers render these in HTML/XML contexts, XSS protection will be needed. Consider adding a comment noting this boundary. -
packages/acp-bridge/src/transcript-replay.ts(agentIdentityFromTaskExecution) — Replay can only reconstruct root-parented identities (/root/${subagentName}, depth=1). Nested subagent hierarchy (depth >= 2) is lost. PR description marks this as a known M1a follow-up; suggest adding an inline TODO referencing the tracking issue. -
packages/cli/src/acp-integration/session/Session.ts(#emitTeammateActivityvs#enqueueTeammateMessage) — Activity frames are dropped whenthis.disposed || this.closing, while teammate messages intentionally do not early-return on closing (queue is the sole carrier). The asymmetry is correct but non-obvious; suggest a comment explaining why activity discard is safe here. -
Test coverage gap —
Integration Tests (CLI, No Sandbox)is skipping in CI. PR description acknowledges no real-model or full daemon/browser E2E was run. Please confirm the CLI integration test skip is unrelated to this change before merge.
Security Review: PASS (with notes)
- Approval flow is fail-closed: host unavailability results in Cancel, not auto-approve
- Binding epoch + AbortController correctly recycles in-flight approvals on manager swap/detach
- Teammate report content is XML-escaped before embedding in
<peer-notification>envelope - BridgeClient validates all notification fields (type, sessionId, bindingId, teammateName) with strict type checks
- No hardcoded secrets or tokens found in diff
- Note: activity display text is unescaped (see suggestion #2 above); safe today but watch downstream consumers
Conclusion
Changes Requested — Core design and implementation quality are high. The only blocker is CI gate status. Once CI failures are confirmed as non-regressions (or fixed), this PR is ready for merge.
|
Qwen Code review request accepted. Review is queued for an available runner; follow the workflow run for progress. A command-triggered review is not listed under the checks of this PR; the result is posted here as a review when it finishes. |
qwen-code-review-bot
left a comment
qwen-code-review-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
16 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- SubAgentTracker.ts:88 — parentAgent/agentIdentity dead production surface — already reported (round 5/7/8 review-body deferral lists)
- team-sink.test.ts:520 — same-instance-guard test unfalsifiable — already reported (round 8 deferral list, team-sink.test.ts:529)
- team-sink.test.ts:153 — deps.signal abort contract untested — already reported (round 8 deferral list)
- team-sink.test.ts:216 — answers passthrough untested — already reported (round 5 deferral list, team-sink.ts:259)
- messageTypes.ts:79 — agentPath doc claims a uniqueness the field lacks — already reported (round 5/8 deferral lists)
- transcriptToMessages.ts:1792 — comment names the wrong first-wins mechanism — already reported (round 5 deferral list, :1756)
- TeamManager.ts:1945 — TEAMMATE_ACTIVITY emit sites untested — already reported (round 5/8 deferral lists)
- Session.ts:12011 — teammate-exited sending-side hop untested — already reported (round 6/7/8 deferral lists)
- transcript-replay.ts:1833 — background_task_completed identity hoist untested — already reported (rounds 5-8 deferral lists)
- Session.ts:13705 — unreachable permissionScope guard untested — already reported (rounds 5-8 deferral lists)
- transcriptToMessages.ts:613 — teammate_activity missing from isHideableStep — already reported (round 8 deferral list, :618)
- Session.test.ts:1353 — single-sender consumer gate unpinned — already reported (round 5/6/7 deferral lists)
- Session.test.ts:1361 — label never pinned nor persisted — already reported (round 6 deferral list, :1455)
- team-sink.ts:260 — host-cancelled maps to bare Cancel with no reason or leader notice — already reported (round 6 deferral list R6-5, team-sink.ts:252)
- agent-identity.test.ts:258 — text-block identity projection unwitnessed — already reported (round 5 deferral list, transcript.ts:913)
- transcriptToMessages.ts:576 — pre-scan marker-before-identity ordering untested — already reported (round 5 deferral list, :570)
Not reviewed: test-efficacy probing — the probe harness could not be validated (vitest globalSetup guard in the prebuilt worktree); all 18 mutant probes and 110 hunk probes were inconclusive or budget-skipped.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": I did not trace the bridge replay → Web Shell backgroundTurn construction end-to-end, so finding 3's "after reload" consequence is verified only up to the per…; "agent reverse-audit (round 2)": executing packages/acp-bridge vitest over the eight new tests — I verified the fixtures, helper signatures and expected _meta shapes by reading both sides, …; chunk 8: none — I did not confirm whether the acp-bridge consumer at bridgeClient.ts:1063 handles the new 'teammate_activity' _meta.source value, because cross-fil…; "agent reverse-audit (round 1)": did not trace whether a same-channel session restore replaces the BridgeClientSessionEntry object (which would reset the per-entry retiredThrough fence whil…; chunk 14: none — every check above ran to completion., and 7 more.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Deferred under the convergence posture (round 9, not a blocker) — recorded, not requested in this round:
packages/acp-bridge/src/transcript-replay.ts:324 — [review] SUBAGENT_NAME_PATTERN comment claims a re-check of SubagentValidator's constraint but re-checks only the charset — reserved 'main', 1-char and 51-char names get replay attribution …packages/cli/src/acp-integration/session/Session.ts:12038 — [review] taskId-uniqueness comment cites the public enqueueBackgroundNotification dedup, but the invoked private #enqueueBackgroundNotification has none — the real work-hold Set in…packages/sdk-typescript/src/daemon/ui/transcript.ts:1710 — [review] discrete-frame exemption (message/failed) removes the maxBlocks bound for teammate_activity rows — never-persisted rows accumulate unbounded and trimTranscriptState evicts …packages/sdk-typescript/src/daemon/ui/transcript.ts:1730 — [review] coalesceTeammateActivity's backward walk has no proximity bound — the next unrelated tool call rewrites an arbitrarily distant row in place; the committed test pins the tim…packages/sdk-typescript/src/daemon/ui/types.ts:112 — [review] the five new public agent-field docs contradict the wire: text-side keys recipient-vs-producer on message class not frame family; status-side fields undocumented; tool-side says …packages/sdk-typescript/test/daemon-ui-transcript.test.ts:663 — [review] the clearActiveText:false opt-out coalesce shape — the only shape the wire emits — is witnessed by no test; an unconditional-clearActiveText mutation stays green while…packages/web-shell/client/adapters/transcriptToMessages.ts:622 — [review] activity rows project blockTime from the origin envelope while the coalescer rewrites text in place — latest activity text renders beside the first activity's stale t…packages/web-shell/client/adapters/transcriptToMessages.ts:1795 — [review] mergeToolCall copies 13 fields but not agentPath — in the approved-permission-first flow the placeholder's undefined wins permanently, so the new tooltip never rende…packages/web-shell/client/adapters/transcriptToMessages.ts:661 — [review] peer turn-started marker names the teammate twice — this PR adds both the @name chip and label:sender, and the row renders label as visible text beside the chip, sque…packages/web-shell/client/components/messages/SystemMessage.test.tsx:1110 — [review] two of four assertions in the new peer-report label test cannot fail for any input (label lands only in aria-label/title; notification.peer.* never looked …packages/web-shell/client/components/messages/SystemMessage.tsx:376 — [review] folding teammate_activity into isTaskNotification reuses the background-task label/tone/icon table — ephemeral activity is announced to screen readers as 'Task n…packages/web-shell/client/components/MessageItem.tsx:302 — [review] the teammate prop passthrough from adapter to SystemMessage is asserted by no test (the dom test stubs SystemMessage as () => null) — dropping the prop ships green while ev…
Convergence: round 9 posted 10 inline comment(s), 10 of them reported for the first time; the previous round posted 3 (3 new). Findings keep coming back to the same files: packages/cli/src/acp-integration/session/Session.ts (findings in round 8; 2 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 keeps the loop from re-deriving the same set; this PR's reviews already resolve to a critical posting floor. (Observation only — nothing was withheld from this review because of this observation.)
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
Residual risk: this loop is persistently critical — Criticals stood in the previous round's work-list and stand again this round (3 Critical(s)), the rate of first-time findings is not falling (this round 10, previous 3), and the standing Critical backlog is not shrinking. The severity floor will not converge it. Recommendation: land-with-residual-risk — the exit is a maintainer risk-acceptance decision (merge, carrying the residual risk), not another review round. Residual-risk inventory for that decision (maintainer to complete):
| standing Critical | attack surface | attacker-dependency | blast radius |
|---|---|---|---|
| (each standing Critical) | … | … | … |
Advisory only — it does not block this review.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 16 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查（原文为英文）：test-efficacy probing — the probe harness could not be validated (vitest globalSetup guard in the prebuilt worktree); all 18 mutant probes and 110 hunk probes were inconclusive or budget-skipped.
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：I did not trace the bridge replay → Web Shell backgroundTurn construction end-to-end, so finding 3's "after reload" consequence is verified only up to the per…；"agent reverse-audit (round 2)"：executing packages/acp-bridge vitest over the eight new tests — I verified the fixtures, helper signatures and expected _meta shapes by reading both sides, …；chunk 8：none — I did not confirm whether the acp-bridge consumer at bridgeClient.ts:1063 handles the new 'teammate_activity' _meta.source value, because cross-fil…；"agent reverse-audit (round 1)"：did not trace whether a same-channel session restore replaces the BridgeClientSessionEntry object (which would reset the per-entry retiredThrough fence whil…；chunk 14：none — every check above ran to completion.，另有 7 条。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
收敛姿态下延后（第 9 轮，非阻断）——已记录，本轮不要求修改：共 12 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 9 轮发布了 10 条行内评论，其中 10 条是首次提出；上一轮发布了 3 条（其中 3 条首次提出）。发现反复回到同一批文件：packages/cli/src/acp-integration/session/Session.ts（第 8 轮已出过发现，本轮又有 2 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，可以避免循环反复推导同一组发现；本 PR 的评审已解析为 critical 发布下限。（仅为观察——本轮评审未因此扣留任何内容。）
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
残余风险：本循环处于 persistently-critical 形态——上一轮工作清单中的 Critical 本轮依然存在（本轮 3 条 Critical），首次发现的速率没有下降（本轮 10，上一轮 3），且未决 Critical 积压没有减少。severity floor 无法使其收敛。建议：land-with-residual-risk——出口是 maintainer 的风险接受决定（合入并承担残余风险），而非再开一轮评审。供该决定使用的残余风险清单（maintainer 填写）：按每条未决 Critical 列出「攻击面 · 攻击者依赖性 · 影响范围」三栏。仅为建议——不阻断本次评审。
— qwen3.8-max via Qwen Code /review (v0.26.1)
|
qwen-code-review-bot
left a comment
qwen-code-review-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
16 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- SubAgentTracker.ts:88 parentAgent/agentIdentity dead production surface — already reported (rounds 5/7/8 deferral lists)
- Session.ts:13705 unreachable permissionScope guard — already reported (rounds 5-8 deferral lists)
- TeamManager.ts:1957 TEAMMATE_ACTIVITY emit sites untested on the core side — already reported (rounds 5/8 deferral lists)
- Session.test.ts:1355 single-sender consumer gate unpinned — already reported (rounds 5/6/7 deferral lists)
- sdk-typescript transcript.ts:913 text-block agent adoption unwitnessed — already reported (round-5 deferral list)
- transcript-replay.ts:1832 background_task_completed identity hoist untested — already reported (rounds 5-8 deferral lists)
- transcriptToMessages.test.ts:785 pre-scan marker-before-identity ordering untested — already reported (round-5 deferral list, :570)
- transcriptToMessages.ts:648 peer turn-started marker names the teammate twice — already reported (round-9 deferral list, :661)
- MessageList.tsx:877 teammate_activity missing from isHideableStep — already reported (round-8 deferral list, transcriptToMessages.ts:618)
- messageTypes.ts:79 agentPath doc claims a uniqueness the field lacks — already reported (rounds 5/8 deferral lists)
- sdk-typescript types.ts:1134 agent-field docs contradict the wire on recipient-vs-producer — already reported (round-9 deferral list, types.ts:112)
- sdk-typescript transcript.ts:1731 coalesce backward walk has no proximity bound — already reported (round-9 deferral list, :1730)
- sdk-typescript transcript.ts discrete-frame carve-out removes the maxBlocks bound — already reported (round-9 deferral list, :1710)
- transcriptToMessages.ts:1795 mergeToolCall does not carry agentPath — already reported (round-9 deferral list)
- transcript-replay.ts:325 SUBAGENT_NAME_PATTERN re-checks only the charset — already reported (round-9 deferral list, :324)
- Session.ts:12011 teammate-exited sending-side hop untested — already reported (rounds 6/7/8 deferral lists)
Unresolved, please confirm:
- [Critical] R9-1 packages/web-shell/client/adapters/transcriptToMessages.ts:1947 — this round's verification REFUTED the behavioural claim: the design doc shipped in this PR (docs/design/2026-10-11-agent-team-identity-projection.md:41,:46) states the f…
Not reviewed: cross-file tracer (1c) — did not verify whether the daemon's in-session event journal replays teammate_activity frames to a reconnecting client — stopped at the agent tool budget.
Not reviewed: test-efficacy probing — the probe harness could not be validated (harnessValidated: null; all 18 mutant probes and every hunk probe inconclusive on the vitest globalSetup prerequisite guard), so coverage efficacy is unmeasured rather than clean.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": whether any frame is actually emitted from SubAgentTracker in the window between constructor and the START event on the ordinary non-aborted path — this det…; "agent 1c": did not verify whether the daemon's in-session event journal ( entry.events.publish / subscribeEvents({lastEventId}) ) replays teammate_activity frames to a…; "agent reverse-audit (round 1)": I did not determine which Config shape the in-process teammate's CoreToolScheduler receives ( new CoreToolScheduler({ config: this.runtimeContext }) , agent-c…; "agent reverse-audit (round 1)": whether any wire frame carrying subagentMeta.agent is emitted before AgentEventType.START replaces the raw args['subagent_type'] identity — I read both e…; "agent reverse-audit (round 1)": emitResult 's _meta construction (tool-call-emitter.ts:236 onward) — I read emitPreparationDiscarded and the start path's ...params.subagentMeta (tool-ca…, and 19 more.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Deferred under the convergence posture (round 10, not a blocker) — recorded, not requested in this round; 1 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:
packages/cli/src/acp-integration/session/Session.ts:5183 — [review] Critical [fails-closed] [new-surface] Shutdown flush snapshots only notificationQueue, so an…docs/design/2026-10-11-agent-team-identity-projection.md:65 — [review] design-doc Acceptance per-suite test counts are stale at…packages/acp-bridge/src/bridgeClient.ts:1071 — [review] bridgeClient.ts:1071-1079 every rejection arm of the new…packages/acp-bridge/src/bridgeClient.ts:2937 — [review] Both private routes' malformed-payload reject blocks are…packages/acp-bridge/src/bridgeClient.ts:2965 — [review] The teammate-exited handler ownership guard and payload…packages/cli/src/acp-integration/session/Session.test.ts:1788 — [review] Session.test.ts:1788-1791 toHaveLength(3) is the only…packages/cli/src/acp-integration/session/Session.test.ts:1794 — [review] Session.test.ts:1794-1797 bakes the literal 'unknown', an…packages/cli/src/acp-integration/session/Session.ts:12410 — [probe] Landing an evicted peer report before tallying flips the…packages/cli/src/acp-integration/session/emitters/tool-call-emitter.ts:372 — [review] tool-call-emitter.ts:372-373 the new _meta.agent lands…packages/cli/src/acp-integration/session/team-sink.test.ts:148 — [review] team-sink.test.ts:148-149 both retirement mocks only…packages/cli/src/acp-integration/session/team-sink.test.ts:157 — [review] No test sets permissionPersistencePolicy, so the sink…packages/cli/src/acp-integration/session/team-sink.test.ts:444 — [review] The manager-change transition to no manager (team…packages/cli/src/acp-integration/session/types.test.ts:16 — [review] types.test.ts:16-22 the ' ' leg pins a live/replay parity…packages/core/src/agents/team/TeamManager.ts:1249 — [review] the 'unknown' sentinel is not reserved against legal…packages/core/src/agents/team/TeamManager.ts:1955 — [review] TeamManager.ts:1955-1963 two unbounded un-coalesced…packages/core/src/agents/team/TeamManager.ts:2174 — [review] Dead else branch: PendingMessage.from can never be empty…packages/sdk-typescript/test/daemon-ui-transcript.test.ts:472 — [review] daemon-ui-transcript.test.ts:472 the…packages/web-shell/client/adapters/transcriptToMessages.ts:589 — [review] transcriptToMessages.ts:589-591 the pre-scan's text-block…packages/web-shell/client/adapters/transcriptToMessages.ts:1938 — [review] Client-side JSDoc calls the recipient chip an 'author',…packages/web-shell/client/components/messages/SystemMessage.tsx:221 — [review] Of the two chip render sites this diff adds, only the…
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 16 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未决，请确认：共 1 条（原文未翻译，列表见上方英文部分）。
未审查（原文为英文）：cross-file tracer (1c) — did not verify whether the daemon's in-session event journal replays teammate_activity frames to a reconnecting client — stopped at the agent tool budget.
未审查（原文为英文）：test-efficacy probing — the probe harness could not be validated (harnessValidated: null; all 18 mutant probes and every hunk probe inconclusive on the vitest globalSetup prerequisite guard), so coverage efficacy is unmeasured rather than clean.
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 2)"：whether any frame is actually emitted from SubAgentTracker in the window between constructor and the START event on the ordinary non-aborted path — this det…；"agent 1c"：did not verify whether the daemon's in-session event journal ( entry.events.publish / subscribeEvents({lastEventId}) ) replays teammate_activity frames to a…；"agent reverse-audit (round 1)"：I did not determine which Config shape the in-process teammate's CoreToolScheduler receives ( new CoreToolScheduler({ config: this.runtimeContext }) , agent-c…；"agent reverse-audit (round 1)"：whether any wire frame carrying subagentMeta.agent is emitted before AgentEventType.START replaces the raw args['subagent_type'] identity — I read both e…；"agent reverse-audit (round 1)"：emitResult 's _meta construction (tool-call-emitter.ts:236 onward) — I read emitPreparationDiscarded and the start path's ...params.subagentMeta (tool-ca…，另有 19 条。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
收敛姿态下延后（第 10 轮，非阻断）——已记录，本轮不要求修改；其中 1 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface：未认证任何错误结果，且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中：共 20 条（原文未翻译，列表见上方英文部分）。
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
— qwen3.8-max via Qwen Code /review (v0.26.1)
|
本地视觉验证（当前 head 按
结论：在当前 head 上，UI 没有可见变化。 这符合改动性质 —— 身份是 ACP/daemon 线上的协议字段，标准视图里看不到差异；此前的视觉预览是旧 head（ 渲染出的界面样本（当前 head，浅色主题）：
不适用项：无「改动前后差异图」可贴 —— 82 个视图逐个对比均为 0%，这正是本次验证要说明的事实。 |
|
Split into three stacked draft PRs without changing this branch:
The cumulative tree at C ( Each draft links its inherited findings: A owns R9-3/R9-4; B owns R8-3/R9-2/R9-5/R9-7/R9-8/R9-9; C owns R9-1/R9-6. The original threads remain open. Build and focused-test evidence is tied to each split commit; runtime/provider E2E, browser verification and a separate typecheck pass have not been run. Review and land in A → B → C order. After a parent lands, reconcile the dependent branch with main before retargeting, so its displayed diff does not accidentally include the parent again. This original PR remains open and unchanged as the reference until the replacement stack is ready. |
qwen-code-review-bot
left a comment
qwen-code-review-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- DaemonAgentIdentity not exported from the sdk-typescript barrels (packages/sdk-typescript/src/daemon/ui/types.ts:345) — already reported: round-1 review-body deferral list (review 5511196624, types.ts:132), re-listed in round 2 (review 5523…
- Replay identity rebuild over-fires via the 'running' status whitelist and its invariant comment is false (packages/acp-bridge/src/transcript-replay.ts:2716) — already reported: round-1 review-body deferral list (review 5511196624, transcrip…
Not reviewed: incidental findings I4 (agentName JSDoc scope) and I5 (vi.fn absorbs unhandled rejections) — no verifier ruled on them (budget stop); carried as low-confidence, terminal-only.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": whether a teammate name can contain / (which would make ${ROOT_AGENT_IDENTITY.agentName}/${name} a 3-segment path at depth: 1 and teammateOfAgent 's .sp…; chunk 3: confirming that the frame builder wrapping extra: { ...subagentMeta } ( createTranscript*Update , path not located before the ceiling) adds only _meta keys …; "agent reverse-audit (round 1)": the end-to-end agent-respawn path in session-control-plane.ts — I confirmed the BridgeClient is constructed inside constructChannelInfo(channel, …) and th…; "agent reverse-audit (round 1)": did not read sanitizeName itself, so I confirmed the 'unknown' collision for the literal name Unknown / unknown but not for other inputs that might saniti…; "agent reverse-audit (round 1)": did not execute coordination-harness.test.ts or api-user-prompt.test.ts ; the falsifiability claims above are from reading the assertions and the production …, and 7 more.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Deferred under the convergence posture (round 11, not a blocker) — recorded, not requested in this round:
packages/acp-bridge/src/bridgeClient.ts:857 — [review] BridgeClient's class JSDoc now attaches to the new PendingTeammatePermission interface; the class is undocumentedpackages/cli/src/acp-integration/session/Session.test.ts:1375 — [review] peer _meta.agent assertion cannot distinguish senders-derived attribution from envelope-text parsing; the hostile-fixture sibling never asserts itpackages/cli/src/acp-integration/session/Session.test.ts:1360 — [review] no test drives leaderCallback with a multi-sender batch, so the senders?.length===1 attribution guard is unpinnedpackages/cli/src/acp-integration/session/Session.ts:14213 — [review] permissionScope==='session' early return in #withBackgroundPermission is unreachable yet cited as the queue-bypass justificationpackages/cli/src/acp-integration/session/team-sink.test.ts:510 — [review] same-instance-guard test cannot detect the guard's removal (mutation-probed); its comment asserts listener-doubling the ordering makes impossiblepackages/cli/src/acp-integration/session/team-sink.test.ts:152 — [review] deps.signal AbortController discarded — session-lifetime abort and AbortSignal.any composition entirely unpinned (mutant-verified)packages/core/src/agents/team/TeamManager.ts:1259 — [review] deliverLeaderMessages senders-array contract half-enforced; the length-1 attributability rule lives only in the one consumer that knows itpackages/qwen-live/src/adaptor/public-activity.ts:23 — [review] display-only source denylist hand-copied in 4+ places; publicActivity fails open — todo_stop_guard/vision_bridge_notice publish (executed sweep)packages/core/src/services/chatRecordingService.ts:601 — [review] persisted backgroundTask.agent.kind declares 'subagent' but the only replay reader accepts 'teammate'; the writer coerces root to subagent (probe)packages/sdk-typescript/test/agent-identity.test.ts:181 — [review] tool-block create-path agent stamping untested; deleting it keeps 46/46 green (mutant executed)packages/web-shell/client/adapters/transcriptToMessages.ts:576 — [review] the marker-before-identity out-of-order arrival the teammateByTask pre-scan exists for is never tested (mutant executed)packages/web-shell/client/adapters/transcriptToMessages.ts:1792 — [review] comment attributes first-wins agentPath adoption to appendSubContent; the real live-side loss point is the normalizer's progress-frame drop (probe)packages/web-shell/client/adapters/transcriptToMessages.ts:1795 — [review] mergeToolCall does not copy agentPath; approved agent tools registered via the permission placeholder never receive it (probe)packages/acp-bridge/src/transcript-replay.ts:1833 — [review] background_task_completed replay hoist untested; deleting it keeps 157/157 green (probe)packages/sdk-typescript/src/daemon/ui/transcript.ts:913 — [review] text-delta to block.agent reducer copy untested; deleting it keeps 46/46 green (probe)packages/core/src/agents/team/TeamManager.ts:1948 — [review] TEAMMATE_ACTIVITY producer payload has zero tests; producer mutations keep 16/16 green (probe)packages/core/src/agents/team/TeamManager.ts:1789 — [review] teardown emits N TEAMMATE_EXITED while the sink is attached, waking the leader N times about a deleted team; overflow can evict real notifications (probe)docs/design/2026-10-11-agent-team-identity-projection.md:65 — [review] all seven design-doc Acceptance witness counts stale (all six suites executed at HEAD)docs/design/2026-10-11-agent-team-identity-projection.md:35 — [review] design doc states reports enqueue with agent+label unconditionally; multi-sender batches drop both; not in Known gapspackages/acp-bridge/src/bridgeClient.ts:1108 — [review] sessionOwned clears promptId with no replacement bound; one unanswered teammate card pins transcript serving to the bounded journal fallback- …and 22 more (see the run report)
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
Residual risk: this loop is persistently critical — Criticals stood in the previous round's work-list and stand again this round (2 Critical(s)), the rate of first-time findings is not falling (this round 1, previous 0), and the standing Critical backlog is not shrinking. The severity floor will not converge it. Recommendation: land-with-residual-risk — the exit is a maintainer risk-acceptance decision (merge, carrying the residual risk), not another review round. Residual-risk inventory for that decision (maintainer to complete):
| standing Critical | attack surface | attacker-dependency | blast radius |
|---|---|---|---|
| (each standing Critical) | … | … | … |
Advisory only — it does not block this review.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查（原文为英文）：incidental findings I4 (agentName JSDoc scope) and I5 (vi.fn absorbs unhandled rejections) — no verifier ruled on them (budget stop); carried as low-confidence, terminal-only.
未审查（原文为英文）：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：whether a teammate name can contain / (which would make ${ROOT_AGENT_IDENTITY.agentName}/${name} a 3-segment path at depth: 1 and teammateOfAgent 's .sp…；chunk 3：confirming that the frame builder wrapping extra: { ...subagentMeta } ( createTranscript*Update , path not located before the ceiling) adds only _meta keys …；"agent reverse-audit (round 1)"：the end-to-end agent-respawn path in session-control-plane.ts — I confirmed the BridgeClient is constructed inside constructChannelInfo(channel, …) and th…；"agent reverse-audit (round 1)"：did not read sanitizeName itself, so I confirmed the 'unknown' collision for the literal name Unknown / unknown but not for other inputs that might saniti…；"agent reverse-audit (round 1)"：did not execute coordination-harness.test.ts or api-user-prompt.test.ts ; the falsifiability claims above are from reading the assertions and the production …，另有 7 条。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
收敛姿态下延后（第 11 轮，非阻断）——已记录，本轮不要求修改：共 42 条（原文未翻译，列表见上方英文部分）。
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
残余风险：本循环处于 persistently-critical 形态——上一轮工作清单中的 Critical 本轮依然存在（本轮 2 条 Critical），首次发现的速率没有下降（本轮 1，上一轮 0），且未决 Critical 积压没有减少。severity floor 无法使其收敛。建议：land-with-residual-risk——出口是 maintainer 的风险接受决定（合入并承担残余风险），而非再开一轮评审。供该决定使用的残余风险清单（maintainer 填写）：按每条未决 Critical 列出「攻击面 · 攻击者依赖性 · 影响范围」三栏。仅为建议——不阻断本次评审。
— qwen3.8-max via Qwen Code /review (v0.26.1)
|
Verdict: findings — behavior verified green at head Verified head: 中文摘要结论：findings。 行为在真实 daemon + 真实模型下通过：head 上 teammate 帧带 唯一合并阻塞：GitHub 报 未覆盖：Web Shell 浏览器里 peer chip 的 DOM 断言（截图只验证了会话加载与 leader 回复）；teammate 审批的真实客户端交互（只有单测 + mutation）；Windows 矩阵；depth ≥ 2；崩溃后重投递。详见下方表格与 Not covered。 Central claim and A/BThe central claim is that the ACP/daemon wire now attributes output to the agent that produced it in two mutually exclusive columns: subagent frames carry Each arm below is a real
The head-team identities are all exactly Findings1. Merge-blocking: the PR is
|
| mutation | witness | result |
|---|---|---|
M1 coarse: replace the constructor identity in SubAgentTracker |
SubAgentTracker "uses START definition" |
survived — equivalent mutant: the START handler re-derives subagentMeta.agent before the assertion reads it |
M1b: remove the frame-level ...(agent ? { agent } : {}) in tool-call-emitter |
same test | killed (2 failed) |
M2: change the teammate peer identity kind: 'teammate' → 'subagent' |
Session "delivers a teammate report as a peer notification" |
killed (1 failed) |
M3: remove emitter.on(TEAMMATE_APPROVAL_REQUEST, approvalListener) |
team-sink "routes a teammate approval to the client" |
killed (1 failed) |
Every mutated file was restored with cp and its git blob hash re-checked against HEAD; the head tree is clean.
Not covered
- Web Shell peer chip as a browser DOM assertion. The real session was opened in a real headless Chromium against the live daemon: the session loaded and the leader reply rendered, but the
@correctnesschip was not visible in the captured collapsed transcript view. The chip and the adapter are covered by 330 passing web-shell unit tests, not by a browser-level presence assertion. I am not calling this a defect; I am saying the browser-level half of the PR's Web Shell claim is unverified here. - Teammate approval through a real ACP client. The routing and fail-closed paths are covered by the unit suite and M3, not by a live client answering a permission request.
- Nested depth ≥ 2 identity. The PR states there is no wire producer today; the run only observed depth 1.
- Crash-safe redelivery after process/session destruction. The PR lists this as follow-up work.
- Windows. The daemon arms ran on macOS; the PR's own tested-on table lists Linux only. No Windows arm was run.
- Full repository suite and full daemon/browser E2E. Only the focused packages and the four real daemon arms above were run.
Methodology
Four real daemon worktrees were built serially (corepack pnpm install --frozen-lockfile, prepare build included): head 6ed7884c, base 9df10f87 (the merge-base, which equals baseRefOid here), and a locally resolved trial merge of origin/main at 8e1ead35. After the merged gates ran, main advanced to 57f4bd14; the conflict set was re-checked at that tip with git merge-tree and is unchanged (one Session.ts import hunk), but the merged-tree build/test numbers above were measured at 8e1ead35. Each arm ran node scripts/dev.js serve --port <p> --hostname 127.0.0.1 --workspace <ws> with an isolated HOME/QWEN_HOME/QWEN_RUNTIME_DIR and a minimal settings copy (user MCP servers stripped); team arms set QWEN_CODE_ENABLE_AGENT_TEAM=1. The driver opened GET /session/:id/events before POST /session/:id/prompt, captured every SSE envelope to events.jsonl, and the assertions read the actual event.data.update._meta fields — no log-text matching for identity. Focused gates were run from each package directory with env -u QWEN_CODE_SERVE -u QWEN_RUNTIME_DIR: CLI 62, core 130, SDK 46, web-shell 330 at head; CLI 62, teammate-filtered Session 9, web-shell 330 on the merged tree. Raw logs and harness scripts are under tmp/pr13902-verify-20261027-201619/.
The two terminal-style images are rendered from the run artifacts with scripts/verify-capture.mjs; the Web Shell image is a real Chromium screenshot of the live head-team daemon at 127.0.0.1:5391.
Evidence captures
Real four-arm A/B, parsed from the SSE captures:
Exact _meta.agent payloads from the head-team / head-subagent captures, and the base zero-signal arm:
Web Shell loaded against the same live head-team daemon (real Chromium):
Verification run by @wenshao with Qwen Code; all counts come from the artifacts under tmp/pr13902-verify-20261027-201619/, and every SHA named above is the exact tree that produced them.
…rge-20261027 # Conflicts: # packages/cli/src/acp-integration/session/Session.ts
…rge-20261027 # Conflicts: # packages/cli/src/acp-integration/session/Session.test.ts
Main's QwenLM#15441 retired Session.getRewindableUserTurnCount and made rewindToTurn resolve its target through the file-history snapshot's promptId plus the recorder's turn index. The two peer-notification rewind pins predicted that accessor and asserted the old numeric contract, so the merged tree built red. Mark the two rewindable turns, leave the delivered peer report unmarked (that is the behaviour under test), seed the snapshots and assert the same cut point through the identity contract. Both cases still pass with the peer report outside the rewind surface.
|
Merged What was merged: Conflicts and resolutions:
Verification (local, on the merged tree): full build pass · This PR stays the reference copy for the split stack: its review findings are being addressed in #15657 / #15658 / #15659 — nothing here addresses them, the merge only unbreaks the branch. |
Two replay findings from the #13902 review, both about trusting the persisted record too much: - `executionMode` alone was treated as proof the subagent started, but two rejection paths persist it for a session that never registered: worktree-provisioning failure (`status: 'failed'`) and the foreground model-slot cancel (`status: 'cancelled'`). Require `subagentSessionReady === true` on that leg, which is the field current writers flip once the subagent session exists; fixtures modelling a genuine run-then-fail now carry it. - the rejection branch was a negative allowlist gated on `typeof status === 'string'`, so a truncated or older record with an absent/non-string status minted an identity the live wire never stamped. Replace it with a positive allowlist over completed/running/background/ cancelled. New cases: both rejection-shaped displays stay unattributed, and absent / non-string / null status stays unattributed. Dropping the session-ready requirement reddens the first two; restoring the negative gate reddens the status cases.
…rge-20261027 # Conflicts: # packages/cli/src/acp-integration/session/Session.ts
…13902 A/3] (#15657) * feat(acp): project subagent identity through live and replay frames * style(sdk): format split agent identity regression test * fix(acp-bridge): mint replay identity only on positive evidence of a run Two replay findings from the #13902 review, both about trusting the persisted record too much: - `executionMode` alone was treated as proof the subagent started, but two rejection paths persist it for a session that never registered: worktree-provisioning failure (`status: 'failed'`) and the foreground model-slot cancel (`status: 'cancelled'`). Require `subagentSessionReady === true` on that leg, which is the field current writers flip once the subagent session exists; fixtures modelling a genuine run-then-fail now carry it. - the rejection branch was a negative allowlist gated on `typeof status === 'string'`, so a truncated or older record with an absent/non-string status minted an identity the live wire never stamped. Replace it with a positive allowlist over completed/running/background/ cancelled. New cases: both rejection-shaped displays stay unattributed, and absent / non-string / null status stays unattributed. Dropping the session-ready requirement reddens the first two; restoring the negative gate reddens the status cases. * style(acp-bridge): format the replay identity test The new union-typed `_meta` casts were left in a shape prettier rewrites, so the PR-stage Prettier gate failed on this file alone.
|
Pull request was converted to draft
|
Converting this PR to draft to reduce review-channel noise while its content is re-landed in smaller, reviewable layers. It is not abandoned — it is being superseded by the three-PR stack. Stack relationship the reviewer should know about:
This PR itself is the union of those three, plus a few things the stack has not yet re-landed: The conflicting state + 12 failing checks on this head are downstream of branching before main integrated the stack and the #15778 export-renderer headroom change; they are not evidence of a defect in the remaining content. |
qwen-code-review-bot
left a comment
qwen-code-review-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Unresolved, please confirm:
- [Critical] packages/cli/src/acp-integration/session/Session.ts:12800 (R8-3) — carried Critical re-checked at this head: the mechanism stands (blanket
escapeXmlover model text core already framed, so the inner<teammate_message>envelope and the s…
Not reviewed: reverse audit — stopped after round 3 without two consecutive dry rounds (rounds 1, 2 and 3 each reported; rounds 4-5 were not run, so the audit did not converge).
Not reviewed: packages/cli test suites — the six changed files never executed at this head because the web-templates build failure aborts the package's vitest global setup (see R12-1).
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; the macOS/Windows Test matrix cells were skipped too, so no cross-platform run exercised the changed web-shell and cli code.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": did not read chunks other than 18; the two anchors sit on added lines immediately above my range (diff lines 6268-6277), reached from the shared source file and…; "agent reverse-audit (round 1)": whether a BridgeClientSessionEntry outlives its child process — a reused entry would keep a stale retiredThrough while a fresh child restarts nextBindingId…; "agent reverse-audit (round 3)": did not execute team-sink.test.ts to confirm green — packages/web-templates/dist/index.js is absent in this worktree and the export-html build is over its c…; "agent reverse-audit (round 3)": did not execute packages/sdk-typescript/test/daemon-ui-transcript.test.ts ; the coalescing trace above is manual.; "agent reverse-audit (round 3)": did not run packages/cli / packages/acp-bridge suites to confirm the new isAgentTeamEnabled: vi.fn().mockReturnValue(false) , getTeamManager: null and o…, and 4 more.
Deferred under the convergence posture (round 12, not a blocker) — recorded, not requested in this round:
packages/cli/src/acp-integration/session/SubAgentTracker.ts:88 — [review] parentAgent option and agentIdentity getter have no production caller or reader; the depth-2 tests pin a shape no producer emitspackages/web-shell/client/components/messages/SystemMessage.tsx:222 — [review] turn-start marker row prints the teammate name twice (chip and turn.label come from the same sender)packages/web-shell/client/adapters/messageTypes.ts:227 — [review] teammate JSDoc says the field names the author, but inter-agent rows mint it from the recipient while data.author (the real sender) has no readerpackages/cli/src/acp-integration/session/team-sink.test.ts:444 — [review] the null-manager teardown leg ( team_delete -> setTeamManager(null) ) has no test
Convergence: round 12 posted 2 inline comment(s), 1 of them reported for the first time; the previous round posted 2 (1 new). The rate of new findings is not falling. Batching the remaining fixes and verifying them before the next push keeps the loop from re-deriving the same set; this PR's reviews already resolve to a critical posting floor. (Observation only — nothing was withheld from this review because of this observation.)
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
Residual risk: this loop is persistently critical — Criticals stood in the previous round's work-list and stand again this round (2 Critical(s)), the rate of first-time findings is not falling (this round 1, previous 1), and the standing Critical backlog is not shrinking. The severity floor will not converge it. Recommendation: land-with-residual-risk — the exit is a maintainer risk-acceptance decision (merge, carrying the residual risk), not another review round. Residual-risk inventory for that decision (maintainer to complete):
| standing Critical | attack surface | attacker-dependency | blast radius |
|---|---|---|---|
| (each standing Critical) | … | … | … |
Advisory only — it does not block this review.
中文说明
仅完成部分审查，审查缺口已披露。
未决，请确认：共 1 条（原文未翻译，列表见上方英文部分）。
未审查（原文为英文）：reverse audit — stopped after round 3 without two consecutive dry rounds (rounds 1, 2 and 3 each reported; rounds 4-5 were not run, so the audit did not converge).
未审查（原文为英文）：packages/cli test suites — the six changed files never executed at this head because the web-templates build failure aborts the package's vitest global setup (see R12-1).
未审查（原文为英文）：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; the macOS/Windows Test matrix cells were skipped too, so no cross-platform run exercised the changed web-shell and cli code.
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 2)"：did not read chunks other than 18; the two anchors sit on added lines immediately above my range (diff lines 6268-6277), reached from the shared source file and…；"agent reverse-audit (round 1)"：whether a BridgeClientSessionEntry outlives its child process — a reused entry would keep a stale retiredThrough while a fresh child restarts nextBindingId…；"agent reverse-audit (round 3)"：did not execute team-sink.test.ts to confirm green — packages/web-templates/dist/index.js is absent in this worktree and the export-html build is over its c…；"agent reverse-audit (round 3)"：did not execute packages/sdk-typescript/test/daemon-ui-transcript.test.ts ; the coalescing trace above is manual.；"agent reverse-audit (round 3)"：did not run packages/cli / packages/acp-bridge suites to confirm the new isAgentTeamEnabled: vi.fn().mockReturnValue(false) , getTeamManager: null and o…，另有 4 条。
收敛姿态下延后（第 12 轮，非阻断）——已记录，本轮不要求修改：共 4 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 12 轮发布了 2 条行内评论，其中 1 条是首次提出；上一轮发布了 2 条（其中 1 条首次提出）。新发现的产出速度没有下降。把剩余修复攒成一批、验证后再推送，可以避免循环反复推导同一组发现；本 PR 的评审已解析为 critical 发布下限。（仅为观察——本轮评审未因此扣留任何内容。）
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
残余风险：本循环处于 persistently-critical 形态——上一轮工作清单中的 Critical 本轮依然存在（本轮 2 条 Critical），首次发现的速率没有下降（本轮 1，上一轮 1），且未决 Critical 积压没有减少。severity floor 无法使其收敛。建议：land-with-residual-risk——出口是 maintainer 的风险接受决定（合入并承担残余风险），而非再开一轮评审。供该决定使用的残余风险清单（maintainer 填写）：按每条未决 Critical 列出「攻击面 · 攻击者依赖性 · 影响范围」三栏。仅为建议——不阻断本次评审。
— deepseek-v4.1-flash-1017@e6bf8ffe via Qwen Code /review (v0.26.2)
| // arrive before the display frame carrying the identity, so filling this | ||
| // inside the loop leaves the marker row — the row the user actually sees — | ||
| // anonymous. | ||
| const teammateByTask = new Map<string, { name: string }>(); |
There was a problem hiding this comment.
[Critical] R12-1: [build] [regression] This diff's added transcript-renderer code pushes the /export html document renderer 1,004 bytes past its hard ceiling, so npm run build --workspace=packages/web-templates fails at this head — and because packages/cli compiles and tests against packages/web-templates/dist, the failure also blocks every packages/cli unit suite.
Witness (run):
$ npm run build --workspace=packages/web-templates # head 1539038057
Document export renderer JS is 2050214 bytes; component CSS moved to export-transcript-document.css is 2395565 bytes
Error: Document export renderer JS is 2050214 bytes; expected <= 2050000.
^ build.mjs:374, npm error code 1
$ same scoped build on merge base 9ef52e153
Document export renderer JS is 2049210 bytes → exit 0
The ceiling carried 790 bytes of headroom at 9ef52e15; this branch spends 1,004. Inside one run web-templates/dist is never produced, so packages/cli's vitest global setup aborts with workspace package "@qwen-code/web-templates" has not been built (missing packages/web-templates/dist/index.js) and none of the six changed packages/cli suites (Session.test.ts, team-sink.test.ts, acpAgent.test.ts, SubAgentTracker.test.ts, types.test.ts, create-sub-session.test.ts) executed at this head; packages/cli, packages/qwen-live and packages/vscode-ide-companion never built at all.
Fix: either trim the added renderer payload, or raise the ceiling deliberately and record the measurement the way that file's own comments do (MAX_DOCUMENT_RUNTIME_BYTES = 2_060_000, with the measured 2,050,214 at this head against 2,049,210 at 9ef52e15).
Acceptance: npm run build --workspace=packages/web-templates must exit 0 at the new head — the build check is its own witness, and it exits 1 without the fix.
Constraint: build.mjs:77-81 states the rule the new number must respect — "Keep the warning close to the measurement and the hard ceiling close above it: a cap left far above the measurement is a ratchet with enough slack for a whole dependency family to come back unnoticed" — with DOCUMENT_RUNTIME_WARNING_BYTES = 1_970_000 staying proportional, so a round multi-megabyte cap is not the fix.
中文说明
本 diff 新增的 transcript 渲染代码把 /export html 文档渲染包推过硬阈值 1,004 字节，导致 npm run build --workspace=packages/web-templates 在当前 head 直接失败；由于 packages/cli 依赖 packages/web-templates/dist 编译与测试，这同时阻断了全部 packages/cli 单测。
证据（已运行）：head 测得 2,050,214 字节 > 2,050,000，build.mjs:374 抛错、退出码 1；merge base 9ef52e15 同一命令测得 2,049,210，退出码 0——即基线只剩 790 字节余量，而本分支用掉 1,004。后果：web-templates/dist 不会生成，packages/cli 的 vitest global setup 直接以「has not been built」中止，本 PR 改动的六个 cli 测试文件在本 head 一个都没能执行；packages/cli、packages/qwen-live、packages/vscode-ide-companion 也完全未构建。
修复：要么削减新增渲染载荷，要么按该文件既有注释的做法显式抬升阈值并记录实测值（如 MAX_DOCUMENT_RUNTIME_BYTES = 2_060_000，注释写明 head 2,050,214 / 9ef52e15 2,049,210）。验收标准就是该构建本身退出 0。约束：build.mjs:77-81 要求阈值贴近实测、警告值保持比例，不能改成粗放的多兆字节上限。
— deepseek-v4.1-flash-1017@e6bf8ffe via Qwen Code /review (v0.26.2)
| // Queued teammate reports are already marked read in the on-disk | ||
| // mailbox, so land them here: the writer-close loop below runs before | ||
| // `dispose()`, and `appendRecord` is a silent no-op once it has closed. | ||
| session.flushPendingPeerNotifications?.(); |
There was a problem hiding this comment.
[Critical] R11-1: [certifies-falsely] [new-surface] Still stands at this head. beginManagedShutdown lands queued teammate reports before peer intake is quiesced, and the landing latches persisted even when the recorder has already stopped accepting writes — so a report arriving in the shutdown window is lost silently while the durable state records it as landed.
session.flushPendingPeerNotifications() runs synchronously right after the unawaited void session.cancelPendingPrompt().catch(…) (this file, 4234), and nothing stops peer intake first. Two entrances put a peer item into notificationQueue after that one-pass flush:
- an in-flight notification turn cancelled by the abort re-queues its already-spliced item on a later microtask (
notificationQueue.unshift(item),Session.ts:13855/:13905); TeamManager's 500 ms leader-inbox poll keeps running untildispose()— no ACP call site callsstopLeaderInboxPolling, anddetachTeamSinkhappens only insidedispose()(Session.ts:5290) — while#enqueueTeammateMessagebails only ondisposed(Session.ts:12786), so a report polled duringcloseManagedRuntime/flushDeferredExternalRecords/recorder.finalize()is pushed and never drained, because both drain paths return onclosing(Session.ts:13594-13596).
dispose() then calls #landUndrainedPeerNotifications() again, whose recordNotification reaches appendRecord's silent early return once acceptingWrites is false (chatRecordingService.ts:1792-1795), while item.persisted = true is still latched (Session.ts:5235) and notificationQueue is cleared (Session.ts:5301). The mailbox row was marked read before the callback ran, so the report exists nowhere and nothing is logged. The sibling stored-session path settles turns first (await session.waitForActiveTurnsToSettle(), this file 5049) and only then flushes (5140); this path has no settle.
Witness: not run — the interleaving was not driven (the review's scratch tree returned available:false on the repository's git include), so both the ordering and the drop are quoted from the code at head (acpAgent.ts:4233-4234, :4243-4250, :4262-4295, :4747; Session.ts:5207-5238, :5290-5301; chatRecordingService.ts:1792-1795). A second auditor and this round's verifier each re-traced all five links and found every one present; the residual uncertainty is how often an event lands inside a sub-second window, not whether the mechanism exists.
Fix: quiesce intake before landing — stop the leader-inbox poll and detach the sink at the top of flushPendingPeerNotifications() — await cancelPendingPrompt() (or waitForActiveTurnsToSettle()) ahead of the flush, and set item.persisted only when the recorder still accepts writes.
Acceptance: extend lands queued teammate reports before managed shutdown closes the writers (acpAgent.test.ts) so the fake session's cancelPendingPrompt returns a promise whose continuation pushes a report into the array the flush reads, and assert that report was landed before closeSessionWriter; it reds against today's ordering and greens once the cancel is awaited and intake quiesced. A Session.test.ts case asserting a peer item enqueued after flushPendingPeerNotifications() is still written (or is refused a persisted latch) pins the second half.
Constraint: #enqueueTeammateMessage must keep accepting reports while closing — TeamManager.pollLeaderInbox() consumes and marks the row read before invoking the callback, so returning early there "would turn 'retained, undelivered' into 'consumed, lost'" (Session.ts:12783-12789), and a reversible close that is released must still deliver, pinned by "still projects a roster change that lands inside a reversible close" (Session.test.ts:1736). The fix therefore has to quiesce intake at the irrevocable point, not reject enqueues.
中文说明
R11-1 在当前 head 仍然存在。beginManagedShutdown 在尚未静默 peer 摄入的情况下就落地排队中的队友报告，且落地时在 recorder 已停止接收写入的情况下仍然锁存 persisted——因此关停窗口内到达的报告会被静默丢弃，而持久化状态却记录它已落地。
session.flushPendingPeerNotifications() 紧跟在未被 await 的 void session.cancelPendingPrompt().catch(…)（本文件 4234 行）之后同步执行。有两个入口会在这次一次性 flush 之后把 peer 项压入 notificationQueue：(1) 被 abort 的进行中通知轮次会在之后的 microtask 把已 splice 的项 unshift 回去（Session.ts:13855 / :13905）；(2) TeamManager 的 500ms leader-inbox 轮询一直跑到 dispose() 为止（ACP 侧无任何调用 stopLeaderInboxPolling，detachTeamSink 只在 dispose() 内，Session.ts:5290），而 #enqueueTeammateMessage 只在 disposed 时返回（Session.ts:12786），于是在 closeManagedRuntime / flushDeferredExternalRecords / recorder.finalize() 期间轮询到的报告被压入却永不被消费——两条 drain 路径都在 closing 时返回（Session.ts:13594-13596）。
随后 dispose() 再次调用 #landUndrainedPeerNotifications()，其 recordNotification 在 acceptingWrites 为 false 时命中 appendRecord 的静默早退（chatRecordingService.ts:1792-1795），但 item.persisted = true 仍被锁存（Session.ts:5235），随后 notificationQueue 被清空（Session.ts:5301）。邮箱行在回调前已被标记为已读，因此报告在任何地方都不存在，也没有任何日志。同级的 stored-session 路径会先 await session.waitForActiveTurnsToSettle()（本文件 5049）再 flush（5140）；本路径没有 settle。
证据：未运行——scratch tree 返回 available:false（仓库 git include 不可读），故顺序与丢弃均引用 head 代码（acpAgent.ts:4233-4234、:4243-4250、:4262-4295、:4747；Session.ts:5207-5238、:5290-5301；chatRecordingService.ts:1792-1795）。另一名审计者与本轮 verify 各自复走了五个环节，全部成立；残余不确定性是事件落入亚秒窗口的频率，而非机制是否存在。
修复：落地前先静默摄入——在 flushPendingPeerNotifications() 开头停止 leader-inbox 轮询并 detach sink；在 flush 之前 await cancelPendingPrompt()（或 waitForActiveTurnsToSettle()）；仅在 recorder 仍接收写入时才置 item.persisted。验收：扩展 acpAgent.test.ts 的 lands queued teammate reports before managed shutdown closes the writers，让 fake session 的 cancelPendingPrompt 在 continuation 中向 flush 读取的数组推入一条报告，并断言该报告在 closeSessionWriter 之前落地——当前顺序为红，await 取消并静默摄入后转绿。约束：不得因为 closing 就拒绝入队（Session.ts:12783-12789：邮箱先被消费并标记已读，早退会把「保留未投递」变成「已消费即丢失」），可逆 close 释放后仍须投递（Session.test.ts:1736 钉住）。
— deepseek-v4.1-flash-1017@e6bf8ffe via Qwen Code /review (v0.26.2)
What this PR does
Projects agent identity and Agent Team coordination onto the ACP/daemon wire, in two deliberately distinct collaboration columns.
Subagent column (parent→child delegation): every projected subagent frame (thought, message, tool call, tool call update) now carries
_meta.agent = { agentName, depth, kind }with a path-style name (/root/general-purpose), mirroring OpenAI Responses' per-itemagentidentity; the Web Shell keeps that path on the subagent panel name as atitletooltip only — no visible path chip ships in this slice (SubAgentPanel.tsx:345; pinned bySubAgentPanel.test.tsxkeeps the path on the name title without rendering it as a chip).agentNameis a readable role path, not a unique execution id: concurrent same-type subagents share it, and renaming the agent definition changes it.parentToolCallIddistinguishes subagent executions; that boundary is documented on both the producer and the normalizer.Agent Team column (peer complete agents): an ACP-hosted session now attaches a leader sink (
team-sink.ts). Before this,TeamManager.pollLeaderInbox()returned before consuming on this host (teammate reports stayed unread in the leader inbox) andTEAMMATE_APPROVAL_REQUESTwas void-emitted — the coordination never reached anyone. Now teammate reports are delivered aspeerbackground notifications carrying structured sender identity (kind: 'teammate', minted from the inbox entry'sfrom, never parsed from message text), roster joins/exits are projected as peer notifications so an already-attached client can track membership transitions (there is no current-state snapshot for late attach or reconnect), and teammate tool approvals route to the client'srequestPermission(fail-closed: Cancel plus a<team_notice>to the leader when the host cannot answer; YOLO auto-approve only when the request carries no renderable details).Why it's needed
#13785 records the gap: the public vocabulary names child agents and teams, but nothing a client receives says which agent produced what. Both reference protocols (OpenAI Responses multi-agent, Anthropic Managed Agents) treat agent identity as contractual. This slice delivers the identity dimension on the daemon/ACP projection and removes the "ACP host has no leader sink" failure class — the zero-signal class among the three host classes in #13785 M4 — giving M1b a reference wire shape. Peer delivery reuses core's existing
'peer'background-notification channel (idle wake, loss accounting, kind-agnostic drain/display), without a new peer runtime. Team-binding approval retirement now uses one private, versionedqwen/notify/session/teammate-binding-endednotification on the existing SDK 0.14.1 connection; it adds no daemon REST route or public mediator API.Reviewer Test Plan
How to verify
At
3499b027c4faa, 2,106 focused cases, targeted ESLint/Prettier, five affected workspace typechecks and integration typecheck passed on the exact committed candidate. Approval retirement, status identity, activity isolation and canonical START behavior were mutation-checked. A serialized ACP component probe verified old-binding cancellation and replacement approval at cap 1; a local macOS browser rendered the real transcript pipeline with injected protocol frames without splitting leader output or taking its usage. Main was synchronized to supply the current CI budget helper; obsolete assistant-activity handling was removed, and the rebuilt transcript's size ceiling was raised from 1,338,000 (main) to 1,340,000 inpackages/web-shell/client/build-artifact.test.ts— merged with main the entry measures 1,338,274 UTF-16 units, past both main's ceiling and the 1,338,500 this branch carried. See the current repair report for build results and verification limits. No real-provider or full daemon/browser E2E was run for this repair. The real-model report and screenshots atdf760052100d1and the runtime observations below are historical evidence, not current-head acceptance.Real machine: boot the daemon from this branch with
QWEN_CODE_ENABLE_AGENT_TEAM=1 node scripts/dev.js serve --port <p> --token <t> --workspace <dir>, create a session, send a team prompt (team_create, then spawn teammatecorrectnessasking it to report one sentence ending in a marker). Expected and observed on SSE: peer notificationsteammate-roster-1(status joined) andteammate-message-2/3, each carrying_meta.agent={"agentName":"/root/correctness","depth":1,"kind":"teammate"}, plus leader-reply frames withsource=background_notification_response; the same capture contains zero subagent-column frames. Contrast run without a team: 47parentToolCallId-chained frames carryingkind:'subagent'and zero peer markers — the two columns are mutually attributable at a glance. The Web Shell rendered the peer rows (ⓘ … · peer) and the verbatim teammate report in a real headless Chromium against the same daemon.Evidence (Before & After)
Before: on this host an ACP session with a team delivered 0 teammate reports (retained unread in the leader inbox) and emitted 0 approval signals; no projected object carried any agent identity. After: the SSE markers and browser rendering described above.
Tested on (earlier runtime acceptance)
Environment (optional)
Linux host; daemon via
node scripts/dev.js serve(no bundle step); headless Chromium (playwright) for Web Shell rendering; unit suites via vitest withenv -u QWEN_CODE_SERVE -u QWEN_RUNTIME_DIR.Risk & Scope
_meta; older readers ignore it. Teammate identity is minted only for single-sender batches — mixed-sender batches stay unattributed rather than misattributed. Status/idle team events are deliberately not projected (each would wake the leader into an automatic turn). Join/exit notifications are incremental, so a client that attaches late still needs a non-waking roster snapshot/query before it can claim authoritative current membership.parentToolCallIdlayer, depth values only 1), so depth ≥ 2 identity has no wire producer yet — nested trees remain record-projection work (M1a); cross-session teammate projection is M1b/M3. Mailbox consumption and model-visible delivery are not transactional; crash-safe redelivery after process/session destruction remains follow-up work, and reversible-close retention does not provide that guarantee.Linked Issues
Refs #13785 (referenced without a closing keyword: the public-contract milestones remain open design work).
中文说明
本 PR 做了什么
把 agent 身份与 Agent Team 协调投影到 ACP/daemon 的 wire 上，分为刻意保持区分的两条协作列。
subagent 列（父子委派）：每个投影出的 subagent 帧（thought、message、tool call、tool call update）现在都带
_meta.agent = { agentName, depth, kind }，名字为 path 风格（/root/general-purpose），与 OpenAI Responses 的逐 itemagent身份同形；Web Shell 本片只把该 path 挂在 subagent 面板名字的titletooltip 上，不渲染可见的 path chip（SubAgentPanel.tsx:345；由SubAgentPanel.test.tsx的keeps the path on the name title without rendering it as a chip钉住）。agentName是可读的角色 path，并不是唯一执行 ID：同类型并发 subagent 会共享它，agent 定义改名也会改变它；parentToolCallId用于区分 subagent 执行，这一边界在生产侧与 normalizer 两侧都有文档。Agent Team 列（对等完整 agent）：ACP 托管会话现在会挂一个 leader sink（
team-sink.ts）。在此之前，本宿主上TeamManager.pollLeaderInbox()会在消费前早退（队员报告留在 leader inbox 里未读），TEAMMATE_APPROVAL_REQUEST空 emit——协调过程到不了任何人手里。现在队员报告以peer背景通知投递，带结构化发送者身份（kind: 'teammate'，从 inbox 条目的from铸造，绝不从消息文本解析）；roster 的加入/退出投影为 peer 通知，已挂载的客户端可跟踪成员变化（晚挂载或重连时没有当前状态快照）；队员的工具审批路由到客户端requestPermission（fail-closed：宿主无法应答时 Cancel 并向 leader 注入<team_notice>；仅当请求不带可渲染细节且处于 YOLO 时才自动批准）。为什么需要
#13785 记录了缺口：公共词表里有 child agent 与 team，但客户端收到的任何东西都不说明「哪条输出是哪个 agent 产的」。两个参照协议（OpenAI Responses multi-agent、Anthropic Managed Agents）都把 agent 身份当契约内容。本片在 daemon/ACP 投影侧交付身份维度，并消掉「ACP 宿主没有 leader sink」这一失效类——即 #13785 M4 三类宿主中的零信号类——为 M1b 给出参考 wire 形状。peer 投递复用 core 既有的
'peer'背景通知通道（空闲唤醒、丢失记账、kind 无关的 drain/display），不增加 peer runtime。队绑定审批回收新增一条私有、版本化的qwen/notify/session/teammate-binding-ended通知，走既有 SDK 0.14.1 连接；不添加 daemon REST 路由或 public mediator API。评审测试计划
如何验证
3499b027c4faa的精确提交候选通过 2,106 条聚焦用例、定向 ESLint/Prettier、五个相关 workspace 的类型检查和 integration 类型检查；审批回收、status 身份、活动隔离与 START canonical 行为做过变异反证。真实 ACP 串行化组件链路以 cap 1 验证旧绑定取消和新绑定审批；本地 macOS 浏览器通过真实 transcript 流水线渲染注入的协议帧，leader 答案与 usage 未被活动切断或夺走。已同步 main 补齐当前 CI 预算脚本，并删除旧 assistant 活动路径；重建 transcript 的体积阈值是上调的——由 main 的 1,338,000 提到packages/web-shell/client/build-artifact.test.ts里的 1,340,000，因为与 main 合并后该 entry 实测 1,338,274 UTF-16 单位，同时超出 main 的阈值和本分支此前携带的 1,338,500。构建结果与验证限制见本轮修复报告。本轮未跑真实模型或完整 daemon/browser E2E。df760052100d1的真实模型报告与截图及下方真机记录仅为历史证据，不作为当前 head 验收。真机：从本分支起 daemon（
QWEN_CODE_ENABLE_AGENT_TEAM=1 node scripts/dev.js serve --port <p> --token <t> --workspace <dir>），建会话，发 team prompt（team_create，再派 teammatecorrectness要求其回一句以 marker 结尾的话）。SSE 上期望并实测到：peer 通知teammate-roster-1（status joined）与teammate-message-2/3，均带_meta.agent={"agentName":"/root/correctness","depth":1,"kind":"teammate"}，以及source=background_notification_response的 leader 回话帧；同一抓帧中 subagent 列帧为零。不带 team 的对照跑：47 帧parentToolCallId链帧带kind:'subagent'、peer 标记零命中——两列形状互斥、可一眼归因。Web Shell 在真 headless Chromium 中对同一 daemon 渲染出了 peer 行（ⓘ … · peer）与队员原话。证据（Before & After）
Before：本宿主上带 team 的 ACP 会话投递 0 条队员报告（留在 leader inbox 未读）、0 个审批信号；任何投影对象都不带 agent 身份。After：上述 SSE 标记与浏览器渲染。
Tested on（较早版本的真机验收）
环境（可选）
Linux 宿主机；daemon 走
node scripts/dev.js serve（无 bundle 步骤）；Web Shell 渲染用 headless Chromium（playwright）；单测走 vitest 并加env -u QWEN_CODE_SERVE -u QWEN_RUNTIME_DIR。风险与范围
_meta，旧读者会忽略它。teammate 身份只为单发送者批次铸造——混合发送者批次宁可无身份也不张冠李戴。status/idle 队事件刻意不投影（每条都会唤醒 leader 起一个自动 turn）。加入/退出通知是增量流，晚挂载客户端仍需要一个不会唤醒 leader 的 roster 快照/查询，才能声称拿到了权威当前成员表。关联 Issue
Refs #13785（不用 closing 关键字引用：公共契约里程碑仍是开放的设计工作）。