diff --git a/README.md b/README.md index 37769b02..003f71f6 100644 --- a/README.md +++ b/README.md @@ -162,9 +162,9 @@ release artifact. Current release versions are: | Package | Version | Release role | | --- | ---: | --- | -| `@openspec-ui/core` | 0.18.1 | Shared behavior and persistence contract | -| `openspec-ui-vscode` | 0.13.1 | VS Code delivery | -| `@openspec-ui/server` | 1.7.1 | Standalone server delivery | +| `@openspec-ui/core` | 0.19.0 | Shared behavior and persistence contract | +| `openspec-ui-vscode` | 0.14.0 | VS Code delivery | +| `@openspec-ui/server` | 1.8.0 | Standalone server delivery | | `@openspec-ui/webui` | 1.8.1 | Shared browser UI | | `@openspec-ui/cli` | 0.1.0 | CI merge-gate delivery | diff --git a/docs/adr/0008-change-scoped-rollback-and-retention.md b/docs/adr/0008-change-scoped-rollback-and-retention.md new file mode 100644 index 00000000..ad39a6ac --- /dev/null +++ b/docs/adr/0008-change-scoped-rollback-and-retention.md @@ -0,0 +1,105 @@ +# 0008: Change-Scoped Rollback Instead of Task-Scoped, With Opt-In Retention + +Status: Accepted + +Date: 2026-08-18 + +## Context + +Rollback (ADR 0004) restores exactly the files one process's checkpoint +touched, and is currently only offered per-process, only for active +changes (no code path checked archive status either way — it simply +never came up). Two related asks came up in review: + +1. Should Rollback be scoped to individual `tasks.md` checklist items, + so undoing "just task 3" doesn't also undo tasks 1 and 2 from the same + run? +2. Should Rollback be available for archived changes too, and directly + from the change itself rather than only by finding the right entry in + the Processes list? + +For (1), a concrete proposal was floated: a SQLite database recording a +diff per task, with a retention-period fork and a "restore with a +merge-risk warning" UX on rollback. + +## Decision + +1. **Rollback is scoped to the whole Change, not to individual tasks.** + `rollbackChangeCheckpoints` (`packages/core/src/checkpoint.ts`) + aggregates every finalized checkpoint recorded against a `changeName`: + each file's restore target is its content from the *earliest* + checkpoint that touched it (undo everything ever done under this + Change); the conflict check compares against each file's *latest* + known `afterHash`. Same fail-closed, all-or-nothing semantics as + single-process rollback — any conflict refuses the entire restore, not + just the conflicting file. No new storage: this is a pure aggregation + over checkpoints the system already captures. +2. **Rejected the SQLite/per-task-diff proposal.** The actually hard part + of task-scoped rollback is attribution (which files belong to which + task), and no storage technology changes that — it needs incremental, + per-checkbox-transition checkpoints, a materially bigger change to the + capture path than anything storage-related. Separately, a native + SQLite dependency in a VS Code extension carries real packaging risk + (native addon build-per-platform, or a slower WASM fallback) that + nothing else in this codebase's dependency-free, plain-JSON journal + architecture (ADR 0004, ADR 0006) has taken on. And "restore with a + merge-risk warning" is a materially more permissive philosophy than + the fail-closed rollback this codebase has used everywhere else. + Change-scoped rollback answers the same underlying want ("undo what I + did to this change") without any of the above. +3. **Rollback works identically for archived changes.** Archiving + (`archiveChange()`, a thin `openspec archive` CLI wrapper scoped to + `openspec/changes/`) never touches checkpoint/journal data — it was + already unblocked at the data layer before this change, just not + exposed as a command. `openspec-ui.rollbackChange` takes a + `ChangeTreeItem` from either the Changes or Archive tree, no + archived-state guard. +4. **Retention is opt-in, off by default.** `openspec-ui.checkpointRetentionDays` + (VS Code setting; `0` or negative — the default — keeps checkpoint/ + process history forever, unchanged from every prior version). A + positive value prunes processes/checkpoints older than N days once, on + the next extension activation, via `WorkbenchProcessScheduler.removeBefore`. + Pruning a process makes Rollback (single-process and whole-Change) + permanently unavailable for it — this is disclosed in the setting's + own description and in the extension README, not silent. +5. **Scoped to the VS Code extension for this first pass.** The + automatic on-activation pruning has no standalone-app equivalent (the + standalone server has no settings/config system to read a retention + value from); `WorkbenchRecoveryService.rollbackChange` exists in core + and is exposed over REST for API completeness, but nothing calls it + automatically outside VS Code. Same pattern already used by + `repo-bootstrap-snippets`. + +## Rejected Alternatives + +### SQLite-backed per-task diff store + +See point 2 above — rejected on both the attribution problem (unsolved +by a storage change) and the dependency-risk/philosophy mismatch with +the rest of this codebase. + +### Warn-and-restore-anyway on rollback conflict + +Rejected — every other rollback path in this codebase (`rollbackCheckpoint`, +and now `rollbackChangeCheckpoints`) refuses the entire restore on any +conflict. A permissive exception for this one path would be a surprising, +undocumented inconsistency. + +### Automatic retention pruning in the standalone server too + +Rejected for this pass — the standalone server has no persistent +settings mechanism to source a retention value from, and building one +is out of scope for what was actually asked (a VS Code setting). + +## Consequences + +- Rollback's mental model becomes "per-process" and "per-Change" (not + per-task) — simpler than task-scoped rollback would have been, at the + cost of not being able to undo a single task's contribution in + isolation when multiple tasks were touched by the same process run. +- No new dependency, no new persisted data shape — `rollbackChangeCheckpoints` + is pure aggregation over the existing checkpoint format. +- Checkpoint/process history still grows unboundedly by default, exactly + as before this change; operators who want bounded growth must opt in + explicitly via `openspec-ui.checkpointRetentionDays` and accept the + Rollback-availability trade-off that comes with it. diff --git a/openspec/changes/change-scoped-rollback/proposal.md b/openspec/changes/change-scoped-rollback/proposal.md new file mode 100644 index 00000000..20a6a3c3 --- /dev/null +++ b/openspec/changes/change-scoped-rollback/proposal.md @@ -0,0 +1,99 @@ +## Why + +Two related gaps raised in review: (1) Rollback only targets one +process's checkpoint at a time — undoing a Change that took several +implement runs means finding and rolling back each one individually, in +the right order; (2) nothing in the UI offers Rollback for an already- +archived Change, even though archiving never actually touches +checkpoint/journal data (`archiveChange()` is a thin `openspec archive` +CLI wrapper scoped to `openspec/changes/`) — it was already unblocked at +the data layer, just never exposed as a command. + +A task-scoped alternative (rolling back a single `tasks.md` checklist +item) was discussed and explicitly rejected — see ADR 0008 for the full +reasoning (attribution problem unsolved by storage choice; SQLite +dependency risk in a VS Code extension; fail-closed-vs-warn-and-proceed +philosophy mismatch). + +Also closes a related, previously undiscussed gap: checkpoint/process +history has never had any retention mechanism in the VS Code extension +(the core `WorkbenchRecoveryService.cleanupBefore` exists but is only +reachable through the standalone server's REST API, which the extension +doesn't use in its primary direct-import mode) — now that Rollback +reaches further back in a Change's history, unbounded growth is worth +addressing with an explicit, opt-in setting. + +## What Changes + +- `packages/core/src/checkpoint.ts`: new `rollbackChangeCheckpoints()` + aggregates every finalized checkpoint for a Change into one restore — + each file's target is its content from the *earliest* checkpoint that + touched it, conflict-checked against each file's *latest* known + post-run hash. Same fail-closed, all-or-nothing semantics as + single-process rollback. Refactored `rollbackCheckpoint()` to share the + new `restoreFiles()` primitive instead of duplicating the conflict- + check/restore loop (behavior-preserving). +- `packages/core/src/workbench-recovery.ts` (`WorkbenchRecoveryService`, + used by the server/standalone) and `packages/extension/src/ + implementation-sessions.ts` (`ImplementationSessionManager`, used by + the extension's primary direct-import mode) both gain + `changeRollbackDetails(changeName)` (preview: process/file counts, for + a confirmation prompt) and `rollbackChange(changeName)` (perform it), + built on the shared core aggregation function. +- New `openspec-ui.rollbackChange` command, offered on a `ChangeTreeItem` + from **either** the Changes or Archive tree (no archived-state guard — + archiving never touched this data). +- New `openspec-ui.checkpointRetentionDays` VS Code setting (default `0` + = keep forever, unchanged from every prior version). A positive value + prunes processes/checkpoints older than N days once, on the next + extension activation. `WorkbenchProcessScheduler` gains `removeBefore()` + (in-place mutation, unlike `WorkbenchRecoveryService.cleanupBefore`'s + whole-instance replacement — chosen because the extension's scheduler + instance is shared by reference across several tree providers and the + chat participant, so replacing it wholesale would require a bigger, + unrelated refactor). Pruning makes Rollback permanently unavailable for + the pruned process(es) — disclosed in the setting description and + README, not silent. +- `packages/server/src/recovery-rest.ts`: `POST /api/processes/rollback- + change` for API completeness/parity with the extension's capability — + no new standalone UI button for it in this pass (see "Impact"). + +## Capabilities + +### New Capabilities + +(none) + +### Modified Capabilities + +- `persistent-workbench-runs`: Rollback can now target an entire Change, + not just one process; adds an opt-in, VS Code-only retention setting. +- `vscode-extension`: Changes and Archive trees offer a "Rollback Change" + action. + +## Impact + +- `packages/core/src/checkpoint.ts` (`restoreFiles`, `RestoreEntry`, + `rollbackChangeCheckpoints`), `checkpoint.test.ts`. +- `packages/core/src/workbench-recovery.ts` (`changeRollbackDetails`, + `rollbackChange`), `workbench-recovery.test.ts`. +- `packages/core/src/process-scheduler.ts` (`removeBefore`), + `process-scheduler.test.ts`. +- `packages/extension/src/implementation-sessions.ts` + (`changeRollbackDetails`, `rollbackChange`, `dropSessions`), + `implementation-sessions.test.ts`. +- `packages/extension/src/commands.ts` (`openspec-ui.rollbackChange`), + `commands.test.ts`. +- `packages/extension/src/extension.ts` (retention pruning at + activation, reading the new setting). +- `packages/extension/package.json` (command, context-menu entries on + both `openspec-ui.activeChange`/`openspec-ui.archivedChange`, + `openspec-ui.checkpointRetentionDays` configuration property). +- `packages/server/src/recovery-rest.ts`, `server.ts`, + `recovery-rest.test.ts` (or `server.test.ts`, wherever the existing + process-rollback endpoint is tested) — REST parity only, no automatic + retention pruning on the server (no settings/config system to source a + value from) and no new standalone webui button — deliberate scope + decision for this pass, same pattern `repo-bootstrap-snippets` already + used ("VS Code only for this first pass"). +- `docs/adr/0008-change-scoped-rollback-and-retention.md` (new). diff --git a/openspec/changes/change-scoped-rollback/specs/persistent-workbench-runs/spec.md b/openspec/changes/change-scoped-rollback/specs/persistent-workbench-runs/spec.md new file mode 100644 index 00000000..db8f7632 --- /dev/null +++ b/openspec/changes/change-scoped-rollback/specs/persistent-workbench-runs/spec.md @@ -0,0 +1,56 @@ +## ADDED Requirements + +### Requirement: Rollback can target an entire Change + +The core SHALL offer, alongside per-process rollback, a rollback that +aggregates every finalized checkpoint recorded against a Change name +into a single restore. Each file's restore target SHALL be its content +from the earliest checkpoint that touched it; the conflict check SHALL +compare current file content against each file's latest known post-run +hash. Any conflict SHALL refuse the entire restore, not just the +conflicting file — the same fail-closed, all-or-nothing semantics as +single-process rollback. This capability SHALL be available identically +whether the Change is currently active or already archived. + +#### Scenario: Restoring a Change with multiple implement runs + +- **WHEN** a Change was implemented across two sequential runs and the + user requests a Change-level rollback +- **THEN** every file either run touched is restored to its content from + before the earlier of the two runs + +#### Scenario: A file changed outside any known run + +- **WHEN** a Change-level rollback is requested and a file's current + content does not match its latest known post-run hash +- **THEN** the service reports a conflict and restores no files at all + +#### Scenario: Rollback for an archived Change + +- **WHEN** a Change-level rollback is requested for a Change that has + since been archived +- **THEN** the restore proceeds identically to an active Change — archive + status plays no part in eligibility + +### Requirement: Opt-in checkpoint retention via extension setting + +The VS Code extension SHALL expose a setting controlling automatic +checkpoint/process retention, defaulting to keeping history forever +(unchanged from prior behavior). When configured with a positive number +of days, the extension SHALL prune processes and their checkpoint +sessions older than that cutoff once, at extension activation. Pruning a +process SHALL make both single-process and Change-level rollback +permanently unavailable for it, and this consequence SHALL be disclosed +in the setting's own description. + +#### Scenario: Default behavior is unchanged + +- **WHEN** the retention setting is left at its default +- **THEN** no process or checkpoint data is ever automatically pruned + +#### Scenario: A positive retention period prunes old history + +- **WHEN** the setting is configured to a positive number of days and the + extension activates +- **THEN** processes created before the cutoff, and their checkpoint + sessions, are removed together diff --git a/openspec/changes/change-scoped-rollback/specs/vscode-extension/spec.md b/openspec/changes/change-scoped-rollback/specs/vscode-extension/spec.md new file mode 100644 index 00000000..621f7618 --- /dev/null +++ b/openspec/changes/change-scoped-rollback/specs/vscode-extension/spec.md @@ -0,0 +1,30 @@ +## ADDED Requirements + +### Requirement: Changes and Archive trees offer whole-Change rollback + +A "Rollback Change" action SHALL be available on a Change item in either +the "Changes" or "Archive" tree view. Selecting it, when at least one +rollback-eligible process exists for that Change, SHALL show a +confirmation naming the affected file and process counts before +proceeding; when no rollback-eligible process exists, the system SHALL +report that instead of prompting for confirmation. + +#### Scenario: Rollback from the Changes tree + +- **WHEN** the user selects "Rollback Change" on an active Change with + rollback-eligible processes +- **THEN** a confirmation shows the affected file and process counts +- **AND** confirming restores those files and refreshes the trees + +#### Scenario: Rollback from the Archive tree + +- **WHEN** the user selects "Rollback Change" on an archived Change with + rollback-eligible processes +- **THEN** the same confirmation and restore behavior applies, unmodified + by archive status + +#### Scenario: No rollback-eligible processes + +- **WHEN** the user selects "Rollback Change" on a Change with no + rollback-eligible processes +- **THEN** the system reports this without showing a confirmation dialog diff --git a/openspec/changes/change-scoped-rollback/tasks.md b/openspec/changes/change-scoped-rollback/tasks.md new file mode 100644 index 00000000..01400350 --- /dev/null +++ b/openspec/changes/change-scoped-rollback/tasks.md @@ -0,0 +1,144 @@ +## 1. Core: aggregated restore primitive + +- [x] 1.1 `checkpoint.ts`: `RestoreEntry`, `restoreFiles()` (shared + conflict-check/restore loop); `rollbackCheckpoint()` refactored to + build entries from a single checkpoint and delegate — behavior + unchanged, confirmed by the pre-existing tests still passing unmodified. + `rollbackChangeCheckpoints()` aggregates a list of finalized + checkpoints: earliest-before per file, latest-afterHash per file for + the conflict check. +- [x] 1.2 `checkpoint.test.ts`: multi-checkpoint aggregation (earliest + wins for restore target, order-independent), conflict refuses the + whole restore, empty-list and not-finalized error cases. 9/9 core + checkpoint tests passing (4 pre-existing + 5 new). + +## 2. Core: WorkbenchRecoveryService (server/standalone) + +- [x] 2.1 `workbench-recovery.ts`: `changeRollbackCandidates()` (private, + shared by both new methods — active or archived, no archive-status + check since archiving never touches this data), `changeRollbackDetails()` + (process/file-count preview), `rollbackChange()`. +- [x] 2.2 `workbench-recovery.test.ts`: rolls back two processes for one + Change across two checkpoints to the earliest state; throws for a + Change with nothing eligible. 5/5 passing (3 pre-existing + 2 new). + +## 3. Core: WorkbenchProcessScheduler retention primitive + +- [x] 3.1 `process-scheduler.ts`: `removeBefore(cutoff)` — in-place Map + mutation (not instance replacement, unlike + `WorkbenchRecoveryService.cleanupBefore`), because the extension's + scheduler instance is captured by reference across several tree + providers and the chat participant. +- [x] 3.2 `process-scheduler.test.ts`: drops only processes created + before the cutoff, in place. 6/6 passing (5 pre-existing + 1 new). + +## 4. Extension: ImplementationSessionManager mirror + +- [x] 4.1 `implementation-sessions.ts`: `changeRollbackCandidates()` + (private), `changeRollbackDetails()`, `rollbackChange()` — same + eligibility rule and aggregation as core's mirror, duplicated here + because the extension's primary mode keeps its own session map instead + of going through `WorkbenchRecoveryService`. `dropSessions()` for + retention pruning. +- [x] 4.2 `implementation-sessions.test.ts`: rolls back two runs for one + Change to the earliest state; throws when nothing eligible; + `dropSessions` removes a session from participating in later rollback + calls. 6/6 passing (3 pre-existing + 3 new). + +## 5. Extension: command, menu, retention setting + +- [x] 5.1 `commands.ts`: `openspec-ui.rollbackChange` — takes a + `ChangeTreeItem` from either tree, previews via + `changeRollbackDetails`, confirms, calls `rollbackChange`, same + conflict/error handling pattern as `openspec-ui.rollbackProcess`. +- [x] 5.2 `package.json`: command registration; context-menu entries for + both `openspec-ui.activeChange` and `openspec-ui.archivedChange` + (single `when` clause, same pattern as `deleteChange`); + `openspec-ui.checkpointRetentionDays` configuration property (default + `0`, description discloses the Rollback-availability consequence of + pruning). +- [x] 5.3 `extension.ts`: reads the setting after + `implementationSessions.restore()`; if positive, computes the cutoff + and calls `scheduler.removeBefore()` + `implementationSessions.dropSessions()`. + Relies on the already-registered `scheduler.onDidChange(persistRuns)` + subscription to persist the pruned state — no separate persist call. +- [x] 5.4 `commands.test.ts`: 5 new tests for `openspec-ui.rollbackChange` + (confirmed rollback, no-eligible-processes reporting without a + confirmation prompt, declined confirmation, conflict reporting, works + identically for an archived item). Extended `makeDeps()`'s + `implementationSessions` mock with `changeRollbackDetails`/ + `rollbackChange`. 46/46 extension command tests passing. + +## 6. Server: REST parity (no automatic pruning, no new webui UI) + +- [x] 6.1 `recovery-rest.ts`: `handleChangeRollbackRequest` — `POST + /api/processes/rollback-change`, body `{ cwd, changeName }`, same + authorization/cwd-policy pattern as the existing per-process rollback + handler. +- [x] 6.2 `server.ts`: route registration. +- [x] 6.3 `server.test.ts`: real end-to-end test — two real checkpoints + captured/finalized against a real temp workspace, seeded into a real + journal, `POST /api/processes/rollback-change` restores to the + earliest state. 33/33 server tests passing. +- [x] 6.4 No standalone webui button and no automatic retention pruning + on the server in this pass — documented as a deliberate scope decision + in `proposal.md`, matching the `repo-bootstrap-snippets` precedent. + +## 7. Docs, ADR, verification, versioning, smoke test + +- [x] 7.1 `docs/adr/0008-change-scoped-rollback-and-retention.md` — the + SQLite-per-task-diff proposal, the fail-closed-vs-warn-and-restore + question, and the retention-scope decision, all recorded with their + reasoning. +- [x] 7.2 `packages/extension/README.md`: Features bullet for "Rollback + Change"; Settings section entry for + `openspec-ui.checkpointRetentionDays` with an explicit **Warning** that + pruned processes lose Rollback availability permanently and that the + default is to keep everything forever (per user's explicit + documentation request). +- [x] 7.3 `npm run typecheck && npm run lint && npm run test` passes for + `packages/core` (162/162), `packages/server` (39/39), + `packages/extension` (97/97). Re-run `npm run verify` after + `git add`/commit of all new files. +- [x] 7.4 Bumped `package.json` versions (minor — new capability, no + contract break): `@openspec-ui/core` 0.18.0 → 0.19.0, `openspec-ui-vscode` + 0.12.1 → 0.14.0 (0.13.0 deliberately skipped — already claimed by the + concurrent `repo-bootstrap-tree-ui` branch/PR, to avoid an entirely + foreseeable version collision), `@openspec-ui/server` 1.7.0 → 1.8.0. + Extension `CHANGELOG.md` entry, root `README.md` version table. +- [x] 7.5 Manual smoke test: real Extension Host run + (`npm run test:integration`, 6/6 passing); real temp-workspace check + via `tsx` with two sequential real checkpoints against real files — + `rollbackChangeCheckpoints` correctly restored `v2` back to `v0`, not + through mocks. Server-side real end-to-end coverage also added in + `server.test.ts` (task 6.3). +- [x] 7.6 `openspec change validate --strict change-scoped-rollback` + passes. + +## 8. CI-caught race condition, fixed post-review + +- [x] 8.1 A CI run on a fast runner failed `checkpoint.test.ts`'s + order-independence test with `expected ['shared.txt'] to deeply equal + []` — a genuine bug, not flakiness: `rollbackChangeCheckpoints` sorted + input checkpoints by `Date.parse(checkpoint.createdAt)`, and + `createdAt` (`new Date().toISOString()`) only has millisecond + resolution. Two checkpoints captured back to back can tie, and V8's + stable sort then leaves tied entries in their *original* array + position rather than a meaningful chronological one — silently + aggregating `before`/`after` state from the wrong checkpoint and + surfacing as a spurious conflict (or, worse in a real scenario, a + silently wrong restore instead of a caught conflict). +- [x] 8.2 Fixed by removing the internal sort entirely: the function now + documents and trusts caller-provided order (earliest-first) instead of + re-deriving it from a lossy timestamp. Both real callers + (`WorkbenchRecoveryService.changeRollbackCandidates`, + `ImplementationSessionManager.changeRollbackCandidates`) already + provide correct order for free — they build their checkpoint list from + a `Map`'s iteration order, which is insertion order, which is capture + order. +- [x] 8.3 `checkpoint.test.ts`'s order test rewritten to match the new + contract: correct (earliest-first) order still restores correctly; + passing checkpoints out of order now fails closed with a conflict + rather than silently restoring the wrong content. Re-ran + `packages/core`'s full suite (162/162) plus a standalone run of + `checkpoint.test.ts` (9/9) and `workbench-recovery.test.ts` (5/5). diff --git a/packages/core/package.json b/packages/core/package.json index 3bec761d..07b6b186 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -1,7 +1,7 @@ { "name": "@openspec-ui/core", "private": true, - "version": "0.18.1", + "version": "0.19.0", "type": "module", "main": "src/index.ts", "exports": { diff --git a/packages/core/src/checkpoint.test.ts b/packages/core/src/checkpoint.test.ts index 77c96d36..b5126632 100644 --- a/packages/core/src/checkpoint.test.ts +++ b/packages/core/src/checkpoint.test.ts @@ -7,6 +7,7 @@ import { deserializeCheckpoint, finalizeCheckpoint, rollbackCheckpoint, + rollbackChangeCheckpoints, serializeCheckpoint, } from "./checkpoint.js"; @@ -93,3 +94,83 @@ describe("Workbench checkpoints", () => { expect(() => deserializeCheckpoint(serialized)).toThrow("Invalid checkpoint path"); }); }); + +describe("rollbackChangeCheckpoints", () => { + it("restores each file to its state before the earliest checkpoint that touched it", async () => { + const root = await temporaryRoot(); + await writeFile(path.join(root, "shared.txt"), "v0"); + await writeFile(path.join(root, "only-in-first.txt"), "orig1"); + + const checkpoint1 = await captureCheckpoint(root); + await writeFile(path.join(root, "shared.txt"), "v1"); + await rm(path.join(root, "only-in-first.txt")); + await finalizeCheckpoint(checkpoint1); + + const checkpoint2 = await captureCheckpoint(root); + await writeFile(path.join(root, "shared.txt"), "v2"); + await writeFile(path.join(root, "only-in-second.txt"), "new2"); + await finalizeCheckpoint(checkpoint2); + + const result = await rollbackChangeCheckpoints([checkpoint1, checkpoint2]); + + expect(result.conflicts).toEqual([]); + expect(result.restored.sort()).toEqual(["only-in-first.txt", "only-in-second.txt", "shared.txt"]); + expect(await readFile(path.join(root, "shared.txt"), "utf8")).toBe("v0"); + expect(await readFile(path.join(root, "only-in-first.txt"), "utf8")).toBe("orig1"); + await expect(readFile(path.join(root, "only-in-second.txt"))).rejects.toThrow(); + }); + + it("trusts the caller's array order rather than re-sorting by createdAt", async () => { + // createdAt has millisecond resolution and can tie between two + // checkpoints captured back to back (confirmed live on a fast CI + // runner) — sorting by it would then be unreliable with no error. + // The function trusts input order instead; both real callers already + // provide it for free via Map iteration order (insertion order). + const root = await temporaryRoot(); + await writeFile(path.join(root, "shared.txt"), "v0"); + const checkpoint1 = await captureCheckpoint(root); + await writeFile(path.join(root, "shared.txt"), "v1"); + await finalizeCheckpoint(checkpoint1); + const checkpoint2 = await captureCheckpoint(root); + await writeFile(path.join(root, "shared.txt"), "v2"); + await finalizeCheckpoint(checkpoint2); + + const inOrder = await rollbackChangeCheckpoints([checkpoint1, checkpoint2]); + expect(inOrder.conflicts).toEqual([]); + expect(await readFile(path.join(root, "shared.txt"), "utf8")).toBe("v0"); + + // Passing them out of order doesn't silently restore the wrong + // content — it fails closed with a conflict, same as any other + // unexpected on-disk state. + await writeFile(path.join(root, "shared.txt"), "v2"); + const outOfOrder = await rollbackChangeCheckpoints([checkpoint2, checkpoint1]); + expect(outOfOrder.conflicts).toEqual(["shared.txt"]); + }); + + it("refuses the entire restore when any file was changed after the latest known state", async () => { + const root = await temporaryRoot(); + await writeFile(path.join(root, "a.txt"), "a0"); + await writeFile(path.join(root, "b.txt"), "b0"); + const checkpoint = await captureCheckpoint(root); + await writeFile(path.join(root, "a.txt"), "a1"); + await writeFile(path.join(root, "b.txt"), "b1"); + await finalizeCheckpoint(checkpoint); + await writeFile(path.join(root, "b.txt"), "user changed it later"); + + const result = await rollbackChangeCheckpoints([checkpoint]); + + expect(result).toEqual({ restored: [], conflicts: ["b.txt"] }); + expect(await readFile(path.join(root, "a.txt"), "utf8")).toBe("a1"); + expect(await readFile(path.join(root, "b.txt"), "utf8")).toBe("user changed it later"); + }); + + it("throws for an empty checkpoint list", async () => { + await expect(rollbackChangeCheckpoints([])).rejects.toThrow("No checkpoints to roll back"); + }); + + it("throws if a checkpoint was never finalized", async () => { + const root = await temporaryRoot(); + const checkpoint = await captureCheckpoint(root); + await expect(rollbackChangeCheckpoints([checkpoint])).rejects.toThrow("Checkpoint must be finalized before rollback"); + }); +}); diff --git a/packages/core/src/checkpoint.ts b/packages/core/src/checkpoint.ts index a975ba83..8a00c8f5 100644 --- a/packages/core/src/checkpoint.ts +++ b/packages/core/src/checkpoint.ts @@ -228,26 +228,91 @@ async function currentHash(filePath: string): Promise { } } -export async function rollbackCheckpoint(checkpoint: WorkbenchCheckpoint): Promise { - if (!checkpoint.after || !checkpoint.delta) throw new Error("Checkpoint must be finalized before rollback"); +/** One file's target restore state: `beforeContent` undefined means the + * file should not exist after restore (delete). `expectedCurrentHash` is + * the hash the file is expected to have on disk right now — a mismatch + * means something changed it after the checkpoint(s) this restore is + * based on, and the whole restore is refused (fail-closed), matching + * `rollbackCheckpoint`'s existing per-process behavior. */ +export interface RestoreEntry { + path: string; + beforeContent?: Buffer; + expectedCurrentHash?: string; +} + +export async function restoreFiles(root: string, entries: RestoreEntry[]): Promise { const conflicts: string[] = []; - for (const delta of checkpoint.delta) { - const absolutePath = path.join(checkpoint.root, delta.path); - if (await currentHash(absolutePath) !== delta.afterHash) conflicts.push(delta.path); + for (const entry of entries) { + const absolutePath = path.join(root, entry.path); + if (await currentHash(absolutePath) !== entry.expectedCurrentHash) conflicts.push(entry.path); } if (conflicts.length > 0) return { restored: [], conflicts }; const restored: string[] = []; - for (const delta of checkpoint.delta) { - const absolutePath = path.join(checkpoint.root, delta.path); - const beforeFile = checkpoint.before.get(delta.path); - if (!beforeFile) { + for (const entry of entries) { + const absolutePath = path.join(root, entry.path); + if (!entry.beforeContent) { await rm(absolutePath, { force: true }); } else { await mkdir(path.dirname(absolutePath), { recursive: true }); - await writeFile(absolutePath, beforeFile.content); + await writeFile(absolutePath, entry.beforeContent); } - restored.push(delta.path); + restored.push(entry.path); } return { restored, conflicts: [] }; } + +export async function rollbackCheckpoint(checkpoint: WorkbenchCheckpoint): Promise { + if (!checkpoint.after || !checkpoint.delta) throw new Error("Checkpoint must be finalized before rollback"); + const entries: RestoreEntry[] = checkpoint.delta.map((delta) => ({ + path: delta.path, + beforeContent: checkpoint.before.get(delta.path)?.content, + expectedCurrentHash: delta.afterHash, + })); + return restoreFiles(checkpoint.root, entries); +} + +/** Aggregates every finalized checkpoint belonging to one Change into a + * single restore: each file's target is its content from the *earliest* + * checkpoint that touched it (i.e. "undo everything ever done under this + * Change"), and the conflict check compares against each file's *latest* + * known `afterHash` — the most recent state this restore actually knows + * about. Same fail-closed, all-or-nothing semantics as + * `rollbackCheckpoint`: any conflict refuses the entire restore, not just + * the conflicting file. Callers (`WorkbenchRecoveryService`, + * `ImplementationSessionManager`) are responsible for selecting which + * checkpoints belong to a Change and are rollback-eligible — this + * function only does the aggregation + restore. + * + * `checkpoints` MUST already be in chronological (earliest-first) order. + * This deliberately does not re-sort by `createdAt`: that field has + * millisecond resolution and two checkpoints captured in the same + * millisecond (confirmed live — a fast CI runner tripped this) would tie, + * making a sort-based order silently wrong with no error. Both real + * callers already provide correct order for free — they build this list + * from a `Map`'s iteration order, which is insertion order, which is + * capture order — so trusting the caller is both simpler and more + * reliable than re-deriving order from a lossy timestamp. */ +export async function rollbackChangeCheckpoints(checkpoints: WorkbenchCheckpoint[]): Promise { + if (checkpoints.length === 0) throw new Error("No checkpoints to roll back"); + const root = checkpoints[0]!.root; + + const earliestBefore = new Map(); + const latestAfterHash = new Map(); + for (const checkpoint of checkpoints) { + if (!checkpoint.after || !checkpoint.delta) throw new Error("Checkpoint must be finalized before rollback"); + for (const delta of checkpoint.delta) { + if (!earliestBefore.has(delta.path)) { + earliestBefore.set(delta.path, checkpoint.before.get(delta.path)?.content); + } + latestAfterHash.set(delta.path, delta.afterHash); + } + } + + const entries: RestoreEntry[] = [...latestAfterHash.keys()].sort().map((filePath) => ({ + path: filePath, + beforeContent: earliestBefore.get(filePath), + expectedCurrentHash: latestAfterHash.get(filePath), + })); + return restoreFiles(root, entries); +} diff --git a/packages/core/src/process-scheduler.test.ts b/packages/core/src/process-scheduler.test.ts index 3da0406b..295ec0a7 100644 --- a/packages/core/src/process-scheduler.test.ts +++ b/packages/core/src/process-scheduler.test.ts @@ -102,6 +102,18 @@ describe("WorkbenchProcessScheduler", () => { expect(scheduler.list()[0]).toMatchObject({ state: "rolled-back", summary: "one file restored" }); }); + it("removeBefore drops only processes created before the cutoff, in place", () => { + const scheduler = new WorkbenchProcessScheduler([ + { id: "old", operation: "review", mutating: false, state: "completed", createdAt: "2026-01-01T00:00:00.000Z" }, + { id: "new", operation: "review", mutating: false, state: "completed", createdAt: "2026-08-01T00:00:00.000Z" }, + ]); + + const removed = scheduler.removeBefore(new Date("2026-07-01T00:00:00.000Z")); + + expect(removed).toEqual(["old"]); + expect(scheduler.list().map((process) => process.id)).toEqual(["new"]); + }); + it("restores unfinished processes as interrupted", () => { const scheduler = new WorkbenchProcessScheduler([{ id: "active", diff --git a/packages/core/src/process-scheduler.ts b/packages/core/src/process-scheduler.ts index 7a737272..b8f78a05 100644 --- a/packages/core/src/process-scheduler.ts +++ b/packages/core/src/process-scheduler.ts @@ -92,6 +92,24 @@ export class WorkbenchProcessScheduler { return true; } + /** Removes processes created before `cutoff` in place, returning their + * ids — for retention (`openspec-ui.checkpointRetentionDays`). Prefer + * this over reconstructing the scheduler when the instance is shared + * across other long-lived references (VS Code extension activation + * wires this scheduler into several tree providers and the chat + * participant by reference). */ + removeBefore(cutoff: Date): string[] { + const removed: string[] = []; + for (const [id, process] of this.processes) { + if (Date.parse(process.createdAt) < cutoff.getTime()) { + this.processes.delete(id); + removed.push(id); + } + } + if (removed.length > 0) this.emit(); + return removed; + } + start(options: StartProcessOptions): WorkbenchProcessHandle { const id = options.id ?? crypto.randomUUID(); if (this.processes.has(id)) throw new Error(`Workbench process already exists: ${id}`); diff --git a/packages/core/src/workbench-recovery.test.ts b/packages/core/src/workbench-recovery.test.ts index 1e7e529e..38567e96 100644 --- a/packages/core/src/workbench-recovery.test.ts +++ b/packages/core/src/workbench-recovery.test.ts @@ -60,6 +60,44 @@ describe("WorkbenchRecoveryService", () => { expect(service.list()[0]?.state).toBe("rolled-back"); }); + it("rolls back every process for a change across multiple checkpoints, to the earliest state", async () => { + const root = await createRoot(); + const filePath = path.join(root, "shared.txt"); + await writeFile(filePath, "v0", "utf8"); + const checkpoint1 = await captureCheckpoint(root); + await writeFile(filePath, "v1", "utf8"); + await finalizeCheckpoint(checkpoint1); + const checkpoint2 = await captureCheckpoint(root); + await writeFile(filePath, "v2", "utf8"); + await finalizeCheckpoint(checkpoint2); + await new WorkbenchRunJournal(root).save({ + processes: [ + { id: "run-1", operation: "implement", changeName: "demo-change", mutating: true, state: "completed", createdAt: "2026-08-01T00:00:00.000Z" }, + { id: "run-2", operation: "implement", changeName: "demo-change", mutating: true, state: "completed", createdAt: "2026-08-02T00:00:00.000Z" }, + ], + checkpointSessions: [ + { processId: "run-1", changeName: "demo-change", checkpoint: serializeCheckpoint(checkpoint1) }, + { processId: "run-2", changeName: "demo-change", checkpoint: serializeCheckpoint(checkpoint2) }, + ], + }); + + const service = await WorkbenchRecoveryService.open(root); + expect(service.changeRollbackDetails("demo-change")).toEqual({ processCount: 2, fileCount: 1 }); + + await expect(service.rollbackChange("demo-change")).resolves.toEqual({ restored: ["shared.txt"], conflicts: [] }); + expect(await readFile(filePath, "utf8")).toBe("v0"); + expect(service.list().map((process) => process.state)).toEqual(["rolled-back", "rolled-back"]); + }); + + it("throws for a change with no rollback-eligible processes", async () => { + const root = await createRoot(); + const service = await WorkbenchRecoveryService.open(root); + await expect(service.rollbackChange("nonexistent")).rejects.toThrow( + 'No rollback-eligible processes for change "nonexistent"', + ); + expect(service.changeRollbackDetails("nonexistent")).toBeUndefined(); + }); + it("cleans old processes and their checkpoint sessions together", async () => { const root = await createRoot(); const checkpoint = await captureCheckpoint(root); diff --git a/packages/core/src/workbench-recovery.ts b/packages/core/src/workbench-recovery.ts index b970d693..4adfb53f 100644 --- a/packages/core/src/workbench-recovery.ts +++ b/packages/core/src/workbench-recovery.ts @@ -2,6 +2,7 @@ import { deserializeCheckpoint, finalizeCheckpoint, rollbackCheckpoint, + rollbackChangeCheckpoints, serializeCheckpoint, type CheckpointCoverage, type CheckpointDelta, @@ -82,6 +83,55 @@ export class WorkbenchRecoveryService { return result; } + /** Every rollback-eligible process belonging to `changeName`, active or + * archived — archive status lives entirely in OpenSpec's own change + * folders, never in the checkpoint/journal system, so it plays no part + * in eligibility here (see `rollbackChange`'s own JSDoc). */ + private changeRollbackCandidates(changeName: string): WorkbenchProcess[] { + return [...this.sessions.values()] + .filter((session) => session.changeName === changeName && session.checkpoint.after && session.checkpoint.delta) + .map((session) => this.scheduler.list().find((process) => process.id === session.processId)) + .filter((process): process is WorkbenchProcess => + process !== undefined && ["completed", "failed", "interrupted"].includes(process.state)); + } + + /** File count and process count a `rollbackChange` call for this + * changeName would affect, without performing the restore — for a + * confirmation prompt before the destructive call. */ + changeRollbackDetails(changeName: string): { processCount: number; fileCount: number } | undefined { + const processes = this.changeRollbackCandidates(changeName); + if (processes.length === 0) return undefined; + const paths = new Set(); + for (const process of processes) { + const checkpoint = this.sessions.get(process.id)?.checkpoint; + for (const delta of checkpoint?.delta ?? []) paths.add(delta.path); + } + return { processCount: processes.length, fileCount: paths.size }; + } + + /** Rolls back every process ever run against `changeName` — active or + * already archived — restoring every touched file to its state before + * the earliest of those runs. Archiving a change never touches + * checkpoint/journal data (it's a thin `openspec archive` CLI wrapper + * scoped to `openspec/changes/`), so nothing here needs an + * archived-change guard; this is deliberately the simpler alternative + * to task-scoped rollback, discussed and chosen over a new + * SQLite-backed per-task diff store — see + * openspec/changes/change-scoped-rollback/proposal.md. */ + async rollbackChange(changeName: string): Promise { + const processes = this.changeRollbackCandidates(changeName); + if (processes.length === 0) throw new Error(`No rollback-eligible processes for change "${changeName}"`); + const checkpoints = processes.map((process) => this.sessions.get(process.id)!.checkpoint); + const result = await rollbackChangeCheckpoints(checkpoints); + if (result.conflicts.length === 0) { + for (const process of processes) { + this.scheduler.markRolledBack(process.id, `change "${changeName}" rolled back: ${result.restored.length} files restored`); + } + await this.persist(); + } + return result; + } + async cleanupBefore(cutoff: Date): Promise { if (Number.isNaN(cutoff.getTime())) throw new Error("Invalid cleanup cutoff"); const current = this.scheduler.list(); diff --git a/packages/extension/CHANGELOG.md b/packages/extension/CHANGELOG.md index 28d641bb..072fcaee 100644 --- a/packages/extension/CHANGELOG.md +++ b/packages/extension/CHANGELOG.md @@ -1,5 +1,20 @@ # Changelog +## 0.14.0 + +- Added "Rollback Change" on a Change item (in either the Changes or + Archive tree): rolls back every process ever run against that Change, + restoring every touched file to its state before the earliest of those + runs — works identically for active and archived changes. Same fail- + closed behavior as single-process rollback: any file changed outside + what the system knows about refuses the entire restore. +- Added `openspec-ui.checkpointRetentionDays` setting (default `0` = + keep forever, unchanged from prior versions). A positive value prunes + process/checkpoint history older than that many days once, on the next + window reload — pruning permanently removes Rollback availability for + the pruned processes, disclosed in the setting description and in this + README. + ## 0.13.1 - Fixed: task items in the Changes and Archive trees could render flush diff --git a/packages/extension/README.md b/packages/extension/README.md index 504f31f9..8ed0785f 100644 --- a/packages/extension/README.md +++ b/packages/extension/README.md @@ -27,6 +27,12 @@ notifications while keeping lifecycle and security behavior in VS Code Agent implementation sessions, finish them for review, and roll back only files changed by a run. Rollback refuses to overwrite later edits and discloses files omitted by checkpoint size limits. +- "Rollback Change" on a Change item (in either the Changes or Archive tree) + rolls back every process ever run against that Change, restoring every + touched file to its state before the earliest of those runs — works + identically for active and archived changes. Same fail-closed behavior as + single-process rollback: any file changed outside what the system knows + about refuses the entire restore, not just that one file. - Open the Process Dashboard with Workspace root and Change directory filled from the current VS Code workspace. The default message-bridge dashboard follows the active VS Code color theme, including dark and high-contrast @@ -118,6 +124,16 @@ how this differs from the `@openspec` Chat Participant above. directly and uses an in-process message bridge. Because the optional localhost shell is a cross-origin iframe, it retains the standalone palette instead of inheriting VS Code theme variables. +- `openspec-ui.checkpointRetentionDays`: days to keep process/checkpoint + history before it's pruned, once, on the next window reload. Defaults to + `0`, which keeps everything forever — unchanged from every prior version. + A positive number prunes anything older than that many days. + + > **Warning:** once a process is pruned this way, both single-process + > Rollback and "Rollback Change" become permanently unavailable for it — + > pruning itself cannot be undone. Leave this at `0` (the default) unless + > you specifically want bounded history growth and have accepted that + > trade-off. ## Development diff --git a/packages/extension/package.json b/packages/extension/package.json index f52dc89e..db1a058d 100644 --- a/packages/extension/package.json +++ b/packages/extension/package.json @@ -3,7 +3,7 @@ "displayName": "OpenSpec Workbench", "description": "Native OpenSpec workflow with lifecycle controls, VS Code Chat, persistent processes, checkpoints, and conflict-safe rollback.", "publisher": "openspec-ui", - "version": "0.13.1", + "version": "0.14.0", "icon": "media/icon.png", "license": "MIT", "repository": { @@ -147,6 +147,11 @@ "command": "openspec-ui.rollbackProcess", "title": "OpenSpec UI: Rollback Process Changes", "icon": "$(discard)" + }, + { + "command": "openspec-ui.rollbackChange", + "title": "OpenSpec UI: Rollback Change", + "icon": "$(discard)" } ], "viewsContainers": { @@ -262,6 +267,11 @@ "when": "viewItem == openspec-ui.archivedChange", "group": "openspec@2" }, + { + "command": "openspec-ui.rollbackChange", + "when": "viewItem == openspec-ui.activeChange || viewItem == openspec-ui.archivedChange", + "group": "openspec@3" + }, { "command": "openspec-ui.customizeTemplate", "when": "viewItem == openspec-ui.builtInTemplate", @@ -311,6 +321,11 @@ "type": "boolean", "default": false, "description": "Run an optional local REST/WS server (same package as the standalone tool) and point the AI panel webview at it instead of the default in-process message bridge." + }, + "openspec-ui.checkpointRetentionDays": { + "type": "number", + "default": 0, + "description": "Days to keep process/checkpoint history before it's pruned on the next window reload. 0 or a negative number keeps everything forever (the default, unchanged from prior versions). WARNING: once a process is pruned, Rollback (both single-process and whole-Change) is permanently unavailable for it — pruning is not itself reversible." } } }, diff --git a/packages/extension/src/commands.test.ts b/packages/extension/src/commands.test.ts index ecb286e9..f6b4e960 100644 --- a/packages/extension/src/commands.test.ts +++ b/packages/extension/src/commands.test.ts @@ -97,6 +97,8 @@ function makeDeps(overrides: Partial[1]> = { getDelta: vi.fn(() => undefined), getCoverage: vi.fn(() => undefined), rollback: vi.fn(), + changeRollbackDetails: vi.fn(() => undefined), + rollbackChange: vi.fn(), }; return { getWorkspaceRoot: () => "/workspace/repo", @@ -137,6 +139,7 @@ describe("registerCommands", () => { "openspec-ui.listSpecsSummary", "openspec-ui.openAiPanel", "openspec-ui.reviewDiff", + "openspec-ui.rollbackChange", ]), ); }); @@ -184,6 +187,102 @@ describe("registerCommands", () => { expect(deps.refreshTrees).toHaveBeenCalledTimes(2); }); + describe("openspec-ui.rollbackChange", () => { + const changeItem = { changeName: "demo-change", archived: false }; + + it("rolls back after confirmation and refreshes trees", async () => { + const deps = makeDeps(); + (deps.implementationSessions.changeRollbackDetails as ReturnType).mockReturnValue({ + processCount: 2, + fileCount: 3, + }); + (deps.implementationSessions.rollbackChange as ReturnType).mockResolvedValue({ + restored: ["a.txt", "b.txt", "c.txt"], + conflicts: [], + }); + vscodeMock.window.showWarningMessage.mockResolvedValue("Rollback"); + registerCommands(makeContext() as unknown as import("vscode").ExtensionContext, deps); + + await vscodeMock._registeredCommands.get("openspec-ui.rollbackChange")?.(changeItem); + + expect(deps.implementationSessions.rollbackChange).toHaveBeenCalledWith("demo-change"); + expect(deps.refreshTrees).toHaveBeenCalled(); + }); + + it("reports no rollback-eligible processes instead of showing a confirmation", async () => { + const deps = makeDeps(); + registerCommands(makeContext() as unknown as import("vscode").ExtensionContext, deps); + + await vscodeMock._registeredCommands.get("openspec-ui.rollbackChange")?.(changeItem); + + expect(vscodeMock.window.showWarningMessage).toHaveBeenCalledWith( + expect.stringContaining("no rollback-eligible processes"), + ); + expect(vscodeMock.window.showWarningMessage).not.toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ modal: true }), + expect.anything(), + ); + expect(deps.implementationSessions.rollbackChange).not.toHaveBeenCalled(); + }); + + it("does not roll back when the confirmation is declined", async () => { + const deps = makeDeps(); + (deps.implementationSessions.changeRollbackDetails as ReturnType).mockReturnValue({ + processCount: 1, + fileCount: 1, + }); + vscodeMock.window.showWarningMessage.mockResolvedValue(undefined); + registerCommands(makeContext() as unknown as import("vscode").ExtensionContext, deps); + + await vscodeMock._registeredCommands.get("openspec-ui.rollbackChange")?.(changeItem); + + expect(deps.implementationSessions.rollbackChange).not.toHaveBeenCalled(); + }); + + it("reports conflicts as an error instead of refreshing trees", async () => { + const deps = makeDeps(); + (deps.implementationSessions.changeRollbackDetails as ReturnType).mockReturnValue({ + processCount: 1, + fileCount: 1, + }); + (deps.implementationSessions.rollbackChange as ReturnType).mockResolvedValue({ + restored: [], + conflicts: ["a.txt"], + }); + vscodeMock.window.showWarningMessage.mockResolvedValue("Rollback"); + registerCommands(makeContext() as unknown as import("vscode").ExtensionContext, deps); + + await vscodeMock._registeredCommands.get("openspec-ui.rollbackChange")?.(changeItem); + + expect(vscodeMock.window.showErrorMessage).toHaveBeenCalledWith( + expect.stringContaining("a.txt"), + ); + expect(deps.refreshTrees).not.toHaveBeenCalled(); + }); + + it("works the same for an archived change item", async () => { + const deps = makeDeps(); + (deps.implementationSessions.changeRollbackDetails as ReturnType).mockReturnValue({ + processCount: 1, + fileCount: 1, + }); + (deps.implementationSessions.rollbackChange as ReturnType).mockResolvedValue({ + restored: ["a.txt"], + conflicts: [], + }); + vscodeMock.window.showWarningMessage.mockResolvedValue("Rollback"); + registerCommands(makeContext() as unknown as import("vscode").ExtensionContext, deps); + + await vscodeMock._registeredCommands.get("openspec-ui.rollbackChange")?.({ + changeName: "archived-change", + archived: true, + }); + + expect(deps.implementationSessions.rollbackChange).toHaveBeenCalledWith("archived-change"); + }); + }); + it("openspec-ui.status: picks a change, runs direct OpenSpec status flow, and reveals the panel", async () => { listChangesMock.mockResolvedValue({ changes: [{ name: "shared-ui", completedTasks: 1, totalTasks: 17, lastModified: "t", status: "in-progress" }], diff --git a/packages/extension/src/commands.ts b/packages/extension/src/commands.ts index d40c726a..b607f441 100644 --- a/packages/extension/src/commands.ts +++ b/packages/extension/src/commands.ts @@ -664,6 +664,31 @@ export function registerCommands(context: vscode.ExtensionContext, deps: Command await showCommandError("rollback", error); } }), + vscode.commands.registerCommand("openspec-ui.rollbackChange", async (item?: ChangeTreeItem) => { + if (!item) return; + const details = deps.implementationSessions.changeRollbackDetails(item.changeName); + if (!details) { + void vscode.window.showWarningMessage(`OpenSpec UI: no rollback-eligible processes for ${item.changeName}.`); + return; + } + const answer = await vscode.window.showWarningMessage( + `Rollback ${item.changeName}? This restores ${details.fileCount} file${details.fileCount === 1 ? "" : "s"} across ${details.processCount} process${details.processCount === 1 ? "" : "es"} to their state before this change was ever implemented.`, + { modal: true }, + "Rollback", + ); + if (answer !== "Rollback") return; + try { + const result = await deps.implementationSessions.rollbackChange(item.changeName); + if (result.conflicts.length > 0) { + void vscode.window.showErrorMessage(`OpenSpec UI: rollback blocked by later changes: ${result.conflicts.join(", ")}`); + return; + } + deps.refreshTrees(); + void vscode.window.showInformationMessage(`OpenSpec UI: restored ${result.restored.length} files.`); + } catch (error) { + await showCommandError("rollback change", error); + } + }), vscode.commands.registerCommand("openspec-ui.cancelProcess", (item?: { process?: { id?: string } }) => { if (item?.process?.id) deps.implementationSessions.cancel(item.process.id); }), diff --git a/packages/extension/src/extension.ts b/packages/extension/src/extension.ts index 67038802..a26880fe 100644 --- a/packages/extension/src/extension.ts +++ b/packages/extension/src/extension.ts @@ -69,6 +69,17 @@ export async function activate(context: vscode.ExtensionContext): Promise("checkpointRetentionDays", 0); + if (retentionDays > 0) { + const cutoff = new Date(Date.now() - retentionDays * 24 * 60 * 60 * 1000); + implementationSessions.dropSessions(scheduler.removeBefore(cutoff)); + } const processesTree = new ProcessesTreeProvider(scheduler); context.subscriptions.push( processesTree, diff --git a/packages/extension/src/implementation-sessions.test.ts b/packages/extension/src/implementation-sessions.test.ts index c1bf4ff2..c4f345ae 100644 --- a/packages/extension/src/implementation-sessions.test.ts +++ b/packages/extension/src/implementation-sessions.test.ts @@ -95,4 +95,60 @@ describe("ImplementationSessionManager", () => { expect((await manager.rollback(process.id)).conflicts).toEqual([]); expect(await readFile(filePath, "utf8")).toBe("before"); }); + + it("rolls back every session for a change across two runs, to the earliest state", async () => { + const root = await mkdtemp(path.join(os.tmpdir(), "openspec-session-")); + roots.push(root); + const filePath = path.join(root, "shared.txt"); + await writeFile(filePath, "v0"); + const scheduler = new WorkbenchProcessScheduler(); + const manager = new ImplementationSessionManager(scheduler); + + await manager.run(root, { + operation: "implement", + changeName: "demo", + mutating: true, + execute: async () => { await writeFile(filePath, "v1"); }, + }); + await manager.run(root, { + operation: "implement", + changeName: "demo", + mutating: true, + execute: async () => { await writeFile(filePath, "v2"); }, + }); + + expect(manager.changeRollbackDetails("demo")).toEqual({ processCount: 2, fileCount: 1 }); + await expect(manager.rollbackChange("demo")).resolves.toEqual({ restored: ["shared.txt"], conflicts: [] }); + expect(await readFile(filePath, "utf8")).toBe("v0"); + }); + + it("throws rollbackChange for a change with no rollback-eligible sessions", async () => { + const scheduler = new WorkbenchProcessScheduler(); + const manager = new ImplementationSessionManager(scheduler); + await expect(manager.rollbackChange("nonexistent")).rejects.toThrow( + 'No rollback-eligible processes for change "nonexistent"', + ); + expect(manager.changeRollbackDetails("nonexistent")).toBeUndefined(); + }); + + it("dropSessions removes a session so it no longer participates in change rollback", async () => { + const root = await mkdtemp(path.join(os.tmpdir(), "openspec-session-")); + roots.push(root); + const filePath = path.join(root, "code.ts"); + await writeFile(filePath, "before"); + const scheduler = new WorkbenchProcessScheduler(); + const manager = new ImplementationSessionManager(scheduler); + + const process = await manager.run(root, { + operation: "implement", + changeName: "demo", + mutating: true, + execute: async () => { await writeFile(filePath, "after"); }, + }); + + manager.dropSessions([process.id]); + + expect(manager.getDelta(process.id)).toBeUndefined(); + expect(manager.changeRollbackDetails("demo")).toBeUndefined(); + }); }); diff --git a/packages/extension/src/implementation-sessions.ts b/packages/extension/src/implementation-sessions.ts index cfe8837c..1bd14dea 100644 --- a/packages/extension/src/implementation-sessions.ts +++ b/packages/extension/src/implementation-sessions.ts @@ -3,10 +3,12 @@ import { deserializeCheckpoint, finalizeCheckpoint, rollbackCheckpoint, + rollbackChangeCheckpoints, serializeCheckpoint, type CheckpointDelta, type CheckpointCoverage, type PersistedCheckpointSession, + type RollbackResult, type StartProcessOptions, type WorkbenchCheckpoint, type WorkbenchProcess, @@ -144,6 +146,47 @@ export class ImplementationSessionManager { return result; } + /** Every rollback-eligible session belonging to `changeName`, active or + * archived — same eligibility rule as `WorkbenchRecoveryService`'s + * mirror of this method (`@openspec-ui/core`), duplicated here because + * the extension's primary mode keeps its own session map instead of + * going through that service (see module header). */ + private changeRollbackCandidates(changeName: string): CheckpointSession[] { + return [...this.sessions.values()] + .filter((session) => session.changeName === changeName && session.delta) + .map((session) => ({ session, process: this.scheduler.list().find((candidate) => candidate.id === session.processId) })) + .filter((entry): entry is { session: CheckpointSession; process: WorkbenchProcess } => + entry.process !== undefined && ["completed", "failed", "interrupted"].includes(entry.process.state)) + .map((entry) => entry.session); + } + + changeRollbackDetails(changeName: string): { processCount: number; fileCount: number } | undefined { + const sessions = this.changeRollbackCandidates(changeName); + if (sessions.length === 0) return undefined; + const paths = new Set(); + for (const session of sessions) for (const delta of session.delta ?? []) paths.add(delta.path); + return { processCount: sessions.length, fileCount: paths.size }; + } + + async rollbackChange(changeName: string): Promise { + const sessions = this.changeRollbackCandidates(changeName); + if (sessions.length === 0) throw new Error(`No rollback-eligible processes for change "${changeName}"`); + const result = await rollbackChangeCheckpoints(sessions.map((session) => session.checkpoint)); + if (result.conflicts.length === 0) { + for (const session of sessions) { + this.scheduler.markRolledBack(session.processId, `change "${changeName}" rolled back: ${result.restored.length} files restored`); + } + } + return result; + } + + /** Retention pruning (`openspec-ui.checkpointRetentionDays`) — drops + * sessions whose process was removed from the scheduler by + * `WorkbenchProcessScheduler.removeBefore`. */ + dropSessions(processIds: readonly string[]): void { + for (const id of processIds) this.sessions.delete(id); + } + getDelta(processId: string): CheckpointDelta[] | undefined { return this.sessions.get(processId)?.delta?.map((item) => ({ ...item })); } diff --git a/packages/server/package.json b/packages/server/package.json index a40c6d41..2bd76066 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -1,7 +1,7 @@ { "name": "@openspec-ui/server", "private": true, - "version": "1.7.1", + "version": "1.8.0", "type": "module", "main": "src/index.ts", "dependencies": { diff --git a/packages/server/src/recovery-rest.ts b/packages/server/src/recovery-rest.ts index b3e2225c..42e343f8 100644 --- a/packages/server/src/recovery-rest.ts +++ b/packages/server/src/recovery-rest.ts @@ -12,6 +12,7 @@ interface RecoveryRequest { cwd: string; processId?: string; cutoff?: string; + changeName?: string; } export type RecoveryServiceResolver = (cwd: string) => Promise; @@ -111,6 +112,18 @@ export function handleProcessRollbackRequest( }); } +export function handleChangeRollbackRequest( + req: IncomingMessage, + res: ServerResponse, + policy: RestRequestPolicy, + resolveService: RecoveryServiceResolver, +): Promise { + return withRecoveryRequest(req, res, policy, resolveService, (request, service) => { + if (!request.changeName) throw new Error("changeName is required"); + return service.rollbackChange(request.changeName); + }); +} + export function handleProcessesCleanupRequest( req: IncomingMessage, res: ServerResponse, diff --git a/packages/server/src/server.test.ts b/packages/server/src/server.test.ts index 50e43c8a..20ac7456 100644 --- a/packages/server/src/server.test.ts +++ b/packages/server/src/server.test.ts @@ -14,6 +14,9 @@ import { getCoreVersion, OpenSpecCliCompatibilityError, WorkbenchRunJournal, + captureCheckpoint, + finalizeCheckpoint, + serializeCheckpoint, type AgentRunner, type Command, type Event, @@ -620,6 +623,38 @@ describe("server — REST /api/status", () => { expect((await new WorkbenchRunJournal(cwd).load()).processes).toEqual([]); }); + it("rolls back every process for a change across two checkpoints, to the earliest state", async () => { + const cwd = await createTempWorkspace(); + const filePath = path.join(cwd, "shared.txt"); + await writeFile(filePath, "v0", "utf8"); + const checkpoint1 = await captureCheckpoint(cwd); + await writeFile(filePath, "v1", "utf8"); + await finalizeCheckpoint(checkpoint1); + const checkpoint2 = await captureCheckpoint(cwd); + await writeFile(filePath, "v2", "utf8"); + await finalizeCheckpoint(checkpoint2); + await new WorkbenchRunJournal(cwd).save({ + processes: [ + { id: "run-1", operation: "implement", changeName: "demo-change", mutating: true, state: "completed", createdAt: "2026-08-01T00:00:00.000Z" }, + { id: "run-2", operation: "implement", changeName: "demo-change", mutating: true, state: "completed", createdAt: "2026-08-02T00:00:00.000Z" }, + ], + checkpointSessions: [ + { processId: "run-1", changeName: "demo-change", checkpoint: serializeCheckpoint(checkpoint1) }, + { processId: "run-2", changeName: "demo-change", checkpoint: serializeCheckpoint(checkpoint2) }, + ], + }); + + const response = await fetch(`${baseUrl}/api/processes/rollback-change`, { + method: "POST", + headers: JSON_HEADERS, + body: JSON.stringify({ cwd, changeName: "demo-change" }), + }); + + expect(response.status).toBe(200); + expect(await response.json()).toEqual({ restored: ["shared.txt"], conflicts: [] }); + expect(await readFile(filePath, "utf8")).toBe("v0"); + }); + it("returns actionable diagnostics without replacing a future run journal", async () => { const cwd = await createTempWorkspace(); const journal = new WorkbenchRunJournal(cwd); diff --git a/packages/server/src/server.ts b/packages/server/src/server.ts index 2fb6a453..56071c81 100644 --- a/packages/server/src/server.ts +++ b/packages/server/src/server.ts @@ -26,6 +26,7 @@ import { import { handleSocketMessage } from "./websocket.js"; import { tryServeStatic, type StaticAssetPaths } from "./static.js"; import { + handleChangeRollbackRequest, handleProcessDetailsRequest, handleProcessRollbackRequest, handleProcessesCleanupRequest, @@ -197,6 +198,10 @@ export function createServer(options: ServerOptions): OpenSpecUiServer { void handleProcessRollbackRequest(req, res, requestPolicy, resolveRecoveryService); return; } + if (req.method === "POST" && req.url === "/api/processes/rollback-change") { + void handleChangeRollbackRequest(req, res, requestPolicy, resolveRecoveryService); + return; + } if (req.method === "POST" && req.url === "/api/processes/cleanup") { void handleProcessesCleanupRequest(req, res, requestPolicy, resolveRecoveryService); return;