[split-1066] U3 - feat(tools): onParameterParseFailure teardown boundary #1936

Activity

1. easonLiangWorldedtech commented on Oct 10, 2026

   

   easonLiangWorldedtech

   on Oct 10, 2026

   Last edited by easonLiangWorldedtech

   ContributorAuthor

   More actions

   Pending row: cancellation-aware diff open

   The review row on PR 1931 asks that DiffViewProvider.open() and openDiffEditor() become cancellation-aware. Today a task disposed while open() is in flight leaves the pending open and its deferred-scroll timeout running, so a later partial delta can resurrect a diff view for a task that no longer exists.

   Acceptance:

   • Task disposal cancels or is awaited by the in-flight open, and the deferred scroll timeout is disposed.
   • A test disposes the task mid-open() and asserts no diff view survives and no timeout fires afterwards.
   • Negative control recorded for this unit: the number of tests the mutation kills, so the port to sibling units can be compared.

   This unit goes first; the change then ports in merge order 1928, 1931, 1929, 1930, 1932.
2. easonLiangWorldedtech commented on Oct 10, 2026

   

   easonLiangWorldedtech

   on Oct 10, 2026

   ContributorAuthor

   More actions

   Defect: the rollback guard silently skips the saveDirectly branch, and reset() forgets instead of cleaning

   Found while checking the coupling with U6 (#1932), at U3 head 5ff7bea73. Both halves are U3 code:
   the guard came in with 5c0f21219 and the swallowing caller with 2a9bfabfb.

   1. A guard keyed on a flag only one path sets. DiffViewProvider.revertChanges() opens with
   if (!this.relPath || !this.isEditing) return, but isEditing = true is assigned only inside
   open(). The PREVENT_FOCUS_DISRUPTION branch never calls open(), so in that branch
   revertChanges() early-returns and nothing rolls back: the directories handed over by
   adoptCreatedDirs() stay on disk and the file saveDirectly() wrote stays written. Worse, the
   caller WriteToFileTool.revertDiffChangesBeforeReset() still returns true, so the teardown is
   reported as complete and reportRevertFailure() never fires. A swallowed failure is worse than a
   reported one. A guard condition must be proven reachable from every entry point; using a flag
   that only one path sets turns the guard off for the other path.

   2. reset() forgets instead of cleaning. reset() sets createdDirs = [] and
   adoptedCreatedDirs = [] without removing anything from disk, so "early return + reset" leaves
   the preflight-created directories on disk permanently: after the reset no code path knows them.

   Acceptance criteria

   1. With experiments: { preventFocusDisruption: true }, a failed or denied saveDirectly write
      removes the directories this call created and unlinks a file it created (modify: content
      restored) - the rollback works on the branch that never opened a diff view.
   2. The rollback return value reflects what actually happened: "nothing to do" and "did the work"
      must not be conflated with "silently skipped", and a skipped rollback must reach
      reportRevertFailure().
   3. reset() may only drop the directory list after the directories are gone, or the caller must
      roll back before it resets - proven by a test that fails when reset() runs first.
   4. Every test added for this runs with the experiment ON as well as OFF: the default branch
      passes today and proves nothing about this defect.

   Ported from the retired fork tracking item 6093538947. Timing: next legitimate push of #1931 (a
   review in flight blocks the push, not this item).
3. easonLiangWorldedtech commented on Oct 10, 2026

   

   easonLiangWorldedtech

   on Oct 10, 2026

   ContributorAuthor

   More actions

   Follow-up on new-file rollback ownership in unit U3 (pull 1931).

   adoptCreatedDirs closes the rollback gap for the execute() preflight only: the streaming path still relies on DiffViewProvider.open() recording the directories it creates itself, and nothing asserts both entry points. A converging guard that only has one writer silently fails on the other path.

   Acceptance: two tests, one per entry point (the execute() preflight, and open() during a partial stream), each proving that a rolled-back new-file write removes every directory that write created and nothing it did not create.

   Owner: unit U3. Scheduled to ride the next legitimate push of that unit.
4. easonLiangWorldedtech commented on Oct 10, 2026

   

   easonLiangWorldedtech

   on Oct 10, 2026

   ContributorAuthor

   More actions

   Contingency note for unit U3 (pull 1931).

   A discarded change is kept on disk rather than deleted: .tmp-cr-audit/p1066-u3-cancellation.patch, 99 additions and 11 deletions, which makes open() and openDiffEditor() cancellation-aware. It was reverted only because the checklist row it answered belonged to an older head, not because the change was wrong.

   Acceptance: if a maintainer asks for cancellation-aware diff opening, apply the patch on the current head, re-run the DiffViewProvider spec and its negative controls, and re-measure how many tests each control reddens - do not re-derive it.

   Owner: unit U3.