[Tracking] guarded-write chain (epic 1375): rules, port sources, chain-level rows

Part of the split-plan bookkeeping for the guarded-write (epic 1375, units U1 to U9) chain.

What belongs here

chain-level rules and design decisions that no single unit owns
port sources and the per-unit derivation notes for content that is ported rather than rebased
chain-level review rows that CodeRabbit raises against a unit but whose cause spans units
unit-scope notes for units that have no per-unit issue yet must name their unit in the first line

What does not belong here

unit-scope defects: those go on that unit's own issue (all nine units now have one - see the unit map below)
anything that requires a maintainer decision: raise it in the PR thread instead

Writing rules for this issue

no @mention in any comment here: mentions notify people who are not subscribed to this issue
do not reference a fork-repo issue number here; a bare number resolves against this repository and silently points at something unrelated
when recording an upstream pull request, write it without a leading hash (for example upstream PR 1066)
prefer editing an existing comment over posting a new one: edits notify nobody and recompute cross-references
this issue's history was consolidated into this body on 2026-10-13; add new chain records as edits to the matching section below rather than as new comments

Unit tracking issues (fws chain)

One per unit, so every PR now has a linked issue in its sidebar: U1 PR 1910 -> #2016, U2 PR 1911 -> #2017, U3 PR 1912 -> #2018, U4 PR 1913 -> #2019, U5 PR 1914 -> #2020, U6 PR 1915 -> #2021, U7 PR 1918 -> #2022, U8 PR 1916 -> #2023, U9 PR 1917 -> #2024. All nine PR descriptions were rewritten for human reviewers following maintainer feedback on PR 1910: each leads with what the PR does and why in plain terms, links its unit tracking issue with a Closes line (U9 additionally closes the epic as the last unit), and keeps the measured verification numbers under Test Procedure / Verification at this head.

Decision requests

One canonical write lock, or an accepted residual window

Two chain registrations converge on the same end state and neither can be closed inside a single unit. The runtime has no atomic compare-and-swap rename, so a pre-rename verification callback followed by a plain rename cannot be made sufficient by any unit, and a symlink refusal that walks ancestors once cannot be made sufficient while publication is path based. The only shapes that make both claims true are a single lock that every writer in the repository honours, including writers outside the extension host, or an explicit, documented residual window on both controls. A unit-level change cannot choose between those, and a registered row stays red at every head until one of them is chosen, because a citation does not clear a pre-merge row - only code at the head does. Until the choice is made, the units that wire these controls cannot reach an approving review at their own head.

Chain-level defect classes

1. A rollback that persists unapproved streamed content (defect class, not one call site)

A rollback path awaits workspace.applyEdit() or TextDocument.save(), discards the boolean, and continues as if the operation succeeded. When the editor refuses the restore, the buffer still holds content the user never approved and the following save writes it to an existing file, while the caller reports a completed rollback.

Units affected: U8 (PR 1916) in this chain, and the same primitive as consumed by PR 1921, PR 1929 and PR 1932 in the other two chains.

Two facts that are different and must not be merged into one attribution: the shared primitive is DiffViewProvider.restorePreStreamBuffer() / saveBufferClean(), whose primitive owner is PR 1930 (commit e41ebe0), with port direction 1930 -> 1916 / 1921 / 1929 / 1932; the semantic baseline is PR 1928 discardUnapprovedStream() and its editorFailure channel, i.e. a false return from applyEdit() means the rollback failed. The baseline was already broadcast, so it is not a new rule.

Acceptance for any unit that carries the row: restorePreStreamBuffer() throws when applyEdit() returns false; document.save() and the tab close are not called; the failure reaches the layer that reports it (revertChanges -> revertDiffChangesBeforeReset -> rollbackError); a test asserts the user does not receive a "successful rollback". A save that resolves false must propagate. For PR 1932 additionally: the discardFileTab() shape (restore and save hoisted above the tab loop) must stay intact, and prettier whitespace changes do not affect it. Fix it in the callee so every caller inherits the contract, rather than adding another inline check at each call site - this is the same lesson as the rollback fix in 9c54765: fixing one call site does not fix the class.

2. A confined publication must pin the directories the write itself created

The caller can only record the identity of directories that already exist, and a recursive mkdir reports nothing it made, so the pre-commit re-check walked a list that omitted exactly the components that appeared during the write. A local process that replaces one of those with a link to outside the scope sends the commit somewhere the confinement decision never made, while every recorded identity still matches.

Fixed on U6 (PR 1915) in commit 3289a72: after the mkdir the primitive records the identity of each directory it created and the re-check walks the recorded list plus those; the stat-with-mapping is shared by both call sites so no second branch set appears.

Inherited by any unit whose branch carries this version of the primitive - U1 and U8 among them. Ports are owed, not copied: each unit's call shape differs, so a port needs its own red-first test and its own negative controls. Carry a port when that unit is pushed for some other legitimate reason; do not push a unit solely to deliver it.

Acceptance for a port: (1) a confined write whose parent chain is created by that write, with one component swapped for an out-of-scope link after the mkdir, fails with the ancestor-replaced error and renames nothing; (2) the same shape without the swap publishes normally; (3) a swap of a pre-existing ancestor still fails. Negative controls, one per half of the fix: skipping the recording turns test 1 red, and recording without feeding the list to the re-check also turns it red.

3. Fail-closed DACL restore must reach the old-generation copies of the publish primitive

In the old-generation copies of src/services/file-safety/safeWriteText.ts the DACL restore runs after the commit rename and, on failure, only records a flag and warns, so a replacement is published even though the captured DACL could not be restored. The caller gets no signal: the primitive returns success and the only post-commit throw is PublishNotDurableError. A call-site preflight would be a second mechanism duplicating the primitive's own capability logic, and requesting a backup does not fail closed because the replacement is still published.

Ownership: the chain already carries the correct shape. The earliest unit in merge order implements a typed DaclRestoreError thrown after the commit rename when the restore fails, plus a fail-closed inspection error before the commit, and its own pre-merge row passes with that shape. The old-generation branches carry independently re-derived copies that predate it, so the fix is a convergence port of one shape, not a new design.

Acceptance, not bound to a commit: (1) a restore failure after the commit rename reaches the caller as a typed error, and no caller can observe success for a publish whose DACL was not restored; (2) the branch converges on the chain's single shape rather than adding a second mechanism in the caller; (3) the direct-save path surfaces the same failure instead of swallowing it; (4) backup retention on restore failure does not regress, and capture stays fail-closed before the commit; (5) every branch carrying its own copy of the block gets the port re-derived for that branch, not copied byte for byte. Negative-control shape: one mutant per production call site, each reddening only the tests naming that behaviour; run each mutant against the pre-fix spec to show the branch was previously uncovered; if a mutant reddens far more tests than the target, assume the mutant is wrong first; restore mutants byte for byte and verify with a hash.

4. Publish must not follow a swapped ancestor link when a caller asks for symlink refusal

The refusal walks the ancestors once, before anything is resolved, staged or locked, while the two later checks look only at the final component and publication is path based - the directory creation and the rename both take a path. An ancestor replaced by a link after the walk therefore redirects the commit, so the control the caller asked for is not what the caller gets. The runtime has no renameat equivalent, so this is a design decision in the primitive rather than a local patch: either publish through handles with no-follow semantics, or stop claiming the protection for paths that cannot be proven stable after the check.

Ownership correction measured at the earliest unit head: the primitive in the earliest unit of the declared merge order has no refusal option, no confinement option, no ancestor walk and no external callers, so the claim this registration narrows is vacuously true there and no change at that head can produce a test that is red first. The earliest unit that actually carries the control is the one whose helper resolves the caller path against a declared scope root and walks ancestors once before anything is staged or locked, and that is where the cheap half belongs - document precisely what the control does not cover, add a test that swaps an ancestor after the final component check and is red before the change, and decide whether the credential path keeps the claim. Handle-relative no-follow publication is not available in this runtime, so the narrowing is the only implementable half.

Carrier correction measured branch by branch: the confinement control is not new to the unit that first documented the window. The unit before it in the declared merge order already carries the identical control - the same scope-root resolution with the missing-ancestor walk, the same pre-lock and in-lock checks in the same order - and the same credential call sites that pass it. The two earliest units carry no such control at all. The documentation narrowing therefore has to be ported to that earlier unit and re-derived: where that copy still decides from a truthiness test, keep the boundary wording and drop the sentence about a declared but empty root, which only exists in the later copy.

Acceptance, not bound to a commit: (1) either publication is handle relative and no-follow for the directory creation and the rename, or the control is removed from the credential path so it no longer claims protection it cannot prove; (2) the chosen boundary is documented on the option, including what it does not cover; (3) a test swaps an ancestor after the final component check and shows the outcome the option now promises, red before the change; (4) the shape lands in the earliest unit that carries the control and is re-derived per branch when ported forward; (5) callers that pass the option keep their existing behaviour for ancestors that do not change under them. Negative-control shape: one mutant per production call site, each reddening only the test naming that behaviour, plus a run of the new test against the pre-fix production file; restore byte for byte and verify with a hash.

5. The symlinked-ancestor walk is not bounded by the authorized scope root (owed on U7, PR 1405)

A symlink above the authorized root - a linked workspace root, a linked temporary directory on a developer machine or a CI agent profile - refuses writes that the caller opted into for a reason. The refusal exists to stop a link redirecting a payload out of the scope; applied above the authorized root it stops the write without protecting anything, because nothing above that root is part of the containment decision.

Acceptance: (1) a write whose ancestor chain contains a symlink strictly above the authorized scope root publishes normally, with the payload at the named path; (2) a symlink strictly inside the authorized scope still refuses, with the same error the refusal gives today; (3) the credential-carrying case keeps its current behaviour: a link at the final component is an error, and a link inside the scope is an error; (4) the walk stops at the scope root rather than at the filesystem root, and the code says which root it stopped at.

Negative control: red first - the test for case 1 must fail against the current walk; then green with the bound in place; then red again with the load-bearing piece removed - drop the lower bound so the walk runs to the filesystem root again, and case 1 must fail. A second control for the other half: widen the bound so it also ignores links inside the scope, and case 2 must fail. Both mutants restored byte-exact.

Notes for whoever takes it: heavy lift, deliberately not bundled with the comment and documentation changes already landed on that unit. Not tied to any commit: carry it on the unit that legitimately pushes this primitive next, re-derived against that unit's call shape rather than copied. Related chain facts: the lock key and the publication destination are different paths, and the fold of a lock key has to match the way the lock entry is named (see the port-source record below).

6. A shared fixed staging-directory name lets a workspace-shipped symlink redirect the staging chmod and file

A repository that ships a directory entry named .file-safety-staging as a symlink to any directory the user owns has that outside directory set to 0o700 by the first confined write into the containing directory, and the staged temp file is created in the outside directory before the commit rename.

The chain's single shape is a per-write unique staging-directory name, which removes the hazard by construction: the entry the write creates, chmods and writes into is one it made. Every unit branch of the declared merge order (U1 through U9) already carries that shape.

Carriers of the defect: the old-generation copies on PR 1395 and PR 1405 still use the shared fixed name. PR 1408 carried the convergence port in commit 87ee84e (raised as inline finding 4240865298 against its head); this records the port still owed on the two earlier old-generation branches.

Acceptance for a port: (1) two self-staged writes into one directory receive different staging names, each of the generated shape, and each write's chmod and staged file land inside its own directory; (2) a pre-existing entry named .file-safety-staging - plain directory or symlink - is never chmod-ed or written into by this layer; (3) the test is red before the change on that branch. Negative-control shape: one mutant - revert the staging-directory name to the shared fixed name and confirm exactly the new ownership test reddens and nothing else; restore byte for byte and verify with a hash.

7. Empty declared root: reject semantics are canonical

Correction after two units fixed this independently: the two copies diverged. The unit that merges earlier in the declared order rejects an empty or whitespace-only declared root with the confinement error naming that root, before the lock key, the lock, directory creation or staging, because the helper's stance is that a failure must surface rather than be decided from a guess about the caller. The later copy currently resolves an empty string to the process working directory instead. The canonical shape is therefore the rejecting one, and the later copy must be re-derived to it when the earlier one lands, since merging the later copy as it stands would silently replace reject semantics with working-directory semantics in the landed default. Until then the later unit stays as it is rather than spending another review cycle, because it cannot merge before the earlier one anyway.

8. U9 teardown: the waiter cannot tell whether the pass it waited for succeeded (PR 1917 / PR 1918 - fix once and port)

runTeardown cannot tell a waiter whether the pass it waited for actually succeeded. The tracked promise is resolved by release() in the finally block, so it settles successfully whether the owner's cleanup or finalize returned or threw. The waiter's await this.teardownInFlight therefore resolves in both cases and it returns false - "not my pass" - with no signal that the pass it just waited through left the document mid-teardown. The owner's own caller does see the throw, so the failure is not swallowed outright; what is lost is the waiter's ability to react, which is exactly the case a cancellation lands in: the second caller arrives, waits, and walks away believing the buffer was reverted and the tabs closed.

The two structural halves of the original note are already in place and should not be re-derived: finalize now runs inside the tracked promise, so a cancellation arriving after the callback but before the restore and reset no longer starts a second full pass, and the gate is held until both settle.

Acceptance: a test where the owner's cleanup rejects and the waiter is already parked: the waiter must either see the failure or run the recovery itself - asserting only that the owner rejects is not enough, because that half already holds. Negative control: make the waiter's path indistinguishable again (resolve the gate unconditionally and drop whatever channel carries the outcome) and that test must fail.

Owed changes and ports

Stop accepting a bare staging path (U6, PR 1915)

The text writer still accepts a caller-supplied staging path by prefix alone, for compatibility, and the wording says it is checked rather than trusted. Verified at the head of U6: no production caller passes a bare path - the JSON writer takes the handle from the staging-file API and hands the handle down. So the compatibility acceptance has no user, and it keeps a bypass in the primitive: a prefix check on a path a caller invented is not the opaque capability the design relies on, because the caller can name a path it did not create.

Acceptance: (1) a bare path is refused with an error that names the handle API; (2) the handle path still works unchanged; (3) three assertions that the handle cannot be forged by naming a path - a path this call did not create, a link at the staging location, and the target itself. Negative control: restore the prefix acceptance and drop the handle identity check, and the unforgeability assertions go red.

Port source: folding a lock key the way the lock is placed (owed on U8, PR 1916)

Source: U6 (PR 1915) commit 5131bc0. Derived into U8 (PR 1916) commit 19aec00 - not byte-identical, because U8 computed the same-identity comparison inline while U6 has it as a helper.

The rule, which is what a port must reproduce rather than the code shape: the advisory lock is the entry <path>.lock beside the file, so a key must be folded the way that entry is named. On Windows the filesystem folds two spellings of one directory entry twice over - case anywhere, and short (8.3) names inside a component, which is how a CI agent hands out its runner profile directory - while a string comparison folds only case. So the fold canonicalises the deepest EXISTING ancestor and appends the segments below it, case-folded on Windows, and the walk starts at the parent, never at the file: resolving the final component through a symlink folds a link together with its referent, and those are two different lock entries.

Owed on U8: the mixed-ancestor acceptance test (an existing ancestor spelled short with a tail that does not exist yet) has not been derived there, because in U8 the same-identity comparison is only consulted when publishing over a link. The test must be re-derived against that shape, not copied. Verification limit recorded on the U8 commit: the four windows failures that motivated the port do not reproduce on a host whose temp directory has no short-name ancestor, so only CI exercises them.

Ownership rulings

The safeWriteText primitive has one owner chain (registered for U4, PR 1913)

Three pre-merge rows raised on PR 1913 at head c36fd64 land on the shared primitive src/services/file-safety/safeWriteText.ts: (1) Security Boundaries - a symlink race requires confineTo to be carried into safeWriteText and enforced at the final publish action; (2) Persistence Integrity - the committed semantics at safeWriteText.ts:509-511; (3) Lifecycle Resource Cleanup - the swallowed backup unlink at safeWriteText.ts:553-558.

Ruling: these are primitive-level, not U4-level. The primitive already has one fixed defect in this chain (the pin-before-commit escape, fixed in U6, with the port owed to U1 and U8 and re-derived per unit rather than copied byte for byte), so a second unit editing the same block independently is exactly the double fix the chain rule forbids. The primitive fix belongs to the earliest unit in the declared merge order that can carry it, and ports forward from there. U4 does not fix these three rows; it references this ruling and resolves its threads.

Acceptance for whoever takes the primitive fix: symlink race - swap an ancestor for an out-of-scope link between the last check and the publish action -> AncestorReplacedError and no rename; no swap -> publishes. Committed semantics - a failure after the commit point must not be reported as a rollback, and must not leave the caller believing the previous content is back. Swallowed backup unlink - ENOENT and ENOTEMPTY stay silent, every other error is surfaced, and the loop continues. Negative control shape per row: mutate the exact production line, expect exactly the tests that name that behaviour to go red, and run the same mutant against the pre-fix spec to prove the branch was uncovered. A mutant that reddens far more tests than the target means the mutant itself is wrong.

Note for the reader: an open-thread count on a PR and the number of rows in that PR's pre-merge table are two different measurements and must not be used interchangeably.

Registrations that are not defects (U3, PR 1912)

1. The Linked Issues check reports an error because the branch is based directly on the repository default branch, not on a preceding unit. CodeRabbit therefore sees the whole stack (13 files, roughly 3.6k added lines, 39 commits) instead of one unit's delta, and its linked-issues validation reports an error for a pull request whose diff spans requirements owned by other units. Handling chosen: the pull request body carries a requirement-to-unit table so a reviewer can tell which requirement this unit serves. Acceptance: when the earlier units of this chain merge, the row disappears without any code change - if it does not, the cause is something else and this record is wrong.

2. A review finding overstated the effect of a cleanup on a committed publication. The finding said the cleanup path can delete the already-committed target. On the POSIX path it cannot: the staged name is gone once the rename lands, so the unlink reaches a name that no longer exists and the only observable effect was a false leftover warning. The shape criticism was still correct - a cleanup that reaches for a path it may not own needs its ownership stated, not inferred - so the guard stayed. Acceptance: none required. Recorded so the next reviewer of a sibling unit does not re-derive the overstated consequence, and so a future row about the same shape is judged on the shape, not on the effect that turned out not to exist.

Closed before registration (verified at head, do not re-derive)

ApplyPatchTool refreshing a stale observation: the hunk read now observes only when there is no prior entry or the prior version equals the token just read, which is the same stale policy ApplyDiffTool applies, and the comment at the site states why an older observation must not be refreshed.
PostCommitDurabilityError leaving the backup behind: the delete path catches it and unlinks error.backupPath before rethrowing.

Both verified by reading the two files at the head of the U9 branch on 2026-10-10, not from the queue entry.

fix: order cancellation against the guarded publish and session restart

test(diffview): keep the open() serialization test inside the lint su…

fix: order cancellation against the guarded publish and session restart

test(diffview): keep the open() serialization test inside the lint su…

fix(file-safety): pin the authorized ancestry before the commit rename

fix(safety, task): cover the directory-failure paths and release the …

Chain budget record - U3 / PR #1912 is over the executable-line cap: advisory, cap-class (planned-and-issued)

Gate line, verbatim (mutation-diff job 128607732918, step Enforce executable-line scope and run advisory mutation testing, PR head 1fb5fcc47; re-run at head 085fbf85e4ad):

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

Three things this record exists to make visible:

Cap-class, not a code signal. Nothing is being carried past the gate on a surviving mutant - the run never reached mutation. Its own log warns core Stryker preflight could not start: spawnSync /tmp/stryker-launch-…/node_modul…, which is the known Stryker 10 + @stryker-mutator/vitest-runner runtime-activation defect. The single advisory survivor the job prints, packages/core/src/value.ts:1: Survived BooleanLiteral mutant, is in a file outside this PR's 13-file delta; it is read as an artifact of that failed preflight. That reading is unverified, and deliberately not chased.
The newest commit does not move the number. 085fbf85e adds a JSDoc @internal tag to the primitive export so the production Knip view stops reporting it as an unused export. It is a comment: 589 before it, 589 after it.
A trim is not available inside this unit. 89 executable lines would have to come out of the extension package's delta. Composition of this delta by my own count (added lines from git diff --unified=0 main..HEAD, blank and comment-only lines excluded - 767 for non-test files against the gate's 589, so the gate's metric is narrower and stays the authority): src/services/file-safety/safeWriteText.ts 538, src/utils/safeWriteJson.ts 177, src/services/mcp/McpHub.ts 22, src/core/task/observationRegistry.ts 23, src/core/task/Task.ts 7, and 2,295 across the six test files (safeWriteText.spec.ts alone 1,466). The only blocks large enough to reach 89 are the primitive itself or its evidence. Cutting either is a decision about where the primitive and its evidence live, not a scope trim, and it collides with this unit's seam ruling - main's protocol owns the JSON write mechanism, our path-safety layer grafts onto it, and both feature sets stay in the unit that introduces them. So the split plan already recorded above (U1-U9, one issue per unit: U3 -> Tracking fws U3: observation registry with read completeness #2018) stands as the plan, the advisory stands with it, and if maintainers want feat(task): observation registry with read completeness (U3, #1375) #1912 under the cap the narrow exclusion is the cheap route - which needs a maintainer signature, not an author one.

Chain-level row to keep next to this one: #1912's src/utils/safeWriteJson.ts now carries main's replacement protocol plus our confinement layer grafted onto it - confinement before any mkdir, lock-key confinement, re-confinement under the lock, the ancestor/dir pins, identity-only re-verification (TargetMovedError, AncestorReplacedError), _copyTargetMode parity, and the POSIX-only best-effort parent-directory fsync that is a logged warning and never a PostCommitDurabilityError out of the JSON path. #1405 (U7) head aa44471e74 resolves that same file at its rows 3 and 7. Whoever lands first should expect the other to re-run the file, not re-decide it. The identity clause this unit depends on is written up on #2356 (comment 6716641767), which extends #2350 and leaves the not-yet-existing-basename half open on purpose.

Merge upstream/main (73931fe) into fws/u4-read-scope-recording

Reconcile the carried lock-key spec with the replacement protocol

Mark safeWriteText internal now that nothing in production calls it

Add the notice-separator row the review asked for on the clipped full…

Stage the confined write's bytes after staging, and copy the target m…

Read the staged bytes after staging, where the escape undo can use them

Copy the target's mode onto the staged file so the publish cannot wid…

Compare the interruption injection against identity so main's rows fi…

Split plan for PR #1914 (guard-core unit). Recorded here before any split PR or unit branch opens, per the repository's own remedy for an oversized unit.

Gate line being answered, verbatim from the mutation-diff run on head 33e9200:

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

No exclusion is requested. #1914 cannot be trimmed under the limit without cutting content, so it is a split; a maintainer-reviewed narrow exclusion remains the human's call, not an author's.

Measured denominators

All with the repository's own counting rules (scripts/stryker-diff.mjs) replayed locally at pinned node v22.23.4 against committed trees; a+d from git diff --numstat. At the current head 74f5eb2 (tree c57a9d64e6d6):

denominator	extension changed executable lines	over 500
cumulative vs main (merge-base `73931fe`)	1006	yes
non-cumulative over PR #1913's head `416179d` - this PR's own content	587	yes
first-parent delta `33e9200`..`74f5eb2` - what the gate's own base resolution reads for a merge head	18	no

All three are recorded because they answer different questions: 1006 is what a reviewer of this PR against main reads, 587 is what this PR adds beyond the write-protocol unit, and 18 is what the gate now happens to measure because the head is a merge whose first parent is the previous head. The split is planned on the content (587), not on the refresh artifact (18).

Per-file counts against main: guardedWrite.ts +394 (A), safeWriteText.ts +344 (A), safeWriteJson.ts +131, ReadFileTool.ts +40, fileLock.ts +34, McpHub.ts +27, observationRegistry.ts +20 (A), indentation-reader.ts +11, Task.ts +5. Against 416179d the first three shrink to guardedWrite +394, safeWriteText +156, fileLock +34, Task +3 because #1913 already carries the rest.

Proposed units and merge order

Declared order: main -> PR #1913 (open) -> U5-B -> U5-A. Each unit branch holds only its own content on top of its predecessor (non-cumulative), and each carries its own tests in the same commit as the code it tests.

