Repository navigation
Open
Description
Problem
The agent's file-write path has no version guard, no atomic publish, and no per-step
visibility:
- Concurrent writers silently overwrite each other's work: parallel subtasks share one
workspace, multiple VS Code windows are separate extension-host processes, and the
JetBrains plugin has confirmed real corruption under two IDE instances ([BUG][regression] Global _index.json full rewrite is unsafe under concurrent tasks (real corruption under JetBrains multi-agent) #1231). - Model writes land directly in the shared workspace with no checkpoint to roll back to;
task start creates no baseline at all. - No staleness detection: a file that changed after the model read it is silently
overwritten on the next write. - The write path carries artificial latency (1 s delay per write) on top of the missing
safety.
Known issues
| # | Title | Status | Fix in this epic |
|---|---|---|---|
| #1371 | MCP settings wiped by concurrent windows | Open | A5 — root cause is the check-then-create TOCTOU in McpHub.getMcpSettingsFilePath() (L496-517), not initializeMcpServers() |
| #1021 | Race in abandonSubtask for saveClineMessages | Open | A5 — fire-and-forget saveClineMessages() re-attaches abandoned subtasks |
| #920 | Concurrent task history updates cause lost entries | Open | A5 — cross-instance (parallel tabs) regression test on the locked-merge write |
| #1231 | Global _index.json full rewrite unsafe (corruption confirmed) | Closed | Already fixed — per-task history_item.json + merge-under-lock; that is now the reference storage pattern |
| #1221 | Truncated tool-call args silently written to disk | Open | A6 (separate story) — parser/finalization defect, not a filesystem-safety problem |
| #376 | File-level write serialization | Open | Superseded by A3 |
| #370 | Extension-side task-scoping guard | Open | Related; stays open |
Current state (verified in code)
Agent write path
- WriteToFileTool.execute() (src/core/tools/WriteToFileTool.ts:29) →
DiffViewProvider.saveDirectly() (src/integrations/editor/DiffViewProvider.ts:1141) =
raw fs.writeFile(absolutePath, content): no lock, no version check, no atomic
publish, no fsync. This is the main agent write path. - EditFileTool (src/core/tools/EditFileTool.ts:139) validates old_string against a fresh
read at write time — a natural staleness detector for edits, but write_to_file (full
overwrite) has no staleness check at all. - ReadFileTool records no mtime/stat/checksum of the files it hands to the model, so no
compare-and-swap is possible anywhere in the codebase. - Auto-approval (src/core/auto-approval/) can skip user review entirely.
- ExecuteCommandTool runs shell commands in the workspace; shell writes bypass any
filesystem guard (out of scope, below).
Internal state
- safeWriteJson (src/utils/safeWriteJson.ts) is the JSON-state entry point (imported by
TaskHistoryStore, taskMessages, apiMessages, McpHub, webviewMessageHandler,
modelCache): proper-lockfile advisory lock, temp + rename, backup/rollback, merge
callback. Missing: fsync, Windows DACL preservation, and it does not cover the
workspace write path. - MCP settings wiped when multiple windows open — race in McpHub.getMcpSettingsFilePath() direct fs.writeFile #1371 root cause: getMcpSettingsFilePath() (McpHub.ts:496-517) does
fileExistsAtPath() then a raw fs.writeFile of an empty stub — two windows both see
"absent" and the second blind write truncates the file to a 122-byte stub (TOCTOU). - fix(task): guard saveClineMessages against abandoned tasks to prevent race in abandonSubtask #1021: fire-and-forget saveClineMessages() calls can resolve after abandonSubtask()
and re-attach a severed subtask.
Concurrency
- Parallel subtasks / new_task children: same process, shared workspacePath
(Task.ts L515-517), no file-level isolation. - Per-task isolation today = checkpoints shadow git at
/tasks//checkpoints (ShadowCheckpointService), triggered on
user message send — not on writes. - Cross-process (VS Code windows / JetBrains) coordination for workspace files: none.
- Worktrees (packages/core/src/worktree/) are user-UI-only: prompts, tools, and
Task.startTask contain no worktree references, and git worktree add is a full checkout
of the base commit. The agent flow does not use it; it cannot serve as the isolation
mechanism for this work.
Solution
A — Safety core: version guard + per-path serialization + atomic publish
- A1 Version token — dev:ino:size:mtimeNs:ctimeNs from one fs.stat (BigInt). Cost of
one stat; the token is a disk fact, so all processes observe the same value. - A2 Observation registry — per-task Map<absolutePath, version>, populated by
ReadFileTool (one extra stat per read). Owner = task, so parent and subtask
observations are independent. - A3 Guarded write and edit — compare-and-swap semantics on the write path:
- unobserved target → createIfAbsent: succeeds for new files; if the file exists the
write fails, forcing a read first — the model re-reads and retries with the observed
version; - observed-absent → createIfAbsent;
- observed-present → replaceIfVersion(version): a mismatch fails the tool call with
"stale version — re-read the file, then retry"; - edit keeps the literal-match check plus the version guard; an unobserved edit
fails with "file not read yet — read the file, then retry". - Failures carry a remediation suffix the model can act on, so the standard loop
self-heals (re-read → retry); the user sees the failure as a step event in chat. - A per-absolute-path tail-promise chain (in-process FIFO) wraps read → guard →
publish, so concurrent subtask mutations to the same file are deterministically
ordered: one wins, the rest fail as stale. - Cross-process stance (explicit): no workspace lockfile. Two processes editing
the same file are detected via the version token; the loser fails as stale and
re-reads. A lockfile would block the user's own editor.
- unobserved target → createIfAbsent: succeeds for new files; if the file exists the
- A4 Atomic publish — replace saveDirectly's raw fs.writeFile with: temp file in a
private per-write staging dir → fsync → rename; on Windows ReplaceFile (with DACL
copy), rename fallback. Generalize safeWriteJson's staging/backup/rollback into a
safeWriteText used by both file writes and JSON state. - A5 Internal-state firebreak
- MCP settings wiped when multiple windows open — race in McpHub.getMcpSettingsFilePath() direct fs.writeFile #1371: replace the raw stub write in getMcpSettingsFilePath() with
safeWriteJson(..., { merge: keep existing if non-empty }) — atomic read-modify-write
under the advisory lock closes the TOCTOU. - fix(task): guard saveClineMessages against abandoned tasks to prevent race in abandonSubtask #1021: guard in-flight saveClineMessages() with this.abandoned (extract from
feature/local-usage-stats, commit 1d1eb91). - [BUG] Concurrent task history updates from parallel tabs may cause lost entries #920: cross-instance (parallel tabs) regression test for updateTaskHistory on the
locked-merge write.
- MCP settings wiped when multiple windows open — race in McpHub.getMcpSettingsFilePath() direct fs.writeFile #1371: replace the raw stub write in getMcpSettingsFilePath() with
- A6 Truncated tool-call args (Truncated tool-call arguments can be silently written to disk (stale partial-parse nativeArgs reused on finalization failure) #1221, separate story) — a NativeToolCallParser
finalization failure must not reuse the streaming-phase nativeArgs (partial-json
output); fail the tool call with an explicit "arguments were truncated" error instead
of writing the truncated content to disk.
Latency — net write-path cost added by A: one stat + one fsync + one rename. On top
of that, remove the artificial delays:
| Current slow point | Location | Action |
|---|---|---|
| 1000 ms delay per write | DEFAULT_WRITE_DELAY_MS (packages/types/src/global-settings.ts:23) | default 0; keep the setting for users who want pacing |
| delay(300) before approval | WriteToFileTool.ts | remove |
| Blocking LSP diagnostics after save | saveDirectly tail | report asynchronously as a follow-up event |
| DiffView open/update/scroll choreography | default approval path | make the chat-diff (PREVENT_FOCUS_DISRUPTION) path the default |
B — Per-step visibility + rollback
- B1 Automatic checkpoint per file write — extend the existing checkpoints shadow
git so every successful write_to_file / edit_file / apply-patch records a checkpoint
(today only user message sends do). Task start becomes a real, O(1) baseline — not a
no-op, and not a worktree. - B2 Per-task change journal — append-only JSONL under the task dir (one entry per
file write: path, operation, checkpoint id, diff stats); torn-tail repair on load. - B3 Per-step change cards + rollback — per-step change cards in the existing chat
flow (reusing the unified diff + computeDiffStats already produced for approval) with
"N files changed this step", and rollback to any checkpoint per file or per step.
Auto-approval paths get the same cards after the fact — the user can always see and
undo what the agent did.
Acceptance criteria
- No raw fs.writeFile / fs.appendFile in the workspace file-write path (all writes
through the A4 atomic-publish helper; JSON state through safeWriteJson). - Stale writes (file changed since this task's last read) fail loudly with the
re-read-then-retry remediation, and the agent recovers automatically in the standard
loop. - A write to an existing file the task never read fails (createIfAbsent conflict) and
the model recovers by reading first. - Every successful file write produces a rollback-able checkpoint and a journal entry;
the user can view each step's diff and roll back per step or per file. - Concurrent subtasks writing the same file: exactly one wins, the others receive stale
errors — deterministic, no corruption, no lost writes. - Two-window regression: concurrent MCP settings initialization never truncates
mcp_settings.json (MCP settings wiped when multiple windows open — race in McpHub.getMcpSettingsFilePath() direct fs.writeFile #1371). - Abandoned-subtask regression: in-flight saves cannot re-attach an abandoned subtask
(fix(task): guard saveClineMessages against abandoned tasks to prevent race in abandonSubtask #1021). - Truncated tool-call regression: a cut-off write_to_file content argument is not
written to disk; the tool call errors instead (Truncated tool-call arguments can be silently written to disk (stale partial-parse nativeArgs reused on finalization failure) #1221). - Added guard latency < 50 ms p95 on a local volume (stat + rename only; no artificial
delays on the default path). - ESLint suppression counts do not increase; narrow Vitest suites for A and B pass.
Related PRs
Open (head: easonLiangWorldedtech's fork, base main):
| PR | Scope |
|---|---|
| #1380 | A5 — #1371: atomic mcp_settings.json stub creation via safeWriteJson + merge under the advisory lock |
| #1381 | Speed: DEFAULT_WRITE_DELAY_MS 1000 → 0, and the delay(300) before scrollToFirstDiff removed |
| #1382 | A5 — #1021: guard saveClineMessages against abandoned tasks (minimal main form of commit 1d1eb91) |
Adjacent PRs to track (open on main):
- feat(task-scheduler): enable concurrent delegated task fan-out #1046 — concurrent delegated task fan-out: increases concurrent writers, so the A3 guards become mandatory once it lands.
- [Fix] Task history disappears when user reopens a task #1319 — task-history safe-write rename-window retry + snapshot merge under the advisory lock: overlaps A5's internal-state scope; do not double-fix.
- feat(dashboard): add local usage statistics dashboard with session detail #1225 — usage stats dashboard: original home of the fix(task): guard saveClineMessages against abandoned tasks to prevent race in abandonSubtask #1021 fix commit (1d1eb91).
- fix(write-to-file): address partial filesystem error review #1066 — write_to_file partial filesystem error handling: the A6/Truncated tool-call arguments can be silently written to disk (stale partial-parse nativeArgs reused on finalization failure) #1221 parser fix should be stacked after it (same author, same code area).
Out of scope
- Shell-tool writes (ExecuteCommandTool).
- Cross-process workspace locking — the version token is the cross-process mechanism.
- Replacing the existing diff-editor approval UX: it is preserved; B adds after-the-fact
visibility. - Truncated tool-call arguments can be silently written to disk (stale partial-parse nativeArgs reused on finalization failure) #1221 is tracked here but implemented as its own story (parser layer).
- Worktrees remain a user-initiated feature; the agent flow uses checkpoints, not
worktrees.
Metadata
Metadata
Assignees
Labels
No labels
Activity
fix(file-safety): derive the version token from exact BigInt stats (A1,
test(experiments): cover unset-experiment default routing in all writ…
test(experiments): add explicit enabled-state routing tests for chat-…
feat(task): per-task file observation registry (A2, Zoo-Code-Org#1375)
feat(file-safety): atomic text publish primitive + safeWriteJson refa…
feat(file-safety): atomic text publish primitive + safeWriteJson refa…
feat(editor): async post-save diagnostics on chat-diff save path (L1, Z…
feat(file-safety): atomic text publish primitive + safeWriteJson refa…
feat(editor): async post-save diagnostics on chat-diff save path (L1, Z…
feat(file-safety): atomic text publish primitive + safeWriteJson refa…
feat(editor): async post-save diagnostics on chat-diff save path (L1, Z…
feat(checkpoints): per-write checkpoints, task-start baseline, and pe…
feat(file-safety): atomic text publish primitive + safeWriteJson refa…
feat(editor): async post-save diagnostics on chat-diff save path (L1, Z…
feat(file-safety): atomic text publish primitive + safeWriteJson refa…
99 remaining items
easonLiangWorldedtech commentedon Oct 5, 2026
Corrected merge order — U8 must land before U6
compilewas red on U6 (#1915) for a real dependency reason, not a test problem:U6's
ApplyPatchToolcallstask.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs, "create"), but thewriteKindparameter is introduced by U8. A consumer cannot build before its provider, so the recorded order U1→…→U5→U6→U7→U8→U9 is not executable.U8 owns the
saveChangessignature; U7 does not touchApplyPatchTool.ts, so U7 does not depend on U6. The executable order is:U1 → U2 → U3 → U4 → U5 → U8 (#1916) → U6 (#1915) → U7 (#1918) → U9 (#1917)
Rebuilt and verified in that order (each unit carries its predecessors' corrections):
U1 97b599d2e → U2 625a976dc → U3 60376ca18 → U4 d7eab3d99 → U5 ca636d6f9 → U8 45b791264 → U6 22b6decf1 → U7 ae52376d8 → U9 667d01dec.Verification at the new chain top: tsc clean, 12 spec files / 489 tests pass (1 skipped), ESLint
--max-warnings=0clean. U8 also verified standalone on U5: tsc clean, 130 DiffViewProvider tests pass, ESLint clean.perf(write-path): remove artificial write delays by default (part of Z…
feat(editor): async post-save diagnostics on chat-diff save path (L1, Z…
fix(editor): reconcile the observation when a guarded publish lands w…
fix(utils): decide confinement once and reject a declared-empty root
fix: order cancellation against the guarded publish and session restart
test(diffview): keep the open() serialization test inside the lint su…
fix(editor): reconcile the observation when a guarded publish lands w…
fix(utils): decide confinement once and reject a declared-empty root
fix: order cancellation against the guarded publish and session restart
test(diffview): keep the open() serialization test inside the lint su…
feat(s4b): guard the legacy diff-editor save path and re-check the ro…
feat(checkpoints): per-task change journal core with torn-tail repair…
fix(tools): take the read version token before the read, and pin the …
revert(utils): drop the out-of-scope safeWriteJson work from the S4b …