[split-1066] U5 - fix(write-to-file): capture the streaming failure once and report it once #1935

Activity

1. easonLiangWorldedtech commented on Oct 10, 2026

   

   easonLiangWorldedtech

   on Oct 10, 2026

   ContributorAuthor

   More actions

   Unit-scope debt for U5, registered rather than swept, so the next round can find it.

   Same defect class as commit 9c54765 (unchecked boolean results on a rollback path), still open at these call sites in the diff-view provider: the save in showEditedFileWithoutDisruptingFocus; the three applyEdit calls in saveChanges and its append and final-edit paths; the save in saveChanges; the save in keepOrCloseEditedFile. Acceptance for each: a false result propagates or is justified where the buffer can still hold unsaved content, with a test that fails when the check is removed, and the fix reuses the existing callee-owned contract instead of adding another inline check.

   Second item: the cancellation guards in handlePartial cover the four await boundaries that exist today. Any await added between acquiring per-task partial state and opening the diff view must be followed by a cancellation re-check and a state release, mirroring the two-window shape used for the stream-failed guard.
2. easonLiangWorldedtech commented on Nov 12, 2026

   

   easonLiangWorldedtech

   on Nov 12, 2026

   ContributorAuthor

   More actions

   Split plan for pull 1930, measured by the campaign on this machine before any of it is pushed.

   Every figure below is a local count at the head named beside it. The pricing instrument is the shipped scripts/stryker-diff.mjs, blob 7dfe0006d2d5, which caps changed executable lines at 500 on its line 11 and instrumented mutants at 400 on its line 12 and excludes test paths at its line 85, so test code charges nothing under either cap. Mutant figures are instrumented counts from my own Stryker 10 dry runs, not run results. Main tip for all of it is 53cb5b5.

   1. The parent shape, and which cap fires

   Pull 1930 as authored heads at 2c94a81. That same content rebased onto main tip is a 13 file shape, 4930 insertions and 130 deletions. Measured with the shipped selector at that tree: 519 changed executable lines and 164 selectors across 6 executable files, and Stryker 10 instruments those same 6 files and reports 460 mutants.
   Per file under the same instrument at that tree: src/core/tools/WriteToFileTool.ts 254 lines, src/integrations/editor/DiffViewProvider.ts 222, src/core/assistant-message/presentAssistantMessage.ts 29, src/core/tools/BaseTool.ts 7, src/core/task/Task.ts 6, src/core/webview/ClineProvider.ts 1.
   Run at that tree, node scripts/stryker-diff.mjs ci --base 53cb5b5 --head 54371c66df196d4097978782f50c1004bd639e07 exits 1 and prints the words, Mutation gate failed: extension has 519 changed executable lines, limit 500, Split the PR or obtain a maintainer-reviewed narrow exclusion. The only characters I changed in that quoted line are the parentheses around the limit.
   Both caps are over: 519 lines against 500, and 460 instrumented mutants against 400. The line cap is the one that stops the run, because selection happens before mutants are counted. So this split is forced by the line cap, which is a different reason than the one that forced the split of pull 1913.

   2. Unit 1

   Head acd836f, tree 6a44df198371bec479ac0ca9355162d2a697cbac, one parent, main tip. Against main tip: 4 files, 2077 insertions, 40 deletions.
   src/integrations/editor/DiffViewProvider.ts, all 15 of the pull hunks for it, 490 insertions and 32 deletions, worth 222 counted executable lines and 54 selectors.
   src/integrations/editor/tests/DiffViewProvider.spec.ts, 1574 insertions and 1 deletion, 0 under both caps because it is test code.
   src/core/task/Task.ts, exactly one hunk, the caller adaptation, header -3823 +3823,6 against main tip, counted executable lines 3825 3826 3827 3828, worth 4 lines and 1 selector.
   src/core/task/tests/Task.dispose.test.ts, 7 insertions and 6 deletions, 0 under both caps because it is test code.
   Price at its own tree with the shipped instrument: 226 counted executable lines and 55 selectors over 2 files, under both the 500 line cap and the 400 mutant cap. The gate command at that tree exits 0 and prints the extension width as 226 lines together with 2 advisory lines. My Stryker 10 dry run at that tree instruments 2 files and reports 192 mutants with 0 error lines.

   3. Unit 2

   Head c3dbed4, parent unit 1, tree ed9e68752b06eba1f56fe1308be2411ba2a72e78. Against unit 1: 10 files, 2853 insertions, 90 deletions. Against main tip it is the pull whole, 13 files and 4930 insertions and 130 deletions, which is what its own pull will display because GitHub diffs a pull against main.
   Executable files and their worth against unit 1 under the shipped instrument: src/core/tools/WriteToFileTool.ts 254 lines and 99 selectors, src/core/assistant-message/presentAssistantMessage.ts 29 and 4, src/core/tools/BaseTool.ts 7 and 3, src/core/task/Task.ts 2 and 2 across 4 further hunks, src/core/webview/ClineProvider.ts 1 and 1. Total 293 counted executable lines and 109 selectors over 5 files, under both caps.
   Its 5 test files, writeToFileTool.spec.ts, writeToFileTool-partial-state-cleanup.spec.ts, presentAssistantMessage-write-to-file-stream-cleanup.spec.ts, removeClineFromStack-delegation.spec.ts and Task.spec.ts, together 2084 insertions and 11 deletions, charge 0 under both caps.
   The gate command at that tree with unit 1 as base exits 0 and prints the extension width as 293 lines with 2 advisory lines. My dry run instruments 5 files and reports 268 mutants with 0 error lines.

   4. Additivity, checked against the parent rather than assumed

   Lines 226 plus 293 equals the parent 519. Selectors 55 plus 109 equals 164. Mutants 192 plus 268 equals 460. Per file, Task.ts is 4 lines in unit 1 and 2 in unit 2 against the parent 6, and DiffViewProvider.ts is 222 in unit 1 and 0 in unit 2 against the parent 222. Nothing is dropped and nothing is counted twice, and neither half rewrites a hunk belonging to the other.

   5. The contract seam, and why the whole-file cut was rejected

   The pull turns DiffViewProvider.revertChanges into a report of whether a revert ran, and Task.ts is the caller that has to adapt. Main tip types that field as a promise resolving void, so the seam hunk and the pull own Task.dispose.test.ts stub typings cannot be left behind in a follow-on pull. I re-derived that instead of inheriting it, with tsc 5.9.3 --noEmit -p tsconfig.json run from src, on this host where main tip is the baseline at exit 0 and 0 error lines:
   unit 1 with Task.ts and Task.dispose.test.ts left at main, object 24b0778c72f8c60ad2f4a2ea8dd7d0f0d16b6f6d: exit 2, 3 error lines, all 3 new against main. TS2322 at core/task/Task.ts line 3823 column 5, Type Promise of boolean or void is not assignable to Type Promise of void. TS2322 at core/task/tests/Task.dispose.test.ts line 253 column 5. TS2345 at core/task/tests/Task.dispose.test.ts line 286 column 93, Promise void is not assignable to Promise boolean.
   seam present, pull test file still at main, object 3b23b6e3a4aa1f1b6dd2b2c1adcaee9fcdcdff32: exit 2, 2 error lines, both in the test file. So the seam alone clears the production error and the pull own stub typings are what clear the other two.
   unit 1 as it will be pushed: exit 0, 0 error lines, 0 new and 0 disappeared.
   The 4 seam lines and the 13 lines of pull test typings are therefore the control that proves the seam must travel in unit 1. The instrument attributes 0 mutants to the seam: instrumenting src/core/task/Task.ts alone over its one selector reports 0 mutants, and instrumenting DiffViewProvider.ts alone reports 192, the same 192 the two files report together.

   6. The one thing I changed in the objects, with its evidence

   The cut as my teammate built it carried src/core/task/Task.ts without its final newline. The repository own prettier 3 run as node prettier --check . from the root flags exactly one file at that tree, src/core/task/Task.ts, exit 1, where main tip and unit 2 each flag 0 files, and the branch diff against main carried a second phantom hunk at the last line, header -6573 +6578. Letting prettier format that file added 1 byte, removed the phantom hunk, and moved the width from 2078 insertions and 41 deletions to 2077 insertions and 40 deletions. The price did not move, 226 lines, 55 selectors and 192 mutants before and after, so the phantom line was never counted as executable. Unit 2 tree is byte identical to the tree my teammate built, ed9e68752b06 on both sides. No knip entry, no config file, no suppression entry, no changeset and no changelog edit travels in either half.

   7. The knip question, measured here rather than inherited

   knip 5.88.1 run the way the repository runs it, knip --treat-config-hints-as-errors: at unit 1, exit 0 with 0 bytes of output; at unit 2, exit 0 with 0 bytes.
   My own control: one added export that nothing imports, placed at src/integrations/editor/DiffViewProvider.ts line 1687, turns the same command to exit 1 and prints Unused exports with 1 entry naming knipDiscriminateProbe1930 at that file and line. The file was restored and blob verified against the shape above.
   Why the zero is weaker than it reads: root knip.json sets ignoreExportsUsedInFile to true, so an export used inside its own declaring file is invisible to the rule. At unit 1 the new diff-view API is self consumed inside its own file, git grep counts 2 mentions of captureRollbackRecoveryContext and 5 of RollbackRecoveryContext in DiffViewProvider.ts and 0 mentions outside tests anywhere else, while at unit 2 WriteToFileTool.ts takes 2 mentions of captureRollbackRecoveryContext and 5 of RollbackRecoveryContext. A clean knip at unit 1 is therefore not evidence that those exports have a consumer outside their own file. It is evidence that nothing knip is configured to look at is unused at that tree.

   8. The boundary call graph, re-derived

   git grep -n over src, excluding test directories and spec files, for the tool level names this pull introduces: 0 references at unit 1, 64 at unit 2 and 64 at the pull as authored. Every consumer of the tool level stream state arrives with unit 2, so merging unit 1 first does not leave a caller reading state that nothing writes. That 64 is my count for that name set and those exclusions and is not the 38 my teammate reported for a different name set.

   9. Which half carries the contract recorded on issue 1935

   Issue 1935 records the unit contract as handlePartial capturing the streaming failure once and reporting it once, with the authoritative execute error being the one surfaced. Unit 2 is what delivers that: WriteToFileTool.ts holds the per task state, BaseTool.ts and presentAssistantMessage.ts thread it, ClineProvider.ts consumes it, and the test text naming handlePartial appears only in unit 2, 2 mentions in presentAssistantMessage-write-to-file-stream-cleanup.spec.ts and 0 mentions at unit 1 and 0 at main tip. Unit 1 delivers what that contract leans on: the report of whether a revert ran, the captured and restored rollback recovery context, and the one caller adaptation in Task.ts.
   Issue 1935 also records that the row named reports a filesystem error only once across the streaming and execute phases was re-attributed from U5 to U6. Searched with git grep at main tip, at unit 1 and at unit 2, that row name appears 0 times at all three, so this cut neither pulls that row back nor invents a substitute for it.
   Neither half alone satisfies the unit contract, and unit 2 cannot compile or run its consumers without unit 1, which is why unit 2 is stacked on unit 1 instead of being run beside it. Merge order stays unit 1 first on the branch pull 1930 already tracks, then unit 2. This plan does not reorder the chain recorded on issue 1935 and issue 1989, it splits one unit inside that chain.

   10. Gates run on each half, with exits

   prettier 3 run as node prettier --check . from the root: exit 0 with 0 offending files at unit 1 and at unit 2, against 0 offending files at main tip.
   tsc 5.9.3 -p tsconfig.json --noEmit from the root: exit 0 with no output at both halves. tsc --noEmit -p tsconfig.json from src: 0 error lines at unit 1.
   eslint from src run as node eslint --prune-suppressions --max-warnings=0 on the 4 files unit 1 touches and the 11 files unit 2 touches: exit 0 with 0 lines of output on every file. The prune re-serialises src/eslint-suppressions.json on this host, so it was restored after each run; compared entry by entry, 265 file and rule pairs, the branch blob and the rewritten blob differ in 0 entries and both total 2868 suppressions, so no suppression count moved in either half.
   vitest 4.1.12 on unit 1 two specs, DiffViewProvider.spec.ts and Task.dispose.test.ts: exit 0, 2 files, 126 tests.
   vitest 4.1.12 on unit 2 five specs listed above: exit 0, 5 files, 316 tests passed and 5 skipped.
   The row level control that unit 1 is not merely reshaped text: unit 1 two spec files laid over main tip source, object 80b3ec5d7eee, 2 files changed, run through vitest 4.1.12, exits 1 with 36 failed and 90 passed of 126, all 36 in DiffViewProvider.spec.ts. The same two files at unit 1 pass 126 of 126. Task.dispose.test.ts passes over main tip source because its rows are type level, which is why that file is the one the seam control in section 5 turns on.
   Carried advisory, not introduced: each half gate run prints 2 advisory lines. One names src/core/task/Task.ts line 4459 at unit 1 and line 4479 at unit 2 about the mutation exclusion directive form, pointing at a directive that already exists at main tip, where git grep finds the same 7 reasoned directives in that file at lines 4375 4428 4445 4454 4873 4890 and 4896, shifted down by the lines this pull adds. The other advisory is this machine reporting that the shipped Stryker launcher could not be spawned, spawnSync ENOENT. The lifted-cap read of the parent shape reports the same single directive advisory, so the split introduces none and resolves none.

   11. What these numbers are not

   No shape above is called green, and none of these numbers is a CI number. a green local preflight over Valid 0, Killed 0, Survived 0 and No coverage 0 with blocking 0 and advisories 2 is not evidence in either direction on this machine. the mutation leg is unmeasured, a PASS over an empty digest is not evidence in either direction, and CI at the first pushed unit is the only proof of the gate.
   A mutant figure quoted here is an instrumented count on a machine where every dry run has to be killed at its initial test run because the shipped launcher cannot spawn the runner here. Two readings of mine were produced against the wrong head-side checkout and are corrected above: I first read 195 mutants for unit 1 and 386 mutants over 6 files for the parent shape, both while a different tree was checked out, and the instrument reads head-side source from the working tree, so a mismatched checkout silently changes the answer instead of failing loudly.

   12. What happens next, and what this comment does not do

   Unit 1 goes onto the branch pull 1930 already tracks, p1066/u5-streaming-failure-capture, under a lease pinned to a fresh transport read, and the title and body of pull 1930 are rewritten to describe unit 1 alone. Unit 2 goes onto a new branch stacked on unit 1 and opens its own pull against main naming pull 1930 as its dependency and this comment as its plan; that pull will display 13 files because GitHub diffs a pull against main, while its own delta is the 10 files listed above.
   No review thread on pull 1930 is resolved by this comment, no re-review is requested, and no thread on any pull in this chain is resolved by silence. The campaign answers threads with evidence and leaves resolving to a reviewer, because the pull author is the account that reviews it.