U5-B - guard machinery in the writer (lands first): src/services/file-safety/safeWriteText.ts (+156 counted executable lines over 416179d), src/utils/fileLock.ts (+34), src/core/task/Task.ts (+3), with safeWriteText.spec.ts (474 additions / 388 deletions), fileLock.spec.ts (169 additions) and Task.spec.ts (50 additions). Totals: 193 counted executable lines, under the 500 limit; a+d 1753, above the 400 soft line - rationale: 693 of those additions are spec text for the same seam, and the seam and its tests may not be separated into different units.

U5-A - the guardedWrite tool (lands second): src/core/tools/guardedWrite.ts (+394 counted executable lines) with guardedWrite.spec.ts (1310 additions). Totals: 394 counted executable lines, under the limit; a+d 1999.

Partition arithmetic: 193 + 394 = 587, exactly the non-cumulative increment over #1913's head; 8 of 8 changed paths assigned, none to two units, none dropped. The order is forced by compilation, not taste: guardedWrite consumes the writer's guard API (the conditional no-replace commit, the authorized-directory pin, the already-authorized target), so U5-A cannot compile without U5-B.

Two questions this plan does not answer

Q1, for the code owner before U5-B's branch is pushed. Merging 416179d into this branch surfaced two mechanisms present in #1913's copy of safeWriteText.ts and absent from this branch's copy: (a) the (dev, ino) refusal of a staging path that is an alias of the target (416179d:341-356; 0 references tree-wide at 33e9200, source and specs alike), and (b) the refusal to publish when the target exists but its access rights could not be read (DaclInspectionError; 0 references tree-wide at 33e9200, and no spec in either lineage asserts the inspection-failure path). The refresh kept this branch's version for both, and recorded the consequence. Either they are deliberately superseded by the guard-core design, in which case the reviewer should see that decision, or they must be re-added in U5-B - roughly 10 to 15 counted lines each, which U5-B's budget absorbs (193 -> at most about 223 of 500). The plan does not decide this.

Q2, for a maintainer. U5-A's a+d (1999) exceeds the 1000 hard line. The only decomposition below it separates guardedWrite.spec.ts (1310 a+d) from guardedWrite.ts (689 a+d), i.e. splits a spec from the single file it tests. Either the spec is split into a state/contract pair inside U5-A, or the hard line is accepted with rationale for a spec that covers one file. Recorded, not decided.

Per-unit verification each unit must pass by once

Own committed tree; prettier --check; eslint --max-warnings=0 with src/eslint-suppressions.json never increasing; tsc -p src/tsconfig.json --noEmit; the vitest shards that own the touched paths (services for file-safety, core for tools/task, misc for utils); knip and knip --production with --treat-config-hints-as-errors; the knip-policy test; and the selector replay above run on the unit head before pushing, so a cap result is never inferred from a sibling unit.

Local results describe this tree only: pnpm 10.8.1 ignores the pnpm field in package.json, so a resolution difference is the first candidate when a local shard disagrees with a hosted leg.

Addendum to the plan above, from the mutation-diff run on the refreshed head 74f5eb2 (job 128631851758, log read, line quoted verbatim):

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

Two things this settles, and one it changes about the plan.

