Skip to content

Tracking B1: per-write checkpoints, task-start baseline, and perWriteCheckpoints setting #1397

Open

Description

@easonLiangWorldedtech

Tracking issue for the B1 PR (branch feat/per-write-checkpoints-b1; PR to be opened after review).

Part of the file-write safety series — plan: easonLiangWorldedtech/Zoo-Code#33, epic: #1375. Item: B1 — Per-write checkpoint. Stacked on #1395 (S3 atomic publish); to be rebased onto main once #1395 lands.

Scope

  • Every successful write_to_file / edit_file / apply_patch records a shadow-git checkpoint after the write succeeds (one per write; apply_patch = one checkpoint after the whole patch), in addition to the existing pre-write and user-message checkpoints. The checkpoint reuses the existing shadow git at <globalStorage>/tasks/<taskId>/checkpoints and the existing checkpoint_saved plumbing — no new say/webview message types.
  • Task start records one O(1) baseline checkpoint (allowEmpty, suppressed, no worktree), exactly once per task.
  • The per-write checkpoint and the baseline are non-blocking (fire-and-forget, crash-safe) so the write path keeps its ≈0 added latency.
  • New setting perWriteCheckpoints (boolean, default true) — the master switch for the B cluster, gating both behaviors; ships with the full persisted-setting round trip (types, ExtensionState, ClineProvider getState/getStateToPostToWebview, webview SettingsView cachedState save flow, CheckpointSettings UI, all locales).

Acceptance

  • One checkpoint per successful write (mock-service assertions per tool); none on rejected or failed writes
  • Task-start baseline recorded exactly once per task instance, cheap (no worktree)
  • Rollback index: the checkpoint id is surfaced through the existing checkpoint_saved / currentCheckpointUpdated plumbing
  • Setting round trip: default true; stored true/false/unset all resolve correctly in getStateToPostToWebview(); UI save via cachedState; import/export round trip
  • CI green (ubuntu unit-test gate), 100% codecov patch coverage

Status

  • PR: to be opened

Corrections measured at 8415547 on feat/per-write-checkpoints-b1, 2026-11-09

  • Bullet 8 above promises the per-write checkpoint and the task-start baseline are non-blocking, fire-and-forget, and keep the write path at about zero added latency. That is not what the code does. The call is awaited — await checkpointSave(task, false, true).catch(() => {}) at ApplyPatchTool.ts:196, :206 and :220, await checkpointSave(task, ...) at EditFileTool.ts:473 and WriteToFileTool.ts:185 — so the write path waits for the save to settle. What is true is narrower and worth saying precisely: a checkpoint failure cannot fail the write, because the rejection is swallowed, but this is awaited-with-swallow rather than fire-and-forget, and the added latency is not approximately zero.
  • The setting and its copy did land completely: perWriteCheckpoints now appears in all eighteen locale files, English at webview-ui/src/i18n/locales/en/settings.json:772-775, naming write_to_file, edit_file and apply_patch and stating that an editor save records no checkpoint. That was proved with a control in which the previous copy fails nineteen of twenty-four rows.
  • Scope note for readers of the diff: the unit delta at this head is 23 files, +273 minus 43, and src/core/task/Task.ts is not among them — it is byte-identical to the parent at 6,587 lines. Three rows of core/task/__tests__/Task.spec.ts nevertheless fail in CI's coverage-core lane at this head and passed at the parent head, so whoever reads this should not infer from the file list that Task behaviour is untouched by this change; the failing rows are TypeError: Cannot set property abort of #<Task> which has only a getter and the open question is cross-file state in that lane, not this file.
  • Two questions stay unowned rather than quietly closed: the bounded wait, expiry and cancellation semantics for queued checkpoint work (Task.ts:3880-3920 with checkpoints/index.ts:236-263, threads PRRT_...7pkQ and PRRT_...7p-r), and the stray .<name>.<pid>.<ts>.backup file left by a crash between the two renames at ApplyPatchTool.ts:588 and :590, which has no sweeper anywhere in the tree and is filed as its own follow-up rather than resolved inside a thread.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions