Skip to content

feat(tools): per-write checkpoint + change card for apply_diff (B3a v2-8a, epic #1375) - #50

Open
easonLiangWorldedtech wants to merge 1 commit into
feat/fws-v2-7-changecard-corefrom
feat/fws-v2-8a-applydiff-wiring
Open

feat(tools): per-write checkpoint + change card for apply_diff (B3a v2-8a, epic #1375)#50
easonLiangWorldedtech wants to merge 1 commit into
feat/fws-v2-7-changecard-corefrom
feat/fws-v2-8a-applydiff-wiring

Conversation

@easonLiangWorldedtech

@easonLiangWorldedtech easonLiangWorldedtech commented Sep 5, 2026

Copy link
Copy Markdown
Owner

What

v2-8a of the file-write safety v2 rebuild (epic #1375): apply-diff tool wiring (B3a) — the apply_diff tool gets the same per-write checkpoint + change-card treatment as write_to_file / edit_file / apply_patch. apply_diff previously wrote with no per-file checkpoint and no change card, so its edits had no rollback surface in chat. This is unit 2 of 4 in the split of the reviewed B3a content (legacy #1411, 1891 lines standalone → v2-7 core, v2-8a apply-diff tool wiring, v2-8b apply-patch/edit/search tool wiring, v2-9 settings/i18n UI).

  • src/core/tools/ApplyDiffTool.ts — after the diff view is saved, apply_diff now calls checkpointSave with the per-write journal + change-card payload: it reuses the unified diff and stats already computed for the tool message (never recomputed), resolves auto-approval with the same checkAutoApproval call shape the other wired tools use (auto-approved steps get the compact card via the v2-7 builder), and honors the live perWriteCheckpoints setting with default-on semantics (skip only when explicitly false). The approval message — built identically in both save branches — is now computed once and reused by the checkpoint's auto-approval resolution. apply_diff only ever modifies an existing file (a missing file errors out before the patch is applied), so the operation is always "update". A checkpoint failure is swallowed (.catch(() => {})) — parity with the other wired tools — so it never blocks the write.
  • src/core/tools/apply-patch/apply.tsApplyPatchFileChange gains diff?, diffStats?, autoApproved? (the approval-diff fields the per-step change card reuses; the apply_patch consumer wiring lands in v2-8b).
  • Spec — new ApplyDiffTool.changeCard.spec.ts (5 tests): per-write checkpoint recorded with the applied diff + stats after a successful write (the success test also pins the checkAutoApproval call-argument shape: live state, cwd, ask: "tool", the approval message text, protection flag); auto-approved step marked so the v2-7 builder renders the compact card; perWriteCheckpoints: false → no checkpoint call; approval declined → nothing recorded and the tool still reports the error; provider state unavailable (providerRef.deref()undefined) → state?.perWriteCheckpoints short-circuits without throwing and the checkpoint still records. The ApplyDiffTool. prefix follows the v2-7 index.checkpointSave.spec.ts convention: the Stryker diff gate's preferDirectTestFiles match is case-sensitive, and without it the suite would be excluded from the mutation test set whenever the related set also contains a direct match of apply.ts.

Gates (local, pre-push)

  • vitest (src): 5 suites / 57 passed — the new ApplyDiffTool.changeCard spec plus the adjacent changeCard / apply.spec / applyPatchTool.execute / applyPatchTool.partial suites.
  • eslint: --max-warnings=0 on all 3 touched files (full zoo-code lint via pre-commit); eslint-suppressions.json counts unchanged.
  • pnpm check-types: clean (11/11 tasks).
  • Stryker (scripts/stryker-diff.mjs ci, base 11eeb8845 = v2-7 head (PR feat(checkpoints): per-step change card core: payload builder, emission hook, changeCardDetail setting (B3a v2-7, epic #1375) #49), head this commit): 21 changed-code mutants (all in the ApplyDiffTool.ts checkpoint block), all Killed — 0 Survived / 0 NoCoverage.

Equivalent mutants

None — the final run is clean (0 Survived / 0 NoCoverage), so no exclusion directives were needed. An intermediate run surfaced 4 survivors, all killed by test/code hardening before push: the dead let completeMessage = "" initializer (both save branches overwrote it before use — the message is now computed once and shared, which removes the unobservable literal) and three checkAutoApproval call-site mutants (the empty-argument object literal, the ask: "tool" channel string, and the state?. optional chaining) killed by the call-argument assertion in the success test and a new test where providerRef.deref() returns undefined (the checkpoint path must stay alive with no provider state).

Line budget

Standalone diff vs the PR base (11eeb8845): 311 lines (294+/17−, 3 files) — within the 400-line soft design target.

Stacking / merge order

Base: feat/fws-v2-7-changecard-core (PR #49). v2-8b (apply-patch/edit/search tool wiring) stacks on this PR; v2-9 (settings UI + i18n) stacks on those. After the whole v2-8a..v2-9 chain lands on main, each base switches to main with an identical diff.

Rebase note (round 23)

Rebased onto the current v2-7 head a5528729c (was 11eeb8845) after v2-7's changeCardDetail optional-field fix. This PR's diff content is byte-identical across the rebase, and the Stryker diff gate mutates only changed diff lines — the outcome above therefore stands; no re-run required.

@easonLiangWorldedtech easonLiangWorldedtech added coderabbit-review-active Required CI passed; CodeRabbit review is active awaiting-coderabbit Waiting for CodeRabbit to approve the latest commit labels Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

Review status

Thanks for contributing. This comment tracks the review sequence and the next action.

Current step: Required CI passed. Waiting for automated review of the latest commit.

If automated review does not start, a maintainer must restart it.

Review-state labels are managed by this workflow; do not edit them manually.

@github-actions github-actions Bot added coderabbit-review-active Required CI passed; CodeRabbit review is active and removed coderabbit-review-active Required CI passed; CodeRabbit review is active labels Sep 5, 2026
@easonLiangWorldedtech
easonLiangWorldedtech force-pushed the feat/fws-v2-7-changecard-core branch from 11eeb88 to a552872 Compare September 6, 2026 00:54
@easonLiangWorldedtech
easonLiangWorldedtech force-pushed the feat/fws-v2-8a-applydiff-wiring branch from 60c76ff to 2f6cb0a Compare September 6, 2026 00:55
@github-actions github-actions Bot added coderabbit-review-active Required CI passed; CodeRabbit review is active and removed coderabbit-review-active Required CI passed; CodeRabbit review is active labels Sep 6, 2026
apply_diff previously wrote with no per-file checkpoint and no change card, so its edits had no rollback surface in chat. Wire it to the v2-4 journal + v2-7 change-card pipeline with the same parity as write_to_file / edit_file / apply_patch: reuse the unified diff and stats already computed for the tool message, auto-approved steps get the compact card, live perWriteCheckpoints setting with default-on semantics, and a checkpoint failure never blocks the write.
@easonLiangWorldedtech
easonLiangWorldedtech force-pushed the feat/fws-v2-8a-applydiff-wiring branch from 2f6cb0a to 77c435f Compare September 6, 2026 02:30
@github-actions github-actions Bot added coderabbit-review-active Required CI passed; CodeRabbit review is active and removed coderabbit-review-active Required CI passed; CodeRabbit review is active labels Sep 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting-coderabbit Waiting for CodeRabbit to approve the latest commit coderabbit-review-active Required CI passed; CodeRabbit review is active

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants