feat(core): address other sessions by name from send_message and list_agents - #10158
feat(core): address other sessions by name from send_message and list_agents#10158qqqys wants to merge 11 commits into
Conversation
…_agents Send side of same-machine cross-session messaging, on top of the receive side that landed in QwenLM#9576. `list_agents` now lists the other Qwen Code sessions on this machine that advertise an inbox (probed, not just registered), each with the same name `qwen sessions ps` prints, a short `ref` derived from its session id, and the session's own name under `self`. `send_message` accepts that name as `to`: a bare name that matches exactly one live session delivers; two sessions sharing a name need `name [ref]`, and an ambiguous bare name is refused rather than guessed. An in-process teammate wins a name collision, `to: "*"` stays a team-only broadcast, and addressing one's own name is named as the mistake it is. Frames carry the recipient's session id; a receiver whose id differs (PID reuse, /clear) refuses with a receipt. Receipts for messages this session sent are surfaced in the transcript — held, denied, expired, and a delivery that ends a hold — and receipts for ids this session never sent are dropped. Claude-Session: https://claude.ai/code/session_01MGkHMaMFhR2gfhbk5koXEC
…g the PR Four independent review passes on the first cut (send-path correctness, receiver-side races and abuse, mutation probes of the new tests, and prompt/docs/spec parity) folded in: - A teammate's report to `leader` (or the lead agent id) is an in-process send and is never routed through the peer directory, where a session named `leaderboard-…` would have been suggested and the report lost. - A frame pinned to a session id this process does not hold is refused with a distinct `misaddressed` receipt instead of `denied`, and the receiver re-asserts its registry record, since a skipped /clear patch can leave the record naming the previous id; that patch now retries. - The send ledger is a state machine: a receipt is surfaced only when it moves the message to a new state, so a repeated or forged receipt can no longer grow the history, and the UI needs no bookkeeping of its own. - The session-id getter and the ledger are wired before the socket binds, closing the post-listen window the file already documents. - A peer whose name a teammate shadows (sanitized equality) is listed with its ref; registry-sourced names are flattened for `self` too; the disabled-feature error says so instead of claiming a lookup; the sent result tells the model it will not learn the outcome and must not re-send; ETIMEDOUT is described as possibly still readable. - Docs: the parity rule stated in full, receipts described as transcript notices to the user rather than tool results, a `/peers` row, settings rows for the two options. - Tests for `probePeerSocket` and `readOwnSessionRecord`, and one per surviving mutant. Claude-Session: https://claude.ai/code/session_01MGkHMaMFhR2gfhbk5koXEC
|
Thanks for the PR! Template: complete — all required sections present, with a real Reviewer Test Plan and tmux evidence. ✓ Problem: this is not a fix but step 3 of the maintainer-tracked roadmap in #8724 (labels Direction: aligned. #8724 deliberately stages the work receive-first and off by default, and this PR stays inside that ladder. The reference agent ships the same capability — Claude Code's changelog carries several cross-session-messaging entries (e.g. a fix for it "silently turning off inside user namespaces and rootless containers after the 2.1.232 socket-directory hardening", and a 30 s inbox connection deadline) — so the area is established rather than speculative. Size: core paths are touched. Breakdown of the 3014 changed lines: 1111 production logic, 1850 test, 2 generated/schema, 51 docs. Per the two-tier rule a Approach: the scope feels right. Discovery reuses the #8969 registry, transport reuses the #9576 frames and Risk: no high-risk path matches from the revert-history analysis (no streaming/MCP/shell/sandbox/ACP surfaces touched). The IPC trust boundary itself was reviewed to rest in #9576; this PR extends it without moving it. Moving on to code review. 🔍 中文说明感谢贡献! **模板:**完整——所有必需小节齐全,且附有真实的 Reviewer Test Plan 和 tmux 证据。✓ **问题:**这不是一个 fix,而是 maintainer 跟踪的路线图 #8724(标签 **方向:**对齐。#8724 刻意按"先收后发、默认关闭"分阶段推进,本 PR 严格停留在这个阶梯内。参考 agent 也已具备同样能力——Claude Code 的 changelog 里有多条跨会话消息相关条目(例如修复其"在 2.1.232 socket 目录加固后于用户命名空间和 rootless 容器中静默关闭"的问题,以及 30 秒收件箱连接截止)——因此该领域是成熟的,而非投机性的。 **规模:**触及核心路径。3014 行变更的构成:1111 行生产逻辑、1850 行测试、2 行生成/schema、51 行文档。按两级规则,触及核心的 方案:范围合理。发现复用 #8969 注册表,传输复用 #9576 的帧与 **风险:**回滚历史分析未命中任何高风险路径(未触及 streaming/MCP/shell/sandbox/ACP 表面)。IPC 信任边界本身已在 #9576 中评审定型,本 PR 是延伸而非改动。 进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewBefore reading the diff I wrote down my own baseline: registry scan + liveness probe + addressable names in Verified against the merged base, not just the diff:
Findings: one suggestion, nothing blocking — in sequenceDiagram
participant P1 as Model in session A
participant P2 as send_message tool
participant P3 as peer directory (registry plus probes)
participant P4 as session B inbox
participant P5 as inbound gate in B
participant P6 as send ledger in A
P1->>P2: send_message(to name, message)
P2->>P3: resolve name against reachable peers
P3-->>P2: one peer, none, or ambiguous with candidates
P2->>P4: frame with fromMode and toSessionId pin
P4->>P5: admit, hold, or refuse (misaddressed when the pin mismatches)
P5-->>P4: receipt (held, delivered, denied, expired, misaddressed)
P4-->>P6: receipt keyed by the original msgId
P6-->>P1: notice only when the receipt moves the message to a new state
Files changed (25 of 25 shown)
Test evidence (the PR's own CI, via API)Unattended run — per triage rules no PR code is built or executed here; the evidence below is the PR's own CI read through the API at the reviewed commit. As of this review the ubuntu unit suite is still running; the macOS/Windows test jobs and the CLI integration job show skipped (fork-PR gating); precheck, both Desktop Shell builds, the dependency CVE audit, the secret scan, and labeling are green. The finalize workflow rewrites the table below once CI settles. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 The author reports 364 core + 205 CLI unit tests covering the new paths, plus clean typecheck/lint — that is the author's claim, not independently re-run in this pass. The tmux transcript in the PR body is likewise the author's own Linux run. Sandboxed verification would settle what static review cannot: the central claims here are behavioural — discovery prints reachable sessions, an ambiguous bare name is refused with candidates, a DEFAULT→YOLO send is held and released via 中文说明代码审查:在读 diff 之前我先写下了自己的基线方案——注册表扫描+存活性探测+在 list_agents 里给出可寻址名字;发送侧按名字解析到记录再落到 socket,携带诚实的 fromMode 并把回执映射成模型可执行的文本; 除 diff 外还对照了已合入的基线代码验证: 发现:一条建议、无阻塞项——send-message.ts 里本属于 trySendToPeer 的文档注释悬空在 peerMessagingOff 字段上方,与字段自身的注释叠在一起,方法丢了注释、字段挂了两条。纯外观问题,下次改动时顺手归位即可。 测试证据:本次为无人值守 CI 运行,按规则不构建、不执行 PR 代码。截至审查时:ubuntu 单元测试仍在运行;macOS/Windows 测试与 CLI 集成任务显示跳过(fork PR 门控);precheck、两个 Desktop Shell 构建、依赖 CVE 审计、密钥扫描、打标均为绿。CI 表格由 finalize 流程在 CI 落定后就地更新。作者报告 364+205 个单测及干净的 typecheck/lint——此为作者声明,本次未独立复跑;PR 正文中的 tmux 记录同样是作者在本机 Linux 上的自行运行。 沙箱验证可补足静态审查看不到的部分:本 PR 的核心主张是行为性的——发现可达会话、歧义裸名拒绝并列出候选、DEFAULT→YOLO 被扣下并经 /peers 放行、两次回执出现在发送方记录、SIGKILL 后会话从目录消失——这些无法仅从 diff 观察,且一套在移除台账或钉扎后依然全绿的测试同样能通过。作者有 write 权限,两条通道均可用: — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 3/5 — clean review on the merits, but the Stage 0 core-size escalation needs a maintainer's sign-off. Stepping back: the approach matches what I would have proposed before seeing the diff — registry-based discovery, socket liveness probes, name addressing with refusal-over-guessing, receipts as a bounded state machine — and the three additions beyond that baseline each close a gap the receive-only step explicitly left open, none of them speculative. Every part of the diff maps to the stated goal; the only surplus I found in the whole pass is one misplaced doc comment. The code is straightforward — small modules with why-comments, failure modes described instead of thrown, and 1850 lines of tests that pin the semantics rather than the implementation. In six months I'd thank the author, not curse them. This is step 3 of the maintainer-tracked ladder in #8724, and the receive side it completes landed today; the direction question was answered by the roadmap itself. The fork-refactor approval guardrail does not apply (this is a What I could not settle from here, and what a maintainer may want before merging:
⏸️ Deferring to @yiliang114 and @LaZzyMan (who reviewed and approved the receive side, #9576) — Stage 0 escalation on core size (1111 production lines ≥ 500), which this run cannot approve past. Static review found nothing blocking; if the size call and CI are satisfactory, this is ready for a human approval. 中文说明置信度:3/5 —— 就审查本身而言是干净的,但 Stage 0 的核心规模升级需要维护者确认。 退后一步看:方案与我读 diff 前独立写下的一致——基于注册表的发现、socket 存活探测、"拒绝而非猜测"的按名寻址、有界状态机式的回执——超出基线的三处补充各自闭合了仅接收侧明确留下的缺口,没有一处是投机性的。diff 的每一部分都对应既定目标,全程唯一发现的多余之处是一条放错位置的文档注释。代码是直白的——小模块、解释原因的注释、描述而非抛出的失败模式,以及 1850 行钉住语义而非实现的测试。六个月后回头看,会感谢作者而不是埋怨。 这是 #8724 维护者跟踪的阶梯中的第 3 步,它要补全的接收侧今天刚合入;方向问题已由路线图本身回答。fork 重构审批护栏不适用(这是 我无法在此确认、维护者合并前可能需要看的三点:
⏸️ 转交 @yiliang114 与 @LaZzyMan(二人审查并批准了接收侧 #9576)——因核心规模的 Stage 0 升级(1111 生产行 ≥ 500),本次运行不能越过它批准。静态审查未发现阻塞项;若规模判断与 CI 均可接受,可进入人工批准。 — Qwen Code · qwen3.8-max Reviewed at |
|
@qwen-code /takeover |
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。本 PR 来自 fork,首轮处理将由下一次定时扫描执行(通常几分钟内)。移除 |
doudouOUC
left a comment
There was a problem hiding this comment.
Review: PR #10158 — cross-session peer addressing
I reviewed this PR through the API (git fetch is blocked on this Windows machine). The triage already covered the code thoroughly, and I agree with its assessment. Here are my own observations:
Summary
This is a well-structured, carefully designed feature. The code is clean, the architecture is sound, and the safety considerations are thorough. The PR stays within the established patterns from #9576 and the roadmap in #8724.
What I verified
I read the full diff for all 25 files via the API, including the new (301 lines), (179 lines), and the modified , , , , , , and .
Key design decisions I checked
-
Routing order — → in-process teammate → peer session. An in-process recipient wins a name collision with a peer. Correct.
-
** stays team-only** — Never fans out across sessions. Correct.
-
Ambiguous names refused — A bare name matching 2+ sessions is refused with candidates rather than guessed. The right call for an irreversible inject.
-
** pinning** — Prevents PID-reuse misdelivery. The is a getter (not a startup value), so swaps are handled correctly.
-
Receipt ledger as a state machine — maps valid transitions. Repeat receipts are silently dropped. Unknown IDs are dropped. Correct.
-
** status** — Distinct from , with a separate text. Correct.
-
Send ledger bounded — , oldest evicted first. Reasonable.
-
Frame tracked before write — A receiver whose loop is stalled accepts the connection and bytes sit in the kernel buffer, so a send that times out can still be receipted. Only failures proving the frame never arrived (ENOENT, ECONNREFUSED, EMSGSIZE) forget it. Correct.
-
Exhaustive switch — on ensures the compiler catches unhandled variants. Good practice.
-
** uses the same predicate as the receiver** — , so two sessions in the same mode always agree on parity. Correct.
-
Teammate-shadow detection — A peer whose name sanitizes to a teammate's gets its appended. Correct.
-
Registry strings flattened — on and before they reach tool output. Correct.
CI note
The ubuntu test suite shows PASS across all three batches (228 + 59 + 201 = 488 tests), but the process exited with code 1. The error annotations show only a generic "Process completed with exit code 1" — this appears to be a CI infrastructure issue (DWS agent unavailable, GitLab 403), not a PR code defect. The macOS/Windows/integration jobs are skipped under fork gating.
Finding
The triage already noted the only finding I saw: in the JSDoc comment for ("Try to deliver to another Qwen Code session…") is stranded above the field, which carries its own doc comment underneath it — the method lost its doc comment and the field has two stacked on it. Cosmetic and worth reattaching on the next touch.
Verdict
No blockers. The code is correct, well-tested, and consistent with the existing architecture. The PR is part of the maintainer-tracked roadmap (#8724, step 3) and completes the send side of the cross-session messaging feature.
— Qwen Code
|
🤖 AutoFix ran out of time before finishing (timeout (2700000ms)) (attempt 1/100) — it will retry on the next scan.
See the Qwen Autofix agent step logs for model/tool output. 中文说明🤖 AutoFix 在完成前耗尽了时间(timeout (2700000ms))(第 1/100 次尝试)—— 将在下次扫描时重试。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/32988204380 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-9 orphaned trySendToPeer JSDoc above peerMessagingOff — already reported (review 5031164222 by @doudouOUC)
中文说明
本轮确认的 1 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| to: | ||
| teammateNames.has(sanitizeName(peer.name)) && | ||
| !formatPeerAddress(peer, peers).endsWith(']') |
There was a problem hiding this comment.
[Critical] The teammate-shadow guard decides "a ref was already appended" by string-sniffing the rendered output of formatPeerAddress (endsWith(']')), and the shadow set is built only from team-file members. Both break when registry-supplied names enter the address machinery. Corner 1: a reachable session whose record carries the name notes [draft] (flattenPeerLabel keeps brackets — registry names are other-process input this PR deliberately treats as attacker-influenced) with a teammate notes-draft in this session: sanitizeName('notes [draft]') === 'notes-draft' marks it shadowed, but the sniff sees ] and falls through, so to prints bare notes [draft]; the model sends that verbatim, route 2's sanitizing findMemberByName matches the teammate, and the message is silently delivered in-process — the peer never sees it, no error is raised. Corner 2: peers docs-cd/ref aaa111 and docs-cd [aaa111]/ref bbb222 with teammate docs-cd active: the first prints docs-cd [aaa111] (shadowed, ref appended), the second prints bare docs-cd [aaa111] — two sessions, one identical address; resolution always picks the first, so the second is unreachable and anything aimed at it is injected into the first. Also: a peer literally named leader prints bare to: "leader" and is intercepted by route 2, because the shadow set omits LEADER_NAME/leadAgentId which route 2 also intercepts. Official clients never produce bracketed names (deriveSessionName strips them), so the trigger is an out-of-band registry writer — the same threat model this PR applies to name/cwd.
Witness (probe against the real ListAgentsTool at this commit):
corner1 printed to: ["notes [draft]"] teammate intercepts: true
corner2 printed to: ["docs-cd [aaa111]","docs-cd [aaa111]"]
corner2 resolve("docs-cd [aaa111]"): {"kind":"one","picked":"sA"}
leader printed to: ["leader"]
Decide from structured data instead of the rendered string: compute contested = peers.filter((o) => o.name === peer.name).length > 1 and print shadowed || contested ? ${peer.name} [${peer.ref}] : peer.name; guarantee printed addresses are injective (append the ref to any session whose rendered address equals another's); mirror all of route 2's in-process recipients (members + LEADER_NAME + leadAgentId) in the shadow set. Note the R1-2 fix alone does not make corner 2 injective — the injectivity guarantee is the load-bearing half here.
Fix witness: add list-agents.test.ts cases for both corners (bracketed-name peer + sanitizing teammate prints a ref'd to; the two docs-cd peers print distinct to values) — removing the structured rule makes them red.
中文说明
teammate 遮蔽守卫通过字符串嗅探 formatPeerAddress 的渲染输出(endsWith(']'))来判断"是否已追加 ref",且遮蔽集合仅由 team 文件成员构成。当注册表提供的名字进入寻址机制时两者都会失效。角落 1:一个可达会话的注册记录名为 notes [draft](flattenPeerLabel 保留方括号——注册表名字是本 PR 刻意按攻击者可影响处理的其他进程输入),本会话存在 teammate notes-draft:sanitizeName('notes [draft]') === 'notes-draft' 使其被标记为遮蔽,但嗅探看到 ] 便跳过追加,于是 to 输出裸名 notes [draft];模型照发,路由 2 的 findMemberByName(先净化)匹配到 teammate,消息被静默投递到进程内——对端永远收不到,也不报错。角落 2:对端 docs-cd/ref aaa111 与 docs-cd [aaa111]/ref bbb222,且存在 teammate docs-cd:前者输出 docs-cd [aaa111](被遮蔽、追加 ref),后者输出裸名 docs-cd [aaa111]——两个会话同一个地址;解析永远选第一个,第二个不可达,发给它的消息会注入第一个。另外:名为 leader 的对端会输出裸 to: "leader" 并被路由 2 拦截,因为遮蔽集合漏掉了路由 2 同样拦截的 LEADER_NAME/leadAgentId。官方客户端不会产生带方括号的名字(deriveSessionName 会剥掉),触发条件是带外注册表写入者——正是本 PR 对 name/cwd 应用扁平化时所针对的威胁模型。
见证(在本提交上对真实 ListAgentsTool 的探针):角落 1 输出 to: ["notes [draft]"] 且 teammate 拦截为 true;角落 2 输出两个相同的 docs-cd [aaa111],解析得 {"kind":"one"};leader 输出裸名。
建议改用结构化数据判断:计算 contested,当被遮蔽或有竞争时输出 name [ref];保证输出地址单射(任何渲染地址与他人相同的会话追加 ref);遮蔽集合同步纳入路由 2 的全部进程内接收者(成员 + LEADER_NAME + leadAgentId)。注意仅有 R1-2 的修复无法让角落 2 单射——单射保证才是这里的关键。
修复见证:为两个角落各加一条 list-agents.test.ts 用例;移除结构化规则后应变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| const match = peers.find( | ||
| (peer) => | ||
| peer.ref === ref!.toLowerCase() && | ||
| (namePart!.length === 0 || peer.name === namePart), | ||
| ); | ||
| return match ? { kind: 'one', peer: match } : { kind: 'none' }; |
There was a problem hiding this comment.
[Critical] The name [ref] branch resolves wrongly in three directions, all confirmed by probe at this commit. (a) Guessing: two reachable sessions whose 6-hex refs collide — [abc123] (a form this function explicitly accepts) resolves via peers.find to the FIRST match and the message is injected into the wrong session, pinned to that session's own id so the receiver accepts it; the bare spelling abc123 correctly returns ambiguous for identical semantics. (b) Swallowing literal names: a peer literally named notes [deadbeef] prints exactly that string as its only address, but the regex matches with namePart='notes', ref='deadbeef', no peer has that ref, and the branch returns none without falling through to the exact-name match one branch later; suggestPeerNames then suggests the identical string, looping the sender. (c) Shadowing: P1 named notes (ref cafe12) and P2 literally named notes [cafe12] — notes [cafe12] is the only address denoting P2, yet the branch resolves it to P1 (name+ref both match), injecting P2's message into P1; with two peers named notes, the contested peer prints an address byte-identical to P2's literal name and resolution picks by list order. Bracketed names need an out-of-band registry writer (official names never contain brackets) — the same premise the PR's own flattening relies on.
Witness (probe at this commit):
bare "abc123" -> {"kind":"ambiguous"}
bracketed "[abc123]" -> {"kind":"one","picked":"s1"} (reversed order picks the other)
"notes [deadbeef]" -> {"kind":"none"}, suggestions ["notes [deadbeef]"]
corner (c) -> "notes [cafe12]" resolves to P1 ("notes"), not P2
Resolve BOTH readings and merge: literal = peers.filter(p => p.name === trimmed), bracketed = withRef matches, then literal ∪ bracketed — one → one, many → ambiguous, zero → none (or continue to the bare-name/bare-ref logic). The simpler "collect all bracketed matches" fix closes (a) and (b) but NOT (c) — probe-verified: with exactly one ref+name match, find still picks P1. Also keep the printed address space injective (append the ref when a bare name parses as another peer's name [ref] address).
Fix witness: peer-directory.test.ts — colliding-ref bracketed input expects ambiguous; a notes [deadbeef] peer round-trips to itself; [notes/cafe12 + 'notes [cafe12]'] does not resolve to the notes peer. Each goes red if the merged-reading fix is removed.
中文说明
name [ref] 分支在三个方向上解析错误,均已在本提交上用探针确认。(a) 猜测:两个可达会话的 6 位 hex ref 相撞时,[abc123](本函数明确接受的形式)经 peers.find 解析到第一个匹配,消息被注入错误会话,且帧固定在该会话自己的 id 上、接收方会接受;裸写法 abc123 对完全相同的语义却正确返回 ambiguous。(b) 吞掉字面名字:字面名为 notes [deadbeef] 的对端只能以这个字符串被寻址,但正则按 namePart='notes'、ref='deadbeef' 匹配,没有对端持有该 ref,分支直接返回 none,不回落到下一分支的精确名字匹配;suggestPeerNames 随后建议同一个字符串,把发送者绕回原处。(c) 遮蔽:P1 名为 notes(ref cafe12),P2 字面名为 notes [cafe12]——notes [cafe12] 是唯一指向 P2 的地址,分支却解析到 P1(名字+ref 都匹配),把本该给 P2 的消息注入 P1;若两个对端都叫 notes,被竞争者输出的地址与 P2 的字面名逐字节相同,解析按列表顺序任选。带方括号的名字需要带外注册表写入者(官方名字不含方括号)——与本 PR 扁平化所依赖的前提一致。
见证(本提交上的探针):裸 abc123 → ambiguous;[abc123] → one(反序选另一个);notes [deadbeef] → none 且建议回原串;角落 (c) 中 notes [cafe12] 解析到 P1。
建议同时解析两种读法并合并:literal(精确名字)∪ bracketed(ref 匹配)——一个 → one,多个 → ambiguous,零个 → none(或继续走裸名/裸 ref 逻辑)。仅收集全部括号匹配的修法能关 (a)(b),但关不掉 (c)——探针已验证:恰好一个 ref+名字匹配时 find 仍选 P1。同时保证输出地址空间单射。
修复见证:为上述三种情形各加一条 peer-directory.test.ts 用例;移除合并读法修复后应变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| cwd: peer.cwd, | ||
| started_at: new Date(peer.startedAt).toISOString(), |
There was a problem hiding this comment.
[Critical] new Date(peer.startedAt).toISOString() is unguarded: the registry parser validates only typeof startedAt === 'number' && Number.isFinite(startedAt), so a finite-but-Date-invalid value (9e15, 1e308, a nanoseconds writer's ~1.7e18 — any |v| > 8.64e15) passes validation and throws RangeError: Invalid time value inside execute(), which has no try/catch. The whole list_agents call fails on EVERY session on the machine — including the pre-existing background-agent listing — for as long as that record stays live. The trigger is a same-user out-of-band registry writer with a listening socket and a valid token — the same threat model this PR applies to name/cwd via flattenPeerLabel; the benign variant is a differently-versioned writer recording nanoseconds. All existing tests use in-range values.
Witness (probe at this commit):
startedAt: 9e15 -> error { message: "Invalid time value", type: "execution_failed" }
startedAt: 1e308 -> same; pre-existing background-agent entry absent from llmContent
in-range baseline -> success, agent listed
with implied guard -> both out-of-range cases succeed (flip)
Guard at the render site or in toPeerSessionInfo: only build the Date when Number.isSafeInteger(v) && Math.abs(v) <= 8.64e15, else omit started_at (or skip the record); alternatively tighten the record parser to treat an out-of-Date-range startedAt as unreadable.
Fix witness: add a list-agents.test.ts case with peerRow({ startedAt: 9e15 }) asserting the call resolves and still lists the healthy session; removing the guard rejects with RangeError.
中文说明
new Date(peer.startedAt).toISOString() 无防护:注册表解析器只校验 typeof startedAt === 'number' && Number.isFinite(startedAt),因此有限但超出 Date 范围的值(9e15、1e308、纳秒写入者的 ~1.7e18——任何 |v| > 8.64e15)都能通过校验,并在没有 try/catch 的 execute() 内抛出 RangeError: Invalid time value。只要该记录存活,本机每个会话的整个 list_agents 调用都会失败——包括既有的后台 agent 列表。触发者是同用户带外注册表写入者(持有监听 socket 与有效 token)——正是本 PR 对 name/cwd 应用 flattenPeerLabel 时所针对的威胁模型;温和变体是记录纳秒的不同版本写入者。现有测试全部使用范围内取值。
见证(本提交上的探针):9e15 与 1e308 → execution_failed("Invalid time value"),既有后台 agent 从 llmContent 消失;范围内基线成功;加上隐含守卫后两种越界取值都成功(翻转)。
建议在渲染点或 toPeerSessionInfo 中加防护:仅当 Number.isSafeInteger(v) && Math.abs(v) <= 8.64e15 时构造 Date,否则省略 started_at(或跳过该记录);或者收紧记录解析器,把超出 Date 范围的 startedAt 视为不可读。
修复见证:加一条 peerRow({ startedAt: 9e15 }) 的用例,断言调用成功且健康会话仍在列表中;移除守卫后应以 RangeError 拒绝。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| const frame = buildUserFrame({ | ||
| content: options.message, | ||
| from: self.ipcPath, |
There was a problem hiding this comment.
[Critical] sendToPeer builds, tracks and reports sent for a frame whose empty content the PR's own wire contract guarantees the receiver silently drops. The send_message schema declares message with maxLength: 65536 and required but no minLength, and there is no validateToolParamValues override, so message: "" passes. Route 3 then builds buildUserFrame({content: ''}), tracks the entry pending, writes the line; the receiver's parsePeerFrame rejects content.length === 0 and uds-inbox drops the frame with no receipt possible. sendPeerFrame resolves (bytes were written), the outcome is sent, and the tool tells the model "Sent … do not re-send" — the message was never delivered, no receipt can ever arrive, the ledger entry stays pending until eviction, and the do-not-re-send instruction makes the loss unrecoverable in-conversation.
Witness (end-to-end probe with a real startPeerInbox at this commit):
sendToPeer({target:'app-ab', message:''}) -> outcome.kind === 'sent'
receiver onFrame calls after 300 ms: 0
with refuse-on-empty applied: no 'sent' (flip)
Refuse before building the frame — return a failure outcome in sendToPeer when options.message.length === 0 — and add minLength: 1 to the message property in send-message.ts's schema so the teammate/task routes are covered too.
Fix witness: peer-send.test.ts — sendToPeer({ target, message: '', approvalMode }) must not return {kind:'sent'}, must not call sendPeerFrame, and must leave lookupSentPeerMessage empty.
中文说明
sendToPeer 会为空内容的帧构建、记账并报告 sent,而 PR 自己的线路契约保证接收方会静默丢弃它。send_message 的 schema 声明 message 为 maxLength: 65536、required 但没有 minLength,也没有 validateToolParamValues 覆写,所以 message: "" 能通过。路由 3 随后构建 buildUserFrame({content: ''})、把条目记为 pending 并写入;接收方 parsePeerFrame 拒绝 content.length === 0,uds-inbox 丢弃该帧且不可能产生回执。sendPeerFrame 正常返回(字节已写出),结果为 sent,工具告诉模型"已发送……请勿重发"——消息从未投递、永远不会有回执、台账条目停留在 pending 直到被驱逐,而"请勿重发"的指示让这次丢失在会话内不可恢复。
见证(本提交上用真实 startPeerInbox 的端到端探针):sendToPeer({target:'app-ab', message:''}) → outcome.kind === 'sent';300 ms 后接收方 onFrame 调用次数为 0;加上空消息拒绝后不再返回 'sent'(翻转)。
建议在构建帧之前拒绝——options.message.length === 0 时 sendToPeer 返回失败结果——并在 send-message.ts 的 schema 中给 message 加 minLength: 1,使 teammate/task 路由同样受保护。
修复见证:peer-send.test.ts——空消息不得返回 {kind:'sent'}、不得调用 sendPeerFrame、lookupSentPeerMessage 保持为空。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| const ownSessionId = this.getSessionId?.(); | ||
| if ( | ||
| frame.toSessionId !== undefined && |
There was a problem hiding this comment.
[Critical] The session-id pin is judged only at frame arrival. A frame HELD before an in-process session-id swap (/clear, /reset, /new, /resume all flow through Config.startNewSession, which sets this.sessionId = nextSessionId in the same process) is never re-judged: InboundGate.reevaluate (fired by the Shift+Tab approval-mode change via peerMessaging?.reevaluate('approval-mode-changed')) and decide (the /peers accept path) release every entry the new state accepts via tryDeliver with no pin re-check, reporting delivered. Concrete sequence: B runs a mode that mismatches sender A; A's frame pinned to B's id X is held, A gets a held receipt; B's user runs /clear (registry patched to new id Y by this PR's own transition) — the held frame stays parked; B cycles the approval mode — the frame pinned to X is injected into session Y, whose context was just cleared and which A never addressed, while A's ledger says delivered. The identical frame arriving one second after /clear is refused as misaddressed: the outcome depends purely on arrival timing. RECEIPT_TRANSITIONS already models held → misaddressed, but nothing in production code ever emits it.
Witness (probe over the real PeerMessaging + sockets at this commit):
after arrival: held=1 submitted=0 receipts=["held"]
after swap+reevaluate: submitted=1 receipts=["held","delivered"] deliveredToNewSession=true
decide-after-swap: submitted=1 receipts=["held","delivered"]
with pin re-check patched: released=0 submitted=0 (flip)
Re-judge parked state when the id changes: give InboundGate/PeerMessaging a hook invoked on the session transition, or make the release paths in reevaluate/decide re-check toSessionId against a getSessionId callback before tryDeliver; every held frame whose pin differs is dropped and settled with the already-modelled misaddressed receipt. Note a faithful fix needs InboundGate.reportStatus to admit the misaddressed status.
Fix witness: peer-messaging.test.ts — hold a pinned frame under getSessionId returning 'session-x'; flip to 'session-y' and reevaluate; assert settleSentMessage receives misaddressed for that msgId and submitFn is never called.
中文说明
会话 id 固定只在帧到达时判断。在进程内会话 id 切换(/clear、/reset、/new、/resume 都经过 Config.startNewSession,它在同一进程内设置 this.sessionId = nextSessionId)之前被 HOLD 的帧不会被重新判断:InboundGate.reevaluate(Shift+Tab 切换审批模式经 peerMessaging?.reevaluate('approval-mode-changed') 触发)与 decide(/peers accept 路径)会通过 tryDeliver 放行新状态接受的每个条目且不再检查固定,并报告 delivered。具体序列:B 的审批模式与发送方 A 不匹配;A 发往 B 的帧固定在 B 的 id X 上被扣下,A 收到 held 回执;B 的用户执行 /clear(本 PR 自己的转换把注册表记录更新为新 id Y)——被扣的帧仍停在原处;B 切换审批模式——固定在 X 上的帧被注入刚刚清空上下文、且 A 从未寻址过的会话 Y,而 A 的台账显示 delivered。同样的帧若在 /clear 后一秒到达则会被以 misaddressed 拒绝:结果完全取决于到达时序。RECEIPT_TRANSITIONS 已经建模 held → misaddressed,但生产代码中没有任何地方发出它。
见证(本提交上用真实 PeerMessaging + socket 的探针):到达后 held=1;swap+reevaluate 后 submitted=1 且 deliveredToNewSession=true;decide 路径同样投递;加上固定重查补丁后不再投递(翻转)。
建议在 id 变化时重新判断停留状态:给 InboundGate/PeerMessaging 一个在会话转换时调用的钩子,或让 reevaluate/decide 的放行路径在 tryDeliver 前用 getSessionId 回调重查 toSessionId;固定不符的被扣帧应丢弃并以已建模的 misaddressed 回执结算。注意忠实的修复需要 InboundGate.reportStatus 接受 misaddressed 状态。
修复见证:扣下一个固定帧后把 getSessionId 从 'session-x' 翻到 'session-y' 并 reevaluate;断言该 msgId 收到 misaddressed 且 submitFn 从未被调用。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| const peer = resolved.peer; | ||
| const address = formatPeerAddress(peer, peers); |
There was a problem hiding this comment.
[Suggestion] The address stored in the ledger and reported to the model is computed with formatPeerAddress(peer, peers), which only detects a contest AMONG PEERS and knows nothing about teammate shadows — but list_agents appends the ref exactly when a teammate shadows the name. With team member worker and a registry record named Worker: list_agents prints to: "Worker [<ref>]"; the model sends exactly that and the bracketed branch resolves; but this computation stores bare Worker — the tool confirms "Sent to Worker", the ledger holds Worker, receipts announce "Message to Worker: …". When the model re-sends using that confirmed/announced address — the natural move after the delivered→expired correction says the message was lost — route 2 sanitizes Worker → worker, matches the teammate, delivers there and reports success; the addressed session never receives it. suggestPeerNames maps through the same shadow-unaware formatPeerAddress, so near-miss suggestions hand out the same poisoned bare form. This is distinct from the list-agents printing finding and the route-2 interception finding: their fixes do not touch this computation, which keeps handing back the un-ref'd form after both.
Witness (probe at this commit): list-agents' own expression prints to: "Worker [<ref>]"; sending that value through the real sendToPeer returns outcome.address === 'Worker' (expected the ref'd form); a re-send of the bare stored form is intercepted by findMemberByName → teammate; with a shadow-aware shared rule the probe passes.
Compute the stored address with the same shadow-aware rule list_agents prints — export one shared printPeerAddress(peer, peers, shadowedNames) used by list_agents' to, sendToPeer's address, and suggestPeerNames, appending [ref] whenever the sanitized name is shadowed or contested.
Fix witness: peer-send.test.ts — shadow set containing worker, single peer named Worker; sendToPeer({ target: 'Worker [<ref>]' }) returns outcome.address === 'Worker [<ref>]' and the ledger address re-resolves to the peer via resolvePeerTarget. Removing the shadow rule makes it red.
中文说明
存入台账并报告给模型的地址用 formatPeerAddress(peer, peers) 计算——它只检测对端之间的名字竞争,对 teammate 遮蔽一无所知——而 list_agents 恰在 teammate 遮蔽名字时追加 ref。当存在团队成员 worker 与名为 Worker 的注册表记录时:list_agents 输出 to: "Worker [<ref>]";模型照发且括号分支成功解析;但此处计算存下裸名 Worker——工具确认 "Sent to Worker"、台账保存 Worker、回执通告 "Message to Worker: …"。当模型用这个确认/通告过的地址重发——在 delivered→expired 纠正说消息丢失后的自然动作——路由 2 把 Worker 净化为 worker,匹配 teammate,投递到那里并报告成功;被寻址的会话永远收不到。suggestPeerNames 也经由同一个不感知遮蔽的 formatPeerAddress 映射,近似匹配建议会给出同样有毒的裸名。这与 list-agents 打印发现、路由 2 拦截发现彼此独立:它们的修复都不触及这处计算,两处修复后这里仍会交出不带 ref 的形式。
见证(本提交上的探针):list-agents 自己的表达式输出 to: "Worker [<ref>]";把该值经真实 sendToPeer 发送,返回 outcome.address === 'Worker'(期望带 ref);用存储的裸名重发会被 findMemberByName 拦截到 teammate;改用感知遮蔽的共享规则后探针通过。
建议用与 list_agents 打印相同的、感知遮蔽的规则计算存储地址——导出一个共享的 printPeerAddress(peer, peers, shadowedNames),供 list_agents 的 to、sendToPeer 的 address 与 suggestPeerNames 使用,凡净化名字被遮蔽或有竞争就追加 [ref]。
修复见证:peer-send.test.ts——遮蔽集含 worker、单个名为 Worker 的对端;sendToPeer({ target: 'Worker [<ref>]' }) 返回 outcome.address === 'Worker [<ref>]' 且台账地址可经 resolvePeerTarget 解析回该对端。移除遮蔽规则后应变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| if ( | ||
| !inProcessRecipient && | ||
| !this.peerMessagingOff && | ||
| /not found/i.test(errMsg) | ||
| ) { |
There was a problem hiding this comment.
[Suggestion] With an active team and cross-session messaging off, this augmentation is suppressed by !this.peerMessagingOff and nothing compensates: the model gets only Failed to send message: Teammate "…" not found. and never learns that messaging is disabled or that agents.crossSessionMessaging exists. The tool description advertises session targets unconditionally, so in default configuration the model sends a session name, route 3 returns disabled (peerMessagingOff = true), the fall-through team send throws the resolution error, and the guard fails on !this.peerMessagingOff. The model retries name variants or reports "no such teammate"; it can never discover that enabling the flag would make sessions reachable. The identical send with NO active team does name the flag — the existing test 'says messaging is off, rather than that a lookup found nothing' pins that branch, and all disabled-state tests run through toolWithoutTeam(), so the team-active branch is genuinely unpinned.
Witness (probe at this commit): final llmContent is exactly Failed to send message: Teammate "app-ab" not found.; with an added branch naming the flag the probe passes.
Add a branch for !inProcessRecipient && this.peerMessagingOff appending the same flag information the no-team branch gives, e.g. "Cross-session messaging is not enabled in this session (agents.crossSessionMessaging), so no other session could receive it."
Fix witness: send-message.test.ts — team config whose sendMessage rejects with Teammate "docs" not found., sendToPeer mocked { kind: 'disabled' }; assert llmContent contains agents.crossSessionMessaging. Removing the branch makes it red.
中文说明
在团队活跃且跨会话消息关闭时,这一增补被 !this.peerMessagingOff 抑制且没有任何补偿:模型只得到 Failed to send message: Teammate "…" not found.,永远不知道消息功能被禁用、也不知道 agents.crossSessionMessaging 的存在。工具描述无条件地宣传会话目标,因此默认配置下模型会发送会话名,路由 3 返回 disabled(peerMessagingOff = true),回落的团队发送抛出解析错误,守卫在 !this.peerMessagingOff 上不成立。模型反复尝试名字变体或报告"没有这个 teammate";它永远无法发现启用开关后会话就可寻址了。无活跃团队时的同样发送确实会点名该开关——现有测试 'says messaging is off, rather than that a lookup found nothing' 固定的是那个分支,且所有禁用态测试都经 toolWithoutTeam() 运行,因此团队活跃分支确实没有任何测试固定。
见证(本提交上的探针):最终 llmContent 恰为 Failed to send message: Teammate "app-ab" not found.;加入点名开关的分支后探针通过。
建议为 !inProcessRecipient && this.peerMessagingOff 增加一个分支,追加与无团队分支相同的开关信息,例如"本会话未启用跨会话消息(agents.crossSessionMessaging),因此没有其他会话能收到它"。
修复见证:团队配置的 sendMessage 以 Teammate "docs" not found. 拒绝、sendToPeer mock 为 { kind: 'disabled' };断言 llmContent 包含 agents.crossSessionMessaging。移除该分支后应变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| 'Send a message to a teammate (use "to") or to a running, paused, or completed background task (use "task_id"); completed tasks are revived. ' + | ||
| 'For teams, set "to" to a bare teammate name (no @) or "*" to broadcast. ' + | ||
| 'Send to a teammate or another Qwen Code session (use "to"), or a running, paused, or completed background task (use "task_id"); completed tasks are revived. ' + | ||
| 'Set "to" to a bare teammate name (no @), to "*" to broadcast within an active Agent Team only, or to a session name from list_agents — append its " [ref]" only when list_agents shows two sessions with that name. ' + |
There was a problem hiding this comment.
[Suggestion] This guidance — "append its [ref] only when list_agents shows two sessions with that name" — contradicts list_agents' own output: list-agents.ts ALSO appends the ref when a unique session's name shadows a teammate (teammateNames.has(sanitizeName(peer.name)), pinned by the test 'appends the ref when a teammate shadows the bare name'). In exactly that case the bare name routes to the teammate via the sanitizing findMemberByName (pinned by 'recognises a teammate by its sanitized name'), so the ref is the ONLY spelling that reaches the session — precisely where this description says it is not needed. Sequence: team with teammate worker; a session in a directory named worker lists as to: "worker [f7a1b2]"; following this description the model sends to: "worker" (only one session bears the name) and the message is delivered to the teammate with a success confirmation; the intended session never receives it. The same wrong rule is repeated in the to schema-property description two lines below — fixing one line leaves the other teaching it.
Align both sites with the actual address space: "use the session's to value from list_agents verbatim — it already includes a [ref] whenever the bare name would not reach the session."
Fix witness: send-message.test.ts already pins description content ('warns the model off permission laundering in the tool description'); pin the corrected guidance at both sites the same way.
中文说明
这条指引——"仅当 list_agents 显示两个会话同名时才追加 [ref]"——与 list_agents 自己的输出矛盾:list-agents.ts 在唯一会话的名字遮蔽 teammate 时同样追加 ref(teammateNames.has(sanitizeName(peer.name)),由测试 'appends the ref when a teammate shadows the bare name' 固定)。恰恰在这种情况下,裸名会经净化的 findMemberByName 路由到 teammate(由 'recognises a teammate by its sanitized name' 固定),ref 是唯一能到达该会话的写法——正是描述说不需要它的地方。序列:团队有 teammate worker;目录名为 worker 的会话列出为 to: "worker [f7a1b2]";模型照此描述发送 to: "worker"(只有一个会话叫这个名字),消息带着成功确认投递给了 teammate;目标会话永远收不到。同样的错误规则在两行后的 to schema 属性描述中重复出现——只改一行,另一行仍在教错规则。
建议两处都对齐实际地址空间:"原样使用 list_agents 给出的会话 to 值——凡裸名无法到达会话时,它已包含 [ref]。"
修复见证:send-message.test.ts 已固定描述内容('warns the model off permission laundering in the tool description');以同样方式在两处固定修正后的指引。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| description: | ||
| 'Recipient: a teammate name, "*" for Agent Team broadcast, or a session name from list_agents (append " [ref]" only when two sessions share a name).', |
There was a problem hiding this comment.
[Suggestion] Second occurrence of the incorrect ref guidance (the top-level tool description carries the same rule and is covered by the sibling comment): "(append [ref] only when two sessions share a name)". list_agents also appends the ref when a UNIQUE session's name shadows a teammate, and in that case the bare name is intercepted by route 2's sanitizing teammate lookup — the ref is the only spelling that reaches the session. A fix anchored on only one of the two lines leaves the other teaching the model to drop the ref exactly when it is mandatory.
Apply the same corrected guidance here: "Recipient: a teammate name, * for Agent Team broadcast, or a session's to value from list_agents, used verbatim."
中文说明
错误的 ref 指引的第二处出现(顶层工具描述携带同样的规则,由相邻评论覆盖):"(仅当两个会话同名时才追加 [ref])"。list_agents 在唯一会话的名字遮蔽 teammate 时同样追加 ref,而那种情况下裸名会被路由 2 的净化 teammate 查找拦截——ref 是唯一能到达该会话的写法。只修两行中的一行,另一行仍会教模型在 ref 必不可少时丢掉它。
建议在此应用同样的修正指引:"收件人:teammate 名字、Agent Team 广播用 *,或原样使用 list_agents 给出的会话 to 值。"
— qwen3.8-max via Qwen Code /review (v0.22.2)
| const name = peer.name.toLowerCase(); | ||
| return name.startsWith(needle) || name.includes(needle); |
There was a problem hiding this comment.
[Suggestion] suggestPeerNames matches the whole target string, so a bracketed name [ref] target whose name part exactly matches a reachable peer produces ZERO suggestions — and sendToPeer's not-found path passes the raw target (peer-send.ts:217) while send-message.ts:148 swallows a suggestion-less not-found. This is the one stale-address shape the ref system GUARANTEES: a peer restart changes the ref (peerRef derives from the session id), the name survives. Sequence: session A holds to: "worker [abc123]" in context (the forced spelling for contested or shadowed names); peer worker restarts with a new ref; A sends "worker [abc123]"; the bracketed branch finds no peer with that ref and returns none; suggestions are [] because no peer name starts with or includes the FULL bracketed string; the model gets 'No reachable session named "worker [abc123]"' while session worker is in fact reachable — one extra discovery round trip on every peer restart. The literal∪bracketed merge fix for the resolution finding does not close this: for a stale ref neither reading matches.
Witness (probe at this commit): resolvePeerTarget([worker], 'worker [abc123]') → {kind:'none'}; suggestPeerNames([worker], 'worker [abc123]') → [] (expected ['worker']); control suggestPeerNames([worker], 'work') → ['worker']; with the suffix-strip fix the probe passes and all 31 existing peer-directory tests still pass.
When the full needle matches nothing, strip a trailing bracketed-ref token (needle.replace(/\s*\[[0-9a-f]{4,12}\]\s*$/i, '').trim()) and match on the bare form when non-empty — suggestions already render current full addresses via formatPeerAddress, so the model gets the fresh ref.
Fix witness: peer-directory.test.ts — peer worker present; expect(suggestPeerNames(peers, 'worker [abc123]')).toEqual(['worker']) — red against the current code.
中文说明
suggestPeerNames 对整个目标字符串做匹配,因此名字部分恰好匹配某个可达对端的括号 name [ref] 目标会得到零条建议——而 sendToPeer 的 not-found 路径传入原始目标(peer-send.ts:217),send-message.ts:148 又吞掉无建议的 not-found。这正是 ref 系统必然产生的陈旧地址形状:对端重启会改变 ref(peerRef 由会话 id 派生),名字却保留。序列:会话 A 的上下文里持有 to: "worker [abc123]"(名字有竞争或被遮蔽时的强制写法);对端 worker 重启后有了新 ref;A 发送 "worker [abc123]";括号分支找不到持有该 ref 的对端、返回 none;建议为 [],因为没有对端名字以整个括号字符串开头或包含它;模型得到 'No reachable session named "worker [abc123]"',而会话 worker 实际可达——每次对端重启都多一次发现往返。解析发现的 literal∪bracketed 合并修复也关不掉这里:ref 已陈旧时两种读法都不匹配。
见证(本提交上的探针):resolvePeerTarget([worker], 'worker [abc123]') → {kind:'none'};suggestPeerNames([worker], 'worker [abc123]') → [](期望 ['worker']);对照 suggestPeerNames([worker], 'work') → ['worker'];加上后缀剥离修复后探针通过,且现有 31 条 peer-directory 测试全部通过。
建议在整个 needle 无匹配时,剥离末尾的括号 ref 记号(needle.replace(/\s*\[[0-9a-f]{4,12}\]\s*$/i, '').trim()),非空时按裸名匹配——建议本就经由 formatPeerAddress 渲染当前完整地址,模型会拿到新的 ref。
修复见证:peer-directory.test.ts——存在对端 worker;expect(suggestPeerNames(peers, 'worker [abc123]')).toEqual(['worker'])——当前代码下为红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
🤖 AutoFix ran out of time before finishing (timeout (2700000ms)) (attempt 2/100) — it will retry on the next scan.
See the Qwen Autofix agent step logs for model/tool output. 中文说明🤖 AutoFix 在完成前耗尽了时间(timeout (2700000ms))(第 2/100 次尝试)—— 将在下次扫描时重试。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/33007578480 🧠 Handled by Qwen Code · model/模型 |
|
🤖 Could not produce a passing fix for this feedback (round 3/100) — the verification gate rejected the attempt. This item now needs a human; the loop stays engaged and still picks up new feedback and base conflicts, but will not retry this item on its own. Autofix round summary — PR #10158 (same-run verification repair)This round is a same-run verification repair under the active budget warning (two prior rounds exhausted the time budget). It preserves the rejected commit Diagnosis of the gate rejectionThe gate re-ran
1. serve-wiring timeout — reproduced, mechanism established, fixed. Reproduced the gate's exact command locally on the same runner fleet unde Why it was not pushed: tests failed in packages/cli 中文说明🤖 未能为该反馈产生可通过验证的修复(第 3/100 轮) —— 验证门拒绝了该尝试。此项现在需要人工处理;循环保持在线,仍会拾取新反馈与 base 冲突,但不会自行重试此项。 验证门的拒绝原因与日志证据见上方英文部分(gate-rejection 不翻译)。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/33018050818 🧠 Handled by Qwen Code · model/模型 |
…ew-fix # Conflicts: # packages/core/src/tools/send-message.test.ts
|
🤖 AutoFix stopped: this counting window now contains 3 agent time-budget exhaustions (pushed rounds in between included; this round itself may have failed differently). That is 3 full agent runs that pushed nothing. A human should split or reduce the PR (or raise the agent time budget AND its step backstop together), then comment
See the Qwen Autofix agent step logs for model/tool output. 中文说明🤖 AutoFix 已停止:当前计数窗口内已累计 3 次时间预算耗尽(含其间推送过的轮次;本轮本身可能以别的方式失败)。即 3 次完整 agent 运行没有推送任何内容。应由人工拆分或缩减该 PR(或同时提高 agent 时间预算与其步骤兜底),然后评论 Run log: https://github.com/QwenLM/qwen-code/actions/runs/33032458575 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
9 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-7 expired receipts announced with held-expiry wording — already reported (comment 3866135151)
- R1-8 EAGAIN/EBUSY failures strand the send ledger — already reported (comment 3866135160)
- R1-9 reassertSessionRegistryRecord/queueRetriedSessionRegistryPatch have zero tests — already reported (comment 3866135171)
- R1-10 production pin wiring asserted by no test — already reported (comment 3866135177)
- R1-12 /peers row splits the approval-mode group — already reported (comment 3866135190)
- R1-13 receipt enumeration omits misaddressed — already reported (comment 3866135201)
- R1-15 name parity with truncated qwen sessions ps output — already reported (comment 3866135213)
- R1-18 team-active + messaging-off error names no switch — already reported (comment 3866135241)
- R1-21 stale bracketed ref gets no suggestion — already reported (comment 3866135263)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI (fork gating) and its suite did not run locally.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/core/src/ipc/peer-send.ts:79 — [review] added ledger/identity fields with no production read sitesdocs/users/configuration/settings.md:411 — [review] crossSessionInbound Type cell says string; schema says enumpackages/core/src/ipc/peer-directory.test.ts:202 — [review] bare-ref/suggestion case-insensitivity pinned by no testpackages/core/src/ipc/peer-send.ts:305 — [review] local send-cap rejection reuses EBUSY; blamed on the targetpackages/core/src/tools/send-message.ts:447 — [review] terminated teammate shadows a same-named peer foreverpackages/core/src/ipc/peer-send.test.ts:428 — [review] held→expired / held→misaddressed corrections unpinnedpackages/core/src/tools/send-message.test.ts:795 — [review] catch-append suppression conjuncts unpinned (mutants survive)
中文说明
仅完成部分审查,审查缺口已披露。
本轮确认的 9 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI (fork gating) and its suite did not run locally。
未审查:反向审计——在 5 轮的反审轮数上限内未收敛。
收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 7 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| const intercepted = (address: string): boolean => | ||
| address.toLowerCase() === LEADER_NAME || | ||
| address === teamFile?.leadAgentId || | ||
| memberNames.has(sanitizeName(address)); |
There was a problem hiding this comment.
[Critical] R1-1: (fix-induced) The round-1 fix for R1-1 replaced the endsWith(']') sniffing with this intercepted() candidate-validation mirror, but the mirror omits send_message's to === '*' broadcast branch, which runs before the peer route. A reachable peer whose registry name is exactly * passes intercepted('*') (not LEADER_NAME, not a leadAgentId, and sanitizeName('*') is '', which no member can have) and resolves uniquely, so list_agents advertises to: "*" while the tool description tells the model to use the to value verbatim. Sending to that address never reaches the session: with an active team, send_message({to: '*'}) broadcasts the content to every teammate — delivered to unintended recipients while the peer receives nothing and success is still reported — and with no team it hard-errors "No active team to broadcast to". Registry names are other-process input this PR deliberately treats as attacker-influenced, and a directory literally named * is benign-reachable too.
Witness (probe at this commit, real ListAgentsTool + SendMessageTool):
list_agents advertises to:"*" for a peer named '*'
send_message({to:'*'}) → broadcastCalls [["meant for the star peer only","leader"]], sendToPeerCalls []
flip (adding address === '*') → advertises "* [abc123]", which routes to the peer
Add address === '*' as the first condition (or extract one shared interception predicate for both tools), so the peer is advertised as * [ref] / [ref], which both route correctly:
| const intercepted = (address: string): boolean => | |
| address.toLowerCase() === LEADER_NAME || | |
| address === teamFile?.leadAgentId || | |
| memberNames.has(sanitizeName(address)); | |
| const intercepted = (address: string): boolean => | |
| address === '*' || | |
| address.toLowerCase() === LEADER_NAME || | |
| address === teamFile?.leadAgentId || | |
| memberNames.has(sanitizeName(address)); |
Fix witness: add a list-agents.test.ts case with a peer named '*' asserting no advertised to equals '*'; removing the added condition must make it red.
中文说明
第 1 轮 R1-1 的修复用这个 intercepted() 候选验证镜像替换了 endsWith(']') 嗅探,但镜像漏掉了 send_message 的 to === '*' 广播分支——该分支先于 peer 路由执行。一个注册名恰为 * 的可达对端能通过 intercepted('*')(不是 LEADER_NAME、不是 leadAgentId,且 sanitizeName('*') 为 '',没有成员能叫这个名)并唯一解析,于是 list_agents 输出 to: "*",而工具描述让模型原样使用 to 值。发送到该地址永远到不了那个会话:有 team 时 send_message({to: '*'}) 把内容广播给全体 teammate——投递给了意料之外的接收者,对端什么都没收到却仍报告成功;没有 team 时直接报 "No active team to broadcast to"。注册表名字是本 PR 刻意按攻击者可影响处理的其他进程输入,而字面名为 * 的目录也可以良性地达到。
见证(本提交上的探针,真实 ListAgentsTool + SendMessageTool):list_agents 对名为 * 的对端输出 to:"";send_message({to:''}) → broadcastCalls [["meant for the star peer only","leader"]],sendToPeerCalls [];翻转(加入 address === '')→ 输出 " [abc123]",路由到对端。
修复:把 address === '*' 作为 intercepted 的第一个条件(或为两个工具提取一个共享拦截谓词),使该对端以 * [ref] / [ref] 输出,两者都能正确路由。修复见证:在 list-agents.test.ts 中加一条名为 '*' 的对端用例,断言输出的 to 不等于 '*';移除该条件应变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| if (!this.pinStillValid(entry.frame)) { | ||
| void this.report(entry.frame, 'misaddressed'); | ||
| this.notifyHeldChange(); | ||
| return 'done'; | ||
| } |
There was a problem hiding this comment.
[Critical] R1-5: (fix-induced) The round-1 fix for R1-5 added this pinStillValid re-check, but the misaddressed drop returns 'done' — the same outcome as a real delivery — and its only consumer, peers-command.ts, maps accept + 'done' to "Released to this session. It will be picked up on the next turn." (accept all counts it in "Released N message(s)"). The message is destroyed while the user is told it will appear on the next turn: hold a pinned frame (mode-parity mismatch), /clear swaps the session id (the held set survives — the PR's own test relies on that), /peers accept <id> drops the frame and receipts the sender misaddressed, yet the user sees "Released"; nothing surfaces in any turn, and reevaluate fires only on approval-mode change, so nothing clears these holds automatically. decide()'s own docstring says claiming 'done' "would report a release that never happened".
Witness (probe through the real InboundGate + peersCommand at this commit):
/peers accept <id> → "Released to this session. It will be picked up on the next turn."
delivered frames: 0, receipts ["held","misaddressed"]
/peers accept all → "Released 1 message.", delivered 0
flip (return 'gone') → "That message is no longer waiting…", "Released 0 messages."
Return a distinct outcome for this branch so peers-command reports the drop honestly:
| if (!this.pinStillValid(entry.frame)) { | |
| void this.report(entry.frame, 'misaddressed'); | |
| this.notifyHeldChange(); | |
| return 'done'; | |
| } | |
| return 'gone'; |
(or add a dedicated 'misaddressed' member and a message naming the drop; 'gone' already renders "no longer waiting", which is true). Fix witness: a peers-command test — hold a pinned frame, swap getSessionId(), /peers accept <id>; assert the response does NOT contain "Released"; removing the distinct outcome makes it red.
中文说明
第 1 轮 R1-5 的修复加上了这个 pinStillValid 重查,但误址丢弃分支返回 'done'——与真实投递相同的结果——而它唯一的消费者 peers-command.ts 把 accept + 'done' 映射为 "Released to this session. It will be picked up on the next turn."(accept all 还把它计入 "Released N message(s)")。消息被销毁,用户却被告知它会在下一回合出现:扣下一个固定帧(模式对不齐),/clear 换掉会话 id(扣下集合仍在——本 PR 自己的测试依赖这一点),/peers accept <id> 丢弃该帧并给发送方 misaddressed 回执,用户看到的却是 "Released";任何回合里都不会出现这条消息,而 reevaluate 只在审批模式变化时触发,没有任何东西会自动清掉这些扣留。decide() 自己的注释就说,声称 'done' "would report a release that never happened"。
见证(本提交上穿过真实 InboundGate + peersCommand 的探针):accept → "Released to this session…",delivered frames: 0,回执 ["held","misaddressed"];accept all → "Released 1 message.",delivered 0;翻转(返回 'gone')→ "That message is no longer waiting…","Released 0 messages."。
修复:让该分支返回一个不同的结果,使 peers-command 如实报告丢弃(或新增 'misaddressed' 成员并给出点名丢弃的文案;'gone' 已渲染 "no longer waiting",符合事实)。修复见证:新增 peers-command 测试——扣下固定帧、翻转 getSessionId()、/peers accept <id>,断言响应不含 "Released";移除该不同结果后应变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| for (const entry of release) { | ||
| if (!this.pinStillValid(entry.frame)) { | ||
| misaddressed += 1; | ||
| void this.report(entry.frame, 'misaddressed'); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
[Critical] The misaddressed drop records no recordSettled tombstone on either release path (here and in decide()), so a dropped msgId re-enters the gate on re-send instead of repeating its verdict — the re-decision hole the settled map's documented contract exists to close. Every other live terminal outcome tombstones (eviction→expired, deny→denied, delivery→delivered). /resume restores an arbitrary prior session id in-process (resumeCommand → config.startNewSession(sessionId)), and PeerMessaging is started once with a live getSessionId getter, so the same gate instance judges the resumed id: a frame pinned to session A is held, /clear swaps to B, reevaluate/decide drops it as misaddressed, the user /resumes session A — and a re-send of the same msgId with a different body (a same-user process under this module's stated threat model, or an honest sender whose best-effort receipt was lost — report() swallows all errors) now passes the arrival pin check and finds no settled entry: under accept policy the swapped body auto-delivers with no review; under hold policy it re-parks under the same handle the user already saw dropped, inviting a familiarity-biased approval. The sender's ledger was told misaddressed (terminal there), so later receipts for that id are silently dropped.
Witness (probe at this commit, real InboundGate):
re-admission after decide-drop: verdict accept, delivered ["body-2"]
re-admission after reevaluate-drop: verdict accept, delivered ["body-2"]
hold policy: verdict held, held bodies ["body-2"]
flip (recordSettled tombstone): verdict refused, statuses [held, misaddressed, misaddressed]
Tombstone the id on both drop paths: widen the settled verdict union with 'misaddressed' and call this.recordSettled(entry.frame.msgId, 'misaddressed') here and in decide()'s pin-invalid branch (a re-send then repeats the honest receipt). Fix witness: inbound-gate.test.ts — admit a pinned frame (held), swap getSessionId, drop via decide/reevaluate (misaddressed), then gate.admit(same frame) again must return 'refused' with nothing delivered; removing the added recordSettled calls makes it red (both existing pin-drop tests stay green without a tombstone, so nothing pins this today).
中文说明
误址丢弃在两条放行路径(此处与 decide())上都没有写 recordSettled 墓碑,被丢弃的 msgId 在重发时会重新进入闸门,而不是重复其判决——这正是 settled 映射的文档契约要堵住的重新裁决漏洞。其他所有活着的终局结果都有墓碑(驱逐→expired、拒绝→denied、投递→delivered)。/resume 会在进程内恢复任意先前会话 id(resumeCommand → config.startNewSession(sessionId)),而 PeerMessaging 只启动一次、带着活的 getSessionId getter,所以同一个闸门实例会评判恢复出来的 id:固定在会话 A 的帧被扣下,/clear 换到 B,reevaluate/decide 以 misaddressed 丢弃,用户 /resume 回 A——此时用不同内容重发同一 msgId(该模块声明的威胁模型下的同用户进程,或回执尽力发送但丢失的诚实发送方——report() 吞掉所有错误)就能通过到达固定检查,且找不到 settled 条目:accept 策略下换了内容的消息自动投递、无人审核;hold 策略下它会重新停靠在用户刚刚看着被丢弃的同一个句柄下,诱发惯性批准。发送方台账已被告知 misaddressed(在那一侧是终局),之后该 id 的回执都会被静默丢弃。
见证(本提交上的探针,真实 InboundGate):decide 丢弃后重收 → verdict accept, delivered ["body-2"];reevaluate 丢弃后重收 → 同样投递;hold 策略 → 重新扣下 ["body-2"];翻转(加墓碑)→ refused,statuses [held, misaddressed, misaddressed]。
修复:在两条丢弃路径上为该 id 写墓碑——把 settled 判决的并集扩上 'misaddressed',并在此处与 decide() 的固定失效分支调用 this.recordSettled(entry.frame.msgId, 'misaddressed')(重发将得到诚实的重复回执)。修复见证:inbound-gate.test.ts——收下固定帧(扣下)、翻转 getSessionId、经 decide/reevaluate 丢弃(misaddressed),再次 gate.admit(同一帧) 必须返回 'refused' 且不投递;移除新增的 recordSettled 调用应变红(现有两条固定丢弃测试在没有墓碑时仍然通过,所以今天没有任何测试钉住这一点)。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| return peers | ||
| .filter((peer) => { | ||
| const name = peer.name.toLowerCase(); | ||
| return name.startsWith(needle) || name.includes(needle); | ||
| }) |
There was a problem hiding this comment.
[Critical] suggestPeerNames hands out bare peer names with no interception check (its only contest check is peer-vs-peer name duplication in formatPeerAddress), but send_message routes teammate/leader/leadAgentId names in-process before the peer route. The typo-corrector thus advertises a bare address that list_agents deliberately refused to advertise bare, and following it routes to the teammate: with teammate docs-cd active (sanitizeName('Docs-CD') === 'docs-cd') and a reachable peer named Docs-CD (advertised only as Docs-CD [ref]), a near-miss send to:'Docs' surfaces "Did you mean: Docs-CD?"; the model follows its own tool's suggestion and sends to:'Docs-CD' — findMemberByName's sanitize-match wins over the peer route, the content is delivered to the teammate subagent with "Message sent", the peer never receives a frame, and no receipt or misaddressed notice can fire because nothing left the process — the wrong-recipient injection resolvePeerTarget's own docstring calls unrecoverable.
Witness (probe at this commit, real SendMessageTool + real sendToPeer/suggestPeerNames):
STEP1 to:'Docs' → "…Did you mean: Docs-CD? Use list_agents to see who is reachable."
sendMessage calls: [], sendPeerFrame calls: 0
STEP2 to:'Docs-CD' → "Message sent to \"Docs-CD\"."
sendMessage calls: [["Docs-CD","ping","leader",null]], sendPeerFrame calls: 0
flip (filter suggestions through the in-process predicate) → the intercepted suggestion no longer surfaces
Filter suggestions through the same interception predicate list-agents already has (per the R1-1 fix): give sendToPeer an isReserved?: (address: string) => boolean option supplied by send-message (team-aware), drop bare suggestions that fail it, and fall back to the name [ref] form for any filtered peer. Fix witness: with an active team member build and a reachable peer named build, a near-miss must suggest build [ref] (not bare build), and sending the suggested address must reach sendPeerFrame, not teamManager.sendMessage; removing the filter makes it red.
中文说明
suggestPeerNames 给出裸对端名时没有任何拦截检查(它唯一的竞争检查是 formatPeerAddress 里的对端之间重名),但 send_message 会先于 peer 路由把 teammate/leader/leadAgentId 名字路由到进程内。这个拼写纠正器因此会给出 list_agents 刻意拒绝裸输出的地址,照做就会路由到 teammate:teammate docs-cd 在场(sanitizeName('Docs-CD') === 'docs-cd')、可达对端名为 Docs-CD(只以 Docs-CD [ref] 输出)时,近似失配 to:'Docs' 给出 "Did you mean: Docs-CD?";模型照着自己工具的建议发送 to:'Docs-CD'——findMemberByName 的净化匹配先于 peer 路由命中,内容投递给 teammate 子代理并报告 "Message sent",对端一帧都收不到,也没有任何回执或 misaddressed 通告——帧根本没离开进程;这正是 resolvePeerTarget 注释里称之为不可恢复的错收件人注入。
见证(本提交上的探针,真实 SendMessageTool + 真实 sendToPeer/suggestPeerNames):STEP1 to:'Docs' → "Did you mean: Docs-CD?…",sendMessage 调用 0、sendPeerFrame 调用 0;STEP2 to:'Docs-CD' → "Message sent to "Docs-CD".",sendMessage 调用 [["Docs-CD","ping","leader",null]],sendPeerFrame 调用 0;翻转(建议过一遍进程内拦截谓词)→ 不再给出被拦截的建议。
修复:用 list-agents 已有的同一个拦截谓词(见 R1-1 的修复)过滤建议:给 sendToPeer 一个由 send-message(感知 team 状态)提供的 isReserved?: (address: string) => boolean 选项,丢弃过不了滤的裸名建议,对被过滤的对端回退到 name [ref] 形式。修复见证:存在 teammate build 与名为 build 的可达对端时,近似失配必须建议 build [ref](而非裸 build),且发送该建议地址必须到达 sendPeerFrame 而不是 teamManager.sendMessage;移除过滤应变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| const byName = peers.filter((peer) => peer.name === trimmed); | ||
| if (byName.length === 1) return { kind: 'one', peer: byName[0]! }; | ||
| if (byName.length > 1) return { kind: 'ambiguous', matches: byName }; |
There was a problem hiding this comment.
[Critical] The bare-target ladder resolves the name reading and the ref reading by priority — byName short-circuits before byRef is computed — instead of merging them, unlike the bracket branch directly above, whose own comment argues "picking one reading silently injects into the wrong session when both have a claim". A string that is simultaneously one peer's name and another peer's ref silently resolves to the name peer, violating the module's documented no-guessing contract. Registry names are self-advertised other-process input and refs are sha256(sessionId).slice(0,6), readable from the shared registry: a malicious local peer registers the name equal to a victim's ref; the victim's ambiguous-error output (or a contested list_agents row) hands the sender the ref, the sender targets bare abc123 intending the victim, and the message is injected into the shadow session — pinned to the shadow's own id, so the receiver's pin check confirms the wrong target — with a delivered receipt for the sender. (Rare benign trigger: a session literally named with a 6-hex string.)
Witness (probe at this commit; victim ref real — sha256 of the session id starting 'abc123'):
resolvePeerTarget([victim, shadow], 'abc123') → {kind:'one', peer: shadow} (the NAME reading)
sendToPeer → frame stamped toSessionId=shadow-session, outcome {kind:'sent'}
flip (merge both readings) → {kind:'ambiguous', matches:[shadow, victim]}, NO FRAME SENT;
all 65 pinned tests (34 peer-directory + 31 peer-send) still pass
Merge the readings like the bracket branch does:
| const byName = peers.filter((peer) => peer.name === trimmed); | |
| if (byName.length === 1) return { kind: 'one', peer: byName[0]! }; | |
| if (byName.length > 1) return { kind: 'ambiguous', matches: byName }; | |
| const byName = peers.filter((peer) => peer.name === trimmed); | |
| const byRef = peers.filter((peer) => peer.ref === trimmed.toLowerCase()); | |
| const matches = [...new Set([...byName, ...byRef])]; | |
| if (matches.length === 1) return { kind: 'one', peer: matches[0]! }; | |
| if (matches.length > 1) return { kind: 'ambiguous', matches }; |
(the bare-ref block below becomes redundant and can be removed). Fix witness: peer-directory.test.ts — peer A with ref 'abc123' plus peer B named 'abc123'; resolvePeerTarget([a, b], 'abc123') must equal { kind: 'ambiguous', matches: [...] }; reverting to the name-first ladder makes it red (probe shows it returns kind: 'one' for B today); no existing test exercises this collision.
中文说明
裸目标的解析阶梯按优先级处理名字读法与 ref 读法——byName 在计算 byRef 之前就短路返回——而不是像紧邻上方的括号分支那样合并两种读法,而括号分支自己的注释就说"两种读法都有主张时任选其一会静默注入错误会话"。一个字符串同时是一个对端的名字和另一个对端的 ref 时,会静默解析到名字一方,违背本模块文档化的"不猜测"契约。注册表名字是自我声明的其他进程输入,ref 是 sha256(sessionId).slice(0,6)、可从共享注册表读到:恶意本地对端把名字注册成受害者的 ref;受害者的歧义错误输出(或存在竞争的 list_agents 行)把 ref 交给发送方,发送方以裸 abc123 指向受害者,消息却被注入影子会话——帧固定在影子自己的 id 上,接收方的固定检查反而确认了错误目标——发送方还收到 delivered 回执。(罕见的良性触发:名字恰为 6 位十六进制串的会话。)
见证(本提交上的探针;受害者 ref 为真实值——会话 id 的 sha256 以 'abc123' 开头):resolvePeerTarget([victim, shadow], 'abc123') → {kind:'one', peer: shadow}(名字读法);sendToPeer → 帧固定 toSessionId=shadow-session,结果 {kind:'sent'};翻转(合并两种读法)→ {kind:'ambiguous', matches:[shadow, victim]},不发送任何帧;全部 65 条既有测试(34 条 peer-directory + 31 条 peer-send)仍然通过。
修复:像括号分支一样合并两种读法(建议块见上;下方的裸 ref 分支随之冗余,可删除)。修复见证:在 peer-directory.test.ts 中,对端 A 的 ref 为 'abc123'、对端 B 名为 'abc123',resolvePeerTarget([a, b], 'abc123') 必须为 { kind: 'ambiguous', matches: [...] };改回名字优先的阶梯应变红(探针显示今天会返回 kind: 'one' 选中 B);现有测试没有任何一条覆盖这个冲突。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| // A ref collision plus adversarial literal names can leave no string | ||
| // in the supported grammar that uniquely selects this peer. Do not | ||
| // advertise a misleading address that would route elsewhere. | ||
| if (to === undefined) return []; |
There was a problem hiding this comment.
[Suggestion] This omission branch — a peer advertised by no address at all — is the only untested branch of the new listing logic. Two sessions sharing a name whose 6-hex refs collide make all three candidates (name, name [ref], [ref]) ambiguous, so both peers must be silently omitted; no test exercises that. A future refactor letting to: undefined through would emit entries whose to key JSON.stringify drops — handing the model unaddressable rows — with nothing going red (probe verified: removing the guard emits entries with no to key while all 16 existing tests stay green). Add a list-agents.test.ts case with two same-name, same-ref peers asserting both are absent from parsed.sessions while a third distinct peer still lists; deleting the guarded return [] must make it red.
中文说明
这个省略分支——一个完全不输出任何地址的对端——是新列表逻辑中唯一没有被测试覆盖的分支。两个同名会话的 6 位十六进制 ref 相撞时,三种候选(name、name [ref]、[ref])全部歧义,两个对端都必须被静默省略;没有任何测试覆盖这一点。未来某个重构若让 to: undefined 通过,会输出 to 键被 JSON.stringify 丢弃的条目——交给模型无法寻址的行——而不会有任何测试变红(探针已验证:移除守卫会输出没有 to 键的条目,16 条现有测试全部保持通过)。在 list-agents.test.ts 中加一条用例:两个同名同 ref 的对端,断言两者都不出现在 parsed.sessions 而第三个不同对端仍然列出;删除带守卫的 return [] 应变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| const intercepted = (address: string): boolean => | ||
| address.toLowerCase() === LEADER_NAME || | ||
| address === teamFile?.leadAgentId || |
There was a problem hiding this comment.
[Suggestion] intercepted() applies the LEADER_NAME branch unconditionally, but the route it mirrors — send-message.ts's inProcessRecipient — gates the entire in-process check on !!teamManager. With no active team the mirror invents an interception send_message does not perform: a reachable peer named leader is advertised only as leader [ref] even though send_message({to:'leader'}) skips the in-process branch entirely and delivers bare leader through the peer route (probe at this commit: list advertises ["leader [aaa111]"] while bare leader resolves {kind:'one'} and sendToPeer is called). The direction is conservative — the advertised bracketed address still round-trips to the right peer, no misdelivery — but it violates the mirror's stated parity contract ("Validate every candidate against both those routes") and no test pins the no-team case. Gate the leader branch on the team's existence:
| const intercepted = (address: string): boolean => | |
| address.toLowerCase() === LEADER_NAME || | |
| address === teamFile?.leadAgentId || | |
| const intercepted = (address: string): boolean => | |
| (teamFile !== undefined && address.toLowerCase() === LEADER_NAME) || | |
| address === teamFile?.leadAgentId || |
Fix witness: the default toolWith() already models no team — add a peer named leader and assert parsed.sessions[0].to === 'leader'; red on current code (gets leader [ref]), green with the gate.
中文说明
intercepted() 无条件应用 LEADER_NAME 分支,但它镜像的路由——send-message.ts 的 inProcessRecipient——把整个进程内检查置于 !!teamManager 之下。没有活跃 team 时,镜像虚构了一个 send_message 并不执行的拦截:名为 leader 的可达对端只以 leader [ref] 输出,尽管 send_message({to:'leader'}) 会完全跳过进程内分支、经由 peer 路由投递裸 leader(本提交上的探针:列表输出 ["leader [aaa111]"],而裸 leader 解析为 {kind:'one'} 且 sendToPeer 被调用)。方向是保守的——输出的带括号地址仍能往返到正确对端,不会误投——但它违背了镜像声明的对齐契约("Validate every candidate against both those routes"),且没有测试钉住无 team 场景。把 leader 分支置于 team 存在性之下:
修复见证:默认 toolWith() 已建模无 team——加一个名为 leader 的对端,断言 parsed.sessions[0].to === 'leader';当前代码下为红(得到 leader [ref]),加上门控后变绿。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| submitQuery, | ||
| submissionInFlightRef, | ||
| submissionSettledRevision, | ||
| peerMessaging, | ||
| }); |
There was a problem hiding this comment.
[Suggestion] This pass of peerMessaging into useQueuedSubmissionDrain — the only thing that makes the drain-time pin check live in production — is pinned by no test. The drain-drop behaviour is tested only via renderHook with an injected { drainQueuedFrame } fake; every full-container peer test mocks useMessageQueue wholesale, and the FakePeerMessaging in the cross-session describe has no drainQueuedFrame member. Mutation-verified at this commit: deleting this line survives all 252 tests across AppContainer/peer-messaging/useMessageQueue, while a control mutation (drainQueuedFrame(undefined)) fails — the suite is alive, it just never covers this call site. Dropping the line silently revives the exact regression the R1-6 fix exists to prevent: a queued envelope whose addressee was replaced by /clear is submitted into the successor session. Add a full-container (or wiring) test — e.g. render with FakePeerMessaging extended by a drainQueuedFrame spy and a real, unmocked queue holding one stale peer entry, asserting the spy is called with the entry's delivery; removing the anchored line must make it red.
中文说明
把 peerMessaging 传入 useQueuedSubmissionDrain 的这一行——让生产环境中 drain 时固定检查生效的唯一接线——没有任何测试钉住。drain 丢弃行为只通过带注入 { drainQueuedFrame } 假对象的 renderHook 测试;所有全容器 peer 测试都整体 mock 了 useMessageQueue,cross-session describe 里的 FakePeerMessaging 也没有 drainQueuedFrame 成员。本提交上经变异验证:删除这一行,AppContainer/peer-messaging/useMessageQueue 的全部 252 条测试保持通过,而对照变异(drainQueuedFrame(undefined))会失败——测试套件是活的,只是从未覆盖这个调用点。删掉这一行会悄悄复活 R1-6 修复要防止的那个回归:收件人已被 /clear 替换的排队信封被提交进后继会话。补一条全容器(或接线)测试——例如用扩展了 drainQueuedFrame spy 的 FakePeerMessaging 加一个真实未 mock、含一条过期 peer 条目的队列渲染,断言 spy 被以该条目的 delivery 调用;移除被锚定的这一行应变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
- One routing rule for "does this address stay in-process": `*` always, and the leader handle, lead agent id and member names (sanitized, as TeamManager matches them) only while a team is active. send_message routes by it, list_agents advertises by it, and the near-miss suggester and the sent address are filtered by it — so a peer named `*` or shadowed by a teammate is never handed to the model bare, and a session named "leader" is reachable when there is no team. - A bare target is read both as a name and as a ref and the readings are merged, like the bracketed form: a name equal to another session's ref is ambiguous, never a silent pick. - A misaddressed drop is tombstoned on both release paths, so a re-sent id with a swapped body repeats the verdict instead of re-entering the gate; and `decide` reports it as 'gone', so /peers says the message is no longer waiting instead of "Released". - EAGAIN/EBUSY sends are forgotten by the ledger (the frame was never written); an accepted message that expired is announced as the session exiting before reading it, not as a held message expiring. - Docs: /peers row moved out of the approval-mode group, misaddressed added to the receipt list, crossSessionInbound typed as enum. Claude-Session: https://claude.ai/code/session_01MGkHMaMFhR2gfhbk5koXEC
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
6 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-9 reassertSessionRegistryRecord/queueRetriedSessionRegistryPatch have zero tests — already reported (comment 3866135171), still open
- peer-send.ts added ledger/identity fields with no production read sites (sentAt, OwnPeerIdentity.sessionId) — already recorded in the round-2 review body's deferral list (review 5038344526)
- held→expired / held→misaddressed settle-transition tests missing — already recorded in the round-2 review body's deferral list (review 5038344526)
- send-message description [ref]-only-when-two-sessions-share-a-name guidance contradicts list_agents' reserved-name refs — already reported (comments 3866135246, 3866135253), still open
- docs claim that list_agents shows the same name qwen sessions ps prints (ps truncates to 20 columns) — already reported (comment 3866135213, R1-15), still open
- R2-6 the peerMessaging wiring into useQueuedSubmissionDrain is pinned by no test — still stands; already reported (comment 3869676540), same-location overlap drop
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI (fork gating) and its suite did not run locally.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/core/src/ipc/peer-send.ts:165 — [review] lookupSentPeerMessage exported with zero production read sitespackages/core/src/tools/send-message.ts:454 — [review] !peerMessagingOff catch-append guard pinned by no testpackages/cli/src/ui/AppContainer.tsx:464 — [review] drain positive path with peerMessaging wired pinned by no test
Convergence: round 3 posted 3 inline comment(s), 3 of them reported for the first time; the previous round posted 8 (8 new). Findings keep coming back to the same files: packages/cli/src/ui/AppContainer.tsx (findings in round 2; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查,审查缺口已披露。
本轮确认的 6 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI (fork gating) and its suite did not run locally。
收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 3 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 3 轮发布了 3 条行内评论,其中 3 条是首次提出;上一轮发布了 8 条(其中 8 条首次提出)。发现反复回到同一批文件:packages/cli/src/ui/AppContainer.tsx(第 2 轮已出过发现,本轮又有 1 条)。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.22.2)
| const detail = | ||
| status === 'expired' && previous !== 'held' | ||
| ? 'That session exited before it read your message; it was not delivered.' | ||
| : describeDeliveryStatus(status); |
There was a problem hiding this comment.
[Critical] R1-7: (fix-induced) The round-2 fix for R1-7 reworded this expired receipt notice, but the new wording asserts the peer session exited for every expired receipt that did not end a hold — and the gate also emits expired while the recipient is alive. When the receiving session's accept backlog is full (MAX_ACCEPTED_BACKLOG = 50), InboundGate.admit resolves accept on policy, tryDeliver throws 'accepted-message backlog is full', and the gate reports expired without ever reporting delivered — its own comment calls that state transient and retryable. The sender ledger then moves pending → expired, which is terminal, and this effect writes Message to <addr>: That session exited before it read your message; it was not delivered. into history even though the peer is alive; a retry under a new msgId has its plain delivered receipt silenced by this same effect, so the false claim is never corrected and the model plans around a peer it believes is gone. The exit-specific text is only guaranteed accurate when previous === 'delivered' (settleUnconsumed at close).
Witness (probe against the real receipt path at this commit):
emit { status: 'expired', previous: 'pending', address: 'docs-cd' }
→ "Message to docs-cd: That session exited before it read your message; it was not delivered."
gate admit: { decision: 'refused', statuses: ['expired'] } (no 'delivered' ever reported)
settleSentPeerMessage(id, 'delivered') === undefined (no later receipt can correct it)
flip (previous === 'delivered' guard): probe passes, existing receipts test still green
| const detail = | |
| status === 'expired' && previous !== 'held' | |
| ? 'That session exited before it read your message; it was not delivered.' | |
| : describeDeliveryStatus(status); | |
| const detail = | |
| status !== 'expired' | |
| ? describeDeliveryStatus(status) | |
| : previous === 'delivered' | |
| ? 'That session exited before it read your message; it was not delivered.' | |
| : 'Your message expired without being delivered; the recipient session was busy or has exited.'; |
Extend the receipts test in AppContainer.test.tsx with an emission of { status: 'expired', previous: 'pending' } asserting the notice does not contain 'exited'; removing the previous === 'delivered' guard must make it red.
中文说明
R1-7:(修复引入)第 2 轮对 R1-7 的修复改写了这条 expired 回执通告,但新措辞对每一个不是结束 hold 的 expired 回执都断言对端会话已退出——而闸门在对端存活时也会发出 expired。当接收会话的接受队列已满(MAX_ACCEPTED_BACKLOG = 50)时,InboundGate.admit 按策略判定 accept,tryDeliver 抛出 'accepted-message backlog is full',闸门在未报告过 delivered 的情况下报告 expired——其自身注释称该状态是暂时且可重试的。发送方台账随后 pending → expired,这是终局状态,而此处仍向历史写入 "That session exited before it read your message",即使对端还活着;以新 msgId 重试时,其普通的 delivered 回执又被同一效果静默,错误断言永远无法被纠正,模型会基于一个它认为已退出的对端做决策。退出专属文案只有在 previous === 'delivered'(close 时的 settleUnconsumed)时才保证准确。
见证(本提交上对真实回执路径的探针):发出 { status: 'expired', previous: 'pending', address: 'docs-cd' } → 渲染出 "That session exited before it read your message";闸门 admit 返回 { decision: 'refused', statuses: ['expired'] }(从未报告 'delivered');settleSentPeerMessage(id, 'delivered') === undefined(后续回执无法纠正);加上 previous === 'delivered' 守卫后探针翻转,既有回执测试仍为绿。
建议把退出断言只保留给真正投递成功过的回执,对 pending 出发的 expired 使用中性文案(见上方 suggestion 块)。
修复见证:在 AppContainer.test.tsx 的回执测试中补发 { status: 'expired', previous: 'pending' },断言通告不含 'exited';移除 previous === 'delivered' 守卫后该测试应变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| const peer = resolved.peer; | ||
| const address = formatPeerAddress(peer, peers, options.isReserved); |
There was a problem hiding this comment.
[Suggestion] sendToPeer recomputes the address it records and reports with formatPeerAddress instead of round-trip-validating it, so the address stored in the ledger, printed in the 'sent' confirmation, and surfaced in every later receipt can be a string that re-resolves ambiguous and that list_agents never advertised — contradicting the SentPeerMessage.address doc ('as list_agents printed it'). Concrete shape: team member docs-cd active, peer s1 named docs-cd (ref aaa111), peer s2 carrying the literal registry name docs-cd [aaa111] (ref bbb222) — registry names are other-process input this PR deliberately treats as attacker-influenced. list_agents advertises [aaa111] for s1 because the candidate docs-cd [aaa111] fails its round-trip check; the model sends [aaa111], resolution correctly picks s1, but the recorded address becomes docs-cd [aaa111] — re-resolving it returns ambiguous (s1 by name+ref, s2 by literal name). Receipts surface that poisoned address, and a model re-using it hits 'Ambiguous recipient'. Nothing is misdelivered (resolution was correct), which is why this is a Suggestion.
Witness (probe at this commit):
sendToPeer({ target: '[aaa111]' }) → outcome.peer === s1, outcome.address === 'docs-cd [aaa111]'
resolvePeerTarget(peers, outcome.address) → { kind: 'ambiguous', matches: [s1, s2] }
flip (round-trip check + fallback): outcome.address === '[aaa111]' → re-selects s1 uniquely
| const peer = resolved.peer; | |
| const address = formatPeerAddress(peer, peers, options.isReserved); | |
| const peer = resolved.peer; | |
| let address = formatPeerAddress(peer, peers, options.isReserved); | |
| if (resolvePeerTarget(peers, address).kind !== 'one') { | |
| const bracketed = `${peer.name} [${peer.ref}]`; | |
| address = | |
| resolvePeerTarget(peers, bracketed).kind === 'one' | |
| ? bracketed | |
| : `[${peer.ref}]`; | |
| } |
Add a peer-send.test.ts case with the setup above sending to [aaa111] and asserting resolvePeerTarget(peers, outcome.address) equals { kind: 'one', peer: s1 }; removing the round-trip check must make it red. (Better still: extract the candidate ladder into peer-directory.ts and share it with list-agents so the two sites cannot drift.)
中文说明
sendToPeer 用 formatPeerAddress 重新计算要记账并报告给模型的地址,而没有先验证它能解析回原对端,因此存入台账、打印在 'sent' 确认里、并在之后每条回执中出现的地址,可能是一个重新解析会得到 ambiguous、且 list_agents 从未输出过的字符串——与 SentPeerMessage.address 的注释("as list_agents printed it")矛盾。具体形态:团队成员 docs-cd 在场,对端 s1 名为 docs-cd(ref aaa111),对端 s2 的注册名字面就是 docs-cd [aaa111](ref bbb222)——注册表名字是本 PR 刻意按攻击者可影响处理的其他进程输入。list_agents 为 s1 输出 [aaa111],因为候选 docs-cd [aaa111] 过不了往返检查;模型发送 [aaa111],解析正确选中 s1,但记账地址变成 docs-cd [aaa111]——再解析返回 ambiguous(s1 按名字+ref、s2 按字面名)。回执会把这个有毒地址带给模型,模型照它重发就会撞上 'Ambiguous recipient'。没有错投(解析是对的),所以这是 Suggestion。
见证(本提交上的探针):sendToPeer({ target: '[aaa111]' }) → outcome.peer === s1、outcome.address === 'docs-cd [aaa111]';resolvePeerTarget(peers, outcome.address) → { kind: 'ambiguous', matches: [s1, s2] };翻转(加往返检查与回退)后 outcome.address === '[aaa111]',可唯一重新选中 s1。
修复见证:在 peer-send.test.ts 中按上述布置发送到 [aaa111],断言 resolvePeerTarget(peers, outcome.address) 等于 { kind: 'one', peer: s1 };移除往返检查后应变红。(更好的做法:把候选阶梯提取到 peer-directory.ts,与 list-agents 共用,两处不再漂移。)
— qwen3.8-max via Qwen Code /review (v0.22.2)
| /** Decides whether `send_message` would keep `address` in-process. */ | ||
| export type InProcessRecipientCheck = (address: string) => boolean; |
There was a problem hiding this comment.
[Suggestion] InProcessRecipientCheck is exported (and re-exported through packages/core/src/index.ts's export * from './ipc/peer-routing.js') but has zero read sites anywhere in the tree — a worktree-wide sweep finds exactly one occurrence, the declaration itself. Every call site uses inline signatures of the exact same shape: send-message.ts passes an isReserved closure built from isInProcessRecipient, list-agents.ts declares its own local predicate, and peer-send.ts / peer-directory.ts declare isReserved?: (address: string) => boolean inline. The alias must be maintained and backward-compatibly versioned while documenting no real contract — dead public API surface in a core package export, contrary to the repo's simplicity-first rule. Its sibling InProcessRoutingTeam IS used (send-message.ts), so this is not a keep-both convention.
Witness (sweep at this commit): 1 occurrence of InProcessRecipientCheck worktree-wide — the declaration; exported in peer-routing.d.ts, absent from the .js (type-only).
| /** Decides whether `send_message` would keep `address` in-process. */ | |
| export type InProcessRecipientCheck = (address: string) => boolean; |
中文说明
InProcessRecipientCheck 被导出(并经 packages/core/src/index.ts 的 export * from './ipc/peer-routing.js' 再导出),但整棵树中没有任何读取点——全工作区扫描只找到一处,就是声明本身。所有调用点都使用形状完全相同的内联签名:send-message.ts 传入由 isInProcessRecipient 构造的 isReserved 闭包,list-agents.ts 声明自己的局部谓词,peer-send.ts / peer-directory.ts 内联声明 isReserved?: (address: string) => boolean。这个别名必须被维护并保持向后兼容,却不承载任何真实契约——核心包导出里的死公共 API 面,与仓库的简洁优先原则相悖。它的兄弟 InProcessRoutingTeam 确实在用(send-message.ts),所以这不是"两个都保留"的惯例。
见证(本提交上的扫描):全工作区 InProcessRecipientCheck 仅 1 处——声明本身;出现在 peer-routing.d.ts,不在 .js 中(纯类型)。
— qwen3.8-max via Qwen Code /review (v0.22.2)
- An `expired` receipt that ended no hold and followed no delivery no longer claims the peer exited: the gate also expires a frame it could not queue (accept backlog full) while the session is alive, so the notice says the message was not delivered and to retry once the session is idle. Only a delivery corrected to expired means an exit. - The address a send records — and every later receipt names — is the one list_agents would print: both sites now share `advertisablePeerAddress`, the shortest candidate that the caller's routing leaves alone and that resolves back to exactly that peer. - `send_message`'s ref guidance matches list_agents' rule (use the `to` value verbatim; the ref is there whenever the bare name would not reach the session); the docs no longer promise the `ps` table's possibly truncated name; a dead type alias and an unread ledger field are gone. - Tests for `reassertSessionRegistryRecord`, the retried /clear patch, the held→expired / held→misaddressed ledger corrections, and the address round-trip. Claude-Session: https://claude.ai/code/session_01MGkHMaMFhR2gfhbk5koXEC
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI (fork gating) and its suite did not run locally.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
packages/core/src/ipc/peer-directory.ts:225 — [probe] suggestPeerNames hands out addresses without round-trip validationpackages/core/src/ipc/peer-send.ts:235 — [probe] ambiguous-match list prints an address that re-resolves ambiguouspackages/core/src/ipc/peer-send.test.ts:498 — [probe] delivered row's repeat/no-op cells unpinned in the receipt matrixpackages/core/src/ipc/peer-send.ts:164 — [review] lookupSentPeerMessage exported with zero production read sitespackages/core/src/tools/send-message.ts:452 — [probe] !peerMessagingOff catch-append conjunct pinned by no testpackages/core/src/ipc/peer-send.ts:325 — [probe] ETIMEDOUT 'retry once' advice duplicates delivery (fresh msgId bypasses dedup)packages/core/src/tools/list-agents.ts:99 — [probe] empty branch claims no session reachable while peers are live but unadvertisabledocs/users/features/commands.md:803 — [review] enable-instruction omits that both keys are workspace-restrictedpackages/core/src/ipc/peer-send.ts:322 — [probe] EBUSY text claims the peer is alive for a pre-dial local-cap rejectionpackages/core/src/tools/send-message.ts:140 — [probe] team mode swallows the cross-session-messaging-disabled hint
Convergence: round 4 posted 3 inline comment(s), 3 of them reported for the first time; the previous round posted 3 (3 new). The rate of new findings is not falling. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI (fork gating) and its suite did not run locally。
收敛姿态下延后(第 4 轮,非阻断)——已记录,本轮不要求修改:共 10 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 4 轮发布了 3 条行内评论,其中 3 条是首次提出;上一轮发布了 3 条(其中 3 条首次提出)。新发现的产出速度没有下降。把剩余修复攒成一批、验证后再推送,或将本 PR 的评审降到 --severity-floor critical,可以避免循环反复推导同一组发现。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.22.2)
| const teamFile = teamManager?.getTeamFile(); | ||
| const inProcessRecipient = isInProcessRecipient(to, teamFile); |
There was a problem hiding this comment.
[Critical] R4-1: The in-process precedence check compares the raw string while the peer resolver trims its target, so a whitespace-padded reserved handle escapes the reservation and is delivered cross-session. isInProcessRecipient(to, teamFile) exact-matches ('leader '.toLowerCase() === 'leader' is false, and sanitizeName('leader ') matches no member), but resolvePeerTarget trims — so to: "leader " (a trailing space or newline, e.g. copied from quoted text) falls through to the peer route and resolves to a peer session that happens to be named leader, writing the report into another session. Pre-diff the same input errored in TeamManager.sendMessage (its leader check is also exact-match), so this diff newly turns an invalid spelling into a cross-session delivery — the leak the adjacent comment says this check exists to prevent. The same asymmetry applies to a padded leadAgentId; member names are unaffected because both sides sanitize.
Witness (probe against the real execute/routing at this commit):
BASELINE to='leader' → sendToPeer never called; teamManager.sendMessage('leader', …) called
PR to='leader ' → sendToPeer called 1× with target 'leader '; sendMessage not called; result.error undefined
with trim fix → sendToPeer called 0× for 'leader ' (flip); 41/41 send-message.test.ts green
Normalize once at the routing boundary, e.g. const to = this.params.to?.trim(); in execute() before the if (!to) check, so the in-process rule, TeamManager, and the peer resolver all see the same string.
Fix witness: extend the loop in send-message.test.ts "never routes a teammate's report to the leader through the peer directory" with 'leader ' — without the fix, sendToPeer is consulted and expect(sendToPeer).not.toHaveBeenCalled() goes red; removing the trim must make it red again.
中文说明
进程内优先判断用原始字符串比较,而 peer 解析器会先 trim,导致带空白填充的保留名绕过预留、被投递到跨会话路径。isInProcessRecipient(to, teamFile) 是精确匹配('leader '.toLowerCase() === 'leader' 为 false,sanitizeName('leader ') 也不匹配任何成员),但 resolvePeerTarget 会 trim——于是 to: "leader "(尾随空格或换行,比如从引用文本里复制)会落入 peer 路由,解析到一个恰好叫 leader 的会话,把消息写进另一个会话。改动之前同样的输入会在 TeamManager.sendMessage 里报错(它的 leader 判断同样是精确匹配),所以本 diff 把一次非法拼写新变成了跨会话投递——正是相邻注释声称本检查要防止的泄漏。同样的不对称适用于带空白的 leadAgentId;成员名不受影响,因为两侧都做 sanitize。
见证(本提交上对真实 execute/路由的探针):基线 to='leader' → 不调用 sendToPeer、调用 teamManager.sendMessage;PR 上 to='leader ' → sendToPeer 被以目标 'leader ' 调用 1 次、sendMessage 未被调用、result.error 为 undefined;加上 trim 修复后 → 'leader ' 不再触发 sendToPeer(翻转),41/41 测试仍绿。
建议在路由边界统一归一化,例如在 execute() 的 if (!to) 之前 const to = this.params.to?.trim();,让进程内规则、TeamManager 与 peer 解析器看到同一个字符串。
修复见证:在 send-message.test.ts 的 "never routes a teammate's report to the leader through the peer directory" 循环里加入 'leader '——没有修复时 sendToPeer 会被调用、expect(sendToPeer).not.toHaveBeenCalled() 变红;移除 trim 后该测试应再次变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| export async function listMessageablePeers(): Promise<PeerSessionInfo[]> { | ||
| const records = await listLiveSessions(); | ||
| const candidates = records | ||
| .map(toPeerSessionInfo) | ||
| .filter((peer): peer is PeerSessionInfo => peer !== null); |
There was a problem hiding this comment.
[Critical] R4-2: listMessageablePeers never dedupes by sessionId, and resolvePeerTarget's new Set merge dedupes by object identity, not value — so two live processes hosting one session id become an ambiguity no address in the grammar can ever resolve, and list_agents drops the session entirely. Nothing prevents that state: the registry is keyed by PID, registerSession guards only same-path collisions, --resume/--continue start a second process without signalling the original, and the only writer lease (SessionWriterLease) is gated behind the experimental Zed integration — the interactive TUI that hosts peer messaging is not covered. A user who runs qwen --resume <id> in a second pane while the original still gets two value-equal peers (same name, same ref derived from the shared session id); sending to name, name [ref], [ref], or the bare ref all resolve ambiguous permanently, advertisablePeerAddress returns undefined for both, and list_agents omits the session while the send error still advises "Re-send with the full 'name [ref]'" — an instruction that can never resolve. The feature blacks out for that session, undetectably, until one process exits.
Witness (probe end-to-end with two live child processes under a temp QWEN_HOME):
listLiveSessions: 2 records, both sessionId 'shared-session-id-0001'
listMessageablePeers: [{name 'app-12', ref '639913', pid A}, {name 'app-12', ref '639913', pid B}]
resolvePeerTarget('app-12' | 'app-12 [639913]' | '[639913]' | '639913') → ambiguous 2 (all four)
advertisablePeerAddress(pid A) → undefined; advertisablePeerAddress(pid B) → undefined
list_agents sessions: 0 while peers: 2
flip (dedupe by sessionId, newest startedAt): 1 peer; resolvePeerTarget('app-12') → one
Dedupe candidates by sessionId in listMessageablePeers before probing, keeping the record with the newest startedAt (consistent with the registry's newest-first sort).
Fix witness: add a listMessageablePeers case in peer-directory.test.ts where listLiveSessions resolves two records with the same sessionId but different pid/ipcPath, asserting a single peer is returned and resolvePeerTarget(peers, name) yields kind: 'one'; removing the dedupe makes it red.
中文说明
listMessageablePeers 从不按 sessionId 去重,而 resolvePeerTarget 的 new Set 合并按对象身份(而非值)去重——于是同一会话 id 被两个存活进程承载时,会形成一个语法中任何地址都无法解析的歧义,list_agents 也会把该会话整个丢掉。这种状态没有任何东西阻止:注册表按 PID 建键,registerSession 只防同路径冲突,--resume/--continue 会启动第二个进程而不通知原进程,唯一的写锁(SessionWriterLease)又只在实验性 Zed 集成下启用——承载 peer 消息的交互式 TUI 不在其覆盖范围内。用户在第二个窗格里 qwen --resume <id> 而原会话仍在运行时,会得到两个值相等的 peer(同名、同 ref,都派生自同一会话 id);向 name、name [ref]、[ref] 或裸 ref 发送都永久解析为 ambiguous,advertisablePeerAddress 对两者都返回 undefined,list_agents 省略该会话,而发送失败提示仍然建议 "Re-send with the full 'name [ref]'"——一个永远无法解析的指令。该会话的功能就此不可见地失效,直到其中一个进程退出。
见证(临时 QWEN_HOME 下、两个真实子进程的端到端探针):两条注册记录同属 'shared-session-id-0001';listMessageablePeers 返回两个同名同 ref 的 peer;四种地址形式全部 ambiguous 2;两个 advertisablePeerAddress 均为 undefined;list_agents 的 sessions 为 0 而 peers 为 2;翻转(按 sessionId 去重、取最新 startedAt)后只剩 1 个 peer,resolvePeerTarget('app-12') → one。
建议在 listMessageablePeers 探测前按 sessionId 去重,保留 startedAt 最新的记录(与注册表的最新优先排序一致)。
修复见证:在 peer-directory.test.ts 增加一个 listMessageablePeers 用例,令 listLiveSessions 返回两条 sessionId 相同但 pid/ipcPath 不同的记录,断言只返回一个 peer 且 resolvePeerTarget(peers, name) 为 kind: 'one';移除去重后应变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| expect(outcome).toMatchObject({ | ||
| kind: 'sent', | ||
| peer: s1, | ||
| address: '[aaa111]', | ||
| }); |
There was a problem hiding this comment.
[Suggestion] R4-3: This test ("records an address that re-resolves to the same session") asserts only the send outcome (outcome.address) and a direct resolvePeerTarget call — it never calls lookupSentPeerMessage, so the ledger half of the round-3 fix (trackSent storing the round-trippable address) is unwitnessed in the only case where advertisablePeerAddress diverges from the old formatPeerAddress. Verified by mutant: recording options.target — or reverting trackSent to the pre-round-3 formatPeerAddress(peer, peers, options.isReserved), which stores the non-round-trippable docs-cd [aaa111] instead of [aaa111] — leaves the whole suite green (Tests 35 passed (35) under both mutants). At receipt time peer-messaging.ts forwards settled.address into the notice the sender sees, so such a regression re-advertises the reserved bare name and a re-send routes to the teammate instead of the peer — re-opening exactly the misrouting the round-3 address change closed.
In this same test, read the frame id (sendPeerFrame.mock.calls[0][1].msgId) and additionally assert lookupSentPeerMessage(frame.msgId) toMatchObject({ address: '[aaa111]', peerName: 'docs-cd', state: 'pending' }).
Fix witness: that added assertion goes red if trackSent records anything other than the advertisable address (e.g. mutate address → options.target); today no test goes red under that mutation.
中文说明
这条测试("records an address that re-resolves to the same session")只断言了发送结果(outcome.address)和一次直接的 resolvePeerTarget 调用——它从未调用 lookupSentPeerMessage,因此在 advertisablePeerAddress 与旧 formatPeerAddress 唯一分叉的场景里,第 3 轮修复的台账那一半(trackSent 存可往返解析的地址)没有任何见证。变异体验证:把 trackSent 改记 options.target——或还原为第 3 轮之前的 formatPeerAddress(peer, peers, options.isReserved)(它会存下不可往返的 docs-cd [aaa111] 而非 [aaa111])——整个测试套件仍然全绿(两种变异下均 Tests 35 passed (35))。回执时 peer-messaging.ts 会把 settled.address 转发进发送方看到的通告,这类回归会重新广告保留的裸名字,重发会路由到 teammate 而非 peer——正好重新打开第 3 轮地址修改关闭的误路由。
建议在同一测试里读取帧 id(sendPeerFrame.mock.calls[0][1].msgId),补充断言 lookupSentPeerMessage(frame.msgId) toMatchObject({ address: '[aaa111]', peerName: 'docs-cd', state: 'pending' })。
修复见证:若 trackSent 记录的不是可广告地址(例如把 address 变异为 options.target),新增断言应变红;目前该变异下没有任何测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
What this PR does
Adds the send side of same-machine cross-session messaging, on top of the receive side that landed in #9576. A session that has cross-session messaging enabled can now discover the other enabled sessions on this machine with
list_agentsand message one of them withsend_messageby name — the same nameqwen sessions psprints.list_agentslists reachable sessions (registered and answering on their socket, probed concurrently with a 250 ms cap) undersessions, each withname, a shortrefderived from its session id,cwd, and atovalue that is the bare name unless two sessions share it, in which case it isname [ref]. It also reports this session's ownname/refunderself, so a model that sees its own name — in a peer's message, or in "reply to X" — recognises it instead of trying to message itself. Named Agent Team teammates stay out of the listing, as on main.send_messagegains a third route, tried aftertask_idand after an in-process teammate: a bare name that exactly matches one live session delivers; two sessions sharing a name requirename [ref]; an ambiguous bare name is refused with the candidates rather than guessed, because a message injected into the wrong session cannot be taken back. An in-process recipient — a teammate, or the leader a teammate reports back to — wins a name collision with a peer (a peer shadowed that way is listed with its[ref], which no teammate name sanitizes to),to: "*"stays a team-only broadcast (it never fans out across sessions), and addressing one's own name is answered as the mistake it is. When messaging is off, the error says so rather than claiming a lookup that never happened. The sender asserts its approval-mode class using the same predicate the receiving gate applies to itself, so two sessions in the same mode always agree on parity. Every failure is a described outcome with a next step for the model (turn the feature on, fix the name, add a ref, re-list, retry).Two receive-side additions close loops the receive-only step had to leave open. Frames now carry the recipient's session id: the socket address is keyed by PID and PIDs get reused, so a receiver whose id differs (PID reuse, or
/clearswapping the id) refuses the frame with a newmisaddressedreceipt — distinct fromdenied, because nobody decided anything — and re-asserts its own registry record, since a/clearpatch skipped under fd pressure can leave the record naming the previous id (that patch now retries, like the inbox-address advertise already did). And receipts for messages this session sent are now surfaced in the sender's transcript —held,denied,expired,misaddressed, and a delivery that ends a hold — so a sender can tell "parked for review" from "delivered and ignored". They pass through a bounded send ledger that only reports a receipt when it moves the message to a new state: receipts for ids this session never sent, and a peer repeating one, are dropped before they reach the UI, so neither a stranger nor a chatty peer can grow the history.Registry-sourced strings (
name,cwd) are flattened the way the envelope flattens a sender's label before they reach tool output. The tool descriptions carry the same clause as the receive envelope and the permission classifier rule: a message to another session carries none of the user's authority there, and must never be used to have another session perform an action this one was denied — the three have to agree, and now all three do.Why it's needed
The receive side alone has no visible outcome: a session could accept messages, but nothing could address it. This completes the smallest useful loop between two already-running local sessions — the payoff of #8724 — while keeping the feature off by default behind
agents.crossSessionMessaging, receive-first (a session can refuse before anything can reach it), and name-addressed (a socket path is an implementation detail that changes every restart; a name survives one and reads back to the user).Reviewer Test Plan
How to verify
Unit level:
cd packages/core && npx vitest run src/ipc src/tools/send-message.test.ts src/tools/list-agents.test.ts src/services/session-registry.test.ts(364 tests) andcd packages/cli && npx vitest run src/peerMessaging/peer-messaging.test.ts src/ui/AppContainer.test.tsx src/ui/startInteractiveUI.test.tsx(205 tests).npm run typecheck,npm run lintandprettier --checkon the changed files are clean.End to end, on one machine with
{ "agents": { "crossSessionMessaging": true } }insettings.json:sessionsshows B withtoequal to B's name fromqwen sessions ps, andselfshows A's own name.<cross_session_message>arrives and the model acts within B's own permissions; A's transcript shows nothing more (delivery is the expected case)./approval-mode yolo) and repeat from A in DEFAULT: B announces a held message; A's transcript showsMessage to <B>: Your message is held …./peers acceptin B: A's transcript shows… released …;/peers deny:… declined ….list_agentsshows both asname [ref]; a bare-name send from A is refused with the two candidates;name [ref]delivers to the right one.kill -9and re-list from A: B is gone (its socket no longer answers), and a send to B's name says the address is stale.Evidence (Before & After)
Before:
list_agentsreported only in-process background agents andsend_messagecould not reach another CLI process.After — two tmux sessions (
alphain/tmp/qwen-lt/alpha,betain/tmp/qwen-lt/beta) on this branch's build, captured from the panes. Steps: discovery; a YOLO→YOLO send that beta answers by name; a DEFAULT→YOLO send that beta holds, announces, and releases with/peers accept, with both receipts arriving in alpha; a self-send; and beta killed withSIGKILL, after which alpha's listing drops it and a send reports no reachable session.tmux transcript
Tested on
Windows: the feature is out of scope there by design (the inbox does not bind on win32, so
list_agentsshows nosessionsandsend_messagefalls through to its existing behavior).Environment (optional)
npm run build && node scripts/start.js, two/three tmux panes, Linux.Risk & Scope
misaddressedstatus, and the docs corrections.name [ref]; the alternative (silently switching between two same-named sessions) is worse. Discovery dials every registered session's socket (bounded at 250 ms, concurrent), solist_agentscosts about one probe however many sessions exist.list_agents(main deliberately excludes them); a registrystatus(busy/idle) field; hardening items already recorded under step 4 of Cross-session messaging: let Qwen Code sessions on the same machine message each other #8724 (socket-directory vetting that fails loudly, a first-line deadline, an inbox auth token, backpressure); Windows named pipes;notify_when_idle.send_message/list_agentsbehave exactly as before when cross-session messaging is off (the peer route is only consulted once this session has an inbox), andto: "*"keeps its team-only meaning. Frames from older senders withouttoSessionIdare still admitted.Linked Issues
Part of #8724 (step 3). Follows #8969 and #9576.
中文说明
这个 PR 做了什么
在 #9576 落地的接收侧之上,补上同机跨会话消息的发送侧。开启了跨会话消息的会话现在可以用
list_agents发现本机其他开启了该功能的会话,并用send_message按名字给其中一个发消息 —— 就是qwen sessions ps打印的那个名字。list_agents在sessions下列出可达的会话(已注册且 socket 有应答,并发探测、每个 250 ms 封顶),每条带name、由会话 id 派生的短ref、cwd,以及to值 —— 名字唯一时就是裸名字,两个会话同名时则是name [ref]。它还在self下报告本会话自己的name/ref,这样模型在对方消息里或"回复 X"里看到自己的名字时能认出来,而不是试图给自己发消息。命名的 Agent Team teammate 和 main 上一样不列出。send_message新增第三条路由,在task_id和进程内 teammate 之后尝试:精确匹配唯一一个活跃会话的裸名字直接投递;两个会话同名时必须用name [ref];有歧义的裸名字会被拒绝并列出候选,而不是猜一个 —— 注入错会话的消息收不回来。进程内的接收者 —— teammate,或 teammate 汇报对象 leader —— 在同名冲突中优先(被这样遮蔽的对端会带[ref]列出,任何 teammate 名字都不会规范化成它),to: "*"仍然只是 team 内广播(绝不跨会话扩散),给自己的名字发消息会被明确指出是误用。功能关闭时,错误信息会直说,而不是假装做过查找。发送方用接收闸门对自身使用的同一个谓词声明自己的审批模式类别,因此同一模式的两个会话对"对等"的判断永远一致。每种失败都是带下一步动作的明确结果(开功能、改名字、加 ref、重新列表、重试)。接收侧补了两处收口。帧现在携带接收方的会话 id:socket 地址按 PID 编址而 PID 会被复用,所以 id 不一致的接收方(PID 复用,或
/clear换了 id)会用新的misaddressed回执拒绝 —— 有别于denied,因为没有任何人做出过决定 —— 并重新写入自己的注册表记录,因为在 fd 紧张时被跳过的/clearpatch 会让记录一直停留在旧 id 上(这个 patch 现在和收件箱地址通告一样会重试)。发送出去的消息的回执现在会显示在发送方的会话记录里 ——held、denied、expired、misaddressed,以及结束一次 hold 的投递 —— 发送方由此能区分"被扣下待审"和"投递后被忽略"。回执经过一个有界的发送台账,只有让消息进入新状态的回执才会上报:本会话从未发出过的 id 的回执、以及对端重复发送的回执,在到达 UI 之前就被丢弃,所以陌生进程和话痨对端都无法让历史记录膨胀。来自注册表的字符串(
name、cwd)在进入工具输出前按信封处理发送方标签的方式做了扁平化。工具描述携带与接收信封、权限分类器规则相同的条款:发给另一个会话的消息在那里不带用户的任何权限,绝不能用来让另一个会话执行本会话被拒绝的操作 —— 三处必须一致,现在三处都一致了。为什么需要
只有接收侧看不到任何结果:会话能收消息,但没有任何东西能寻址它。这补齐了两个已运行本地会话之间最小可用的闭环 —— #8724 的收益所在 —— 同时保持默认关闭(
agents.crossSessionMessaging)、先收后发(会话可以在任何人能寻址它之前拒绝)、按名寻址(socket 路径是每次重启都变的实现细节;名字能跨重启存活并且用户读得懂)。评审测试计划
如何验证
单元测试:
cd packages/core && npx vitest run src/ipc src/tools/send-message.test.ts src/tools/list-agents.test.ts src/services/session-registry.test.ts(364 个)和cd packages/cli && npx vitest run src/peerMessaging/peer-messaging.test.ts src/ui/AppContainer.test.tsx src/ui/startInteractiveUI.test.tsx(205 个)。npm run typecheck、npm run lint和改动文件的prettier --check全部干净。端到端,在一台机器上、
settings.json里设{ "agents": { "crossSessionMessaging": true } }:sessions里出现 B,to等于qwen sessions ps里 B 的名字,self是 A 自己的名字。<cross_session_message>,模型在 B 自己的权限内处理;A 的记录里没有更多输出(投递成功是默认情况)。/approval-mode yolo),A 在 DEFAULT 下重发:B 通告有消息被扣下;A 的记录出现Message to <B>: Your message is held …。B 里/peers accept:A 出现… released …;/peers deny:… declined …。list_agents把两者都显示为name [ref];A 用裸名字发送被拒绝并列出两个候选;用name [ref]投递到正确的那个。kill -9掉 B,A 重新列表:B 消失(socket 不再应答);再给 B 的名字发送会提示地址已失效。证据(前后对比)
之前:
list_agents只报告进程内后台 agent,send_message无法到达另一个 CLI 进程。之后:见英文部分的 tmux 记录 —— 两个会话(
alpha、beta)在本分支构建上的实录:发现;YOLO→YOLO 直投并按名回信;DEFAULT→YOLO 被 hold、通告、/peers accept放行,两次回执均到达 alpha;自我寻址;SIGKILL掉 beta 后 alpha 列表中消失、发送报告不可达。测试平台
Windows:该功能按设计不在范围内(win32 上收件箱不绑定,所以
list_agents没有sessions,send_message回落到原有行为)。环境(可选)
npm run build && node scripts/start.js,两到三个 tmux 窗格,Linux。风险与范围
misaddressed状态、文档修正。name [ref]再调一次;另一种做法(在两个同名会话之间悄悄切换)更糟。发现过程会拨号每个已注册会话的 socket(250 ms 封顶、并发),所以无论有多少会话,list_agents的开销大约是一次探测。list_agents列出 teammate(main 上刻意排除);注册表的status(忙/闲)字段;已记录在 Cross-session messaging: let Qwen Code sessions on the same machine message each other #8724 第 4 步的加固项(失败时大声报告原因的 socket 目录审计、首行截止、收件箱鉴权 token、背压);Windows 命名管道;notify_when_idle。send_message/list_agents行为与之前完全一致(只有本会话有收件箱时才会查询 peer 路由),to: "*"保持 team 内含义。不带toSessionId的旧发送方的帧仍然被接受。关联 Issue
属于 #8724(第 3 步)。承接 #8969 和 #9576。
https://claude.ai/code/session_01MGkHMaMFhR2gfhbk5koXEC