Repository navigation
Tracking fws U1: atomic text publish primitive (safeWriteText) #2016
Description
What this unit delivers: one primitive for publishing text files atomically: safeWriteText. The payload is written to a private staging file and fsynced before a single rename puts it in place, so a reader never sees a half-written file and a crash mid-write leaves the previous content intact.
Why it matters: today a crash or power loss during a write can leave a config or state file truncated or half-written. Every later unit in the chain publishes through this primitive.
Implemented by: PR #1910 (branch fws/u1-atomic-publish).
Acceptance:
- Staged write + fsync before publish; a single rename commits the file; POSIX parent-directory fsync after commit, with failure surfaced as
PostCommitDurabilityErrorinstead of silent success. - Existing file permissions preserved on the staged file (CWE-732); Windows DACL saved and restored best-effort and reported through
onWarningwhen it cannot be done. - Optional backup is a copy, never a move, so the canonical path is never absent during the commit window; the copy is removed after a successful commit.
- Pre-commit failure cleans up the staging file and directory and leaves the target untouched.
Part of the file-write-safety epic #1375, landed as a nine-PR chain tracked on #1991. Merge order: U1 -> U2 -> U3 -> U4 -> U5 -> U8 -> U6 -> U7 -> U9.
Activity
easonLiangWorldedtech commentedon Nov 9, 2026
Cross-unit items recorded from #1911's review round (head
2e79e37ac0e7), written once here because they are one decision aboutsafeWriteText's exported surface and not four findings.One: an option that says "commit exactly here". Three findings want the same thing.
safeWriteText.ts:789(thread 4504630292): publish the staging file with the target's descriptor instead of saving and restoring a DACL after the rename, so there is no window where the published file has the wrong access rights. Today a failed restore is a warning atsafeWriteText.ts:795; sibling head303af45776a1threwDaclRestoreErrorat its:709.safeWriteJson.ts:197(thread 4504630302): the caller'sresolvedTargetPathis passed tosafeWriteTextas a path, not as a binding, so a target that becomes a symlink to a sibling between the strict resolution atsafeWriteJson.ts:143and the commit is committed to the sibling rather than rejected.safeWriteJson.ts:217(thread 4504630305): the EPERM fallback moves the target before the second write, so that write has nothing to read a mode or a DACL from - the mode read falls into its ENOENT branch and keeps the staging file's own creation mode (safeWriteText.ts:541against the target-carrying branch at:506), and the win32 DACL save is skipped becausefs.access(targetPath)at:577fails, so no dump exists for the restore.All three are the same shape: an explicit input saying which file, with which permissions, at the moment of publication. One owner decision covers them; three separate patches to the same exported option would not agree with each other.
Two: make reader-side recovery hold the writer's lock. Threads 4504630298 and 4504632760 on
safeWriteJson.ts:150.recoverMissingJsonFile(jsonFileRecovery.ts:57) canonicalises nothing - it resolves and locks the alias - and the staged-replacement scan (:152) walkspath.dirname(absoluteFilePath), while the writer's key is canonicalised throughresolveLockKey(safeWriteJson.ts:124) and its publish target throughresolvePublishTarget. For an alias that means recovery and the writer hold two locks on one file and the restore lands on the alias path. The bot's ordering point is also right: the strict dangling rejection can fire before any recovery that could have restored the referent runs under the same key. Fixing the ordering from the reader side alone, while the writer still holds a different lock, would leave the two-lock problem intact.Three: what is fixed on this unit instead of deferred.
safeWriteText.ts:23and the commit-time guard now carry bigint identities (authorizedTarget?: { dev: bigint; ino: bigint }, read withfs.stat(targetPath, { bigint: true })), because a plain stat returns a 64-bit NTFS file id rounded to a double and the lost bits cannot be recovered afterwards. It could be taken here and only here: the option has no production caller in the tree (the uncapped search at5559f1f67097returns 7 hits: the declaration, three guard lines, three rows insafeWriteText.identity.spec.ts), and it costs 0 counted executable lines - the unit is 496 of 500 at base73931fe8be253b93ec11103a0672c615e849f895withbase...headand--unified=0both before and after, measured with node v22.23.4 - whereas the four items above do not fit in the four lines of headroom this unit has.Measured limits, so nobody over-reads this record. On this box's NTFS volume three real identities read back as 281474990289684, 4503599627490809 and 5629499534337367 - all below 2**53 - so this volume cannot exhibit the bigint collision, and no test here claims to.
icacls /savesucceeds unprivileged here andicacls /restorefails at exit 1300 ("Not all privileges or groups referenced are assigned to the caller."), so any future DACL row must probe the scratch entry and skip with a reason rather than branch on a platform name. And no Linux runtime claim rests on this box: WSL Ubuntu is present (Linux 6.6.87.2-microsoft-standard-WSL2,/mnt/cmounted as v9fs) but has no node binary, so vitest cannot execute there and the ubuntu CI lane is the instrument for Linux behaviour.Recorded by u9-1917 from threads 4504630292, 4504630298, 4504630302, 4504630305, 4504632760 and 4512968639 on #1911.
easonLiangWorldedtech commentedon Nov 9, 2026
Review map for the fifteen open family pull requests - a map, not a request
This is written for a reviewer who has not followed the campaign: what each open unit is, what reading it costs, what it owes its author, and where it sits in the merge order, with every number tied to the commit and the base it was measured against and every blob id tied to the commit it was read at. Throughout, "main" means e487cd7, because the local ref named main in the working clone is stale at 7c291bb. I ask for nothing in this comment: no label, no review, no resolution, no closing.
What the board is actually waiting for, measured at 13:35Z and re-read at 15:23Z on 2026-11-09
Across these fifteen pull requests I read 2,113 review records (lists fetched per_page=100 page by page until a short page, so the totals are sums of pages, not counts of first pages). There are two approving reviews in the entire family, both from coderabbitai[bot] - #1917 and #2384 - and zero approvals from any of the four names on the repository's single CODEOWNERS wildcard line:
.github/CODEOWNERS, 119 B, one line,* @taltas @JamesRobert20 @navedmerchant @edelauna(the root CODEOWNERS is a 404). Every one of the fifteen reports mergeable true and mergeable_state blocked. Every one of our heads is unverified: commit.verification.verified false with signature_type null, on every head, while main tip is verified true.The arithmetic that follows from that, stated once and neutrally: the branch ruleset Protect Main (ruleset 15494382) sets required_approving_review_count 1 and require_extra_approval_for_unattributed_changes true, require_code_owner_review true, require_last_push_approval false and required_review_thread_resolution false, with dismissal_restriction disabled. So an unverified commit by a human author needs two approving reviews, and because
.github/CODEOWNERSis a wildcard over every path, at least one of them must come from taltas, JamesRobert20, navedmerchant or edelauna. Main's own merged history shows what that looked like in practice: #2327, #2343, #2034 and #2120 are unverified, human-authored, merged with two approvals (coderabbitai[bot] plus edelauna); #2324 and #2277 are verified and merged with one (edelauna); #2341 is unverified with a bot author and merged with one (JamesRobert20, a listed owner). Our own token cannot supply an approving review - it is read-only for pulls (pull:false, and triage, push, maintain and admin are all false) - so the second approval is not something this campaign can give itself. #2384 shows the residue precisely: it carries an approving review from the bot, zero unresolved threads, and still reports reviewDecision REVIEW_REQUIRED, because the code-owner requirement is unsatisfied.A sweep of the fifteen older tracked pull requests (#2016 to #2024 and their siblings) was read by the coordinating lane, not by me, and it points the same way: all fifteen mergeable true and blocked with no failing required context anywhere, and the only red marks on them are advisory - mutation-diff on #1408, #1565 and #1921, webview-visual on #1321, extension-host-visual on #1404 - so eleven of them sit awaiting-author because nobody has answered their threads. On both boards, what is left is review input, not builds.
What reading each unit costs, measured on the project's own gate
Numbers below are the project's own counted-executable-lines measure:
scripts/stryker-diff.mjsrun on node v22.23.4 (C:/work/Zoo-Code-fork/.tmp-node22/node_modules/node/bin/node.exe) from C:/work/Zoo-Code-fork/Zoo-Code with--baseand--headgiven explicitly, base in every case the merge base of that head with main tip e487cd7, semantics the executable changed lines charged to the extension package - test paths charge nothing, comments charge nothing,knip.jsoncharges nothing. The gate's cap is 500 such lines, and there is a second, separate cap: the mutation run refuses to start above 400 generated mutants and then reports nothing at all.Thread counts come from GraphQL
reviewThreadswith cursor pagination to the last page, so a pull request with more than 100 threads is counted over every page; the unresolved figure is threads whose isResolved is false. The counted-line figures for #1915, #1916, #1917, #2359, #1913, #2384 and #2368 are CI's printed numbers rather than my own runs; the rest I ran myself on the instrument named above, and my runs reproduce CI exactly wherever both exist (672 for #1912, 1,044 for #1914, 1,853 for #1918, 770 for #2357, 355 for #1910, 496 for #1911, 386 for #2383, 448 for #2385).The second cap, and why it decides the merge order more than the first does
The mutation run has its own limit of 400 generated mutants, and unlike the line cap it does not fail the job: the job exits success, uploads a 795 to 884 byte report, and the report reads "Advisory incomplete" with zero valid mutants and an advisory naming the count. Measured from CI's own reports, each read at the head named: 512 mutants at 2e79e37 on 496 lines (#1911), 535 at 4132582 on exactly 500 lines (#1913), 425 at fc2dc75 on 448 lines (#2385), 510 at the earlier 5559f1f. Under the cap the run measures: 372 valid mutants at 3edac64 on 386 lines (#2383), 318 valid at d5c31a1 on 347 lines (#1910 then), 21 at 685ddda, 11 at 3ce2d05. So the practical ceiling is about 400 mutants, which in this code sits between 386 counted lines, which measured completely, and 448, which measured nothing - and #1913, sitting exactly on the line cap and still measuring nothing, is the proof that the two caps are separate instruments.
Read first, and what each costs a reviewer
#1910 at eb15d53, 355 counted lines against merge base 73931fe (333 of them in the new
src/services/file-safety/safeWriteText.ts, blob 2842acbeb817 read at that head, and 22 insrc/core/tools/ApplyDiffTool.ts, blob 559a48c30fbd at c5556a5 and 2e1e114665d2 at d5c31a1). It is the only one of the rival safeWriteText implementations that CI has ever measured: 347 lines, 318 valid mutants, 255 killed, 4 timeout, 57 survived, 2 no coverage, at d5c31a1 - a 80.2% kill rate and 0.195 unsound mutants per counted line, my arithmetic on CI's printed row, which is recorded there as blocking:59 advisories:0. It has one unresolved thread left of 34, and its reviewDecision is still CHANGES_REQUESTED on a review pass at 14:02:39Z that postdates the 13:45:17Z commit.#2384 at 3ce2d05, 25 counted lines against main tip: the cheapest read on the board, 11 mutants valid and 11 killed, zero threads of any kind, an approving review from coderabbitai[bot] at 12:43:45Z, and the only thing it lacks is one of the four code-owner approvals.
#1911 at 2e79e37, 496 counted lines against 73931fe (416 in its safeWriteText flavour, blob e4fda9f0c777 read at that head, 67 in safeWriteJson.ts, 13 in jsonFileRecovery.ts). It is the largest unit that still clears the line cap, and it is over the mutant cap, so it will never carry a mutation measurement at this shape; 11 unresolved threads of 39.
The choice the campaign cannot make for the maintainers
Four different implementations of the same primitive are open at once, and they are distinct blobs, each read at the commit named: 2842acbeb817 at eb15d53 (#1910), e4fda9f0c777 at 2e79e37 (#1911), cd715dcb9e57 at 9d420ad (#1912), 9701d4b3b688 at 3edac64 (#2383) and at 7209a5f (#1914), 495595ebdcd2 at fc2dc75 (#2385) and at d6fa711 (#2357). Main has no copy of this file at all - ls-tree at e487cd7 prints nothing for that path - so none of these is a modification of something main owns; each is a different answer to the same question.
#2383 at 3edac64 is open, its knip check is red, and its mutation reading is the only complete one its flavour has. The knip redness has an answer that costs zero counted executable lines, because
knip.jsonis not executable and charges nothing: this family already ships that answer twice, 8b51352a3585 at eb15d53 (#1910, which adds an ignore line) and 34450fc441a3 at 7209a5f (#1914, which adds an entry line), and #2385 carries a third spelling of it, 111f02210204 at fc2dc75. #2383's own reading is 386 counted lines, 372 valid mutants, 274 killed, 12 timeout, 78 survived, 8 never covered - 73.7% killed and 0.223 unsound per counted line, my arithmetic on CI's row - and its 86 advisory mutants have been triaged on that pull request into 5 equivalent with the reason named per mutant, 81 test-gap entries across 59 locations each naming the assertion that is missing, and 0 needing a reasoned exclusion. Its state is: 386 counted lines, 386 under the cap, 372 valid mutants under the mutant cap, six required arms green, and zero reviews of any kind, so nobody has read it yet. Choosing between the rival implementations - and there are five spellings of it among these fifteen pull requests - is the maintainers' call to make with this evidence in front of them.Parked behind those, and why each one sits there
src/core/task/Task.tsas c72d5368859f andsrc/core/task/__tests__/Task.spec.tsas 0d8ee8b97899 - a third shape, neither main's (aa6026e56b1e, c56d5291f621) nor the head's (3b50be26cfcf, 58a288233daa). CI tests that tree, not the head.Merge order, and the reason each unit sits where it does
Three of these pull requests have main tip as their direct parent commit - I measured it with rev-parse on the first parent of each head: #2383 at 3edac64, #2384 at 3ce2d05 and #2385 at fc2dc75 all have e487cd7 as parent. Their order relative to one another is therefore a choice about reviewer attention, not a topological necessity, and the cheapest ordering by measured cost is #2384 (25 lines, one owner approval away), then #2383 (386 lines, measurable, the only complete mutant reading of its flavour), then #2385 (448 lines, arms green, no measurement).
The five writer-side units are not a strict stack any more either. #1910 at eb15d53 has one parent, its own previous head d5c31a1, and #1911 at 2e79e37 likewise has 5559f1f as its parent - those are pushes, not stack joints. #1914, #1915, #1916, #1917 and #1918 each have two parents, because each of them merged main into itself (their own commit messages say so: "Merge upstream main 5319fe9 into fws/u5-guard-core at d2b65a5" at 7209a5f, "Merge upstream/main (a0d03a0) into fws/u7-tool-wiring" at 1d993fb, and the same shape at a71761a, da2b913 and d82d63e). So the remaining ordering between them is declared by the campaign, not forced by git, and the campaign's declared order is by measured cost and by what can be measured at all: #1910, then #1911, then #1913 and #1912 (which need a candidate-tree read before a head read), then #1914, then #1915, #1916, #1917, #1918 last with the largest thread debt and the largest diff.
Instruments, and what I did not measure
Measured by me: the counted-line ladder for #1910, #1911, #1912, #1914, #1918, #2357, #2383 and #2385 (scripts/stryker-diff.mjs on node v22.23.4, base named in each run); thread counts and reviewDecision by GraphQL with cursor pagination; parentage by rev-parse; blob identity by git ls-tree -r and absence by git cat-file -e; merge candidates by git merge-tree --write-tree with main tip as the first parent, git 2.54.0.windows.1 at C:/Program Files/Git/bin/git.exe, working directory C:/work/Zoo-Code-fork/Zoo-Code. Read from CI and not re-run by me: the mutation report rows, the six required arms, and the counted-line numbers for #1913, #1915, #1916, #1917, #2359, #2384 and #2368. Read from the coordinating lane and not by me: the sweep of the fifteen older tracked pull requests. Every head date, thread count and line count in this comment was read between 13:35Z and 15:24Z on 2026-11-09; heads in this family move while a comment is being written, which is why each row above carries the sha it describes.
Nothing is asked for in this comment: no label, no review, no thread resolution, no closing.