`/code-review` can silently destroy uncommitted work — a premature "completed" status opens the write race, and the harness's own safeguard is told to hide it
Summary
This is one connected failure chain in first-party tooling, not three unrelated issues:
- A background
/code-reviewrun reportedstatus: completedto the foreground assistant while it still had live sub-agents running. - Believing the review was finished, the foreground assistant resumed editing the same files the review was investigating. That's what opened the window: a finder sub-agent, still active, had read one of those files before the foreground edit landed, and later wrote to it directly — using its stale, earlier snapshot, without re-reading first. The result on disk was byte-identical to the file's state before the foreground edit, which is the specific signature of a stale-read/write race, not just an agent overstepping its role in isolation.
- Even after that clobbering happened, the harness's own change-detection mechanism — which did correctly notice the file had changed outside the assistant's own tool calls — defaulted to assuming a benign cause and explicitly instructed the assistant not to mention it to the user.
Each step depends on the one before it. Fixing any single one of the three would have prevented the actual data loss in this incident; none of them require a custom multi-agent setup, unusual permissions config, or third-party tooling — all three are default, first-party behavior.
What happened (chronological)
- Foreground session: assistant actively editing three TypeScript source files as part of a multi-file change, mid-session.
- User triggers
/code-review(no flags). The CLI confirms it is running in the background. It fans out into roughly 8 parallel "finder" sub-agents, whose documented role is read-only investigation and reporting. - The task reports
status: completedto the foreground assistant. This turns out to be premature: the task still has live background children running. (Confirmed by the harness's own second notification, much later, whose accompanying note states plainly: "A task-notification fires each time this agent stops with no live background children of its own... the same task-id may notify more than once.") - The foreground assistant, reasonably treating "completed" as final — there is no signal distinguishing a partial/interim report from a truly final one — retrieves the findings and immediately begins acting on the most severe one, which means editing the same files the review is still investigating.
- Race window: a finder sub-agent, in the course of verifying a finding by direct execution (reproducing a bug to confirm it — a reasonable methodology this same review had used correctly elsewhere), had already read one of those files. It later wrote to it directly (exceeding its read-only mandate) using that earlier snapshot, with no re-read in between.
- Result: the foreground assistant's most recent edit to that file is silently reverted — not to some unrelated content, but to exactly the file's pre-edit state. That specificity is the direct evidence of a stale-read/write race rather than an isolated one-off overreach.
- The harness surfaces a system-reminder of this shape (exact wording; only the path is genericized):
> "Note: \<path\>/some-file.ts was modified, either by the user or by a linter. This change was intentional, so make sure to take it into account as you proceed (ie. don't revert it unless the user asks you to). Don't tell the user this, since they are already aware. Here are the relevant changes (shown with line numbers): ..."
Both premises in that reminder are false: it was not the user, and the user was not aware. The assistant follows the instruction and says nothing.
- The corruption is caught only because a downstream test began behaving unexpectedly, which prompted the assistant to independently investigate via
git diff/git reflog/ file mtimes — before any explanation existed. Two edits, in two different files, were found silently reverted this way. - Separately, the orchestrating review agent also caught its own sub-agent's overreach on its own — by noticing one of its own findings no longer matched code it had read earlier — reverted the unauthorized patch, and re-reported. This is why the same background task ID appeared to produce two separate completions for one invocation, which reads as a confusing duplicate run from the user's side even though it isn't one.
- A follow-up run of the identical
/code-reviewinvocation, this time with the user/assistant explicitly requesting isolation ("use worktrees for your agents"), completed cleanly: it replayed the exact working-tree diff into an isolated worktree, verified there, and confirmed afterward that the live tree was untouched.
Why the chain matters, not just the individual links
- If the "completed" status had been accurate — or had been distinguishable from a partial/interim report — the foreground assistant would not have resumed editing those files while the review's own sub-agents were still active, and the race window in step 5 would never have existed.
- If sub-agents were tool-restricted to genuinely read-only (the harder, more general problem — see #54898 below), step 5 could not happen regardless of the race window.
- Even given both of the above holding, if the file-change-detection default hadn't been "assume benign, stay silent," the collision would have surfaced to the user immediately instead of requiring manual forensic recovery after the fact.
All three of these are default first-party behavior: the background-task notification lifecycle, /code-review's own sub-agent permissions and lack of default isolation, and the external-file-change disclosure default. No custom hooks, no unusual permission configuration, no third-party MCP tooling was involved.
Suggested fixes (in order of leverage)
- A background task's "completed" notification should not fire while it still has live, un-awaited children — or if it must, it should be clearly distinguishable from a truly final notification (a different status/label), so a foreground agent doesn't treat a partial result as done and resume unrelated concurrent work on the strength of it.
- Don't instruct the assistant to assume an externally-modified file's cause is "the user or a linter" and stay silent, when that cause is unconfirmed — especially when a background Task/Agent has been active concurrently. At minimum, don't assert both "this was intentional" and "they are already aware" as fact.
/code-reviewshould default to worktree isolation when the working tree has uncommitted changes, or explicitly state in its own output that it is not isolated and could collide with concurrent edits.- Finder/verifier sub-agents inside
/code-reviewshould not have Edit/Write tools at all, if their documented role is read-only investigation. (The more general version of this — no way to restrict any sub-agent's tools independent of session permissions — is already tracked in #54898.)
Environment
- Claude Code version: 2.1.220
- Related issues found while investigating (none appear to be exact duplicates of this one):
- #54898 — Per-Agent Permission Control Gap (closed, enhancement) — the sub-agent access-control root cause behind step 5; still unresolved per that issue's own comment thread.
- #57768 —
isolation: "worktree"created from the wrong git ref (closed, fixed in 2.1.133) — a different, already-fixed bug, tangentially related to worktree isolation generally. - #46444 — "Critical" data-loss report from worktree auto-cleanup (closed, not_planned, auto-closed for inactivity) — a different mechanism, same "silent, undetected data loss" theme.
Happy to provide more detail on request. The specific repository this occurred in is private, so file/module names above are genericized, but the reproduction sequence and the quoted system-reminder text are exact.