Repository navigation
safeWriteText's staging-name guard and jsonFileRecovery's leftover prefixes are mutually exclusive #2350
Description
Two write-side contracts in this repository currently disagree about the name of a staging file, and the disagreement is unresolvable from inside a PR that has to satisfy both.
The two contracts, as they read today
- The guarded-write primitive refuses any caller-supplied temp path whose basename does not match its own staging convention —
src/services/file-safety/safeWriteText.ts(present on PR branches, absent at main73931fe8be):
// Ownership convention: a caller-staged file must carry the staging name this
// library's own writers use (.new_<ts>_<rand>.tmp). An arbitrary regular file
// beside the target (e.g. credentials.json) passes every check above, yet
// renaming it onto the target publishes an unrelated file's content and deletes
// its original name. Only files created under the staging convention may be
// published through this parameter.
if (!/^.new_.*.tmp$/.test(path.basename(supplied))) { // :405
throw new StagingPathError(
`Staging file must carry the staging name convention .new_*.tmp, got ${path.basename(supplied)}`,
supplied,
)The behaviour is pinned by its own test: safeWriteText.spec.ts:1696 it("rejects a caller-staged file that does not carry the staging name convention"), asserting the message at :1705.
- Main's JSON recovery recognises leftovers only by a target-prefixed name —
src/utils/jsonFileRecovery.ts:9-19:
function getTempFilePrefix(absoluteFilePath: string, kind: "new" | "bak"): string {
return `.${path.basename(absoluteFilePath)}.${kind}_`
}
function isReplacementLeftover(absoluteFilePath: string, entry: string): boolean {
return (
entry.endsWith(".tmp") &&
(entry.startsWith(getTempFilePrefix(absoluteFilePath, "new")) ||
entry.startsWith(getTempFilePrefix(absoluteFilePath, "bak")))
)
}Why they are mutually exclusive for a caller-staged file
For target settings.json the two accepted shapes are:
| who accepts it | basename | verdict |
|---|---|---|
| recovery only | .settings.json.new_1730000000.tmp |
rejected by the primitive — basename does not start .new_ |
| primitive only | .new_1730000000_a1b2c3.tmp |
invisible to recovery — does not start .settings.json.new_ |
So a caller that stages a file beside the target can satisfy the primitive or be recoverable, not both. This is not theoretical: passing main's own recovery-recognised staging name through the primitive fails its guard with Staging file must carry the staging name convention .new_*.tmp, got .settings.json.new_…tmp (safeWriteText.ts:406-409), which is how 48 tests failed in a conflict resolution that tried to keep main's name for main's recovery.
The reconciliation being used in #1911, and the gap it leaves
The resolution on #1911 stages .new_<basename>_<ts>_<rand>.tmp — anchored so the primitive accepts it, and target-scoped so recovery can never unlink another target's in-flight staging file in the same directory — and extends isReplacementLeftover with a third accepted prefix derived from the target basename. Two things follow that a maintainer should decide on rather than inherit:
- Recovery's accepted set grows from two prefixes to three. It is pinned by tests for the new prefix plus a negative test that a bare
.new_anything.tmpis still not collectable, but the boundary is now a policy choice, not an accident. - Direct
safeWriteTextcallers are still not covered. A unit that writes text through the primitive creates.new_<ts>_<rand>.tmpnames that are not target-scoped, so recovery still will not garbage-collect its leftovers after a crash. Guarded writes in the same series that go through that path therefore leave files a restart cannot clean up.
Asks
- Decide which convention wins, once, in one place: either the primitive accepts the target-prefixed name, or recovery accepts the unscoped name, or both derive the name from a single exported helper (e.g. one
stagingNameFor(targetBasename)used by writer and recovery alike). Today a PR has to invent a third answer. - Pin the accepted-prefix set with a test in
jsonFileRecoverycovering all accepted shapes and at least one rejected look-alike, so the next writer cannot widen it silently. - Note the divergence this creates on the file itself:
src/services/file-safety/safeWriteText.tscurrently exists in four independent, non-ancestor shapes across open PRs — 819 lines at1f8ae7fd0a(feat(file-safety): guarded text publish module + specs (A4, #1375) #1395), 833 lines at0580426aa2(feat(file-safety): canonical advisory lock key (U2, #1375) #1911's head), 446 lines ataa44471e74(feat(tools): guarded write CAS core with per-path FIFO chain (S4a, #1375) #1405), 469 lines ate9490e555d(feat(tools): wire guarded writes into the diff-view save paths (S4b, #1375) #1408) — and main has no version of the file at all. Whichever merges first sets the convention every later PR must satisfy, so this naming decision is best taken before the first of them lands.
No code change is proposed here; this is the record of a contract conflict measured while resolving merge conflicts, filed so the decision has one home.
Activity
fix(safe-write): compare directory identity in the safeWriteText stag…
Merge upstream main 73931fe into fws/u9-task-history-delete
Merge upstream main 73931fe into fws/u9-task-history-delete
Merge upstream main 73931fe into fws/u9-task-history-delete
fix(utils): keep the pre-commit ancestry assertion out of the EPERM r…
Merge upstream main 73931fe into fws/u9-task-history-delete
Mark safeWriteText internal now that nothing in production calls it
Copy the target's mode onto the staged file so the publish cannot wid…
easonLiangWorldedtech commentedon Nov 8, 2026
Series note carried here from #1913 (comment 6717029801), because #2350 decides which primitive main
keeps and that decision is what makes this fix land main-side or in eight lanes.
Measured at #1913 head
8fa7d858c:platform-unit-test (windows-latest)failed with 12 failures inmain's
src/utils/__tests__/safeWriteJson.interruption.test.ts(blobc9c5993f847b…at73931fe8), allexpected 'WRITE_COMPLETED\n' to contain 'EXIT_AT_REPLACEMENT_RENAME'at its line 187, while ubuntu wasgreen. Cause measured on pinned node v22.23.4: the spec's kill point is whole-path equality (child line
40); a lane that grafts path safety publishes to a realpath-resolved target (
fs.realpathexpands anNTFS 8.3 alias,
path.resolvepreserves spelling); on a runner whose TEMP is an alias the injectionnever matches and every row in the file fails. Reproduced locally by pointing TEMP/TMP at
C:\work\ZOO-CO~1\TMP-AL~2\LONG-D~1(12 failed, CI's assertion verbatim) and back at the long spelling(12 passed).
Consequence for the naming/decomposition decision: the fix's canonical home is main-side. Today it exists
in two lane copies (
1fb5fcc47on #1912,f739a5fa3on #1913) and every lane that takes the seam mustcarry it or be red on windows until main does. Merge order decides which text becomes canonical.
Merge the landed u4 head into u5 and record each conflict resolution
Merge the landed u4 head: main's write protocol plus our path-safety …
easonLiangWorldedtech commentedon Nov 8, 2026
Requirement for the guard-core writer, recorded from the #1914 refresh merge (split unit U5-B: branch
vps2f-1914-u5b@120cb057a9f0, builds on #1913's head416179d17de8).The guard-core writer has to carry two refusals. Neither is a rival design; both are the consumer-side application of contracts already ruled on, and I am recording them here because dropping them was the alternative.
(dev, ino)read as bigint stats (file identities on NTFS/ReFS exceedNumber.MAX_SAFE_INTEGER, so a rounded number either hides an alias or rejects a valid staging file). OnlyENOENTmay skip the comparison;EACCES/ELOOP/ENOENT's neighbours mean the comparison could not be made, and that is itself a refusal. Why it cannot be a log line: an alias that reaches the commit leaves the failure handler unlinking the only remaining copy of the content, so a failed write deletes the file it was meant to protect. This is Advisory lock key must be identity-backed, not realpath text: an NTFS 8.3 alias and its long form take different locks #2356's canonical-after-write identity rule applied on the consumer side.icacls /savefails, including after writing a partial dump), or whose access rights cannot be read, must yieldDaclInspectionErrorcarryingphaseandtargetPathbefore anything is committed - not a warning beside a committed file. Publishing without knowing the target's access rights can widen who reads the file, which is the same reasoning behind the_copyTargetModeruling in fix(task-persistence): delete under the canonical lock key (U9, #1375) #1917.Measurement behind the "silently lost" claim:
target itselfandDaclInspectionErroreach return 0 matching lines tree-wide at33e9200900bb(the commit the refresh merged into), so under guard-core's shape the absence would have looked like agreement.Both arrive with tests that exercise the refusal paths: the alias test, the non-
ENOENTreason test, the failed-save test (partial dump unlinked, no rename, oneicaclsattempt), and the inspection test. The inspection test injectsplatform: "win32"and stubsicacls, so it executes on either hosted leg rather than only on the windows one; the hosted windows leg passessafeWriteText.spec.ts. Two pre-existing tests that asserted the opposite behaviour were replaced rather than kept, and that is called out in the unit's commit message.Reversibility: if #2350 rules otherwise for either one, it comes out in a single commit on
vps2f-1914-u5b.easonLiangWorldedtech commentedon Nov 8, 2026
The generation map behind the safeWriteText/guardedWrite convergence (from the #1916 U8 resolution, vps2-f lane)
safeWriteTextexists in incompatible generations;guardedWriteexists in three. Which copy a branch carries is decisive for what can pair with what. Map as shipped on the stack today:retainedTargetoption, ACL/DACL capture + windows publish branches, exportsresolveLinkPathLockKey; json predates main's #2297 trace contractcancellationGenerationlineage,backupoption, noretainedTarget, noresolveLinkPathLockKeyexport; json carriesSafeWriteJsonTraceStage+trace(what main-sidetaskMessages.tsconsumes)canonicalizeForApproval(U7 containment)GuardStaleVersionErrorCAS contract +create/update/editkinds; publishes with{ backup: true };canonicalDirKey/resolveLinkPathLockKeytransplanted as module-local helpersPairing rule (adopted, not advisory): json + its specs and text + its specs travel as ONE generation, always. A branch may adapt a generation's consumers; it may not mix one generation's text with another's json and call the result converged.
The experiment that proves the rule: on #1916 I built the chimera - era
safeWriteTexttrio + 74f5-erasafeWriteJsonquartet - to keep the era surface the era call-site cluster expects. It passedtsc --noEmit, the full ubuntu unit legs locally, and every misc shard locally and on ubuntu CI. It failed only onplatform-unit-test (windows-latest)misc shards: 15 tests insafeWriteText's windows-only branches (3 backup-stat rows, 12 interruption kill-timing rows expectingEXIT_AT_REPLACEMENT_RENAME), because the era windows ACL/publish branches re-route the fs-mock expectations the 74f5 specs encode and shift interruption kill timing. (Code QA run 42459507387, windows job 128665547657.) A reviewer looking at the diff would have attributed that red to the change; it was the pairing. Pinning json+text together at 74f5 and renaming the 43 mock-argument expectations (retainedTarget: true->backup: true) turned the same suite green on every platform.Convergence consequence (#2357 review input): the convergence target is the 25a1a generation, and the sequencing is a human review call, not another push. The U8 tree is queue-proof for it:
guardedWriteneeds onlybackup/resolveLockKey/resolvePublishTarget/PostCommitDurabilityError- all present in the 25a1a generation - and its era-only helpers are already module-local, so once #2357 lands the guardedWrite import swap is mechanical. Taking 25a1ad6 into U8 before #2357 lands was rejected deliberately: it would import the U7-era containment shape mid-stack and re-open pairings already merged (U1-U5).Numbers stated with their bases so they can be compared: local changed-executable-line recount vs the pre-merge branch base 2e00da1 gives types 15/500, webview 401/500, extension 1190/500 on 711ada7; the CI mutation gate computes against the merge base (main, 73931fe) and its reason line reads extension 1797/500 - both over-cap, one advisory class either way.
Merge upstream main (73931fe) into fws/u6-apply-patch-wiring
easonLiangWorldedtech commentedon Nov 8, 2026
Follow-up to the four-generation table above — the CI discriminator fired on the windows leg and it refined the json row, so the table needs this addendum for anyone reviewing the convergence:
What failed.
711ada709(then-final #1916) failed 2 rows on the windows platform-unit misc shard with the reason lineAssertionError: promise resolved "undefined" instead of rejectingatsafeWriteJson.test.ts:675(run 42461215177, job 128670729075). The same row family was already red on74f5eb287's own windows CI — run 42448302148, job 128631851822, 3 failed rows, same file. That rules generation mixing out: json and text were pinned to the same generation there (74f5eb2), and the chimera's 15-row era-text class was gone. The defect is inside the 74f5 generation's test double: the backup-stat injection keyed onos.tmpdir()'s spelling of the leftover path while the writer stats the canonical form of the same file — on windows realpath spells them differently, so the injection missed and the write resolved instead of rejecting.The fix and its provenance.
025fb7d0d(#2359 head) carriessafeWriteJson.tsbyte-identical to74f5eb287(verified via git diff) with a hardenedsafeWriteJson.test.ts(+31/−9: injection keyed on the leftover's dev+ino instead of path spelling, and the fs.stat mock implementation restored in afinallybecause the OS recycles inodes). Its windows platform-unit passed this exact test against the identical implementation — run 42455872567, job 128654644966.What shipped. Test-only adoption across the three carriers, each named in its commit message with the copy source:
281c24565onfws/u5-guard-core(fix commit, parent74f5eb287)680bdc2d142a1bbb86All three now carry a byte-identical copy of that test file, and #2357/#2359 already carried it, so the whole stack holds exactly one copy. The pairing rule from the table above stands untouched — json and its specs travel as one generation; what changed is that the 74f5 json generation is now paired with its own windows-proven spec instead of the version with the path-spelling bug. A full pin bump to
025fb7d0dwas rejected deliberately: it would drag #2359'ssafeWriteText.ts+64/−4 (and the rest of that unit's content) into ancestor branches across the unit boundary, while the json implementation is identical — test-only keeps merge order intact. The generation rows (era /74f5eb287/25a1ad65f/ U8-guardedWrite) and the four-generation pairing rule are unchanged.easonLiangWorldedtech commentedon Nov 9, 2026
Split plan for the 2357 head — six units, measured, not yet opened
This issue is the plan's home rather than #1991 or #2357: #1991 is the guarded-write chain and never mentions 2357, and #2357 is the pull request under measurement. #2350 is the right home because it is the defect about the interaction between the plan's first two units — the writer's staging-name guard here (
services/file-safety/safeWriteText.ts) and the recovery scan's leftover prefixes (utils/jsonFileRecovery.ts) — and that defect constrains the order below.The units. Built locally from the head tree onto this chain by plumbing (
commit-tree+update-ref), nothing pushed, nothing opened. Boundaries were drawn on the measured import edges among the changed production files —safeWriteText←safeWriteJson←McpHub←Task→ReadFileTool→indentation-reader, andjsonFileRecovery→fileLock— not by dividing the file list. Each unit's counted executable lines come from the gate's ownselectFromGitagainst its own parent:refs/tmp/2357-cut/…)u1-writer4b8398f453ee0a3542660935f208ce7425fe3fe1safeWriteText.ts, its 2 specs,knip.jsonu2-lockkey5e9d5a188c2a1f3878bd7db36a95ef2f7c22912cfileLock.ts,jsonFileRecovery.ts, their 3 specsu3-jsonwriter473cacf7772af3287a49e88387f866c6ae93eef5safeWriteJson.ts, its 3 specsu4-mcp72a6f5685e29d737567809a7eaaba8c8496c1991McpHub.ts,McpHub.spec.tsu5-task3c4e158b9d943d1995948a810b4c106b992a18adTask.ts,observationRegistry.ts, 3 specsu6-read0070f2be96a9819be7293034d002ae03a4b67fa4ReadFileTool.ts,indentation-reader.ts, 3 specsThe sum is 770, which is what the whole head reports from
5319fe91f, so nothing is double-counted or dropped. The disproved candidate — the whole head as one unit — is kept as a ref (refs/tmp/u5a-2357-unit-head = d6fa7117e07b70455da24e4722b47eb0b202fa6e), because 770 against a 500 cap is the fact that motivated the split.Merge order, and the constraint #2350 imposes. Quoting both files rather than paraphrasing the defect. Recovery recognizes a leftover by
getTempFilePrefix, defined atutils/jsonFileRecovery.ts:9-11as`.${path.basename(absoluteFilePath)}.${kind}_`and applied at:13-19(entry.endsWith(".tmp") && (startsWith(…"new") || startsWith(…"bak"))), scanning the target's own directory at:33/:43and under the lock at:155-171. The writer in this unit stages a different spelling:_tempNameatservices/file-safety/safeWriteText.ts:365-367produces"." + prefix + "_" + Date.now() + "_" + rand + ".tmp"— no target basename — and:369-373puts it inside a private per-write staging sub-directory, not beside the target. So the two spells are mutually exclusive in both respects: the name and the directory. That is #2350, restated with anchors.Whether the split can land that state depends on wiring, and the wiring is measurable: at this head there are two mentions of
safeWriteTextoutside its own file and its specs —utils/fileLock.ts:53(a comment pointing atresolveLockKey) andutils/safeWriteJson.ts:6(import { resolveLockKey, resolvePublishTarget } from "../services/file-safety/safeWriteText") — and zero at the merge base. No production code in this diff publishes throughsafeWriteText, andsafeWriteJsonstill stages its own leftovers with the recovery-visible spelling (safeWriteJson.ts:97-102, applied at:371and:420), which is textually identical at the merge base and at this head. Therefore:safeWriteJson.ts:6importsresolveLockKeyandresolvePublishTargetfrom it; u3 also must merge after u2, because it callsrecoverInterruptedReplacementand locks throughacquireFileLock/resolveLockKeysemantics that u2 defines.safeWriteText. It has to land with whichever change first publishes throughsafeWriteText, by teaching recovery that staging spelling and sub-directory or by making the writer stage with the recovery-visible prefix beside the target. Saying the split preserves behaviour is only true on that condition, and the condition is stated here rather than assumed.path.resolve's parent — costs +2 counted executable lines (86 to 88 in u2's terms; 770 to 772 on the whole head) and belongs to u2 at 88, where it has room against the 500 cap. It is not free at the later unit that also carries this file (1,283 to 1,285).One thing deliberately unassigned:
src/eslint-suppressions.json, the 26th file in the diff. Its per-file counts belong to files owned by several units, so it is partitioned at split time and not dumped on one unit.Verification disclosure, because a mapped typecheck means nothing without it. The typechecks above ran in a worktree whose
src/node_modulesis a junction farm resolving through exactly one root,C:/work/Zoo-Code-fork/wt-r119-2235/node_modules, with 135 distinct packages. Of the units verified so far, u1 and u2 import no@roo-codepackage at all (u1's own files import onlychild_process,fs,fs/promises,os,path; u2's importfs,fs/promises,os,path,proper-lockfile), so the donor junction cannot be typechecking a different copy of the unit under test and their exit 0 counts as-is. u3 is in the same position (json-stream-stringify,stream,url,module, and the node builtins). u4 (@roo-code/types), u5 (@roo-code/cloud,@roo-code/telemetry,@roo-code/types,@roo-code/types/provider-identifiers) and u6 (@roo-code/types) do import workspace packages, so their verification has to run with the overlay that points@roo-codeinto the unit under test; a plain junction would type them against the donor, and an exit 0 from that would be the 25-phantom-error failure mode rather than a result.Nothing opens before this plan is agreed here. Pointer comments pointing to this issue have been left on #2357 and #2359.
easonLiangWorldedtech commentedon Nov 9, 2026
Named obligation, unit u3, test-side only:
vi.unmock("proper-lockfile")insrc/utils/__tests__/safeWriteJson.test.tsis hoistedPosting it here so it cannot be lost in a lane queue: another lane inherits u3, and a queue entry in someone's scratch notes is not a durable home.
The row.
src/utils/__tests__/safeWriteJson.test.ts:404callsvi.unmock("proper-lockfile") // Ensure the mock is removed after this test, inside the test that starts at:384(vi.resetModules(), thenvi.doMock("proper-lockfile", …)at:392, then the dynamic re-import at:398). Vitest says so itself, verbatim, during the pinned run of u3 (node v22.23.4,node_modules/vitest/vitest.mjs run --config vitest.misc.config.ts --no-file-parallelism utils/__tests__/safeWriteJson.test.ts utils/__tests__/safeWriteJson.interruption.test.ts utils/__tests__/safeWriteJson.lockKey.spec.ts, exit 0, 3 files / 69 passed | 4 skipped):Why it is latent rather than broken. Hoisting means the call runs at module evaluation, before the
vi.doMockat:392, so it currently undoes nothing: the test still gets the mocked module through:384vi.resetModules()plus its ownawait import("../safeWriteJson")at:398, and the leak the comment intended to prevent is handled by the other tests' ownvi.resetModules()at:887,:1136and:1157. That is why the file passes today. The day vitest promotes the warning to an error, the file stops collecting and the whole misc lane on it fails.The file already contains the right shape, 55 lines later. In the compromise-lock test's
finallyblock (:957-965) the author wrote the reason out: "Cleanup must run even when an assertion fails: a leaked mock registration or console spy changes later tests.vi.doUnmockis the non-hoisted counterpart of thevi.doMockabove;vi.unmockis hoisted to the top of the file, so it would not undo this registration from here", and then doesvi.doUnmock("proper-lockfile")at:963followed byvi.resetModules()at:964. So the fix for:404is that same pair — replace the hoistedvi.unmockwithvi.doUnmock("proper-lockfile")+vi.resetModules()— and not what the warning suggests literally: moving thevi.unmockto the top level would hoist it above thevi.doMockat:392and change what later tests in this file see, which is the opposite of what the inline comment at:404intends.Cost under the gate, measured:
packageForPath("src/utils/__tests__/safeWriteJson.test.ts")returnsundefined(the gate's own function,scripts/stryker-diff.mjs:194), so a fix here adds 0 counted executable lines to whichever unit carries it — it is excluded as a test path, like the other 17 test entries in this diff.Ownership. The blob is
2995f26710ff…both at the 2357 headd6fa7117e07b70455da24e4722b47eb0b202fa6eand atrefs/tmp/2357-cut/u3-jsonwriter = 473cacf7772af3287a49e88387f866c6ae93eef5(same object id, checked byrev-parseon both), so the row travels with u3's spec file and nowhere else. It is test-side and costs no counted lines, so no owner decision is needed for it; it is not being fixed here, because u3 is not opening while this plan is unexecuted.How whoever takes it should verify it: re-run that same pinned command, and additionally re-run the two lock-sensitive tests it could disturb (
utils/__tests__/safeWriteJson.test.tsalone with--no-file-parallelism, then the misc lane as a whole) — the point of the change is that mock registrations stop leaking between tests in one file, which is exactly the thing a single-test run cannot show.easonLiangWorldedtech commentedon Nov 9, 2026
Plan rows added by
u5a-2359(round 9), keyed to the cut that has to carry them. These are additions to the plan in 6735171826, not edits of it.Row 1 -
u3-jsonwriter(refs/tmp/2357-cut/u3-jsonwriterat473cacf7772a): the staged-file mode copy must fail closed before the publish. Priced +1 counted executable line, test at 0 (test paths are not charged bypackageForPath). The decision is already taken and already on a pushed head:easonLiangWorldedtech/Zoo-Codevps2f-1914-u5aat3901239982bc8fa3ad96996539b4845aaa25bb31carriesthrow chmodErrorin_copyTargetModeplus the rewritten doc block, measured5319fe91fb...390123998 = 1,284where the same base against the previous head was 1,283. The cut inherits the decision, not the argument. Two facts the taker needs:src/utils/safeWriteJson.tsis the same blobf4edc7ee9c8c168da844730742a54f682bd00a70atu3-jsonwriter, at 2357's headd6fa7117e0, and at 2359 before that commit, andu1-writer4b8398f453eecarries a third versione54b8c51d3c34a7f1b3c7eb4b21f45c4bb9b9e90- so the mode-copy fix exists in two open sibling PRs (2357 and 2359) in two states, and whichever merges first forces the others to drop their copy or merge with a stale function. The row that proves it isaborts before the publish when the staged file's mode cannot be setinsrc/utils/__tests__/safeWriteJson.test.ts(rejects, target keeps its previous content, no.new_leftover); it goes red when the throw is removed, which I measured rather than asserted.Row 2 -
u1-writer(refs/tmp/2357-cut/u1-writerat4b8398f453ee): a never-observed ancestor gets its own error class,AncestorUnverifiableError. Cost intentionally left as "measure from its own merge base" - it is unprototyped and I will not put an unmeasured number in the plan. Ruling: do not reuseAncestorReplacedError(its name asserts observation-then-replacement); do not widen it with a reason field either. Export from wherever the other file-safety errors are exported and document both ancestor failures in the same block. This one row also covers the follow-up thatdiscussion_r4509387068waits on and the threeMcpHub.tscall sites at:2142,:2232,:2445with theconfineForMcpWritedeclaration at:670- named once here so four threads do not each spawn their own unit.Row 3 - the duplication itself, which is a plan-level fact rather than a unit's. These pull requests are siblings against
main, not stacked:src/services/file-safety/safeWriteText.tshas a different blob at every head, whilesrc/utils/fileLock.tsis identical at 2357d6fa7117e0and 23592c3f744f2(0705f771ccac98167f71562ed418da4106b8b31d) and a different one atu1-writer(49807ea154d4), andjsonFileRecovery.tsis shared withu3-jsonwriter. Anything fixed on one sibling during review has to be enumerated against the others before the next unit is cut, or the plan ships the fix in one unit and a stale copy in another.Standing items, unchanged: the 770-vs-500 question on 2357 and the 1,284-vs-500 question on 2359 are both with a maintainer (split or reviewed narrow exclusion); nothing in this chain opens while this plan is unexecuted.
easonLiangWorldedtech commentedon Nov 9, 2026
Row 4 for the plan (from 4509387063-thread
PRRT_kwDOSK-bwM609gmS, the confined-JSON writer's lock key) — booked onu3-jsonwriterwith both numbers, re-measured on this box rather than relayed.The shape: bind the JSON write to the lock key the caller already holds instead of re-deriving one inside the writer, via two statements plus a caller-supplied
heldLockKeyoption that refuses rather than re-derives. Measured with the gate's ownbuildManifest,--unified=0, per-path diffs and sources fromgit show, against merge base5319fe91fb0caf4aa4ec66aac68fa03744ebbc23(the same base CI takes):The pin is already 167 lines over the limit on its own before this fix lands, and its per-file charge is
safeWriteText.ts448,fileLock.ts62,jsonFileRecovery.ts26,safeWriteJson.ts131. The candidate moves onlysafeWriteJson.ts, 131 to 133. My loop and the gate's own sentence agree on both numbers, and the candidate commit is pinned asrefs/tmp/u5a-2357-cand-heldlockkey… no: pinned asfd27be57ed(subjectcandidate shape for PRRT_kwDOSK-bwM609gmS: bind the JSON write to the lock key the caller holds), whosesafeWriteJson.tsis bloba56d534cafb1e17026bb1a1a742691cd354803c8.What matters about those numbers: the fix is cheap and the unit it lands in is not. +2 against a 500 limit is nothing; +2 on a unit that already reads 667 is a rounding error on a head that needs its own decision first - the 667-vs-500 question on
u3-jsonwriter(and the 770-vs-500 on 2357, 1,284-vs-500 on 2359) is a maintainer's call to split further or to grant a reviewed narrow exclusion, and no row in this plan should be executed as if it were the thing that unblocks the unit.Blob it carries. The pin's
safeWriteJson.tsisf4edc7ee9c8c168da844730742a54f682bd00a70- byte-identical to 2357's headd6fa7117e0, and a different blob from thed09092fab0d1now at 2359's head3901239982(which carries the fail-closed mode copy) and frome54b8c51d3atu1-writerand7514b9242at 1913's head. AheldLockKeypatch written against the pin's copy is not the same patch as one written against 2359's: the pin's writer re-derives the key fromabsoluteFilePathin a three-attempt loop (:304-318), 2359's writer takes the path and resolves it once. Whoever takes this row writes it against the blob the unit actually carries, and the row above (Row 1, the mode copy) is the same situation in the other direction.easonLiangWorldedtech commentedon Nov 9, 2026
Correction to Row 4 (6739891079): one garbled clause and one missing ref. I wrote "the candidate commit is pinned as
refs/tmp/u5a-2357-cand-heldlockkey… no: pinned asfd27be57ed" - drafting left in. The true statement: the candidate isfd27be57ed…exactly as u9 made it, and I have now pinned it on this box asrefs/tmp/u5a-2357-cand-heldlockkeyso it cannot be garbage-collected while the plan sits unexecuted. Both measured numbers in that row stand as written: 667 counted executable lines at the pin473cacf7772aand 669 at the candidate, against merge base5319fe91fb0caf4aa4ec66aac68fa03744ebbc23with--unified=0, each one the gate's ownbuildManifestsentence rather than a hand count.easonLiangWorldedtech commentedon Nov 9, 2026
Unit note for the plan, from the 2368 ruling: the leaf-spelling helper is its own unit, and it is cheaper than the file it would otherwise live in.
Measured on 2368's own tree with
buildManifest,--unified=0, sources fromgit show, against merge base5319fe91fb0caf4aa4ec66aac68fa03744ebbc23, formatted first and measured second: this PR's head20343f818areads 26 counted executable lines; the documentation correction alone reads 26 (+0); the shared helpersrc/utils/lockLeafSpelling.tsplus its call site and the new real-library row read 33 - 26 infileLock.ts, unchanged, plus 7 in the new module. Put the same helper insidefileLock.tsand the file goes to 39, which matters only because 2357'su2-lockkeyunit is where this fix would otherwise land.What each sibling owes if it wants the fix: one
importand one call-site edit. The gate charges neither - adding the import tofileLock.tsleft its count at 26, measured - so a sibling's price for this fix is its own diff's price plus zero. That is the difference between one shared fix and five copies, and it is worth naming in the same breath as the blob, because the branches do not carry the same file:fileLock.tsis0ea669e1578cat 2368,0705f771ccacat 2357d6fa7117e0and 23593901239982,49807ea154d4atu1-writerand at main (1910, 1911, 1911-prev, 1912, 1917),6ca70045661cat 1913,2ab8179b00cfat 1914,9f7cad7653c3at 2311 - six distinct blobs counting main's copy, five inside the series, counted over 12 named heads.Two things this unit does not claim, so nobody has to rediscover them: a leaf that does not exist yet is still spelled by the caller (there is no volume spelling to read,
realpathSync.nativethrowsENOENTand the old behaviour stands), and a leaf that is a link whose target is gone does the same. And the Windows-only row that pins the converged case skips on the POSIX arm rather than passing - forced here it reported 14 passed and 1 skipped - so a green linux lane says nothing about it.easonLiangWorldedtech commentedon Nov 9, 2026
Four designs in one family, counted by blob rather than by file name — the sentence maintainers need before they read any of these PRs as versions of each other. Measured with
git rev-parse <ref>:<path>and existence checked withgit cat-file -e, over 14 named refs: maine487cd7b2a, 1910303af45776, 1911f2847c4298, 19129d420ad0c3, 19134132582f4d, 19147209a5f168, 1917d82d63ef3e, 2311d241d22e3, 2357d6fa7117e0, 23593901239982, 2368ff5fbc5870, plus the three pinned cutsu1-writer 4b8398f453ee,u2-lockkey 5e9d5a188c2a,u3-jsonwriter 473cacf7772a. main's copy is inside the count and is named as such.2842acbeb817e54b8c51d3c32350f306850c49807ea154d48e185b28a9ce632184a3c8f2b843d182168d49807ea154d4cd715dcb9e57414fd4dd81882350f306850c49807ea154d4523fa5aaf4a87514b924265c2350f306850c6ca70045661c9701d4b3b688740a20630b282350f306850c2ab8179b00cf0ed4a1970725fbe8b07563aa8db128ffecd449807ea154d4(absent)773ba57c06e0(absent)9f7cad7653c3495595ebdcd2f4edc7ee9c8c1ad95dbdd8a90705f771ccacf5acc5ea4018d09092fab0d11ad95dbdd8a90705f771ccac(absent)e54b8c51d3c32350f306850ccdd0ad1b282e(absent)e54b8c51d3c32350f306850c49807ea154d4495595ebdcd2e54b8c51d3c32350f306850c49807ea154d4495595ebdcd2e54b8c51d3c31ad95dbdd8a90705f771ccac495595ebdcd2f4edc7ee9c8c1ad95dbdd8a90705f771ccacsafeWriteText.ts: 8 distinct blobs across the 8 refs that carry the file at all — 1910, 1911, 1912, 1913, 1914, 1917, 2357 and 2359 each carry a different one (
2842acbeb817,8e185b28a9ce,cd715dcb9e57,523fa5aaf4a8,9701d4b3b688,0ed4a1970725,495595ebdcd2,f5acc5ea4018); main, 2311 and 2368 do not carry the file at all, and all three pinned cuts carry 2357's495595ebdcd2unchanged, so the cuts are re-versions of content already under review on #2357 rather than new work.fileLock.ts: 7 distinct blobs counting main's
49807ea154d4, 6 excluding it. main's copy is shared verbatim by 1910, 1911, 1912 and 1917 — the four PRs whose lock-key work is insidesafeWriteJsoninstead (1911's is632184a3c8f2, withjsonFileRecovery.tsb843d182168d). 1914 carries2ab8179b00cf, and that is the blob 2357's u-series and 2368's identity key are built on:0705f771ccacat 2357, 2359 and the three cuts,cdd0ad1b282eat 2368's headff5fbc5870(the leaf-spelling commit landed there),6ca70045661cat 1913,9f7cad7653c3at 2311. Two rival fileLock designs, therefore, not two versions of one: path-identity work routed throughsafeWriteJsonon main's untouched49807ea154d4(1911, 1912), and file-identity work insidefileLock.tson 1914's2ab8179b00cf(2357, 2359, 2368 and the u-series). A reviewer who reads them as the same change will ask the wrong questions of all four PRs.Where the shared leaf-spelling helper fits, restated so it is not mistaken for a sixth design: it is one module of 7 counted executable lines on 2368 at
ff5fbc5870(src/utils/lockLeafSpelling.ts), and each sibling that wants it owes one import line and one call-site edit — neither charged by the mutation gate, measured on 2368's tree with--unified=0against merge base5319fe91fb0caf4aa4ec66aac68fa03744ebbc23. A price measured against one blob is not the same patch against another, which is the whole reason this table exists.easonLiangWorldedtech commentedon Nov 9, 2026
Correction to comment 6740624778: my 1911 row was read off a PR number, and the head moved. This table replaces that row, and it cites commits instead of PR numbers from here on.
src/services/file-safety/safeWriteText.ts, each id read withgit rev-parse <commit>:<path>and existence withgit cat-file -e, at the commit named beside it — not at a PR head, because PR heads move under a table like this one and nobody tells it:safeWriteText.ts303af45776a12842acbeb8179d420ad0c3abcd715dcb9e574132582f4de4523fa5aaf4a87209a5f1681b9701d4b3b6883edac64036ebfws/u9-1914-cut1-safe-write-file-lock, 1914's lineage)9701d4b3b688— same blob as 1914d82d63ef3e8c0ed4a197072571691733c7a48e185b28a9ce780966d8ae0064e3a2479d935559f1f6709764e3a2479d932e79e37ac0e7ls-remote refs/pull/1911/headand APIpulls/1911agreed at read time)e4fda9f0c777— a third state of 1911's own filed6fa7117e07b495595ebdcd2fc2dc75ab3d1495595ebdcd2— 2357's blob, re-versions of reviewed content4b8398f453eeu1-writer495595ebdcd23901239982bcf5acc5ea4018e487cd7b2a49cat-file -efails, so main is inside the commit set and outside the blob countd241d22e3355ff5fbc58709bThe count that survives, with its base and semantics attached: 10 distinct
safeWriteText.tsblobs across the 14 of these 17 named commits that carry the file (2842acbeb817,e4fda9f0c777,64e3a2479d93,8e185b28a9ce,cd715dcb9e57,523fa5aaf4a8,9701d4b3b688,0ed4a1970725,495595ebdcd2,f5acc5ea4018). My published figure was "8 distinct blobs across the 8 refs that carry it" — that was measured at the commits I then had, and it is now an undercount on both axes: 1911 alone accounts for three of the ten because its head moved twice more since I read it, and 2383 and 2385 carry blobs of files 1914 and 2357 already carry. The number is only ever as good as the commit it was read at, which is the reason for this correction.A second error of mine in the same comment, corrected here rather than left standing. I wrote that
fileLock.tshas "7 distinct blobs counting main's49807ea154d4, 6 excluding it". Across these 17 commits it is 6 distinct blobs counting main's and 5 excluding it:49807ea154d4(main tipe487cd7b2a49, shared verbatim by303af45776a1,9d420ad0c3ab,d82d63ef3e8c, all three 1911 states and 2385'sfc2dc75ab3d1),6ca70045661c(4132582f4de4),2ab8179b00cf(7209a5f1681band 2383's3edac64036eb),9f7cad7653c3(d241d22e3355),0705f771ccac(d6fa7117e07b,3901239982bc),cdd0ad1b282e(ff5fbc58709b). Off-by-one in my arithmetic, not in the ids.Three designs, one of them main's, and each one now names a commit. (1) main's own
49807ea154d4ate487cd7b2a49— and 2385 carries exactly that blob, verified at its headfc2dc75ab3d1, so the new unit does not movefileLock.tsat all; 1910, 1912, 1917 and every 1911 state carry it too, and their lock-key work lives insafeWriteJson.tsinstead (1911's is632184a3c8f2withjsonFileRecovery.tsb843d182168d, read at the same 1911 commits above). (2) 1914's2ab8179b00cfat7209a5f1681b, carried unchanged by 2383's3edac64036eb. (3) 2357 and 2359's own0705f771ccacatd6fa7117e07band3901239982bc, which the pinned cuts inherit, plus 2368's leaf-spelling variantcdd0ad1b282eatff5fbc58709band 1913's6ca70045661c. Four open lanes, one file, three lineages: read them as designs and the questions make sense; read them as versions of each other and they do not.And the mutation sentence that belongs under this table, because a blob table without it invites the wrong inference. Nobody has measured surviving mutants for any of these
safeWriteText.tsblobs except the 1914 lineage's. As measured by the lead on this campaign's instrument: 1911's mutation-diff concluded success while its own report said 0 valid mutants with "the mutation run was incomplete; selected test file names were not recorded", and 2383 carries the only mutant reading of a rival blob, at 78 survivors — 1911 is meanwhile sitting underawaiting-authorapplied bygithub-actions[bot]at 12:59:03 after its six required contexts went green at 12:48:39, i.e. CodeRabbit has reviewed it and thread answers are owed on the one unit in this family with no mutation reading at all. A green conclusion on that gate has meant changed-line width and nothing else every time it has been checked here, so "measured" and "green" are different words and the table above should be read with that distinction.easonLiangWorldedtech commentedon Nov 9, 2026
Deferred items recorded, with the measurement behind them. The split plan for this series lists six units, and nothing in the tracking record mentions the three deferred items: issue body (4,951 bytes) and the plan comment
6735171826(7,126 bytes, authored 2026-11-09T08:03:14Z, both read now atZoo-Code-Org/Zoo-Code#issuecomment-6735171826) return zero hits forDACL,orderingandbackup. So those three items are untracked by any unit rather than deferred by one.ENOENTswallow (src/services/file-safety/safeWriteText.ts:892-:893) now sits inside 2385's unit, in the same change as the lock-artefact pin: decision and two-process evidence in pull 2385, thread#discussion_r4513340001. It lands with that unit's next push; nothing is pushed yet.DACL/ write-ordering on the recovery pass is still unowned: it is not in any unit's scope and this issue's own question (staging-name guard vsjsonFileRecoveryleftover prefixes being mutually exclusive) is where it has to be decided, because the ordering answer changes which of the two guards may run first.