feat(tools): wire guarded writes into the diff-view save paths (S4b, #1375) - #1408

Automatic approval withheld

5 CodeRabbit review threads are still open and must be resolved first:

• Thread 1
• Thread 2
• Thread 3
• Thread 4
• Thread 5

Resolve them on the pull request, or post @coderabbitai approve as a new top-level comment to resolve them and request approval.

Commit: ca3a5ef76d62.

Merge Risk: Moderate · up to f2f78

This change guards file writes with version checks, but a failed rollback in apply_diff can leave a stale observation in place. A later write could then overwrite content the tool never diffed. Several earlier concerns remain open, including observation cleanup when a direct save fails, dirty-buffer handling and some test gaps, and the changed-code mutation gate currently fails. Fix the rollback path and resolve or explicitly accept the remaining items before merging.

Failed checks: 1 error · 3 warnings

Check name	Status	Explanation	Resolution
Persistence Integrity		The standing FAIL still holds. I re-checked its claims against the current code (checkout HEAD ca3a5ef7), and the changed persistence paths still have the reported gaps. 1. **Post-commit errors reje…	1. In src/core/tools/guardedWrite.ts, handle computeVersionToken failures after the rename (lines 197 and 287). Return the committed state or log the stat error, and record the observation when the token is known. Do not reject a commit…
Description check		The description has all template sections and gives detailed implementation, test, and checklist content. Two problems reduce its quality. First, the Related GitHub Issue section links #1400 but the b…	Rewrite the description in short, active sentences. Keep one linked issue in the Closes line. Move review history and corrections out of the body or into comments. State the current CI status clearly. Either split the PR to meet the 500-lin…
Regression Evidence		Duplicate submission placeholder; the final verdict is the first submit_verdict call above.	Ignore this record. See the preceding verdict.
Lifecycle Resource Cleanup		Summary: A changed lifecycle path leaks a file descriptor. src/services/file-safety/safeWriteText.ts is a new file in this PR (git diff --name-status against `refs/coderabbit/pre-merge-check/b…	1. In src/services/file-safety/safeWriteText.ts, move fsSync.fchmodSync(fd, targetMode) (currently line 306) inside the try whose finally calls fsSync.closeSync(fd). Alternatively, close fd in a catch before rethrowing. Keep t…

Passed checks: 4 passed

Check name	Status	Explanation
Linked Issues check		Active target: #1400 only. #1991, #1375, #1383, #1394, #1395, and #1405 are background context, so their coding requirements do not apply. Whole-PR base 53cb5b5 to head f2f785d. [#1400] Cod…
Out of Scope Changes check		No unrelated change was demonstrated at head f2f785d relative to merge base 53cb5b5. Changes that map to #1400 wiring: - DiffViewProvider save paths, open-time observation, and rollback on …
Title check		The title names a feature (guarded writes wired into the diff-view save paths) that matches the core of the changeset. The changeset also includes many supporting pieces: guardedWrite.ts, safeWriteTex…
Security Boundaries		Summary: No changed path in PR #1408 meets the Security boundaries condition: leaks secrets or PII, trusts or executes unvalidated input, or bypasses approval or allowlist controls. The prior FAIL's c…

Full details: Linked Issues check

Explanation

Active target: #1400 only. #1991, #1375, #1383, #1394, #1395, and #1405 are background context, so their coding requirements do not apply. Whole-PR base 53cb5b5 to head f2f785d.

[#1400] Coding requirements checked at head:

• DiffViewProvider.saveDirectly (src/integrations/editor/DiffViewProvider.ts) calls assertWithinCanonicalRoot before creating directories, then guardedWrite(task, relPath, content, writeKind). It fails closed when the owning task is unavailable.
• DiffViewProvider.saveChanges publishes a dirty buffer through guardedWrite(..., "update"). On guard rejection it restores the original content in the editor, keeps user edits in the rethrown remediation, and rethrows. A successful guarded publish does not call updatedDocument.save().
• DiffViewProvider.open records the version token before reading original content. For a new file it records a token after creation.
• ApplyPatchTool delete path is unchanged, so it stays unguarded as the issue requires. Update and move publishes use saveDirectly with "create".
• Write tools, EditTool, EditFileTool, SearchReplaceTool, ApplyDiffTool, and ReadFileTool pass guard kinds and read observations. The new commits refuse an apply_diff direct save when no version token exists, and forget the stale path's observation after a diff rejection.
• guardedWrite.ts keeps the per-path FIFO chain, createIfAbsent and replaceIfVersion guards, and the remediation text the issue requires (not-read-yet and stale). The tool specs and DiffViewProvider.spec.ts cover unobserved, stale, created, and deleted-then-written cases.

Limits of this review:

• I read the head source of DiffViewProvider.ts and guardedWrite.ts only up to the evidence limit. The disposal-guard tail of guardedWrite.ts was not re-read in this round. Its behavior rests on the prior assessment and the change summary.
• The merged main change to observationRegistry (close, retired flag) does not alter the guard wiring evidenced above.
• The issue acceptance items for a green Ubuntu unit-test gate and 100% Codecov patch coverage are CI outcomes, not coding requirements. No check-run or coverage data was inspected. The PR text reports the Windows platform arm as failing at an earlier head because of a main-side test file. This does not change the code verdict.
• The registry completeness flag (complete) is not read by product code. The issue does not require that gate, so it does not affect this verdict.

Full details: Out of Scope Changes check

Explanation

No unrelated change was demonstrated at head f2f785d relative to merge base 53cb5b5.

Changes that map to #1400 wiring:

• DiffViewProvider save paths, open-time observation, and rollback on guard rejection.
• Write tools (WriteToFile, EditFile, Edit, SearchReplace, ApplyDiff, ApplyPatch): guard kinds, read-token capture, and the new apply_diff direct-save refusal when no token exists.
• ReadFileTool observation capture.
• guardedWrite.ts, observationRegistry.ts (including forget), and the Task.ts observationRegistry property.
• Test changes in the tool specs, DiffViewProvider.spec.ts, guardedWrite.spec.ts, and the task observation tests.

Carried primitives (safeWriteText.ts with its spec, safeWriteJson-related lock use):

• The guarded publish depends on these primitives. Backup retention, retries, and symlink resolution are chain-level concerns that the PR text names. The link to #1400 is indirect, so this is recorded as a risk, not an out-of-scope change.
• The carried copies duplicate S1 to S4a. This is a merge-order and rebase risk, not a scope violation.

Size and governance items (mutation-gate line count of 628 against a 500 cap, the local three-unit split proposal, and CI arm status) are not code changes and are not assessed here.

The eslint-suppressions.json change (96 to 95 for readFileTool.spec.ts) matches the test change.

Full details: Title check

Explanation

The title names a feature (guarded writes wired into the diff-view save paths) that matches the core of the changeset. The changeset also includes many supporting pieces: guardedWrite.ts, safeWriteText, observation registry changes, and tests across the file-write tools. The title covers a real and central part of the change, so it is related to the main change. The title's reference to S4b and issue #1375 does not match the linked issue #1400 in the objectives, but this does not make the title off-topic.

Full details: Security Boundaries

Explanation

Summary: No changed path in PR #1408 meets the Security boundaries condition: leaks secrets or PII, trusts or executes unvalidated input, or bypasses approval or allowlist controls. The prior FAIL's code observations are accurate, but they describe a permission-preservation weakness, not one of the three listed conditions. Primary PR: Zoo-Code-Org/Zoo-Code #1408, author easonLiangWorldedtech (provider-reported identity; author not needed for this check). Checked at the current head (ca3a5ef): 1. Prior claim, DACL fall-through (confirmed in code). src/services/file-safety/safeWriteText.ts lines 354-370: if _saveDaclWindows fails, daclDumpPath is set to null and the write continues to fs.rename at line 445. The new file then takes the directory's inherited DACL. The spec deliberately asserts this ("never fails the write"). - Why it does not meet the condition: exposure needs three conditions together: an icacls save failure, a target with a non-inherited restrictive DACL, and a directory ACL that grants a principal the file DACL excluded. The PR shows no concrete leak of secret or PII content for that combination. It is a fail-open access-control weakening, not a disclosure path shown by the evidence. 2. Prior claim, rename replaces the inode (confirmed). The commit at line 445 replaces the inode, so owner, group, and ACL are not preserved. The staged file keeps only the mode bits (fchmodSync, lines 306 and 336). This is a permission-semantics regression for file replacement. It is not an approval, allowlist, or untrusted-input bypass under the stated condition. 3. Staging-directory symlink (review finding, now fixed). _stagingDir (lines 121-140) uses a generated name (.file-safety-staging_<time>_<random>). A workspace-committed symlink at a fixed name can no longer redirect the chmod or the staged file outside the workspace. 4. Workspace boundary. guardedWrite calls assertWithinCanonicalRoot(task.cwd, absolutePath) before publishing (src/core/tools/guardedWrite.ts line 407). DiffViewProvider.saveDirectly also checks the root (DiffViewProvider.ts line 1252). safeWriteText is called only from guardedWrite (lines 173 and 253). 5. Approval flow. Every changed write tool still calls askApproval before saveDirectly. For example, WriteToFileTool.ts lines 129 then 137, and ApplyPatchTool.ts lines 234 then 249. The PR adds write guards and does not remove approval. 6. McpHub/safeWriteJson concern. src/utils/safeWriteJson.ts is not in the changed-file list, so the symlink concern it raised is outside this PR. Prior failure disposition: the claims are accurate descriptions of current behavior, but they do not establish a leak, untrusted-input trust, or approval/allowlist bypass under the check's stated conditions. The DACL fail-open and inode replacement remain valid correctness and permission-preservation concerns. They should be fixed separately, for example by failing closed when the DACL save fails for an existing target. This verdict does not treat them as a Security boundaries failure. Prior failure refuted: The prior claims describe real code: DACL save failure falls through to rename (safeWriteText.ts 354-370, 445), and the rename replaces the inode. They do not meet the check's conditions. The leak needs an icacls failure plus a directory ACL broader than the file's, with no concrete secret exposure shown. The staging symlink is fixed (_stagingDir uses a generated name, lines 121-128). Writes ar...

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Codecov Report

❌ Patch coverage is 92.68293% with 27 lines in your changes missing coverage. Please review.

📢 Thoughts on this report? Let us know!

Review status

Thanks for contributing. This comment tracks the review sequence and the next action.

Current step: Required CI passed. Waiting for automated review of the latest commit.

If automated review does not start, a maintainer must restart it.

Review-state labels are managed by this workflow; do not edit them manually. community-approved is managed the same way — do not add or remove it manually. It signals a fresh community code approval for the current head as an advisory priority only; maintainer review is still required.

Branch refreshed: upstream/main (e487cd7b2a497aa1d9b60f9b0cb497f7839bb629) merged into feat/guarded-write-wiring-s4b; new head c46714d6c2bf5fd0968502b49ac2bf3b0ae9dd57 (parents 9fa7737142fcf25d254d6af9bca838eaf7d3f459 and e487cd7b2a497aa1d9b60f9b0cb497f7839bb629). The five main commits it brings in are 1b9cb7d35937859803bcbe40eccd457e8ba80b2e (apply_diff auto-approval outside the workspace), 5319fe91fb0caf4aa4ec66aac68fa03744ebbc23 (task abort latches), a0d03a06a (Turbo cache input contracts), e487cd7b2 (visual-theme CSS transitions) and d956405c2 (aimock 1.47.0). Two of them overlap this PR's files: the apply_diff one touches src/core/tools/ApplyDiffTool.ts and src/core/tools/__tests__/applyDiffTool.failureReporting.spec.ts, and the task one touches src/core/task/Task.ts and src/eslint-suppressions.json. The merge itself is textually clean; ApplyDiffTool.ts lands as main's content with this branch's hunks applied on top.

Required contexts: six of six green at the new head - check-translations 129025480538, knip 129025480605, compile 129025481145, e2e-mock 129025481268, platform-unit-test (ubuntu-latest) 129025480682 and platform-unit-test (windows-latest) 129025480580, all completed/success. theme-fixtures 129025480343, red at the previous head, is completed/success after the refresh. On the previous head the windows leg was red for a reason that is now measured rather than assumed: main's new applyDiffTool.outsideWorkspace.spec.ts (absent at this branch's old base and at the old head) ran against the synthetic merge tree, and its diff-view double stubbed open() as a no-op while leaving originalContent at an empty string. The stale-base guard this branch adds compares the baseline open() re-reads, so that double reported a stale base that never happened and execute returned before asking for approval - AssertionError: expected "vi.fn()" to be called 1 times, but got 0 times at that spec's line 135, nine failures. The reconciliation is in that double only: it now re-reads through the same fs mock, mirroring the pattern already used in applyDiffTool.failureReporting.spec.ts, and main's assertions are untouched. Three controls were run on the tree, with numbers: the merged tool plus main's unfixed double reproduces the nine failures; deleting main's two isOutsideWorkspace payload lines makes the same spec fail 9 of 11; deleting this branch's 17-line stale-base block turns this branch's own applyDiffTool.guardedWrite.spec.ts red (1 of 10) while main's spec stays green. Locally the core lane is 201 files / 4,363 passed / 9 skipped, the misc lane 119 files / 2,117 passed / 13 skipped, and tsc --noEmit from src exits 0 over a 3,463-file program (1,163 files from this worktree, 2,300 reached through the node_modules junction). ESLint on the single file this round's fix touched reports nothing and the suppression map is unchanged - its --prune-suppressions rewrite is whitespace only (30,910 to 33,294 bytes, parsed JSON identical) and was reverted. The ubuntu leg had been cancelled at 05:16:13Z in the same run as the windows failure (matrix fail-fast, not a pass) and now has its own green run.

The advisory mutation arm is red and the refresh did not move its number. Its own output at c46714d6c2bf5fd0968502b49ac2bf3b0ae9dd57 (job 129025480401):

Mutation gate failed: extension has 640 changed executable lines (limit 500). Split the PR or obtain a maintainer-reviewed narrow exclusion.

The core Stryker preflight could not start: spawnSync .../stryker ENOENT line in the same log is an advisory row, not this verdict. Recomputed independently with the gate's own functions (packageForPath, parseChangedLines, executableChangedLines) from the base the arm resolves for this head - merge-base(main tip, new head) = e487cd7b2a497aa1d9b60f9b0cb497f7839bb629, verified - gives 640 changed executable lines in the extension package across 12 counted files; the other 15 changed src files are test or fixture paths the counter excludes. Partition: src/core/tools/guardedWrite.ts 263, src/services/file-safety/safeWriteText.ts 191, src/integrations/editor/DiffViewProvider.ts 48, src/core/tools/ApplyPatchTool.ts 41, src/core/tools/ApplyDiffTool.ts 20, src/core/tools/EditTool.ts 17, src/core/tools/SearchReplaceTool.ts 17, src/core/task/observationRegistry.ts 12, src/core/tools/ReadFileTool.ts 12, src/core/tools/EditFileTool.ts 10, src/core/tools/WriteToFileTool.ts 8, src/core/task/Task.ts 1.

This PR is 140 counted lines over the cap on its own content, so it is a size case rather than a candidate for an exception. The split this branch would cut, with each proposed unit's own counted-line budget measured the same way against the same base:

1. safeWriteText - src/services/file-safety/safeWriteText.ts plus its test file: 191 counted executable lines. The compare-and-swap writer service; it depends on nothing else in this PR.
2. guardedWrite - src/core/tools/guardedWrite.ts plus its test files: 263 counted executable lines. The tool-level guard that consumes unit 1's writer, so it merges after unit 1.
3. wiring - the ten remaining executable files (DiffViewProvider 48, ApplyPatchTool 41, ApplyDiffTool 20, EditTool 17, SearchReplaceTool 17, observationRegistry 12, ReadFileTool 12, EditFileTool 10, WriteToFileTool 8, Task 1): 186 counted executable lines. Merges after units 1 and 2 because it calls their APIs.

191 + 263 + 186 = 640, so the three units partition the currently counted content exactly. Two caveats stated plainly: each unit's budget must be re-measured from its own merge base once the earlier units land, because a child unit stops re-declaring what its parent already contains, and each unit must also clear the separate 400-mutant preflight cap, which this log cannot speak to because the line cap throws before the mutant preflight is asked.

Request to a maintainer: choose between that three-unit split and a narrow exclusion of specific lines with a signature. Nothing has been cut - no split branch exists, and nothing was pushed for the mutation arm; the single push this round was the refresh the gating windows context required. The chain tracking issue carries this unit, its boundary and its budget.

Correction to the split proposed above, from cutting and testing it rather than restating it.

The three units have now been cut locally (three commits on scratch refs, nothing pushed, no pull requests
opened) and each unit was typechecked and lane-run in isolation. The middle unit as first proposed -
src/core/tools/guardedWrite.ts plus its spec, 263 counted lines - does not pass alone. Measured on
that cut: tsc --noEmit exits 2 with four rows,

core/tools/__tests__/guardedWrite.spec.ts(20,37): error TS2307: Cannot find module '../../task/observationRegistry'
core/tools/__tests__/guardedWrite.spec.ts(343,16): error TS2339: Property 'observationRegistry' does not exist on type 'Task'
core/tools/guardedWrite.ts(413,19): error TS2339: Property 'observationRegistry' does not exist on type 'Task'
core/tools/guardedWrite.ts(450,8): error TS2339: Property 'observationRegistry' does not exist on type 'Task'

and the core lane reports Test Files 1 failed | 197 passed (198), the failure being that spec. The cause
is a dependency the line budget did not show: the guard reports into the observation registry and reads
task.observationRegistry, so src/core/task/observationRegistry.ts (12 counted lines) and the one
Task.ts line that owns the registry belong in the guard's unit, not in the wiring unit. With those two
files and the registry's two spec files moved into it, the same unit exits 0 on typecheck and passes both
lanes (core 200 passed (200), 4,296 passed | 9 skipped; misc 118 passed | 1 skipped (119),
2,096 passed | 13 skipped).

The budget the maintainer is choosing between is therefore 191 / 276 / 173, still summing to 640, with
each number measured by the gate's own functions from that unit's own merge base:

1. safeWriteText - src/services/file-safety/safeWriteText.ts + its spec - 191 counted lines from
   main tip e487cd7b2a… - imports nothing from the other units - merges first. Commit
   04644bd33876b1bcc9d829558cc5ae67f41a6903.
2. guardedWrite + observation registry - guardedWrite.ts 263, observationRegistry.ts 12, the
   Task.ts registry field 1, plus three specs as free budget - 276 counted lines from unit 1's head -
   merges second. Commit efb46d8cea1943db37f499d3118a1eb669485473.
3. wiring - DiffViewProvider 48, ApplyPatchTool 41, ApplyDiffTool 20, EditTool 17,
   SearchReplaceTool 17, ReadFileTool 12, EditFileTool 10, WriteToFileTool 8, plus 11 specs and the
   suppression map as free budget - 173 counted lines from unit 2's head - merges third. Commit
   616b676359677e1fb6f2a1459a5e6ff161e9e562.

191 + 276 + 173 = 640, and the third unit's tree is byte-identical to this PR's head tree (git diff
between them lists zero paths and the two tree oids match), so the three units together are exactly this
PR's content, no more and no less. Each unit's typecheck exits 0 and each unit's core and misc lanes exit 0
run alone; the line-cap arithmetic and the 400-mutant preflight caveat stated above are unchanged. Nothing
is pushed and no pull request exists for any of the three.

Correction appended 08:33Z - what this plan cannot promise, and the state of the three cuts.

Two things the numbers above do not cover:

1. The mutant count is unmeasured. scripts/stryker-diff.mjs caps a package at 400 mutants as well as 500 counted executable lines, and the Stryker preflight that produces a mutant count has never been run against any of these three cuts on this machine. Line arithmetic says unit 2 is the largest at 276 counted lines; it does not say how many mutants those 276 lines generate. A unit under the line cap can still exceed the mutant cap, and if it does, that unit's mutation-diff row comes back red for a reason this comment has not measured. The claim here is about line budgets, which is a different measurement.
2. Two of the reviewer's asks landed inside the cuts rather than staying review notes. The fresh CodeRabbit pass on the current head opened three rows against src/core/tools/tests/guardedWrite.spec.ts (:467, :483, :634), all of them about what the concurrency tests actually prove. Unit 2 owns that file, so unit 2 has been re-cut to carry them: refs/tmp/s3-1408-u2-guardedwrite is now 2fffcfa49468e1daa4b25dd93e5c92c7f376c6a1 (the previous cut is pinned as refs/tmp/s3-1408-u2-guardedwrite-before-test-rows = efb46d8cea1943db37f499d3118a1eb669485473, so no work is discarded) and refs/tmp/s3-1408-u3-wiring was re-picked onto it as eeb89a3136d9293e4f70cb21da672326773b9145. Unit 1 is unchanged at 04644bd33876b1bcc9d829558cc5ae67f41a6903.

Measured on the re-cut units, each in isolation: unit 2 - typecheck exit 0 with 0 error rows, core lane exit 0 (200 files, 4,298 passed / 9 skipped), misc lane exit 0 (118 files + 1 skipped, 2,096 passed / 13 skipped). Unit 3 - typecheck exit 0, misc lane exit 0 (118 + 1 skipped, 2,117 passed / 13 skipped), core lane exit 0 (201 files, 4,365 passed / 9 skipped); an earlier run of that same tree exited 1 while reporting the same 201 files and 4,365 tests passing with no failing test file, reported here as a teardown transient whose cause I have not pinned rather than presented as a clean first pass.

Budgets are unchanged by the test rows - 191, 276 and 173 counted executable lines, per unit, measured from each unit's own merge base - because the gate's counter excludes __tests__ paths; the totals still sum to 640. Completeness control for the re-cut: unit 3's tree differs from this PR's head tree by exactly one path, the spec file those three rows are about, and by nothing else.

Nothing has been pushed and no unit PR has been opened. The cuts wait on the answer to 6734402338.

Decision, numbers, pointer - no request prose. This unit takes the three-unit split rather than a narrow signed exclusion. Reason is arithmetic, not preference: the unit content measures 640 counted executable lines against the 500-line cap, so the PR as it stands is over the cap whatever a reviewer prefers, and the exclusion remedy needs a maintainer signature on named lines that no lane here can write.

The three units, measured at base e487cd7b2a: unit 1 safeWriteText at 191 counted executable lines (head 04644bd338), unit 2 guardedWrite at 276 (2fffcfa494), unit 3 wiring at 173 (eeb89a3136). 191 + 276 + 173 = 640, so the three partition the counted content exactly and no unit's executable code crosses 500. Merge order 1 then 2 then 3, non-cumulative, each unit re-measuring from its own merge base after its parent lands.

The plan - contents per unit, fidelity contracts, verification each unit carries, and the costs the split adds (unit 2 needs a knip exception for guardedWrite.ts because its only consumer arrives in unit 3, and the mutant half is unpriced because no preflight runs on this machine) - is issued on the tracking issue #1400 as comment 6746983166, before any split PR opens.

Nothing is pushed here while the plan is unexecuted, and no split PR is opened, because 1910 and 1911 are this chain's queue heads. This comment states a campaign position, not a request for review.

Execution prerequisites for the three-unit split of this PR (#1400 carries the plan and the measurements; nothing below has been pushed or opened).

1. Merge order is fixed by the chain, not by preference. The cut commits are 04644bd338 (unit 1, the compare-and-swap writer) <- 2fffcfa494 (unit 2, the guarded writer and the registry it reports into) <- eeb89a3136 (unit 3, the eight writers and the diff-editor path). Unit 1's base is e487cd7b2a49; unit 2 sits on unit 1 and unit 3 sits on unit 2, so unit 2's PR base is unit 1's branch and unit 3's PR base is unit 2's branch. Each unit is priced against its own parent - 191, 276 and 173 counted executable lines, with baseSha and mergeBase printed before each number in Tracking S4b: wire guarded writes into the diff-view save paths #1400 - and the whole 640 is the gate's own throw ('extension has 640 changed executable lines (limit 500)'), which is why this PR cannot merge as one unit.
2. What has to land before unit 1 can be pushed. e487cd7b2a49 is today's upstream/main; the queue ahead of it in this fork carries heads 1910 and 1911, so unit 1 must be pushed (or rebased) against whatever upstream/main is at push time, and units 2 and 3 must then be re-based onto their parents in that order and re-measured, because each unit's width is a per-parent quantity that a rebase moves.
3. No unit has been pushed. All three live in private worktrees of the fork at the commits above, each with porcelain 0 rows, and the split has opened no PR. 1408 itself stays as it is until the units are opened.
4. The mutant band is unpriced for all three units on this box. No local Stryker preflight ran: node_modules/.bin/stryker is a #!/bin/sh pnpm shim and spawnSync returns status null / ENOENT here, so no mutant count printed in Tracking S4b: wire guarded writes into the diff-view save paths #1400 or on this PR is a local measurement - the bands belong to CI. What is measured locally is the counter's line selection and the gate's throw.
5. Arms: what I can and cannot claim. The six required contexts are not declared anywhere the API prints them (ruleset 15494382 returns an empty checks array with the strict flag unset), so every 'six required arms' sentence on this stack is inference from the label workflow's scope list plus observed runs, not a per-PR declaration. 1408's six arms were green in an earlier round at an earlier head; each unit's arms must be read from the check-runs endpoint at that unit's own head once it is pushed, and I have not read runs for any unit commit.
6. One carried decision, recorded rather than silently applied. src/core/tools/__tests__/guardedWrite.spec.ts differs between the cut chain (blob 04c21d4c47a4, 39,706 B, 964 line-endings, 44 it rows) and this PR's head (blob 292fbe90694f, 35,372 B, 853 line-endings, 42 it rows): delta -4,334 B, -111 line-endings, -2 it rows. Dating says the head's blob is the older content - the newest head-side edit of that file is 9fa7737142 at 2026-11-09T04:56:15Z, while the chain's blob was introduced by 2fffcfa494 at 07:31:34Z, after rows 4510300277 / 295 / 305 were filed at 07:24:55Z - and the six removed-and-replaced rows are exactly the three tests those rows ask for (they survive on the chain's blob at lines 472, 520 and 716) against the head's single weaker cross-path test at its line 472. I built the re-cut anyway, on the ruling's instruction: 89769801055c carries the head's blob, its own delta against its parent prices 0 counted lines (the spec is a test path, excluded from the counter's package map), its core lane is exit 0 but 4,363 passing against the cut's 4,365 - two fewer tests - and its knip output is byte-identical to the chain's 373 B / sha256 3a7bbd5e9c39fc59. The file is unit 2's, not unit 3's: unit 3's own 19-path delta does not contain it, so a head-side spec has to be applied in unit 2 or it becomes a downgrade commit inside unit 3. The choice is a maintainer's and the replies on those three rows say where the answering tests are, in which blob, and that they are not on this head.
7. Owed rows now named on [Tracking] guarded-write chain (epic 1375): rules, port sources, chain-level rows #1991 so they are not only thread replies: 1408 row 4510300315 (no per-path invalidate exists in the registry - observe() at :30 and clear() at :42 are the whole API) and 1408 row 4504137371 (pre-publish dirty-buffer handling is a maintainer decision the author has answered three times and left open). The four rows whose answers exist inside built units got threaded replies naming the tree and the blob; none is resolved and nothing is pushed.

One omission in the four replies I posted on this PR minutes ago, corrected here rather than by editing them: the evidence they cite is not fetchable yet.

Replies 4516372204 (row 4510300277), 4516372691 (4510300295), 4516373140 (4510300305) and 4516373495 (4510300309) each name a tree and a blob - 2fffcfa49468e1daa4b25dd93e5c92c7f376c6a1 and, for the guardedWrite.spec.ts rows, blob 04c21d4c47a4 - and neither has a URL, because units 1-3 of the split exist only as local refs in private worktrees under the plan on #1400 and the prerequisites comment I wrote on this PR at 2026-11-09T18:26:56Z (6748043210). Nothing on this stack has been pushed: git ls-remote --heads origin tmp/s3-1408 returns zero refs, and the four worktrees report porcelain 0 rows at 04644bd338, 2fffcfa494, eeb89a3136 and 89769801055c (the last one is the re-cut, kept as evidence on tmp/s3-1408-u3-recut-head and not to be pushed). So the tests named in those replies - the third-write queue test at line 472, the publish-open race at 520 and the detach-pending-link test at 716, plus the unobserved-edit refusal tested at applyDiffTool.guardedWrite.spec.ts:160, :199 and :231 - become visible to a reviewer when unit 2 lands, not now. As far as the rows go they are still open and the decision about which spec blob the split carries is recorded as a maintainer's call in that prerequisites comment.

Size-cap supplement for this pull's open review threads

Posted once for the pull rather than six times. All six open threads are listed and none is resolved; each already carries an author reply and stays open because the remedy is owed.

Threads this covers

• comment 4504137371 on integrations/editor/DiffViewProvider.ts:1273 (thread -bwM607s7w)
• comment 4510300277 on core/tools/__tests__/guardedWrite.spec.ts:467 (thread -bwM61KUmV)
• comment 4510300295 on core/tools/__tests__/guardedWrite.spec.ts:483 (thread -bwM61KUml)
• comment 4510300305 on core/tools/__tests__/guardedWrite.spec.ts:634 (thread -bwM61KUmq)
• comment 4510300309 on core/tools/ApplyDiffTool.ts:208 (thread -bwM61KUmx)
• comment 4510300315 on core/tools/ApplyDiffTool.ts:238 (thread -bwM61KUm3)

The arithmetic

• Deciding package: extension 640 executable changed lines against the cap of 500 - 140 over - at head c46714d6c2bf. Whole pull: 640, so this pull's deciding package is the whole pull. Printed by the 2026-11-10T22:08:58Z cap-table run (ct2) on main tip 18861b2e357203e6cb4d93930aa2ed122b1eecea; the abort sentence it produced is extension has <n> changed executable lines (limit 500). Split the PR or obtain a maintainer-reviewed narrow exclusion. - scripts/stryker-diff.mjs, line 268 of the classifier blob 7dfe0006d2d5c7e4d2282f65f68b8ed5f16866bc
• Per-file split as printed: core/tools/guardedWrite.ts 263, services/file-safety/safeWriteText.ts 191, integrations/editor/DiffViewProvider.ts 48.
  The counter excludes test paths: line 85 of that blob declares EXCLUDED_PATH_SEGMENTS = ["/__mocks__/", "/__tests__/", "/fixtures/", "/test-utils/"], line 206 applies it, line 207 drops any *.test|spec|visual.<ext>.ts|js file, lines 209-212 drop vitest.config.*, vite.config.*, vitest.setup.* and test-utils.*. So an assertion added inside a __tests__ file is not counted against this figure, and an edit in a source file is.
• 3 of the six open threads sit in __tests__ files and 3 in source files; under the exclusion above the spec threads do not move this figure in either direction and each source thread can only add to it.

The cheapest drawable cut, priced as a reduced tree

• core/tools/guardedWrite.ts 157-452: 296 file lines, 213 counted, tax 83. Priced on a real reduced tree: 640 to 427 (tree 6f78e7987ffa52a1bf58ca1468a1168ce5ec38c2), leaving guardedWrite.ts at 50 counted lines and moving the package's largest-file row to safeWriteText.ts 191. The deepest drawable window 85-462 (378 file lines, 248 counted, tax 130) prices to 392. Search breadth for this unit as the run recorded it: guardedWrite.ts generated 861 candidate windows and 848 parsed cleanly; across the unit's searched files 5,611 splices were generated, 848 parsed, 175 priced and 88 rejected for stranding a declaration or a spec row, 4,750 dropped for crossing a unit boundary.
• What it deletes: createIfAbsent (157-207), replaceIfVersion (217-299), unobservedEditGuard (309-311), resolveAbsolutePath (324-326), assertWithinCanonicalRoot (338-383) and guardedWrite (397-452) - six exported functions, which is the unit itself.
• Spec consequence measured at the reduced tree: guardedWrite.spec.ts line 16 imports createIfAbsent, guardedWrite, replaceIfVersion and resetChain, so the cut removes the spec's imported subjects rather than trimming them.
• Carrier counts beside it, re-measured 2026-11-11 with git grep -n -w -I --no-color -F -e <plain name> over 71 campaign candidate trees and main tip 18861b2e35: fileIsAbsent, createIfAbsent and currentVersion occur in 8 trees each (feat(tools): guarded write CAS core with per-path FIFO chain (S4a, #1375) #1405 feat(tools): wire guarded writes into the diff-view save paths (S4b, #1375) #1408 feat(tools): guarded write core under the shared lock (U5, #1375) #1914 feat(tools): publish apply_patch through the guard (U6, #1375) #1915 feat(editor): route the diff-view save through the guard (U8, #1375) #1916 U9 re cut, rung 2 of 4: guarded write core and its recovery readers (416 counted lines, 57 selector ranges, headroom 84) #1917 feat(tools): route the remaining write tools through the guard (U7, #1375) #1918 feat(tools): the guardedWrite tool on top of the writer's guard API (U5-A, #1375) #2359) and in 0 occurrences in main; read from feat(tools): guarded write CAS core with per-path FIFO chain (S4a, #1375) #1405's own candidate tree, the copies there are async function fileIsAbsent(absolutePath: string): Promise<boolean> at guardedWrite.ts:117 and export async function createIfAbsent(absolutePath: string, content at :163. So a deletion here leaves the same machinery standing in the sibling write-guard units.
• Relocation was priced in the same reduced-tree run and it is not relief for this row: moving safeWriteText.ts's 191 counted lines to core/tools/ApplyPatchTool.ts leaves the package at 640; moving them to integrations/editor/DiffViewProvider.ts leaves 640; moving them to guardedWrite.ts raises that file 263 to 454 and still leaves 640. Each equality was re-derived rather than carried forward - the destination rose by exactly the lines the source dropped - because the counter reads executable changed lines per package, not per file, so moving content inside its own package is arithmetically a no-op against the cap.

What this means for the threads

Six threads, six authored remedies owed; none is answered by this comment, and the two priced windows both remove the unit the threads are reviewing. This is the row where the honest answer is a routing answer rather than a reduction: 140 lines over a per-package cap, the only drawable windows deleting the whole exported unit, and relocation priced at zero relief because the cap is a package cap. That is a scope decision for maintainers, not a review problem for this pull.

An exclusion needs a human approving review, and because these paths fall to code owners one of those reviews must come from a code owner (taltas, navedmerchant, edelauna, JamesRobert20). A width figure clears the size row only once that approval exists. A cut listed here is a priced deletion and not an acceptance: nothing in this comment is a verdict, and no cut is acceptable to the size gate and unacceptable to its own specs at the same time.

Basis correction to the arithmetic in this comment, with the replacement figure

The figure published above — extension 640 executable changed lines against the cap of 500, at head c46714d6c2bf5fd0968502b49ac2bf3b0ae9dd57 — was taken against a base the gate does not price from. c46714d6c2bf is itself a merge commit (its subject is "Merge upstream/main (e487cd7) into feat/guarded-write-wiring-s4b"), and the gate substitutes a head's first parent whenever the head has two or more parents. The measurement therefore ran from 9fa7737142fcf25d254d6af9bca838eaf7d3f459, which is an ancestor of this pull's stated base e487cd7b2a497aa1d9b60f9b0cb497f7839bb629 — an older base, so trunk commits that predate the stated base were charged to this unit.

Replacement, taken the way CI prices it — main at 53cb5b5e7c8014d3226e5636bbca8999ed3b8526 has now been merged into this branch as df1047b5c042508d319a7d8afcbdf057cf175a64 (parents c46714d6c2bf and the sha), and the gate was run against a single-parent head whose tree is that merge's tree and whose only parent is the sha, which is the shape refs/pull/1408/merge produces and which keeps trunk's 107-file contribution out of the unit's own ledger:

Mutation gate failed: extension has 628 changed executable lines (limit 500).
Split the PR or obtain a maintainer-reviewed narrow exclusion.

So the deciding figure is 628 against 500, 128 over, deciding package extension, merge base 53cb5b5e7c80, measured head 60d83690f06ea4a9c39f4b7d83ac5bd3e5ac07f5 (tree-identical to the pushed df1047b5c0, zero rows between the worktree and that commit). The line cap aborts the run before Stryker starts, so no mutant count was reached on this head and none is claimed.

The 640 is withdrawn rather than restated. Nothing else above is affected: the six unresolved threads, their file and line locations, the unit boundary and the split arithmetic all stand as written. This pull is now mergeable: true with base_sha equal to 53cb5b5e7c80, so the gate measures this unit rather than trunk's drift, and mergeable_state is blocked on review and standing checks, not on a dirty merge.

Five open rows answered as code: three commits on feat/guarded-write-wiring-s4b, tip f2f785d130

Fast-forward from df1047b5c0 (the head this replaces). Control exits below are printed as measured: red at the tree each row replaces, green at the commit that carries it.