fix(core): make the failed-spawn compensating-write gate commit-aware - #10522
Conversation
The failed-spawn compensating-write gate from #10223 compares teamFileWritesStarted against the value captured at member push, but a write that rejected (atomic temp+rename persisted nothing) still counts as started. In the solo case the member's own write starts and throws ENOSPC, the catch rolls the member back, and the gate reads 1 > 0 — firing a compensating write that cannot repair anything. If the disk is still full that write fails too, notifying the leader about a possible ghost member that cannot exist on top of the spawn error that already carries the real cause (#10297). Decrementing the counter on rejection is unsafe: the counter is a monotonic high-water mark, and a decrement lets a later write reuse the value and hide below an earlier member's push watermark, re-introducing the #10208 ghost. Make the gate commit-aware instead: - teamFileWritesStarted becomes the per-write sequence number, still assigned synchronously at the snapshot point and never decremented. - A new teamFileWritesCommitted watermark records the sequence number of the most recently committed write; the queue is serial, so commits land in sequence order and the watermark stays monotonic. - The compensating write is queued with onlyIfCommittedAfter set to the push-time watermark; the queued task runs after every earlier write (including any still in flight at gate time) has settled and writes only if a write above the watermark committed — a rejected window write persists nothing, so the redundant write is skipped. Witness tests: the solo rejected-write case asserts exactly one write attempt and no leader notice (red before the fix: the compensating write fired), and the issue's five-step interleaving pins that a later committed write still triggers the repair, guarding against a decrement-on-reject regression. Fixes #10297 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
github-actions
Bot
added
the
review/self-reported
|
Thanks for the PR! Re-run on request. The head moved since the last pass ( Template looks good ✓ Problem: observed and documented, not theoretical. #10297 (filed by the same author, triaged as Direction: aligned. This is the follow-up explicitly split out of the #10223 review thread so the minimal #10208 ghost fix stayed small, and #10297's "Scope for a safe fix" section prescribes exactly this: make the gate commit-aware while keeping the watermark monotonic, landed separately with its own interleaving tests. Size: core path ( Approach: scope feels right. Risk: no elevated risk signals — no high-risk paths matched, and the Moving on to code review. 🔍 中文说明感谢贡献！ 应请求 re-run。头部自上次审查后移动过（ 模板完整 ✓ 问题：已观测、有记录，不是理论问题。#10297（同一作者提交，已标记 方向：对齐。这是从 #10223 review 线程中明确拆出的后续项（当时为了让 #10208 幽灵成员的最小修复保持精简），#10297 的"安全修复范围"一节给出的正是这个方案：让门控感知提交（commit-aware）、同时保持水位单调，并单独落地、配自己的交错测试。 规模：核心路径（ 方案：范围合理。 风险：无升级风险信号——未命中任何高风险路径， 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewIndependent proposal first, for the record: keep What I verified against the diff and the surrounding code (re-checked this run; the head is
One non-blocking note: every failed spawn now waits for queued roster writes to settle before its rejection propagates, even when no window write existed at all (the old synchronous gate skipped instantly). The wait is bounded by writes already in the serial queue and is the option #10297's triage explicitly endorsed — just worth knowing it applies to the common no-window-write path too, not only the racy one. Since the last pass, two independent reviews landed on this head and found the same: CanReader stress-tested the gate looking for a spurious-fire interleaving (a sibling's compensating write dragging the committed watermark above another member's push watermark) and concluded it cannot happen — a committed write above a member's watermark must have snapshotted after that member's push, so it genuinely needs the repair; and the E2E review found no Critical. sequenceDiagram
participant P1 as spawnTeammate
participant P2 as write queue (serial)
participant P3 as compensating task
P1->>P1: push member, capture watermark N
P1->>P2: window write seq N+1 starts
Note over P2: resolves (commit) or rejects (nothing persisted)
P1->>P1: spawn fails, member rolled back
P1->>P2: queue compensating write (onlyIfCommittedAfter N)
P2->>P3: runs after every earlier write settled
alt committed watermark above N
P3->>P3: write post-rollback roster
else no window write committed
P3->>P3: skip, no write, no notice
end
Test evidence (PR's own CI, fetched via API — no PR code executed)CI on the reviewed head
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 This change has no user-visible surface (internal gate in team-file persistence), so real-scenario TUI testing is N/A — the unit-level interleaving tests are the right oracle here. Sandboxed verification is already in flight at the time of this re-run (a 中文说明代码审查：先给出独立方案作为基线——保持 本轮重新核对（头部为 一个非阻塞提醒：现在所有失败 spawn 的拒绝都要等队列中的写入落定后才传播，即使窗口内根本没有写入。等待受串行队列限制、且是 #10297 分诊明确认可的方案。 上次审查之后又有两个独立评审落在同一头部并得出相同结论：CanReader 专门构造"兄弟成员的补偿写把提交水位拖过另一成员 push 水位"的误触发交错并确认不可能发生——水位之上的提交写入必然在该成员 push 之后才快照，因此确实需要修复；E2E 评审未发现 Critical。 测试证据：未执行任何 PR 代码，以上为通过 API 读取的 PR 自身 CI。审查头部 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 5/5 — clean across every stage, CI green on the reviewed head; approving now. Stepping back: this is exactly the shape of follow-up you want after a tricky concurrency fix. #10223 landed the minimal #10208 ghost fix; the known wart was split into its own issue (#10297) with the safe-fix scope written out, including why the obvious shortcut (decrement-on-reject) is a trap; and this PR implements precisely that scope — no more, no less. The mechanism is the right one: sequence numbers stay monotonic, a separate watermark tracks what actually committed, and the decision moves to the one point where it is safe — when the queued task runs and every earlier write has settled. And the new gate is strictly more conservative than the old one: it fires only when a window write actually committed, a subset of the old fire set, so the #10208 protection can only be preserved. My independent proposal before reading the diff was the same construction, and I could not find a materially simpler one; the diff carries nothing unrelated, the public surface is untouched, and the comments document the invariants rather than the mechanics. The tests are the strongest part: one pins the fix (exactly one write attempt, no misleading notice — pre-fix code does both), and the five-step interleaving actively guards against the tempting wrong fix. Since the last pass, a human reviewer independently tried to break the watermark argument and could not, and the E2E review found no Critical. If I am maintaining this in six months, the watermark comments and those tests tell me everything. What changed since the deferred approval on 中文说明总体评价：5/5——各阶段都干净，审查头部的 CI 全绿；现在批准。 退一步看：这正是复杂并发修复之后理想的后续形态。#10223 先落地了 #10208 幽灵成员的最小修复；已知瑕疵被拆成独立 issue（#10297），写明了安全修复范围、以及为什么"显而易见"的捷径（拒绝时递减计数）是陷阱；本 PR 精确实现该范围——不多不少。机制正确：序号保持单调，独立水位记录真正提交过的写入，决策移到唯一安全的时点——排队任务运行、所有更早写入落定之时。而且新门控比旧门控更保守：只在窗口内确有写入提交时触发，是旧触发集的子集，#10208 的保护只会被保留。 我在读 diff 之前的独立方案与此相同，也没有找到更简单的构造；diff 无无关改动，公共接口不变，注释记录的是不变量而非机械步骤。测试是最强的部分：一个锁定修复本身（恰好一次写入尝试、无误导性通知——修复前两者都会发生），五步交错测试主动防住那个诱人的错误修法。上次审查之后，人类评审独立尝试推翻水位论证未果，E2E 评审未发现 Critical。半年后维护这段代码时，水位注释和这些测试能说明一切。 自 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 6b": running TeamManager.ghost-member.test.ts — no node_modules in the review worktree; full npm ci + build not attempted within tool budget.
Not reviewed: reverse audit — stopped before round 4 by the review time budget.
Test Plan (not a blocker): src/agents/team/TeamManager.ghost-member.test.ts — no such file or directory.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："agent 6b"：running TeamManager.ghost-member.test.ts — no node_modules in the review worktree; full npm ci + build not attempted within tool budget。
未审查：反向审计——评审时间预算不足，未能开始第 4 轮。
Test Plan（非阻断）：src/agents/team/TeamManager.ghost-member.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| if ( | ||
| options?.onlyIfCommittedAfter !== undefined && | ||
| this.teamFileWritesCommitted <= options.onlyIfCommittedAfter | ||
| ) { |
There was a problem hiding this comment.
[Suggestion] The gate's skip branch is never tested with teamFileWritesCommitted strictly between 0 and the push watermark — a committed write below the watermark plus a rejected write that advanced teamFileWritesStarted past it. Alpha's write seq 1 commits (committed=1); beta's success write seq 2 rejects with ENOSPC and beta rolls back; gamma is pushed (watermark 2) and its spawn fails with no further writes. The correct behaviour is skip (1 ≤ 2), but the one-line mutation <= → === passes all nine existing tests — the new tests pin (0,0)-skip and (2,1)-fire, the pre-existing tests pin (1,1)-skip, (0,0)-skip and (1,0)-fire, so nothing hits this strict-between region — yet the mutant fires the redundant compensating write in this interleaving, and with the disk still full that write rejects and emits the misleading <team_error> ghost-member leader notice, the exact symptom issue 10297 removes.
Witness:
Probe in an isolated scratch tree (strict-between scenario, committed=1, watermark=2):
intact code: writeTeamFile called exactly 2 times, no leader notice
mutant <= -> ===: all 9 existing tests still pass (9/9), but the probe flips:
AssertionError: expected "writeTeamFile" to be called 2 times, but got 3 times
leaderSpy: <team_error>Compensating team-file write after failed spawn of gamma@test-team failed: ENOSPC: no space left on device</team_error>
Suggested fix: add a test in the new describe block for issue 10297 — alpha's write commits normally; arm vi.spyOn(teamHelpers, 'writeTeamFile').mockRejectedValueOnce(new Error('ENOSPC: no space left on device')) so beta's success write rejects and beta's spawn rejects; push gamma and reject gamma's backend gate so gamma fails with no new write; assert the total writeTeamFile call count shows no compensating write ran, leaderSpy was not called, and disk/memory contain only alpha.
That new test doubles as the fix witness: under the <= → === mutation the compensating write fires, so its write-count and leader-notice assertions go red; with the current code they pin the strict-between skip.
中文说明
门控的跳过分支从未测试过 teamFileWritesCommitted 严格介于 0 与 push 水位之间的场景——即水位之下有一笔已提交的写入，同时有一笔被拒的写入把 teamFileWritesStarted 推过了水位。设 alpha 的写入序号 1 提交（committed=1）；beta 的成功写入序号 2 以 ENOSPC 被拒、beta 回滚；gamma 被 push（水位 2）且其 spawn 失败、没有新写入。正确行为是跳过（1 ≤ 2），但单行变异 <= → === 能通过全部 9 个现有测试——新测试钉住 (0,0)-跳过 与 (2,1)-触发，既有测试钉住 (1,1)-跳过、(0,0)-跳过 与 (1,0)-触发，没有任何一组覆盖这个严格介于区间——而该变异体在此交错下会执行那次多余的补偿写，磁盘仍满时写入被拒并发出误导性的 <team_error> 幽灵成员 leader 通知，正是 issue 10297 要消除的症状。
建议修复：在 issue 10297 的新 describe 块中补一个测试——alpha 的写入正常提交；用 vi.spyOn(teamHelpers, 'writeTeamFile').mockRejectedValueOnce(new Error('ENOSPC: no space left on device')) 让 beta 的成功写入被拒、beta 的 spawn 失败；push gamma 并拒绝 gamma 的后端门控使其失败且无新写入；断言 writeTeamFile 总调用次数表明没有补偿写执行、leaderSpy 未被调用、磁盘与内存中只有 alpha。
该新测试同时充当修复见证：在 <= → === 变异下补偿写会触发，其调用次数与通知断言变红；当前代码下它们钉住严格介于区间的跳过。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| // No write that started inside the failed member's window | ||
| // committed, so nothing on disk can contain the rolled-back | ||
| // member — skip the compensating write. | ||
| return; |
There was a problem hiding this comment.
[Suggestion] The gate's skip path returns silently — no debug log, no trail — so a skipped compensating write is indistinguishable in logs from one that never ran. Any time the gate's premise is violated in production (the uid-mismatch in-place write fallback, an external editor, a future refactor that breaks the watermark invariant), a ghost member or corrupt roster resurfaces days later and oncall greps the debug log for the Compensating team-file write trail and finds nothing — "the gate evaluated and skipped" cannot be told apart from "the spawn-failure path never executed". The diff's own comment on the adjacent failure path states the requirement — "leave a trail so a resurfaced ghost member can be told apart from a compensating write that itself failed" — but the skip branch satisfies neither half of that distinction.
Witness:
Probe recording every debugLogger call through the real code:
skip arm: ARM_A_SKIP team-related debug entries: [] leader notices: []
fire-fail arm: warn "Compensating team-file write after failed spawn of beta@test-team failed: ENOSPC: no space left on device"
plus the <team_error> leader notice
| // No write that started inside the failed member's window | |
| // committed, so nothing on disk can contain the rolled-back | |
| // member — skip the compensating write. | |
| return; | |
| // No write that started inside the failed member's window | |
| // committed, so nothing on disk can contain the rolled-back | |
| // member — skip the compensating write. | |
| debug.warn( | |
| `Skipping compensating roster write: no write committed ` + | |
| `above watermark ${options.onlyIfCommittedAfter} ` + | |
| `(committed=${this.teamFileWritesCommitted})`, | |
| ); | |
| return; |
The solo test ('skips the compensating write when the only write in the window rejected (solo)') doubles as the fix witness once it additionally asserts the skip trail is emitted — removing the log line reddens that assertion.
中文说明
门控的跳过路径静默返回——没有 debug 日志、没有任何痕迹——于是被跳过的补偿写在日志中与从未执行过的补偿写无法区分。只要门控前提在生产中被破坏（uid 不一致的原地写回退、外部编辑器、未来某次破坏水位不变量的重构），幽灵成员或损坏的 roster 会在数天后重新浮现，而值班同学 grep debug 日志找 Compensating team-file write 痕迹时什么都找不到——"门控评估过并跳过了"与"spawn 失败路径根本没执行"无法区分。diff 自己在相邻失败路径上的注释说明了这一要求——"留下痕迹，以便重新浮现的幽灵成员能与自身失败的补偿写相区分"——但跳过分支两半都不满足。
建议修复：在跳过分支发出一条 debug.warn，同时带上两个水位（见上方 suggestion 块）。
单独用例（'skips the compensating write when the only write in the window rejected (solo)'）可以在补充断言跳过痕迹被发出后充当修复见证——删掉这行日志会使该断言变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| options?.onlyIfCommittedAfter !== undefined && | ||
| this.teamFileWritesCommitted <= options.onlyIfCommittedAfter |
There was a problem hiding this comment.
[Suggestion] Because the gate reads teamFileWritesCommitted at queued-task run time, it also trips on writes that only started after the failed member's rollback and never snapshotted it — preserving, in one narrower interleaving the pre-PR synchronous check correctly skipped, the redundant compensating write (and its misleading leader notice on failure) this PR exists to remove. Z's final roster write W0 hangs in its fs await (seq 1 assigned); X is pushed (watermark 1), then Y (watermark 1); Y's spawn resolves and its final write T queues behind W0; X's spawn fails, rollback removes X and queues compensating task C behind T; W0 commits (committed=1); T then runs after X's rollback, takes seq 2, snapshots {Z, Y} without X, commits (committed=2). C sees committed 2 > watermark 1 and fires, rewriting state that is already on disk — and if that redundant write itself fails under the same ENOSPC onset issue 10297 targets, the leader receives a ghost-member <team_error> about X despite nothing on disk containing X. Pre-PR, the synchronous teamFileWritesStarted > writesStartedAtPush check ran at rollback, when T had not started, and correctly skipped. No ghost or data loss results — C's snapshot is the authoritative post-rollback state — so this is a residual of the exact symptom, not a correctness regression.
Witness:
Probe in an isolated scratch tree:
PR arm: writeLog: [call#1 members=[zeta], call#2 members=[zeta,beta], call#3 members=[zeta,beta]]
(call#3 is the redundant post-rollback rewrite)
leaderSpy: <team_error>Compensating team-file write after failed spawn of alpha@test-team failed: ENOSPC: no space left on device</team_error>
persisted members: [zeta, beta]
Reverted to the pre-PR synchronous gate:
writeLog: [call#1 members=[zeta], call#2 members=[zeta,beta]] leaderSpy: []
Proportionate fix: extend the gate's doc comment above persistTeamFile to state that commits from writes started after the rollback also trip the gate — conservative over-fire, never a ghost. An exact gate would additionally require knowing which committed writes started at-or-before rollback, which is not derivable from a single high-water mark across rejected-sequence gaps, so the comment is the simpler correctness-preserving option.
Any added bookkeeping must preserve seq monotonicity and the serial-commit order the gate relies on — TeamManager.ts:231-233: "Deliberately monotonic — a rejected write keeps its number so a later write cannot reuse the value and hide at or below an earlier member's push watermark (#10297)".
中文说明
由于门控在队列任务运行时才读取 teamFileWritesCommitted，它也会被那些在失败成员回滚之后才启动、从未快照到该成员的写入触发——在本 PR 要消除的"多余补偿写（及其失败时的误导性通知）"上保留了一个更窄的交错场景，而 PR 前的同步检查在该场景下是正确跳过的。设 Z 的最后一笔 roster 写入 W0 挂在 fs await 中（已分配序号 1）；X 被 push（水位 1），随后 Y（水位 1）；Y 的 spawn 成功、其最终写入 T 排在 W0 之后；X 的 spawn 失败，回滚移除 X 并把补偿任务 C 排在 T 之后；W0 提交（committed=1）；T 在 X 回滚之后运行，取序号 2，快照 {Z, Y}（不含 X）并提交（committed=2）。C 看到 committed 2 > 水位 1 便触发，重写一份已在磁盘上的状态——若这次多余写入在 issue 10297 针对的同一次 ENOSPC 下失败，leader 会收到一条关于 X 的幽灵成员 <team_error>，而磁盘上根本没有任何包含 X 的内容。PR 前同步的 teamFileWritesStarted > writesStartedAtPush 检查在回滚时执行，当时 T 尚未启动，因此正确跳过。不会产生幽灵或数据丢失——C 的快照即回滚后的权威状态——所以这是同一症状的残留，而非正确性回归。
建议修复（相称的修法）：把 persistTeamFile 上方的门控文档注释扩展为"回滚之后才启动的写入若有提交同样会触发门控——保守的过度触发，绝不产生幽灵"。精确门控还需要知道哪些已提交的写入是在回滚时或之前启动的，这在单一高水位下无法跨越被拒序号的间隙推导出来，因此注释是更简单且不损害正确性的选项。
任何新增记账都必须保持序号单调与门控依赖的串行提交顺序——TeamManager.ts:231-233："Deliberately monotonic — a rejected write keeps its number so a later write cannot reuse the value and hide at or below an earlier member's push watermark (#10297)"。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| try { | ||
| await this.persistTeamFile({ | ||
| onlyIfCommittedAfter: writesStartedAtPush, | ||
| }); | ||
| } catch (writeErr) { |
There was a problem hiding this comment.
[Suggestion] Moving the skip/fire decision into the serialized write queue makes a failed spawn's rejection block behind unrelated pre-push roster writes that never settle — where the pre-PR synchronous gate rejected immediately — so the spawn error can hang indefinitely behind a wedged write. A success-path write that started before the failed member's push hangs without settling (writeTeamFile is await fs.mkdir + atomic rename with no timeout anywhere on the write path; an unresponsive NFS/FUSE mount or wedged disk can hang it indefinitely). A teammate spawn then fails; the member is already rolled back and the gate's outcome is a foregone skip — nothing committed in the window — but the gate task chains behind the hung write, so spawnTeammate never reaches throw err and the leader's spawn tool call hangs indefinitely instead of surfacing the spawn error. Pre-PR the synchronous teamFileWritesStarted > writesStartedAtPush check was false in this exact interleaving (the only started write is pre-push, so started == writesStartedAtPush), nothing was queued, and the rejection was immediate. The PR's endorsed "await in-flight writes" tradeoff bounds the count of queued writes, not wall-clock, when one of them never settles. Two mitigating facts keep this at Suggestion: the hang class pre-existed in the sibling in-window interleaving pre-PR (the diff widens an existing failure class rather than adding a new one), and no wrong state results — the skip is the correct outcome once the write settles.
Witness:
Probe in an isolated scratch tree:
PR arm: spawnX outcome while W0 hung: STILL_PENDING_AFTER_1500MS
writeCalls at race time: 1; after release+drain: 1 (the foregone skip materialized)
Reverted to the pre-PR synchronous gate:
spawnX outcome while W0 hung: REJECTED: spawn failed
Suggested fix: don't let the decision wait block error propagation — race the await against a generous timeout and proceed to throw err on expiry, leaving the gate task queued (the serial queue still lands or skips the repair in order), and move the warn/leader-notice handling into the task's settlement path since catch (writeErr) no longer observes it:
await Promise.race([
this.persistTeamFile({ onlyIfCommittedAfter: writesStartedAtPush }),
new Promise((r) => setTimeout(r, COMPENSATING_WRITE_SETTLE_TIMEOUT_MS)),
]);The fix must keep the compensating task queued — never dropped — so a late-landing repair still lands last, and the existing tests that read the disk immediately after spawnTeammate rejects pin that in the normal fast case the repair settles before the rejection (teamFileWriteQueue doc comment, TeamManager.ts:209-220). A new test can pin the fix: hold write 1 on a gate that is never released, push beta after it starts, reject beta's spawn, and assert spawnTeammate settles with the spawn error within a bounded time — removing the timeout race from the fix makes that test hang/red.
中文说明
把跳过/触发决定移入串行写队列后，失败 spawn 的 reject 会被阻塞在那些永不落定的 push 前 roster 写入之后——而 PR 前的同步门控在这种交错下会立即 reject——于是 spawn 错误可能被一次卡死的写入无限期挂起。设一笔在失败成员 push 之前启动的成功路径写入挂起不落定（writeTeamFile 是 await fs.mkdir + 原子 rename，整条写路径没有任何超时；无响应的 NFS/FUSE 挂载或卡死的磁盘可以无限期挂住它）。随后某个 teammate 的 spawn 失败；成员已经回滚、门控结果是注定的跳过——窗口内没有任何提交——但门控任务排在那笔挂起写入之后，spawnTeammate 永远到不了 throw err，leader 的 spawn 工具调用被无限期挂起，而不是得到 spawn 错误。PR 前同步的 teamFileWritesStarted > writesStartedAtPush 检查在这个交错下为假（唯一启动的写入在 push 之前，started == writesStartedAtPush），不入队任何东西，reject 立即发生。PR 所采纳的"等待在途写入"权衡约束的是已入队写入的数量，而非墙钟时间——当其中一笔永不落定时。两个减轻因素使其保持 Suggestion：该挂起类别在 PR 前已存在于同族的窗口内交错中（本 diff 扩大了一个既有失败类别而非新增），且不会产生错误状态——写入落定后跳过就是正确结果。
建议修复：不要让决定等待阻塞错误传播——将 await 与一个宽裕的超时竞速，超时后即继续 throw err，让门控任务继续留在队列中（串行队列仍会按序落定或跳过修复），并把 warn/leader 通知处理移入任务的落定路径，因为 catch (writeErr) 不再能观察到它（代码示意见上）。
修复必须让补偿任务保持在队列中——绝不丢弃——以保证迟到的修复仍然最后落盘；既有测试在 spawnTeammate reject 后立即读盘，钉住了正常情况下修复先于 reject 落定（teamFileWriteQueue 文档注释，TeamManager.ts:209-220）。可以用一个新测试钉住该修复：把写入 1 挂在一个永不释放的门控上，在其启动后 push beta，拒绝 beta 的 spawn，断言 spawnTeammate 在有限时间内带着 spawn 错误落定——把超时竞速从修复中移除会使该测试挂起/变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
CanReader
left a comment
CanReader
left a comment
There was a problem hiding this comment.
Spent most of the review on the gate itself, since a counter change in a serial write queue is easy to get subtly wrong. I could not break it.
The specific interleaving I tried to construct was: member Z's window legitimately has no committed write, but some other failed member's compensating write commits and drags teamFileWritesCommitted above Z's watermark, so Z's repair fires spuriously and serialises a live roster containing a sibling whose spawn is still pending — the exact harm the old comment says the gate exists to prevent. It cannot happen. For a committed write to carry a sequence number above Z's watermark it has to have taken its snapshot after Z's push, which means the snapshot contains Z, which means Z genuinely needs repairing. The watermark comparison is doing precisely the right thing, and the "monotonic, a rejected write keeps its number" note in the teamFileWritesStarted doc is what makes it hold.
Splitting started-vs-committed is also the right correction to the original gate. The old teamFileWritesStarted > writesStartedAtPush test could only ask whether a write ran, and a write that ran and rejected persisted nothing — so the solo disk-full case was doing a redundant second write and, when that failed too, reporting a ghost-member notice that told the operator the opposite of what happened. The first new test pins exactly that (one write attempt, no leader message), which is the behaviour change that matters.
One suggestion. The commit point does this.teamFileWritesCommitted = writeSeq, and the doc comment above the field asserts the watermark is monotonic — but that is an invariant of the queue being serial, not something the assignment enforces. Math.max(this.teamFileWritesCommitted, writeSeq) would make it structural, so a future change that lets two writes overlap (or an added retry that resolves out of order) degrades into a redundant compensating write rather than a silently skipped repair, which is the direction you want this to fail. Cheap insurance for a gate whose failure mode is a ghost member on disk.
Nit: the five-step test leans on two setTimeout(50) sleeps to sequence the interleaving. vi.waitFor is already used for the write-count checkpoint; using it for the push ordering too would make the test less dependent on timing under a loaded CI runner.
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 6b": executing packages/core/src/agents/team/TeamManager.ghost-member.test.ts — blocked by the shared worktree's build prerequisite ( packages/core/dist ) being re….
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Test Plan (not a blocker): src/agents/team/TeamManager.ghost-member.test.ts — no such file or directory.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/core/src/agents/team/TeamManager.ghost-member.test.ts:531 — [review] D2-1 harness boilerplate duplicated verbatim across the two describe blocks
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："agent 6b"：executing packages/core/src/agents/team/TeamManager.ghost-member.test.ts — blocked by the shared worktree's build prerequisite ( packages/core/dist ) being re…。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
Test Plan（非阻断）：src/agents/team/TeamManager.ghost-member.test.ts — no such file or directory。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| this.teamFileWritesCommitted <= options.onlyIfCommittedAfter | ||
| ) { |
There was a problem hiding this comment.
[Suggestion] R1-1: Still stands — round 2 re-verified this with an independent re-derivation and an executed mutation. The gate's skip branch is never tested with teamFileWritesCommitted strictly between 0 and the push watermark — a committed write below the watermark plus a rejected write that advanced teamFileWritesStarted past it. Alpha's write seq 1 commits (committed=1); beta's success write seq 2 rejects with ENOSPC and beta rolls back; gamma is pushed (watermark 2) and its spawn fails with no further writes. The correct behaviour is skip (1 ≤ 2), but the one-line mutation <= → === on the anchored comparison passes all nine existing tests, and the mutant then fires the redundant compensating write in this interleaving — with the disk still full that write rejects and emits the misleading <team_error> ghost-member leader notice, the exact symptom #10297 removes. A future edit to this comparison could regress the fix with the whole suite green.
Witness:
Round-2 mutation run (isolated scratch tree, TeamManager.ghost-member.test.ts):
BASE (<=): 9 passed
MUT (===): 9 passed <- mutant survives
MUT (<): 3 failed | 6 passed <- comparator live, so survival is real
with the proposed strict-between test added:
BASE (<=): 10 passed
MUT (===): 1 failed | 9 passed - AssertionError: expected 3 to be 2
(the mutant runs the redundant compensating write)
Suggested fix: add one test to the #10297 describe block — alpha's write commits normally; arm vi.spyOn(teamHelpers, 'writeTeamFile').mockRejectedValueOnce(new Error('ENOSPC: no space left on device')) so beta's success write rejects and beta's spawn rejects; push gamma and reject gamma's backend gate so gamma fails with no new write; assert the total writeTeamFile call count shows no compensating write ran and leaderSpy was not called.
Fix witness: that new strict-between test is the acceptance criterion — mutate <= to === and run it; it must go red, proving it pins the comparison the suite currently leaves unguarded.
中文说明
仍然成立——第二轮通过独立重新推导与执行变异重新验证了该结论。门控的跳过分支从未测试过 teamFileWritesCommitted 严格介于 0 与 push 水位之间的场景——即水位之下有一笔已提交的写入、同时有一笔被拒写入把 teamFileWritesStarted 推过水位。设 alpha 的写入序号 1 提交（committed=1）；beta 的成功写入序号 2 以 ENOSPC 被拒并回滚；gamma 被 push（水位 2）且其 spawn 失败、无后续写入。正确行为是跳过（1 ≤ 2），但把锚定比较单行变异为 <= → === 能通过全部 9 个现有测试，而该变异体在此交错下会执行多余的补偿写——磁盘仍满时该写入被拒并发出误导性的 <team_error> 幽灵成员 leader 通知，正是 #10297 要消除的症状。未来对这处比较的改动可能在整个套件全绿的情况下让修复回归。
建议修复：在 #10297 describe 块中补一个测试——alpha 的写入正常提交；用 vi.spyOn(teamHelpers, 'writeTeamFile').mockRejectedValueOnce(new Error('ENOSPC: no space left on device')) 让 beta 的成功写入被拒、spawn 失败；push gamma 并拒绝其后端门控使其失败且无新写入；断言 writeTeamFile 总调用次数表明没有补偿写执行且 leaderSpy 未被调用。
修复见证：该新的严格介于测试即验收标准——把 <= 变异为 === 后运行它必须变红，证明它钉住了当前套件未设防的比较。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| // No write that started inside the failed member's window | ||
| // committed, so nothing on disk can contain the rolled-back | ||
| // member — skip the compensating write. | ||
| return; |
There was a problem hiding this comment.
[Suggestion] R1-2: Still stands — re-verified at this commit. The gate's skip path returns silently — no debug log, no trail — so a skipped compensating write is indistinguishable in logs from one that never ran. The only observability on this path is the debug.warn and the leader <team_error> notice, and both sit inside the compensating-write failure catch — unreachable when the gate skips. When a ghost member surfaces in config.json after a failed spawn (the exact #10208/#10297 bug family), the oncall greps debug output and finds nothing for the skipped case — identical to the catch path never having run at all — so validating or falsifying the gate in production requires reproducing the exact interleaving blind. The adjacent failure branch already leaves a trail precisely so a resurfaced ghost member can be told apart from a compensating write that itself failed; the new skip branch is the one outcome that trail cannot now distinguish.
Witness:
Read-based re-check at HEAD 176ed09b:
skip branch (TeamManager.ts:376-380) contains no debug.* call;
the only debug.warn on the path is at :736 and the only leaderMessageCallback at :745
— both inside catch (writeErr), unreachable when the gate skips.
Suggested fix: log the skip before returning via the existing debug logger, naming both watermark values, e.g. Skipping compensating team-file write after failed spawn: committed watermark ${this.teamFileWritesCommitted} did not pass push watermark ${options.onlyIfCommittedAfter}.
Fix witness: add a debug-logger spy assertion to the solo skip test asserting the skip message and both watermark values — deleting the log line must turn that test red.
中文说明
仍然成立——已在该提交上重新验证。门控的跳过路径静默返回——没有 debug 日志、没有任何痕迹——因此被跳过的补偿写在日志中与从未运行过无法区分。该路径上唯一的可观测性是 debug.warn 与 leader <team_error> 通知，而两者都在补偿写失败的 catch 内——门控跳过时不可达。当失败 spawn 后 config.json 中出现幽灵成员（正是 #10208/#10297 一族缺陷）时，oncall 检索 debug 输出，对"被跳过"这种情形什么都找不到——与 catch 路径从未执行完全一样——于是在生产中验证或证伪该门控只能盲目复现那个精确交错。相邻的失败分支特意留下痕迹，正是为了让重新出现的幽灵成员能与"补偿写本身失败"区分开；新的跳过分支是这条痕迹目前无法区分的唯一结果。
建议修复：在返回前用现有 debug 日志器记录跳过，并给出两个水位值，例如 Skipping compensating team-file write after failed spawn: committed watermark ${this.teamFileWritesCommitted} did not pass push watermark ${options.onlyIfCommittedAfter}。
修复见证：在单独跳过用例中加一个 debug-logger spy 断言，断言跳过消息与两个水位值——删除该日志行必须使该测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| options?.onlyIfCommittedAfter !== undefined && | ||
| this.teamFileWritesCommitted <= options.onlyIfCommittedAfter |
There was a problem hiding this comment.
[Suggestion] R1-3: Still stands — independently re-derived by this round's reverse audit and re-executed as a flip probe at this commit. Because the gate reads teamFileWritesCommitted at queued-task run time, it also trips on writes that only drained after the failed member's rollback and never snapshotted it — preserving, in one narrower interleaving the pre-PR synchronous check correctly skipped, the redundant compensating write (and its misleading leader notice on failure) this PR exists to remove. Concretely: K's write V is in flight; L's successful spawn queues write W behind V; M is pushed (watermark N = V's seq) and M's spawn fails fast — rollback removes M and queues compensating task C_M with onlyIfCommittedAfter: N. V settles after the rollback; W drains next, takes seq N+1, snapshots the post-rollback roster (M already removed), and commits. C_M then sees N+1 > N and fires, rewriting state that is already on disk — and if that redundant write rejects under the same degraded-disk onset #10297 targets, the leader receives a ghost-member <team_error> about M despite nothing on disk containing M. No ghost or data loss results — C_M's snapshot is the authoritative post-rollback state — so this is a residual of the exact symptom, not a correctness regression.
Witness:
Round-2 flip probe (isolated scratch tree):
PR arm: writeLog=["#1[alpha]","#2[alpha,beta]","#3[alpha,beta]"]
notices=["<team_error>Compensating team-file write after failed spawn of gamma@test-team failed: ENOSPC..."]
persisted=["alpha","beta"] (#3 is the redundant compensating write)
Reverted to the pre-PR synchronous gate:
writeLog=["#1[alpha]","#2[alpha,beta]"] notices=[]
Suggested fix: proportionate fix (round 1): extend the gate's doc comment above persistTeamFile to state that commits from writes started after the rollback also trip the gate — conservative over-fire, never a ghost. Alternative exact fix (round-2 auditor): sample teamFileWritesStarted in the catch block after rollback() and advance a second watermark at the commit point only for writes at-or-below that sample, comparing the gate against it. Any added bookkeeping must preserve seq monotonicity and the serial-commit order the gate relies on — TeamManager.ts:231-233 ("Deliberately monotonic — a rejected write keeps its number so a later write cannot reuse the value and hide at or below an earlier member's push watermark (#10297)").
Fix witness: a new test that holds an initial write in flight, queues a second write behind it, pushes a member whose spawn fails immediately, releases the in-flight write only after the rollback settles, and asserts the compensating write did not run — it goes red on the current gate, and any exact fix must keep it green alongside the existing five-step interleaving test (persistedNames must not contain 'beta').
中文说明
仍然成立——本轮反向审计独立重新推导了该交错，并在该提交上以翻转探针重新执行验证。由于门控在队列任务运行时才读取 teamFileWritesCommitted，它也会被那些在失败成员回滚之后才流出、从未快照到该成员的写入触发——在一个更窄的交错下保留了本 PR 要消除的"多余补偿写（及其失败时的误导性通知）"，而 PR 前的同步检查在该交错下是正确跳过的。具体：K 的写入 V 在途；L 的成功 spawn 把写入 W 排在 V 之后；M 被 push（水位 N = V 的序号）后 spawn 快速失败——回滚移除 M 并把补偿任务 C_M 以 onlyIfCommittedAfter: N 入队。V 在回滚后落定；W 随后流出，取序号 N+1，快照回滚后的名单（M 已移除）并提交。C_M 看到 N+1 > N 便触发，重写一份已在磁盘上的状态——若这次多余写入在 #10297 针对的同一次磁盘劣化下失败，leader 会收到一条关于 M 的幽灵成员 <team_error>，而磁盘上根本没有任何包含 M 的内容。不会产生幽灵或数据丢失——C_M 的快照即回滚后的权威状态——所以这是同一症状的残留，而非正确性回归。
建议修复（相称修法，第一轮）：把 persistTeamFile 上方的门控文档注释扩展为"回滚之后才启动的写入若有提交同样会触发门控——保守的过度触发，绝不产生幽灵"。替代的精确修法（第二轮审计）：在 catch 块 rollback() 之后采样 teamFileWritesStarted，提交点仅为不超过该采样的写入推进第二个水位，门控与该水位比较。任何新增记账都必须保持序号单调与门控依赖的串行提交顺序——TeamManager.ts:231-233（"刻意单调——被拒写入保留其序号，后续写入无法复用该值藏到更早成员的水位之下（#10297）"）。
修复见证：新增测试——挂起一笔初始写入、在其后入队第二笔、push 一个立即 spawn 失败的成员、仅在回滚落定后释放初始写入，断言补偿写未执行——该测试在当前门控下变红；任何精确修法必须与既有五步交错测试（persistedNames 不含 'beta'）一起保持其绿色。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| await this.persistTeamFile({ | ||
| onlyIfCommittedAfter: writesStartedAtPush, | ||
| }); | ||
| } catch (writeErr) { |
There was a problem hiding this comment.
[Suggestion] R1-4: Still stands — the code here is byte-identical to round 1, and this round's performance walk independently re-derived the behaviour. Moving the skip/fire decision into the serialized write queue makes a failed spawn's rejection block behind unrelated pre-push roster writes that never settle — where the pre-PR synchronous gate rejected immediately — so the spawn error can hang indefinitely behind a wedged write. A success-path write that started before the failed member's push hangs without settling (writeTeamFile is await fs.mkdir + atomic rename with no timeout anywhere on the write path; an unresponsive NFS/FUSE mount or wedged disk can hang it indefinitely). A teammate spawn then fails; the member is already rolled back and the gate's outcome is a foregone skip — nothing committed in the window — but the gate task chains behind the hung write, so spawnTeammate never reaches throw err and the leader's spawn tool call hangs indefinitely instead of surfacing the spawn error. Pre-PR the synchronous check was false in this exact interleaving and the rejection was immediate. The endorsed "await in-flight writes" tradeoff bounds the count of queued writes, not wall-clock, when one of them never settles. Two mitigating facts keep this at Suggestion: the hang class pre-existed in the sibling in-window interleaving pre-PR (the diff widens an existing failure class rather than adding a new one), and no wrong state results — the skip is the correct outcome once the write settles.
Witness:
Round-1 probe (isolated scratch tree):
PR arm: spawnX outcome while W0 hung: STILL_PENDING_AFTER_1500MS
writeCalls at race time: 1; after release+drain: 1 (foregone skip materialized)
Reverted to the pre-PR synchronous gate:
spawnX outcome while W0 hung: REJECTED: spawn failed
Round-2: empty git diff e11d980..HEAD on both changed files (byte-identical).
Suggested fix: don't let the decision wait block error propagation — race the await against a generous timeout and proceed to throw err on expiry, leaving the gate task queued (the serial queue still lands or skips the repair in order), and move the warn/leader-notice handling into the task's settlement path since catch (writeErr) no longer observes it. The fix must keep the compensating task queued — never dropped — so a late-landing repair still lands last; the existing tests that read the disk immediately after spawnTeammate rejects pin that in the normal fast case the repair settles before the rejection (teamFileWriteQueue doc comment, TeamManager.ts:209-220).
Fix witness: a new test that holds write 1 on a gate that is never released, pushes beta after it starts, rejects beta's spawn, and asserts spawnTeammate settles with the spawn error within a bounded time — removing the timeout race from the fix makes that test hang/go red.
中文说明
仍然成立——该行代码与第一轮字节一致，本轮性能走查独立重新推导了该行为。把跳过/触发决定移入串行写队列后，失败 spawn 的 reject 会被阻塞在那些永不落定的 push 前 roster 写入之后——而 PR 前的同步门控在这种交错下会立即 reject——于是 spawn 错误可能被一次卡死的写入无限期挂起。设一笔在失败成员 push 之前启动的成功路径写入挂起不落定（writeTeamFile 是 await fs.mkdir + 原子 rename，整条写路径没有任何超时；无响应的 NFS/FUSE 挂载或卡死的磁盘可以无限期挂住它）。随后某个 teammate 的 spawn 失败；成员已经回滚、门控结果是注定的跳过——窗口内没有任何提交——但门控任务排在那笔挂起写入之后，spawnTeammate 永远到不了 throw err，leader 的 spawn 工具调用被无限期挂起，而不是得到 spawn 错误。PR 前同步检查在这个交错下为假，reject 立即发生。所采纳的"等待在途写入"权衡约束的是已入队写入的数量而非墙钟时间——当其中一笔永不落定时。两个减轻因素使其保持 Suggestion：该挂起类别在 PR 前已存在于同族的窗口内交错中（本 diff 扩大了一个既有失败类别而非新增），且不会产生错误状态——写入落定后跳过就是正确结果。
建议修复：不要让决定等待阻塞错误传播——将 await 与一个宽裕的超时竞速，超时后即继续 throw err，让门控任务留在队列中（串行队列仍会按序落定或跳过修复），并把 warn/leader 通知处理移入任务的落定路径，因为 catch (writeErr) 不再观察到它。修复必须让补偿任务保持在队列中——绝不丢弃——以保证迟到的修复仍然最后落盘；既有测试在 spawnTeammate reject 后立即读盘，钉住了正常情况下修复先于 reject 落定（teamFileWriteQueue 文档注释，TeamManager.ts:209-220）。
修复见证：新增测试——把写入 1 挂在永不释放的门控上，在其启动后 push beta，拒绝 beta 的 spawn，断言 spawnTeammate 在有限时间内带着 spawn 错误落定——把超时竞速从修复中移除会使该测试挂起/变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
E2E review report (no Critical found) — head
|
| Probe | Result |
|---|---|
TeamCreate e2eteam → spawn teammate alpha (write hello.txt) |
✅ "Teammate alpha is now running concurrently"; alpha tab appears in the tab bar |
| Teammate work + report | ✅ alpha created the file, reported via send_message; leader independently verified content (13 bytes, exact) |
RequestShutdown → TeamDelete cleanup |
✅ both ran; team dir removed with the delete |
Second team rostercheck kept alive for inspection |
✅ config.json persisted with exactly one member (beta@rostercheck, full record: agentId/prompt/joinedAt/cwd/backendType) — no ghost entries; leader tracked via leadAgentId/leadSessionId/leadPid |
/quit on the leader |
✅ clean exit; --resume UUID matches the roster's leadSessionId; no orphaned teammate/CLI processes afterwards |
CI at head
All product lanes green on 176ed09b61: Test (ubuntu), Integration Tests (no-AK, No Sandbox), web-shell E2E Smoke, Desktop Shell (ubuntu/windows), review-pr, security/secret scans (16 success / 65 skipped / 0 failure).
Verdict
No Critical; the fix works end-to-end in the shipped bundle and its tests discriminate both directions. Not approving from this account — no formal APPROVED review is on record (the bot's Stage-3 5/5 verdict was deferred pending CI green on the pre-merge sha, and CI is green only now on the merge head); a /triage re-run or maintainer sign-off should land it. The open R1-* Suggestions remain for the author/maintainer to weigh.
— automated e2e pass by qqqys (tmux + real model on the built head); no code changes.
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 371 passed · 0 failed · 371 total Flakiness gate: ✅ 1 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：371 通过 · 0 失败 · 371 总计 抖动门：✅ 1 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #10522 Deep Verification ReportVerdict: 中文摘要
Central claim and A/BCentral claim: the failed-spawn compensating-write gate becomes commit-aware — when the only roster write inside the failed member's window rejected (persisted nothing), the compensating write is skipped (exactly one write attempt, no Secondary claims: (a) the monotonic sequence numbering rules out the decrement-on-reject naive fix (the issue's five-step interleaving); (b) the seven existing #10208 interleaving tests pass unchanged. The PR's invariant (ghost on disk ⇒ a write with seq > push-watermark committed ⇒
Witnesses: The base-cell red is the expected control failure — the predicted test ( CorrectionsNone (first verification round; no earlier claims to correct). FindingsNo blockers. Two design-boundary notes, both measured: 1. (Note) Residual false-positive fire when a post-rollback write commits — inherent to the watermark design, harmlessProbe S2 (not in the shipped suite): alpha's write hangs in flight; beta is pushed (watermark 1) and then fails and rolls back while nothing has committed; gamma's write then commits a snapshot taken after the rollback (
Repro: 2. (Note) Spawn rejection is now delayed until in-flight roster writes settle — accepted tradeoff, observed liveDocumented in the PR body and endorsed by the #10297 triage. My probes observed it directly: on base, Mutation matrix (vacuity + positive control)Run at head in a scratch worktree; unmutated control 9/9 green first (
Zero survivors. Every kill landed on the predicted test(s) — attribution checked per mutant. Two rows carry the PR's own claims:
Sibling probes (novel interleavings beyond the suite)Harness:
S1 is the issue's complaint generalized to N concurrent failures sharing one rejecting write: head eliminates every redundant write while base fires one per failed member. Write-path census: the only roster writers are Targeted gates (at head)
Not covered
MethodologyEnvironment: the CI verify container ( Flakiness gate logEvidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Real-environment verification at head
|
| Head verified | 176ed09b61 (e11d980ac2 + merge of main); unchanged during the run |
| Platform | macOS 15 (Darwin 25.6.0), Node v24.18.1, npm ci + npm run build + npm run bundle |
| Disk-full source | 64 MiB HFS+ RAM disk (hdiutil + diskutil) filled to 0 free blocks, QWEN_HOME on it |
| Instrumentation | DYLD_INSERT_LIBRARIES interpose on open/openat/rename — counts and, where stated, delays or fails syscalls. The code under test is unmodified. |
| A/B method | same tree, git apply -R of the PR's TeamManager.ts hunks, rebuild packages/core (and re-bundle for the TUI arm) |
1. End-to-end through the bundled CLI — the leader really does get the bogus notice
dist/cli.js in a real PTY, scripted OpenAI-compatible server (team_create, then agent with a teammate name), QWEN_CODE_ENABLE_AGENT_TEAM=1, --approval-mode yolo, hermetic HOME/QWEN_HOME. Every atomic roster write (config.json.<hex>.tmp) is failed with a real ENOSPC at open(2) — the "disk filled up after the team was created" case from #10297. team_create itself uses flag: 'wx' (no temp file), so the team is created and only the teammate spawn hits the full disk.
BEFORE — one extra line on screen, and a <team_error> about a member that cannot exist:
AFTER — the spawn error alone, carrying the real cause:
| BEFORE | AFTER | |
|---|---|---|
| roster-write attempts (syscall count) | 2, both errno=28 |
1, errno=28 |
| extra TUI line | ● Team roster write after failed spawn of "alpha" failed |
— |
<team_error> reaches the model |
yes — injected as a user message, present in requests #4 and #5 |
no |
| persisted roster | empty | empty |
The notice is not just cosmetic: it lands in the leader's history as a user-role turn and is re-sent on every subsequent request, so the model keeps reasoning about a ghost member that never existed, on top of the spawn error that already says "no space left on device".
2. Real ENOSPC, no mocks, syscall-level A/B
Same behaviour in-process against the built packages/core, with the roster on a truly full volume: real writeTeamFile → real atomicWriteJSON → real kernel ENOSPC.
3. Mutation matrix — 11 of 12 killed, and the survivor is exactly R1-1
Each mutant applied to the head source, TeamManager.ghost-member.test.ts (9 tests) run, file restored from a byte copy.
Both directions of the fix are pinned, including the trap the issue calls out (M5, decrement-on-reject, killed by the five-step test) and the onlyIfCommittedAfter: 0 handling (M4). M12 survives: a mutant that behaves correctly everywhere except when 0 < teamFileWritesCommitted < watermark, where it writes instead of skipping, passes 9/9. That is R1-1, reproduced as an executed mutant rather than an argument.
4. Concurrency rigs on a real filesystem — the four open Suggestions, measured
Real TeamManager, real serial write queue, real atomic temp+rename; only the agent backend is a double, and write timing / transient ENOSPC are injected at open(2).
- repair direction — no regression. Beta's spawn hangs, gamma's write commits a snapshot that still contains beta (disk shows
[beta, gamma]), beta then fails: both arms end at[gamma]. - R1-1 — real gap, correct behaviour. Rig
r11drives0 < committed(1) < watermark(2)with a syscall-injectedENOSPCon beta's write. Head skips correctly (2 write attempts, 0 notices); the surviving mutantM12writes (3 attempts). One test in this shape closes the gap. - R1-3 — real, one redundant write, same bytes. Rig
r13: alpha's write is in flight when Z is pushed (so BEFORE's synchronous check skips), gamma's write is queued before Z's catch and commits after Z's rollback. BEFORE: 2 roster writes. AFTER: 3. The extra write persists exactly what gamma just committed —[alpha, gamma]on disk in both arms. The window where it could serialize a still-pending sibling is the ~6 ms between two back-to-back queue tasks measured here, and such a sibling's own compensating write would then repair it. - R1-4 — real, quantified. Rig
r14wedges a roster write for 6 s insideopen(2). Z's spawn rejection: 30 ms BEFORE, 5993 ms AFTER. Bounded by writes already queued, and a wedged roster write already stalls every later write today — but the number is worth having on record. - R1-2 — confirmed by construction. The skip path is a bare
return;. In the AFTER arm of §1 and §2 there is no debug line, no notice, nothing at all: a skipped compensating write is indistinguishable from one that never ran.
5. Suites and build at head
TeamManager.ghost-member.test.ts— 9/9; all ofpackages/core/src/agents/team/— 322/322 (13 files).npm ci+npm run build(tsc --build) +npm run bundle— green.- CI on
176ed09b61— all product lanes green; bot review isAPPROVEDas of 2026-09-01.
Verdict
No Critical. Good to merge as a reference for this reviewer. The mechanism does what the PR says on a real filesystem and in the shipped bundle: it removes a user-visible, model-visible spurious <team_error> on a full disk, halves the write attempts in that path, and does not regress the repair direction that #10208 exists for.
Non-blocking follow-ups, in the order I'd rank them:
- One test for the
0 < committed < watermarkskip (killsM12, closes R1-1). - A one-line
debug.debugon the skip branch (R1-2) — the cheapest way to keep this path diagnosable. - R1-3 and R1-4 are inherent to deciding inside the queue, which is the option team: make the failed-spawn compensating-write gate commit-aware (avoid redundant write when the only window write rejected) #10297 endorsed; both fail on the safe side.
Reproduce
# same tree, both arms
git fetch origin pull/10522/head && git checkout 176ed09b61
npm ci && npm run build && npm run bundle
# AFTER arm as-is; BEFORE arm:
git show e11d980ac2 -- packages/core/src/agents/team/TeamManager.ts | git apply -R
npm run build --workspace=packages/core # + npm run bundle for the TUI armFull-disk volume: DEV=$(hdiutil attach -nomount ram://131072 | awk '{print $1}'); diskutil erasevolume "Case-sensitive HFS+" QWENTINY "$DEV", then fill it and point QWEN_HOME inside. The syscall probe is a ~120-line DYLD_INTERPOSE dylib over open/openat/rename with optional per-Nth delay and ENOSPC injection; the TUI arm uses the repo's own integration-tests/terminal-capture + fake-openai-server.
Caveats: macOS only (no Windows/Linux arm here). The agent backend is FakeBackend, so a teammate process is not actually started — the roster-write path under test is fully real. Timing and transient faults in §4 are injected at the syscall boundary, not by patching the code under test.
中文说明
在 head 176ed09b61 上的真实环境验证 —— 合并参考
在 macOS 本地验证，同一棵树内回退生产 hunk 作为 BEFORE 臂，两臂共用同一个 checkout、同一份 node_modules、同一条构建流水线。这份报告与 @qqqys 早前的 e2e 互补，方法不同：roster 写入是被真正写满的文件系统弄失败的（并发场景里则是在 open(2) 边界注入故障），全程没有 vi.mock。同时对四条未决的 R1-* Suggestion 给出有证据的判断。
| 验证的 head | 176ed09b61（e11d980ac2 + 合并 main），验证期间未变 |
| 平台 | macOS 15（Darwin 25.6.0）、Node v24.18.1，npm ci + npm run build + npm run bundle |
| 磁盘满的来源 | 64 MiB HFS+ 内存盘（hdiutil + diskutil）填到剩余 0 块，QWEN_HOME 放在上面 |
| 插桩方式 | DYLD_INSERT_LIBRARIES interpose open/openat/rename——计数，并按需延迟或让其失败。被测代码本身未做任何修改。 |
| A/B 方法 | 同树 git apply -R PR 对 TeamManager.ts 的 hunk，重建 packages/core（TUI 臂再重新 bundle） |
1. 走打包 CLI 的端到端 —— leader 确实收到了那条假通知
真实 PTY 里跑 dist/cli.js，用脚本化的 OpenAI 兼容服务端（先 team_create，再带 teammate name 的 agent），QWEN_CODE_ENABLE_AGENT_TEAM=1、--approval-mode yolo、隔离的 HOME/QWEN_HOME。所有原子 roster 写入（config.json.<hex>.tmp）都在 open(2) 处被注入真实 ENOSPC——正是 #10297 里「建好团队之后磁盘写满」的情形。team_create 自身用 flag: 'wx'（不走临时文件），所以团队能建起来，只有 teammate spawn 撞上满盘。
修复前：屏幕上多一行，并发出一条关于不可能存在的成员的 <team_error>（见上方第一张图）。
修复后：只剩 spawn 错误本身，且携带真实原因（第二张图）。
| 修复前 | 修复后 | |
|---|---|---|
| roster 写入尝试（syscall 计数） | 2 次，均 errno=28 |
1 次，errno=28 |
| 多出的 TUI 行 | ● Team roster write after failed spawn of "alpha" failed |
无 |
<team_error> 是否进模型 |
是 —— 作为 user 消息注入，出现在第 4、5 次请求 |
否 |
| 落盘 roster | 空 | 空 |
这条通知不只是观感问题：它以 user 轮的形式进入 leader 历史，之后每一轮请求都会带上，模型会持续围绕一个根本不存在的幽灵成员推理——而 spawn 错误本身早就说清楚了「磁盘没空间」。
2. 真 ENOSPC、零 mock、syscall 级 A/B
同样的行为在进程内对构建产物 packages/core 复现：roster 放在真正写满的卷上，真 writeTeamFile → 真 atomicWriteJSON → 内核真 ENOSPC（第三张图）。
3. 变异矩阵 —— 12 个杀掉 11 个，存活的那个正好是 R1-1
每个变异体打到 head 源码上，跑 TeamManager.ghost-member.test.ts（9 个用例），再用字节备份还原（第四张图）。
修复的两个方向都被钉住，包括 issue 明确点名的陷阱（M5 拒绝时递减，被五步交错用例杀掉）和 onlyIfCommittedAfter: 0 的处理（M4）。M12 存活：一个只在 0 < teamFileWritesCommitted < watermark 时写入（其余场景都正确）的变异体，9/9 全绿通过。这就是 R1-1，用一个真跑出来的变异体而不是论证复现出来。
4. 真实文件系统上的并发装置 —— 四条未决 Suggestion，量化
真 TeamManager、真串行写队列、真原子 temp+rename；只有 agent backend 是替身，写入时序与瞬时 ENOSPC 在 open(2) 处注入（第五张图）。
- 修复方向 —— 无回归。 beta 的 spawn 挂住，gamma 的写入提交了仍含 beta 的快照（盘上是
[beta, gamma]），随后 beta 失败：两臂最终都是[gamma]。 - R1-1 —— 缺口真实，行为正确。 装置
r11用注入到 beta 写入上的ENOSPC造出0 < committed(1) < watermark(2)。head 正确跳过（2 次写入尝试、0 条通知）；存活变异体M12会写（3 次）。补一个这种形状的用例即可闭合。 - R1-3 —— 真实，多一次写入，字节相同。 装置
r13：Z 被 push 时 alpha 的写入在途（所以 BEFORE 的同步判断会跳过），gamma 的写入在 Z 的 catch 之前入队、在 Z 回滚之后提交。BEFORE：2 次 roster 写入；AFTER：3 次。多出的那次写的正是 gamma 刚提交的内容——两臂盘上都是[alpha, gamma]。它可能把仍在 pending 的兄弟成员序列化进去的窗口，实测只有两个背靠背队列任务之间的约 6 ms，而那个兄弟自己的补偿写随后也会修复它。 - R1-4 —— 真实，已量化。 装置
r14在open(2)里把一次 roster 写入卡住 6 秒。Z 的 spawn 拒绝耗时：BEFORE 30 ms，AFTER 5993 ms。上界是已入队的写入，且今天一次卡死的 roster 写入本来就会拖住之后所有写入——但这个数字值得留档。 - R1-2 —— 由构造确认。 跳过分支就是一个裸
return;。在 §1、§2 的 AFTER 臂里没有任何 debug 行、没有通知、什么都没有：被跳过的补偿写与「压根没跑过」在日志上无法区分。
5. head 上的测试与构建
TeamManager.ghost-member.test.ts—— 9/9；packages/core/src/agents/team/全部 —— 322/322（13 个文件）。npm ci+npm run build（tsc --build）+npm run bundle—— 全绿。176ed09b61上的 CI —— 产品泳道全绿；机器人评审已于 2026-09-01APPROVED。
结论
没有 Critical，从本次复核看可以合并。 该机制在真实文件系统和打包产物里确实做到了 PR 所述：消除了满盘场景下用户可见、模型也可见的假 <team_error>，把该路径的写入尝试减半，且没有回归 #10208 所依赖的修复方向。
非阻塞的后续，按我的优先级：
- 补一个
0 < committed < watermark的跳过用例（杀掉M12，闭合 R1-1）。 - 在跳过分支加一行
debug.debug（R1-2）——保持该路径可诊断的最低成本做法。 - R1-3、R1-4 是「在队列内做判断」这一选择的固有代价，而这正是 team: make the failed-spawn compensating-write gate commit-aware (avoid redundant write when the only window write rejected) #10297 认可的方案；两者都朝安全侧失败。
复现
命令见上方英文小节。满盘卷：DEV=$(hdiutil attach -nomount ram://131072 | awk '{print $1}'); diskutil erasevolume "Case-sensitive HFS+" QWENTINY "$DEV"，填满后把 QWEN_HOME 指进去。syscall 探针是一个约 120 行、对 open/openat/rename 做 DYLD_INTERPOSE 的 dylib，支持按第 N 次调用延迟或注入 ENOSPC；TUI 臂用的是仓库自带的 integration-tests/terminal-capture + fake-openai-server。
局限： 仅 macOS（没有 Windows/Linux 臂）。agent backend 是 FakeBackend，因此没有真正拉起 teammate 进程——但被测的 roster 写入路径完全是真的。§4 中的时序与瞬时故障是在 syscall 边界注入的，而不是改被测代码。
5 participants
What this PR does
Makes the failed-spawn compensating-write gate commit-aware instead of started-aware.
teamFileWritesStartedbecomes a monotonic per-write sequence number (still assigned synchronously at the snapshot point, never decremented), and a newteamFileWritesCommittedwatermark records the sequence number of the most recently committed write (writeTeamFileresolved). The compensating write is queued withonlyIfCommittedAfterset to the watermark captured at member push; because the write queue is serial, the queued task runs after every earlier write — including any still in flight at gate time — has settled, and it writes only if a write above the push watermark actually committed. The compensating-write failure handling (debug warn + leader<team_error>notice) is unchanged.Why it's needed
Follow-up to #10223 (split out from its review thread). The gate compares
teamFileWritesStartedagainst the value captured at member push, but a write that rejected (atomic temp+rename persisted nothing) is still counted as "started". In the solo case the member's own write starts (counter 0→1) and throws ENOSPC, the catch rolls the member back, the gate reads1 > 0, and a compensating write fires even though nothing was persisted. If the disk is still full that write fails too, emitting a<team_error>notice about a possible ghost member that cannot exist — on top of the spawn error that already carries the real cause. The naive fix (decrement on reject) is ruled out in the issue: the counter is a monotonic high-water mark, and a decrement lets a later write reuse a value and hide below an earlier member's push watermark, re-introducing the #10208 ghost (the issue's five-step A→B→C interleaving).Reviewer Test Plan
How to verify
Run the ghost-member suite in
packages/core:npx vitest run src/agents/team/TeamManager.ghost-member.test.ts. Two new tests land in a#10297describe block: (1) the solo rejected-write repro — the member's own write rejects with ENOSPC and the compensating write would reject too; pre-fix this calledwriteTeamFiletwice and delivered a<team_error>leader notice, post-fix it makes exactly one write attempt and emits no notice (red before the fix, green after); (2) the issue's five-step interleaving — alpha's write hangs then rejects, beta is pushed while it is in flight, gamma's write commits a snapshot that still contains beta, and beta's spawn fails; the compensating write must still fire and remove beta from disk (this test fails under a decrement-on-reject implementation). The seven existing #10208 interleaving tests pass unchanged — the gate still fires when a window write committed (including a write in flight at gate time) and still skips when no write landed at all. Alsonpx tsc --noEmitinpackages/coreand Prettier on the two changed files.Evidence (Before & After)
N/A — internal correctness change with no user-visible UI; verified through the unit tests above. Before the fix the new solo test fails with
expected "writeTeamFile" to be called 1 times, but got 2 times; after the fix the full suite is 9/9 green.Tested on
Environment (optional)
Unit tests only (
vitest run), Node v24.19.0, Linux.Risk & Scope
Linked Issues
Fixes #10297
References #10223, #10208
中文说明
本 PR 做了什么
把失败 spawn 补偿写门控从"已启动感知"改为"已提交感知"。
teamFileWritesStarted变为单调的每写入序号（仍在快照点同步赋值、绝不递减），新增teamFileWritesCommitted水位记录最近一次真正提交成功（writeTeamFileresolve）的写入序号。补偿写入队时带上onlyIfCommittedAfter（member push 时捕获的水位）；由于写队列是串行的，该任务运行时所有更早的写入——包括门控评估时仍在途的写入——都已落定，只有当水位之上的写入确实提交过才会真正写入。补偿写失败的处理（debug warn + leader<team_error>通知）保持不变。为什么需要
这是 #10223 的后续项（从其 review 线程拆出）。原门控把
teamFileWritesStarted与 member push 时捕获的值比较，但被拒绝的写入（原子 temp+rename 实际什么都没落盘）也被计入"已开始"。单独场景下：成员自己的写入启动（计数 0→1）后抛 ENOSPC，catch 回滚成员，门控读到1 > 0，于是执行了一次其实没有任何东西可修复的补偿写；若磁盘仍满，这次写入也会失败，在已经携带真实原因的 spawn 错误之上，又向 leader 发出一条关于不可能存在的幽灵成员的<team_error>通知。issue 已排除朴素修法（拒绝时递减计数）：该计数是单调高水位，递减会让后续写入复用计数值、藏到更早 member 的水位之下，重新引入 #10208 幽灵（issue 中的五步 A→B→C 交错）。审阅者测试计划
如何验证
在
packages/core运行幽灵成员测试套件：npx vitest run src/agents/team/TeamManager.ghost-member.test.ts。新增两个测试位于#10297describe 块：(1) 单独被拒写入复现——成员自己的写入以 ENOSPC 被拒、补偿写也会被拒；修复前writeTeamFile被调用两次且发出<team_error>leader 通知，修复后恰好一次写入尝试且无通知（修复前红、修复后绿）；(2) issue 的五步交错——alpha 的写入挂起后被拒、beta 在其在途时被 push、gamma 的写入提交了仍含 beta 的快照、beta 的 spawn 失败；补偿写必须仍然触发并把 beta 从磁盘移除（该测试在"拒绝时递减"的实现下会失败）。原有 7 个 #10208 交错测试全部不变通过——窗口内有写入提交时门控照发（包括门控评估时仍在途的写入），完全没有写入落盘时照旧跳过。另在packages/core运行npx tsc --noEmit，并对两个改动文件跑 Prettier。证据（修复前后）
N/A——内部正确性修改，无用户可见 UI；通过上述单元测试验证。修复前新增的单独用例以
expected "writeTeamFile" to be called 1 times, but got 2 times失败；修复后整套 9/9 全绿。测试环境
环境（可选）
仅单元测试（
vitest run），Node v24.19.0，Linux。风险与范围
关联 Issue
Fixes #10297
参考 #10223、#10208