The gate measures the unit cumulative against the PR base, not against the previous head of the chain: the number printed, 1006, is the cumulative count I measured against merge-base with main, while the same head measures 587 against PR feat(read-file): record read scope and report clipping separately (U4, #1375) #1913's head 416179d and 18 against its own first parent 33e9200. The three-denominator table above stands; what matters for execution is that 1006 is the one the gate prints.
PR feat(read-file): record read scope and report clipping separately (U4, #1375) #1913's mutation-diff is green at 416179d (job 128630734515, conclusion success, no gate line in its log). The 499 measurement is what moved it, and no exclusion was requested to get there.

Consequence for execution, which the plan text above did not spell out: because the reading is cumulative, each unit can only come back green after its predecessor lands in main. U5-B measures 193 on its own but 499 + 193 = 692 while PR #1913 is still open, so U5-B's cap result is not decidable until #1913 merges; U5-A measures 394 on its own and is only decidable after U5-B merges. Declared order main -> #1913 -> U5-B -> U5-A is therefore load-bearing for the gate, not just for compilation.

Split executed locally; one amendment to the plan, measured.

Units built from #1913's head 416179d17de8, in the declared order, each with its own head and no cumulative parentage:

unit	branch @ head	files	a+d	gate lines (own delta)	cumulative vs main
U5-B (writer: guard machinery + the two refusals)	`vps2f-1914-u5b` @ `120cb057a9f0`	7	1637	191 / 500	638
U5-A (tool: `guardedWrite` + its spec)	`vps2f-1914-u5a` @ `9a981e261ce7` (parent `120cb057a9f0`)	3	2006	394 / 500	1032

Line counts are the repository's own scripts/stryker-diff.mjs rules (executable source lines, spec files not counted, import declarations excluded) at pinned node v22.23.4, measured on committed trees. "Own delta" is measured against the unit's parent, which is what a unit PR carries; "cumulative" is measured against 73931fe8be25, which is what the hosted gate prints for a PR whose base is main - the gate uses merge-base, so a stacked unit reads as the whole series until its predecessor lands. #1914's head 74f5eb287d1b printed 1006 for exactly that reason while its own first-parent delta was 18.

Plan amendment (this is new information, not a preference). Each unit has to carry one knip.json line, or the intermediate unit fails a required check:

knip is a required status check on main (Protect Main ruleset: check-translations, platform-unit-test (ubuntu-latest), platform-unit-test (windows-latest), compile, knip, e2e-mock). mutation-diff is not in that list, so the 500-line gate is advisory; a red there does not block merge.
knip --production does not credit imports from files knip itself reports as unused. guardedWrite.ts has no production consumer (3 non-test matches, the definition plus two doc mentions), so a unit holding the writer without the tool leaves the exported safeWriteText function with no reachable production importer and fails knip --production (Unused exports (1): safeWriteText ... safeWriteText.ts:439:23).
A writer-only unit cannot borrow the tool's entry line either: with --treat-config-hints-as-errors, an entry that matches no file is itself a failure (core/tools/guardedWrite.ts! ... Refine entry pattern (no matches)).
So U5-B declares services/file-safety/safeWriteText.ts! as a src entry and U5-A replaces it with core/tools/guardedWrite.ts!. Measured on the committed trees: both knip modes plus the 8 knip-policy tests pass on each unit alone. Neither unit changes knip.json's other settings, and U5-A's final config equals 74f5eb287d1b's.
The tool's only dependency on the writer unit is Task.cancellationGeneration, read at six sites in guardedWrite.ts; a variant with the writer and the tool but without Task.ts fails tsc with eight TS2339 errors (built as vps2f-1914-u5c to measure it, not kept). That is why Task.ts sits in the writer unit rather than the tool unit.

Per-unit gates (local, committed trees, pinned node v22.23.4): prettier, eslint (--max-warnings=0, run from src against the flat config), tsc -p src/tsconfig.json --noEmit, knip, knip --production, knip-policy 8/8. Tests: U5-B services 1686 passed with the one pre-existing environmental red (services/rules 5 failures, all EPERM ... symlink, a file neither unit touches), misc 2125 passed / 17 skipped, core 4282 passed (197 files - guardedWrite.spec.ts is not in this unit); U5-A core 198 files / 4344 passed / 9 skipped, identical totals to 74f5eb287d1b, services and misc unchanged from U5-B.

Q2 as ruled, recorded verbatim in substance: U5-A stays whole - the spec is not split from the one file it tests. Its size is 1999 added+deleted lines, the only subdivision being 1310 spec lines against 689 implementation lines; the alternative (splitting the spec from the file) is a diff a reviewer cannot read against the behaviour it pins. Acceptance of the 1000-line hard line versus a maintainer-reviewed narrow exclusion is the maintainer's call, with this measurement attached. Separate point, stated so the two gates are not conflated: the mutation cap is not a problem for U5-A once #1913 lands - its own delta is 394 counted lines and the whole increment over #1913's head is 587, while U5-A standalone is 394.

Nothing is held back waiting on me except the push: both heads are built and verified locally. Once the required legs are green on a head, what remains is code-owner review - a human.

Split plan for PR 1917 (U9, task-history delete). Recorded here before any split branch or split PR opens, per the repository's own remedy for an oversized unit.

Gate line being answered, verbatim from the mutation-diff run on the current head adb8187235187788989baf4db21a7bcc1005dfb8 (job 128638518710, step "Enforce executable-line scope and run advisory mutation testing"; the step before it, "Test mutation gate logic", passed):

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

The same step at the head before this unit's last four commits, c4bd9a2ec (job 128614757619), printed:

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

No exclusion is requested and none is taken: the enforcement was 1134 lines past its limit before those four commits, so an exclusion would hide pre-existing chain size rather than this unit's work, and a narrow exclusion is a maintainer's decision anyway.

Measured denominators

All with the repository's own counting rules (scripts/stryker-diff.mjs) replayed locally at pinned node v22.23.4 against committed trees; a+d from git diff --numstat. The selector is the script's own: packageForPath skips spec/test/visual files, .d.ts, config files and test-utils. files, and executableChangedLines walks the TypeScript AST, skips type-only nodes (imports, interfaces, type aliases, export declarations), counts an expression over its whole span and a statement at its first line.

denominator	extension changed executable lines	over 500
cumulative vs main (merge-base `73931fe`) at `adb818723518778`	1705	yes
cumulative vs main at `c4bd9a2ec` (before this unit's last four commits)	1634	yes
this unit's own content (the paths of U9-A and U9-B below)	316	no

The replay reproduces both hosted numbers exactly (1705 and 1634), so the plan is written on the composition of the count, not on an estimate of it.

Composition of the 1705, file by file under the same selector: guardedWrite.ts 401, DiffViewProvider.ts 338, TaskHistoryStore.ts 278, safeWriteText.ts 266, safeWriteJson.ts 113, ApplyPatchTool.ts 92, ClineProvider.ts 38, ReadFileTool.ts 35, ApplyDiffTool.ts 28, WriteToFileTool.ts 26, EditTool.ts 23, SearchReplaceTool.ts 23, EditFileTool.ts 16, observationRegistry.ts 14, jsonFileRecovery.ts 9, indentation-reader.ts 4, Task.ts 1. Those 17 files sum to 1705. Of that total 1389 lines belong to units that already have their own open PRs on this chain - U1 (PR 1910), U2 (PR 1911), U3 (PR 1912), U4 (PR 1913), U5 (PR 1914), U6 (PR 1915), U7 (PR 1918), U8 (PR 1916) - where they are already being reviewed; 316 lines are this unit's own (278 in the store plus 38 in the provider).

The gate reads cumulative because this unit branch is built on predecessors that have not landed and the gate resolves the base through merge-base against main, so a stacked unit reads as the whole series until its predecessors land. That is the same mechanism recorded above for PR 1914, where the cumulative read was 1006, the unit's own content 587, and the first-parent delta 18.

Proposed units and merge order

Declared order: main -> U1 through U8 in the order already recorded for the chain -> U9-A -> U9-B. Each unit branch holds only its own content on top of main (non-cumulative) and carries its own tests in the same commit as the code it tests.

U9-A - delete, certify and retarget a history deletion in the store (lands first; PR 1917 is restacked to carry exactly this content):

src/core/task-persistence/TaskHistoryStore.ts - 338 counted executable lines (corrected twice on 2026-11-08: it read 278 when this plan was written, then 329 after the three pre-split commits 25e4fb8f8, bc352ea07 and 2281ac082, then 338 after 60f9c8c1c; the same selector every time);
src/core/task-persistence/index.ts - exports TaskHistoryDeleteError next to TaskHistoryStore; a type-only export line, so it counts 0;
src/core/task-persistence/__tests__/TaskHistoryStore.deleteSemantics.spec.ts - 31 tests, 1310 lines, of which 25 tests and 1009 lines are this unit's growth over main (main has 6 tests, 301 lines).
Totals: 338 of 500 counted executable lines at 6093d2a07 (278 at writing, 329 at 0fae602e2; see the dated notes below); a+d 1574 measured at writing.

U9-B - the provider's batch delete call-site and the artifacts that belong to a deleted task (lands second):

src/core/webview/ClineProvider.ts - 38 counted executable lines: the batch call-site switched on the store's per-id verdict, and the artifact cleanup (shadow-checkpoint repository, task directory, post-state) that has to run in the same order the store's verdicts report;
src/core/webview/__tests__/ClineProvider.partialTaskDelete.spec.ts - new, 119 additions;
src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts - 6 additions.
Totals: 38 of 500 counted executable lines; a+d 217.

The order is forced by compilation, not taste: U9-B adds TaskHistoryDeleteError to the barrel import block at ClineProvider.ts (main already imports TaskHistoryStore from that barrel at ClineProvider.ts:146) and switches on that error type per id, so U9-B does not type-check against main until U9-A has landed. U9-A needs nothing from U9-B.

Partition arithmetic: 278 + 38 = 316, this unit's own content; 316 + 1389 = 1705, the 1389 being the chain-carried remainder that already includes Task.ts (+1) and observationRegistry.ts (+14). All 17 counted files are assigned, none to two units, none dropped. Task.ts (+1 counted line) belongs to U7 (PR 1918) and is not moved here, and observationRegistry.ts (+14) belongs to U3 (PR 1912).

Rulings on the open points, recorded as given

The points left open by the plan above were ruled on 2026-11-08, before any split branch exists. They are recorded here in the words they were given in, so the split is executed against them rather than re-argued at branch-cutting time.

One: U9-A stays whole; the size is the maintainer's choice, not an argument. A unit that lands an implementation in one PR and the 1083 lines of tests for that one file in another is reviewable in neither, and the subdivision seam measured above - 14 tests and 442 lines against 8 tests and 295 lines, plus this unit's 7 tests and 312 lines - is a line-count seam, not a behavioural one. So U9-A is planned whole: 1574 added+deleted lines carrying 278 of 500 counted executable lines, its spec travelling with the one file it tests. What is left to a maintainer is the choice between accepting that size and a maintainer-reviewed narrow exclusion, and this paragraph plus the numbers above are what that choice should be made on. This is the ruling already given for U5-A on this issue, applied to these numbers.

Two: the epic-closing line moves only when the split opens. Until a split PR exists, PR 1917 keeps the epic-closing line exactly where it is. When the split opens, the line goes onto whichever unit merges last, so the epic cannot close before the last of the content that closes it has landed. Recorded here so that nobody has to rediscover it when the branches are cut.

Three: suppressions are attributed per earning unit. Each pruned entry across the 0 additions and 125 deletions this head shows in src/eslint-suppressions.json is credited to the unit whose own change earned it - a shared file credited to a single unit is how a count looks earned by the wrong content - and no unit raises a count.

And for the split PR bodies. Each split PR body states both denominators for its own head: the count the gate prints cumulatively against main, and the count the unit itself carries. For these two units that is 1705 cumulative today against 316 of U9's own content (278 in the store, 38 in the provider), with the 1389 remainder living in the eight unit PRs listed above. A reader who sees a four-figure number on a unit branch will otherwise read it as the unit's own size, and that misreading is what makes an inside-the-limit unit look outside it.

Per-unit verification each unit must pass by once

Own committed tree; prettier --check; eslint --max-warnings=0 run from src with src/eslint-suppressions.json never increasing; tsc -p src/tsconfig.json --noEmit; knip and knip --production with --treat-config-hints-as-errors plus the knip-policy tests; the vitest shards that own the touched paths (core for task-persistence and webview); and the selector replay above re-run on each unit head before pushing, so a cap result is never inferred from a sibling unit. Expected, to be measured rather than assumed: U9-A keeps a reachable production importer on its own, because main's ClineProvider.ts already imports the store through the barrel, so no new knip.json entry line should be needed - the U5 split above showed a unit can need one, so it is checked on the unit tree.

State at recording time

Nothing is split yet: no split branch and no split PR exist for this unit. PR 1917 stands at head `adb8187235187788989baf4db21a7bcc1005dfb8`; its 21 hosted legs, read across both pages of the check-runs endpoint, are 19 success, the mutation-diff failure quoted above, and CodeRabbit still running at recording time. Both platform unit-test legs (ubuntu and windows) and the extension-host visual leg completed success at this head. The mutation advisories inside the failing step (the core Stryker preflight failing to spawn, the job summary writer hitting a directory, and one survivor advisory at `packages/core/src/value.ts`) are identical at both heads and are recorded in the sweep log; they are outside this plan's scope. No local Stryker preflight has been run for this unit.

Correction to the size figures in this plan, 2026-11-08, measured with the gate's own selector over the same base (73931fe8b). Three commits landed on the U9-A branch after this plan was written - 25e4fb8f8 (name the lock-key re-derivation when it fails under a held lock), bc352ea07 (re-derive the lock key inside the certification and scan both spellings), 2281ac082 (keep a duplicate id's entry when its later attempt fails after a removal) - and they move the numbers a reader of the split will want:

TaskHistoryStore.ts: 329 counted executable lines, not 278. U9-A is still inside the 500 line cap, so nothing about the plan changes - U9-A is still planned whole, its spec travelling with the one file it tests.
U9's own content: 367 (329 in the store plus the 38 in the provider), not 316.
Cumulative over main at head 0fae602e2: 1756 counted executable lines, of which 1427 are chain-carried, against the 1705 / 1389 this plan printed at writing time.

The figures in the body above are left as they were written, because they are measurements of the composition at that moment; where a number is quoted as U9-A's own budget it now reads 329. A plan that understates by 51 lines gets believed, so the correction is stated here rather than silently absorbed.
Second amendment, same day, same selector, head 6093d2a07. Two more commits landed on this unit after the correction above: 60f9c8c1c (keep a duplicate id's served entry through a second success) adds 9 counted executable lines to TaskHistoryStore.ts - 329 to 338 - and 6093d2a07 (assert the values a duplicate-id restore puts back) is spec-only and adds 0. So U9-A now counts 338 of 500, U9's own content is 376 (338 in the store plus 38 in the provider), and the cumulative figure over main on the pre-split branch is 1765 against the 1756 quoted above. The unit is still inside the cap and the split plan above is unchanged by this; only the numbers move, and the numbers are the part a reader of the split relies on.

Work-order note (chain #1991 tracking, no change to declared merge order).

The declared merge order for the fws units stays U1 U2 U3 U4 U5 (with U5-A #2359, U5-B #2357) -> U8 -> U6 -> U7 -> U9. The refresh order for the two dirty units is now U8 (#1916) first, then U6 (#1915), Lead-confirmed.

Reason: U6's copies of the diff-view content (DiffViewProvider.ts and its spec) are defined by U8's unit content. #1916 is itself dirty/has-conflicts because main advanced 212 files past the shared unit base 3ee042d91 (incl. #2297 safeWriteJson trace/recovery, #2309 any-removal rewrite of DiffViewProvider.spec.ts, #1309). If U6 were refreshed first, its DiffViewProvider resolution would be cut against unreconciled U8 content and then re-cut after U8 settles - two versions of one rule in one series. Refreshing U8 first means every copy source that U6 pulls (and later U7/U9) is settled before anything copies from it.

Copy sources for the refresh (measured, Lead-signed): safeWriteText.ts + its two specs <- 25a1ad65f (post-review U5-B head); guardedWrite.ts + spec <- 025fb7d0d (U5-A/#2359 head); safeWriteJson.ts + safeWriteJson.test.ts + lockKey spec <- 74f5eb287 (U5 head, already carries #2297 trace/recovery); observationRegistry.ts + specs <- 085fbf85e (U3); ReadFileTool.ts/indentation-reader.ts + specs <- 416179d17 (U4). Suppression-file rule: take main's eslint-suppressions.json first, then eslint-verify - never a prune carried across a merge.

Merged-tip measurement that unblocked the U5 pair is logged in the sweep ledger: at probe commit dcbf1b2b5 (U5-B + U5-A) both knip modes exit 0 in the real installed environment - the guardedWrite entry is a real consumer, U5-A's entry-line deletion is safe for the queue.

Label state: author-side versus admin-side (measured this pass)

Of the 168 open PRs in this repo: awaiting-author 110, stale-awaiting-author 25, awaiting-coderabbit 33, awaiting-maintainer 11, has-conflicts 44.

PRs authored by me that still carry awaiting-author (34): #2359, #2357, #2149, #1932, #1931, #1930, #1924, #1921, #1918, #1917, #1916, #1915, #1913, #1912, #1911, #1763, #1653, #1652, #1567, #1565, #1562, #1557, #1550, #1538, #1537, #1523, #1522, #1521, #1408, #1405, #1403, #1394, #1384, #1311.

On the following ones the review threads were re-read thread-by-thread at the live heads and every unresolved thread already carries an author reply, so nothing there is waiting on the author (15): #2359, #2357, #1930, #1917, #1916, #1915, #1913, #1912, #1911, #1565, #1557, #1550, #1408, #1405, #1403. Their mergeable state is blocked on required approvals, not on the author.

I cannot move the label myself. Removing a label is admin-gated for this account, measured on seven PRs — three authored by other contributors (#1523, #1521, #1394) and four of my own (#1917, #2357, #2359, #1913). Each DELETE /issues/N/labels/awaiting-author returned 403 with the body Must have admin rights to Repository. and the label was still present when the issue was re-read.

Least-effort order to make the label true:

An account with label rights drops awaiting-author on the PRs listed above, or applies the repo's existing stale-awaiting-author marker as it does on other PRs.
A human approving review including one from a code owner (taltas, navedmerchant, edelauna, JamesRobert20) — that is the gate actually blocking merge on all of them.
Admin re-run of the zombie Actions jobs: a pushed head re-triggers the legs, an already-green head does not.

On the ones I have not re-read this pass I am not asking for anything (19): #2149, #1932, #1931, #1924, #1921, #1918, #1763, #1653, #1652, #1567, #1562, #1538, #1537, #1523, #1522, #1521, #1394, #1384, #1311.

Correction to the previous comment: I wrote that three of the seven label-removal attempts were on PRs authored by other contributors. That was wrong. #1523, #1521 and #1394 are also authored by me, so all seven attempts — #1523, #1521, #1394, #1917, #2357, #2359, #1913 — were on my own PRs, and all seven returned 403 Must have admin rights to Repository.. That makes the finding stronger rather than weaker: label removal is admin-gated for the author on this repo, not merely on third-party PRs.

[Tracking] guarded-write chain (epic 1375): rules, port sources, chain-level rows #1991

Description

What belongs here

What does not belong here

Writing rules for this issue

Unit tracking issues (fws chain)

Decision requests

One canonical write lock, or an accepted residual window

Chain-level defect classes

1. A rollback that persists unapproved streamed content (defect class, not one call site)

2. A confined publication must pin the directories the write itself created

3. Fail-closed DACL restore must reach the old-generation copies of the publish primitive

4. Publish must not follow a swapped ancestor link when a caller asks for symlink refusal

5. The symlinked-ancestor walk is not bounded by the authorized scope root (owed on U7, PR 1405)

6. A shared fixed staging-directory name lets a workspace-shipped symlink redirect the staging chmod and file

7. Empty declared root: reject semantics are canonical

8. U9 teardown: the waiter cannot tell whether the pass it waited for succeeded (PR 1917 / PR 1918 - fix once and port)

Owed changes and ports

Stop accepting a bare staging path (U6, PR 1915)

Port source: folding a lock key the way the lock is placed (owed on U8, PR 1916)

Ownership rulings

The safeWriteText primitive has one owner chain (registered for U4, PR 1913)

Registrations that are not defects (U3, PR 1912)

Closed before registration (verified at head, do not re-derive)

Activity

easonLiangWorldedtech commented on Nov 8, 2026

Chain budget record - U3 / PR #1912 is over the executable-line cap: advisory, cap-class (planned-and-issued)

easonLiangWorldedtech commented on Nov 8, 2026

Measured denominators

Proposed units and merge order

Two questions this plan does not answer

Per-unit verification each unit must pass by once

easonLiangWorldedtech commented on Nov 8, 2026

easonLiangWorldedtech commented on Nov 8, 2026

easonLiangWorldedtech commented on Nov 8, 2026

Measured denominators

Proposed units and merge order

Rulings on the open points, recorded as given

Per-unit verification each unit must pass by once

State at recording time

easonLiangWorldedtech commented on Nov 8, 2026

easonLiangWorldedtech commented on Nov 8, 2026

Label state: author-side versus admin-side (measured this pass)

easonLiangWorldedtech commented on Nov 8, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions