Repository navigation
feat(file-safety): serialized publish primitive over the shared file lock (split 1 of #1914, #1375) - #2383
Conversation
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one): 1
Please check the settings in the CodeRabbit UI or the
 |
Review statusThanks for contributing. This comment tracks the review sequence and the next action. Current step: Fix the failing required CI checks; awaiting-maintainer requires CI and automated review completion. Review-state labels are managed by this workflow; do not edit them manually. |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Changed-code mutation testingReport for PR head
Details: mutants, guidance, and reproductionChanged-code mutation testing
Focused tests
All surviving and uncovered mutantsWarning annotations highlight up to 20 unique locations (maximum 7 per file). This summary lists every advisory mutant. extension
|
Line | Status | Mutator | Replacement |
|---|---|---|---|
102 | Survived | StringLiteral | "" |
123 | Survived | StringLiteral | `` |
124 | Survived | StringLiteral | `` |
125 | Survived | ObjectLiteral | {} |
127 | Survived | StringLiteral | "" |
137 | Survived | StringLiteral | `` |
139 | Survived | StringLiteral | `` |
154 | Survived | StringLiteral | "" |
168 | Survived | BooleanLiteral | true |
171 | NoCoverage | StringLiteral | "" |
172 | Survived | StringLiteral | "" |
173 | Survived | ObjectLiteral | {} |
175 | Survived | StringLiteral | "" |
188 | Survived | StringLiteral | `` |
189 | Survived | StringLiteral | "" |
205 | Survived | StringLiteral | `` |
206 | Survived | ObjectLiteral | {} |
208 | Survived | StringLiteral | "" |
225 | Survived | StringLiteral | "" |
241 | Survived | StringLiteral | "" |
249 | Survived | StringLiteral | "" |
249 | Survived | StringLiteral | "" |
249 | Survived | MethodExpression | Math.random().toString(36) |
249 | Survived | StringLiteral | "" |
279 | Survived | ObjectLiteral | {} |
279 | Survived | BooleanLiteral | false |
295 | Survived | ObjectLiteral | {} |
295 | Survived | BooleanLiteral | false |
358 | Survived | ConditionalExpression | false |
359 | Survived | MethodExpression | missing |
361 | Survived | CallExpression | ; |
363 | Survived | ConditionalExpression | true |
363 | Survived | EqualityOperator | parent !== cursor |
385 | Survived | ConditionalExpression | true |
385 | Survived | EqualityOperator | missing.length >= 0 |
388 | NoCoverage | ConditionalExpression | true |
388 | NoCoverage | EqualityOperator | code === "ENOTDIR" |
388 | NoCoverage | StringLiteral | "" |
388 | Survived | ConditionalExpression | false |
472 | Survived | StringLiteral | `` |
482 | Survived | StringLiteral | `` |
487 | Survived | LogicalOperator | stagingStat.isSymbolicLink() && !stagingStat.isFile() |
489 | NoCoverage | StringLiteral | "" |
489 | Survived | StringLiteral | `` |
489 | Survived | StringLiteral | "" |
501 | Survived | BooleanLiteral | true |
574 | Survived | CallExpression | ; |
576 | Survived | CallExpression | ; |
586 | Survived | OptionalChaining | options.execFileRunner |
612 | Survived | ObjectLiteral | {} |
612 | Survived | BooleanLiteral | false |
613 | Survived | ConditionalExpression | true |
618 | Survived | ConditionalExpression | false |
633 | NoCoverage | BlockStatement | {} |
633 | Survived | ConditionalExpression | false |
633 | Survived | LogicalOperator | code === "ENOENT" && code === "ENOTDIR" |
633 | Survived | ConditionalExpression | false |
633 | Survived | StringLiteral | "" |
633 | Survived | ConditionalExpression | false |
633 | Survived | StringLiteral | "" |
638 | Survived | ConditionalExpression | false |
670 | Survived | CallExpression | ; |
679 | Survived | CallExpression | ; |
695 | Survived | ConditionalExpression | false |
695 | Survived | StringLiteral | "" |
705 | Survived | ConditionalExpression | true |
737 | Survived | ConditionalExpression | false |
762 | Survived | ConditionalExpression | true |
762 | Survived | StringLiteral | "" |
787 | Survived | OptionalChaining | options.execFileRunner |
791 | Survived | LogicalOperator | releaseBackupOnSuccess || backupPath |
802 | Survived | ConditionalExpression | false |
802 | Survived | StringLiteral | "" |
836 | Survived | ConditionalExpression | true |
836 | Survived | LogicalOperator | backupPath || releaseBackupOnSuccess |
859 | Survived | ConditionalExpression | true |
859 | Survived | ConditionalExpression | false |
859 | Survived | EqualityOperator | daclDumpPath === null |
src/utils/fileLock.ts
Line | Status | Mutator | Replacement |
|---|---|---|---|
61 | Survived | EqualityOperator | depth <= frameBudget |
73 | Survived | MethodExpression | Math.min(0, remainingLinkHops - frameBudget) |
80 | Survived | ConditionalExpression | true |
80 | Survived | EqualityOperator | missing.length >= 0 |
86 | Survived | ConditionalExpression | true |
86 | Survived | StringLiteral | "" |
173 | NoCoverage | StringLiteral | `` |
183 | NoCoverage | StringLiteral | `` |
Resolve a mutation gap
Add or strengthen a focused test that fails under the mutation. If the mutant is equivalent, request maintainer approval for the narrowest mutator-specific exclusion and explain why it cannot change behavior:
// Stryker disable next-line ConditionalExpression: normalized input cannot reach the alternate branch
const result = condition ? value : fallbackBroad all exclusions and exclusions without a concrete reason are reported as advisories.
Reproduce locally
From a full checkout containing both commits:
pnpm install --frozen-lockfile
node scripts/stryker-diff.mjs ci --base e487cd7b2a497aa1d9b60f9b0cb497f7839bb629 --head 5b0de4adacd6e8d27e130e43abde7fc1b7b15581Mutation reports
- extension:
reports/mutation/extension/mutation.html
The workflow uploads generated reports in the changed-code-mutation-report artifact. A direct artifact link appears below after upload.
Advisory findings
- extension timed out 12 of 372 valid mutants. The result is inconclusive; consider fixing flaky or slow tests, or reducing the changed scope.
knip is the only failing arm, and the reason is the consumer graph, not the buildThe arm. Code QA Roo Code run 42617459566 concluded failure at 12:38:39Z on exactly one of its eight jobs. The job is knip (job 129160792900), and the failing step is its sixth, "Run production Knip checks". Its log, lines 394-400 of 444, printed: The other seven jobs of that run completed success - invisible-chars, dependency-review, check-translations, compile, Build test VSIX, platform-unit-test (windows-latest) and platform-unit-test (ubuntu-latest) - as did Changed-code mutation testing, E2E Tests (Mocked), Visual Regression and CodeQL Advanced on this head. The knip job's own policy suite passed 8 of 8 first, and two of its subtest names are the rule it enforces: "a production file imported only by tests is still reported as unused" and "tests cannot make an otherwise unused production export pass the production check". knip is one of the six contexts the Protect Main ruleset requires, so this arm, not the width and not the mutants, is what holds the pull request. Why, measured at this head. At 3edac64, Four open pull requests carry four different implementations of this same primitive, and main carries none of them (blob ids read with
The shape that does clear knip, measured rather than asserted. At #1911's current head, What I am proposing. That this pull request be closed as superseded by #1911, on the arithmetic rather than on preference: the only sub-500 shape in the family that contains a safeWriteText with a production importer in the same unit is #1911's, at 496 of 500, and widening this cut to carry safeWriteJson.ts and jsonFileRecovery.ts reaches 1911's own width while shipping a rival blob of the same file. Widening it to carry the guardedWrite.ts consumer instead costs 394 counted executable lines on top of these 386 (measured per file: guardedWrite.ts 394, fileLock.ts 34, safeWriteText.ts 352, base = main tip e487cd7), which is 780 against a 500 line cap. Nothing is being pushed on this branch, and I am not asking anybody to label or review anything. Merge order this produces, which is the useful part. The family starts at #1910 (333 counted executable lines) and #1911 (496), because those are the two units whose production files have consumers inside themselves. Once #1911 is on main, #1914's remaining content re-cuts to guardedWrite.ts at 394 counted lines plus the canonical-key change in fileLock.ts at 34, about 428, and that unit is knip-viable precisely because safeWriteText will already be on main and guardedWrite will import it. The companion pull request #2384 (observation registry plus the Task run-state fields, 25 counted executable lines, 11 mutants generated and 11 killed, no survivors) is unaffected by any of this and stays open as it is. The mutation reading on this pull request, for the record, since the conclusion alone does not carry it. Changed-code mutation testing succeeded and uploaded its report (artifact changed-code-mutation-report, 1,493,396 B, containing extension/mutation.html, extension/mutation.json and pr-comment.md). The report row reads Measured by me on node v22.23.4 with git from C:/Program Files/Git/bin; the CI lines quoted above are from the job logs I read at 12:47-12:52Z, and the blob ids and counted-line figures are my own measurements against the bases named beside each of them. |
The mutation reading on this pull request, and how it compares with the three rival unitsReport set named before any verdict. CI generated and ran mutants on this head once: artifact 274 + 12 + 78 + 8 = 372 reconciles. The report locates every advisory mutant: 70 of them in Where this leaves the four implementations of the same primitive. These are all CI-printed rows, read from each unit's own artifact with
The kill percentages and the per-line ratios in the next sentence are my arithmetic on those printed rows, not CI's. Of the two rival implementations that have ever been mutation-measured, #1910's kills 78.4% of its valid mutants and leaves 0.174 surviving-or-uncovered mutants per counted executable line; this pull request's (the blob #1914 carries) kills 73.7% and leaves 0.223. #1911's row is The knip arm, priced both ways, at zero counted lines. Two answers already exist in this family for a publish file with no production importer yet. #1910 carries one: its knip.json goes 1f4956faa6cc to 8b51352a3585 by inserting What is still owed on this pull request before it is mergeable in anyone's eyes, stated as owed and not as done. The 78 surviving and 8 uncovered mutants have not been triaged into real gaps and equivalent mutants. That work is cheap in the currency this gate spends - I am asking for nothing here: no label, no review, no thread resolution. My measurements are mine on node v22.23.4 (git from C:/Program Files/Git/bin), the CI rows are quotations from the artifacts and job logs I downloaded at 12:47-13:14Z on 2026-11-09, and the percentages are labelled as my arithmetic where they appear. |
Triage of the 78 surviving and 8 uncovered mutants: three named piles, nothing in a fourthRead, not pushed: this comment changes no file and asks for nothing. The 86 advisory mutants named in CI's own report for head 3edac64036eb6666aa1d9b60f9b0cb497f7839bb629 (78 survived, 8 never covered) are sorted here against the source as it stands at that commit and against the two specs that unit ships - Pile 1 - equivalent mutant, reason named per mutant (5)All five are the same class: a StringLiteral emptied inside a comparison against a constant that no reachable value can equal. They are equivalents by argument about the value range, and I state the argument rather than the label. Nothing else in the 86 lands in this pile, and in particular I am not asserting equivalence for the Pile 2 - test gap, the specific missing assertion named per mutant (81 entries across 59 locations)Each line below is one or more mutants at one source line, with what the suite fails to assert. Where a line carries several mutants, the named assertion kills them together and I say so.
Pile 3 - needs a reasoned exclusion (0)Empty, and deliberately so. Every one of the 86 has an observable difference that an assertion could catch on the two runners CI actually has, so I am not asking for any disable comment and I am not offering a blanket one. If a maintainer decides the five pile-1 items should be silenced rather than left as advisory findings, the exclusion route is a reasoned What the shape of the gap says, stated as mine and not as CI's78 survivors against 372 valid mutants is a 73.7% kill rate (my arithmetic on CI's printed row), and the gaps cluster in three places rather than spreading evenly. The largest is error identity: eight of the classes in this file set One limitation stated before anyone reads this as a completeness claim: this triage judges what CI's report says survived against what the committed specs assert. It is not a re-run of Stryker, and the 12 timeouts are not in any pile because they are neither survivors nor uncovered - CI's own advisory calls the result inconclusive on their account. And the win32-gated sites (279, 295, 586, 762, 787, 859) are judged against a suite that contains exactly one Interpreter and instruments: node v22.23.4 with git from C:/Program Files/Git/bin/git.exe; survivor and uncovered rows parsed from CI's |
Correction to my own triage comment (6741531527): the pile counts were wrong in the published text, and here is the pile it droppedThe comment I posted at 13:42:30Z on this pull request announced "5 equivalent with the reason named, 81 test-gap with the specific missing assertion named, 0 needing a reasoned exclusion" and then rendered zero equivalent entries and 86 test-gap entries across 59 locations. The counts and the list disagree, and the fault is mine alone: the classification step compared a mutated-label string against a key built differently, matched nothing, and silently left every entry in pile 2 while the heading kept the number I had intended. The per-location descriptions in that comment are unaffected and stand as written; what was missing is pile 1, and the totals below are the ones the earlier comment should have carried. Pile 1, as it should have read - equivalent mutant, reason named per mutant (5)
Corrected totalsEquivalent mutant with the reason named: 5. Test gap with the specific missing assertion named: 81 entries across 59 locations. Needs a reasoned exclusion: 0. Sum 5 + 81 + 0 = 86, which is CI's 78 surviving plus 8 uncovered for head 3edac64036eb6666aa1d9b60f9b0cb497f7839bb629, with nothing in a fourth category. Five of the entries that comment listed under pile 2 belong to the same source lines as the five above, and they stay in pile 2 because their observable difference is real: src/services/file-safety/safeWriteText.ts:633 (ConditionalExpression "false"; LogicalOperator "code === "ENOENT" && code === "ENOTDIR""; ConditionalExpression "false"; ConditionalExpression "false"; BlockStatement "{}"); src/services/file-safety/safeWriteText.ts:695 (ConditionalExpression "false"); src/services/file-safety/safeWriteText.ts:737 (ConditionalExpression "false"); src/services/file-safety/safeWriteText.ts:762 (ConditionalExpression "true"); src/services/file-safety/safeWriteText.ts:802 (ConditionalExpression "false"). A rendering defect from the same comment, since it makes the entries hard to read: I wrapped CI's replacement values in quotes when several of them are already quoted, so sixteen rows printed a run of three quote characters instead of naming the value. From here on I write it as words - "the StringLiteral replaced by the empty string" - rather than nesting quotes. Nothing else in that comment is being restated, retracted or amended; a published comment is append-only for me, so this correction sits beside it rather than inside it. Nothing asked: no label, no review, no thread resolution. Same instruments as before - node v22.23.4, git from C:/Program Files/Git/bin/git.exe, mutant rows parsed from CI's pr-comment.md inside artifact 13675510301, source read with git show at 3edac64. |
What this is
Unit 1 of the five-way split of #1914, planned and issued on #2020. Whole-file module cut, 6 files, no partial hunks:
src/services/file-safety/safeWriteText.tswith its spec and integration spec,src/utils/fileLock.tswith its two spec files, andsrc/services/file-safety/__tests__/safeWriteText.integration.spec.ts. It is one of five independent pull requests rather than a stack - nothing in it depends on any other unit, and it compiles on thismaintip alone.This unit introduces no new version of anything.
safeWriteText.tsis blob9701d4b3b688andfileLock.tsis blob2ab8179b00cf, and both are #1914 own blobs, already under review there. Nothing in this pull request is a fresh take on a contested file. What it does is make 386 of #1914 1,044 counted executable lines reviewable at a size one person can read in a sitting.Numbers, with the base named
Base:
maintipe487cd7b2a497aa1d9b60f9b0cb497f7839bb629, which is this commit own parent (git merge-base --is-ancestorexits 0 against it, so the gate merge-base and the base coincide).Counted executable lines: 386 against the 500 limit, headroom 114. Per file:
safeWriteText.ts352 andfileLock.ts34. The four__tests__files count for nothing, becausepackageForPathreturns no package for test paths.Added and deleted lines, beside that number and not instead of it: 2,942 (2,936 added, 6 deleted) over 6 files.
zdt split measureagainst this base printed the following, quoted exactly as measured, with the exit code exactly as measured:exit code 1. That exit code is reporting the 2,942 against the 1,000 hard added-deleted cap, not the executable-line cap; the executable-line number, which is what the CI gate measures against 500, is 386.
Typecheck:
tsc --noEmit -p src/tsconfig.pcmap.jsonfrom the verification worktree root (node v22.23.4): exit 0, 0 error lines.Tests, run from the same worktree
src:vitest.services.config.tsprintedTest Files 2 passed (2)andTests 76 passed (76);vitest.misc.config.tsprintedTest Files 2 passed (2)andTests 9 passed (9). Both exit 0, with 0 failed and 0 skipped. Named rather than left silent: this unit has no skipped tests, and the sixtest.skipIf(process.platform === "win32")skips in this split chain live insrc/utils/__tests__/safeWriteJson.test.ts, which belongs to held unit cut5 - POSIX-only behaviour that does run on Linux CI.Why 2,942 is not pushed lower, stated so it reads as a decision and not an oversight: five of the six files are whole new files, and the largest is a test file whose single top-level
describespans lines 113-1486, 1,374 added-deleted lines on its own. Any cap-legal re-cut of that file needs a test-layout change onmainfirst, and that prerequisite is booked as a named zero-counted-line row on Tracking fws U5: guarded-write core (create-if-absent and compare-and-swap) #2020 (comment 6739719219) with its evidence - a slice ending inside one of those describes does not parse (TS1005 at safeWriteText.spec.ts:819).Gate numbers and what they mean
Two different questions get two numbers, and the base is named for both. Counted executable lines come from the repository own
buildManifestinscripts/stryker-diff.mjs, run through.tmp-cr-audit/u8-measure-chain.mjs(node v22.23.4), with per-path diffs at--unified=0becauseparseChangedLinesmarks every line a hunk covers andselectFromGitpasses--unified=0at scripts/stryker-diff.mjs:306. Added-and-deleted lines are the plaingit diff --shortstatfigure, printed beside the counted number and not instead of it.The base named below is the current
maintip, which is also this commit own parent.git merge-base --is-ancestorexits 0 against it, so the CI gate merge-base computation and my base coincide exactly and no two-dot approximation is involved in these figures.Mutation gate: local attempt, and an honest deferral
A local
zdt mutation preflightwas attempted at this base. The wrapper printedpreflight exit code: 0and=> PASS (local gate green)while writingno reports (mutation testing not applicable or run incomplete)and a digest with an empty package list, and the wrapped Stryker log says why: it instrumented the changed files and then died withCannot find module 'vitest/config'from its.stryker-tmpsandbox copy ofvitest.config.ts, whose require stack resolvesviteout of the parent clone.pnpmstore rather than through this worktree junctionednode_modules. A green verdict with an empty report set is not a green verdict, so this pull request carries no local mutation reading. CI mutation-diff will be the first real one.That deferral is safe for two measured reasons rather than evasive ones. First, #1914 mutation-diff has never produced a mutant verdict for this content at all: it fails on the 1,044-versus-500 width gate before the runner reports, so there is no mutation history being inherited here. Second, if a surviving mutant does appear, the answer is test hardening, which lands in
__tests__paths and therefore costs zero counted executable lines, becausepackageForPathreturns nothing for test paths. A mutation-red cannot move this unit further from mergeable than it already is.Content fidelity
Every file here is #1914 verbatim content, taken from the merge commit that closes it (
7209a5f1681b387e397434172707cb028e33ec3b, tree3c919730a813) with no edits. The five whole-file units of this split are path-disjoint and their file sets sum to the 26 files #1914 changes. Applied onto thismaintip they produce tree83cdd9cb0223, andgit diff --name-only 83cdd9cb0223 7209a5f1681b387e397434172707cb028e33ec3bprints 14 files - every one of them newer content that arrived onmainafter the merge base5319fe91fb0caf4aa4ec66aac68fa03744ebbc23, none of them among the 26. The split therefore neither loses nor invents content, and each unit measures the same counted executable lines against the merge base and against this tip.Series housekeeping
This split was planned and issued on #2020 before any split pull request opened (comments 6738707507, 6739312036, 6739376708, 6739428842, 6739719219), and the version question for the contested files is asked there in comment 6739877102. #1914 stays open until the units of its split have been reviewed, because its review history is the only record of what has already been said about this content; its 14 open review rows were answered on the pull request itself. On #1914 there is no review event from anyone except the bot and the author, and no non-empty review body from anyone except the bot (140 review events over 2 pages, 81 from the bot and 59 from the author account, 0 from anyone else).
No
.changesetfile and no CHANGELOG edit, per AGENTS.md.src/eslint-suppressions.jsonis not touched here: the 96-to-94 count change forcore/tools/__tests__/readFileTool.spec.tsbelongs to held unit cut4 and to that unit alone, so no two pull requests in this split edit that file.knip.jsonis likewise not touched here - held unit cut3 owns its one entry, and the fact that #1914 and #2359 both carry blob34450fc441a3whilemain, #1911, #1912, #1913, #1917 and #2368 carry1f4956faa6ccis a known conflict cost of the series, recorded on #2020, not a mistake of this unit.