#268 — Atomic (transactional) plot save: commitPlotSave / reconcilePlotSave#658
Merged
Conversation
The `.debrief-session` sidecar was retired by #249/spec-261, so item #268 ("VS Code save atomicity") no longer describes the code it cites: saveSession.ts now performs two write phases (features.geojson + thumbnail/STAC assets), not three, and the old line references (163-208) and "FC<->sidecar pair" framing are dead. Rewrite the row to reflect the current two-write flow with correct line numbers, keep it as a parked transactional-save tech-debt item (the non-atomic save risk survives the sidecar's removal), and note the cross-host StacWriter angle that keeps complexity medium-to-high. Trimmed the estimate to ~5-7 dev-days and bumped the Updated date. https://claude.ai/code/session_01Q6WZYZRVLVmHdwpWLSNV5x
Contributor
📋 Backlog NavigatorReview this PR's BACKLOG.md edits interactively: Updated for 850b864 — the navigator reads BACKLOG.md live from this PR's head branch, so no rebuild is needed. |
Author the feature spec for backlog item #268 (rescoped in the previous commit). Captures the real gap after the spec-261 sidecar retirement: a save is several independent writes (feature collection + STAC item + thumbnail assets), the feature-collection write bypasses the shared persistence boundary with a non-atomic raw file write, and there is no way to commit the writes as one unit — so a failure or interruption mid-save can leave a partial / internally-inconsistent plot. Spec defines all-or-nothing save semantics (US1), honest success/failure reporting with safe retry (US2), and coherent recovery after an uncatchable interruption (US3), enforced at the shared writer boundary so both hosts benefit. Commit mechanism (staging+atomic-move vs last-good recovery file) is deliberately left to /speckit.plan. Also link the spec from BACKLOG #268 and move its status approved -> specified (preserves the ID <-> spec-dir invariant). Spec dir is pinned via .specify/.active-feature for this cloud session (branch stays claude/eloquent-fermi-bzLml). https://claude.ai/code/session_01Q6WZYZRVLVmHdwpWLSNV5x
Contributor
📋 Spec NavigatorReview this PR's specs interactively:
|
IanMayo
changed the title
Record the /speckit.clarify Session 2026-06-01 decisions in the spec and tighten the affected requirements: - Host scope = BOTH hosts (desktop FS + browser storage), enforced at the shared persistence boundary (FR-009 already mandated this; now confirmed). - Interrupted-save recovery = automatically restore the last good version + non-blocking notice, no analyst prompt (FR-008) — keeps this non-UI. - Reliability target = atomicity with a coherent fallback, NOT guaranteed power-loss durability of the newest save (FR-005 reworded off "durably committed"; FR-007 clarified; removed the contradicting US2 wording). Also: add the ## Clarifications section, update the checklist notes to mark the two flagged assumptions resolved, and move BACKLOG #268 status specified -> clarified. Commit mechanism stays deferred to /speckit.plan. https://claude.ai/code/session_01Q6WZYZRVLVmHdwpWLSNV5x
/speckit.plan output for 268-save-atomicity. Resolves the deferred
commit-mechanism decision and designs the multi-write transaction at the
shared StacWriter boundary.
Artifacts:
- plan.md — summary, technical context, constitution check (PASS,
no violations), source-file map, media=none
- research.md — grounding (current save/load/writer call sites) +
8 decisions; FS write-ahead intent journal as the
atomic commit point, web-shell single IDB transaction
- data-model.md — CommitPlotSaveInput/Result, SaveJournal, ReconcileResult
(boundary DTOs derived via Pick per Article IV.5)
- contracts/ — stac-writer-commit.ts (type contract, C1-C5 behaviour)
+ save-flow.md (save + reconcile-on-open sequence)
- quickstart.md — how to verify (fault-injection acceptance tests)
- evidence/opening-context.md — cached blog opener (mermaid Hook)
Design: add commitPlotSave + reconcilePlotSave to StacWriter; FS adaptor
stages temps -> writes journal (commit point) -> applies renames -> clears,
reconcile rolls back before the journal / forward after; web-shell wraps
the save in one multi-store transaction. saveSession reports success only
after commit; openPlot reconciles before the read. No new dependencies;
no schema change. BACKLOG #268 clarified -> planned.
https://claude.ai/code/session_01Q6WZYZRVLVmHdwpWLSNV5x
/speckit.tasks output for 268-save-atomicity. 31 tasks across 6 phases:
- Phase 1 Setup (2) — ADR for the save-journal decision; fault-injection helper
- Phase 2 Foundation (4) — extend StacWriter (commitPlotSave/reconcilePlotSave +
DTOs), SaveJournal type, stub doubles, Pick-derivation guard
- Phase 3 US1/P1 (8) — commitPlotSave atomicity on both adaptors + route both
save call sites through it (the MVP)
- Phase 4 US2/P2 (3) — honest reporting: success only after commit, retryable failure
- Phase 5 US3/P3 (7) — reconcilePlotSave + reconcile-before-read on open + notice
- Phase 6 Polish (7) — full gate, web-shell smoke, evidence (test-summary,
usage-example, fault-injection-matrix), blog post, PR
Tests-first per story (contracts C1-C5 <-> SC-001..005). Final task T031 runs
/speckit.pr. BACKLOG #268 planned -> tasked.
https://claude.ai/code/session_01Q6WZYZRVLVmHdwpWLSNV5x
…ave (#268) Phase 1-2 (Setup + Foundation): - ADR-039: write-ahead intent journal as the FS commit point; one IDB transaction on the browser host - Add commitPlotSave/reconcilePlotSave + DTOs to the StacWriter interface (thumbnails derived via Pick; featureCollection reuses the generated RawGeoJSONFeatureCollection) and export them from the package index - Add the FS-only SaveJournal type + typed parseSaveJournal() validator - Add a shared Proxy-based fault-injection writer wrapper for the saveSession/open integration tests (auto-covers new write methods) - Add a compile-time Pick-derivation guard (mutation-verified)
… it (#268 US1) Phase 3 (User Story 1 — a failed save never corrupts my plot): - FS adaptor commitPlotSave: stage temps → write intent journal (commit point) → apply renames → clear; pre-commit failure rolls back (originals byte-identical, temps cleaned); apply-phase failure leaves the journal for roll-forward on open. + buildItemWithThumbnails / applyJournalRenames helpers. - IDB adaptor commitPlotSave: one readwrite transaction over items+payloads (+meta); resolves existing/bundled/minimal item; aborts roll back cleanly. reconcilePlotSave returns clean (IDB never exposes partial state). - VS Code saveSession now commits FC + thumbnails through commitPlotSave in one unit (raw fs.writeFileSync of features.geojson gone — FR-004); success reported only after the commit resolves; capture failure is non-fatal. - web-shell createStandaloneItem routes its item+payload write through commitPlotSave (closes the two-transaction gap). - Re-export RawGeoJSONFeatureCollection from @debrief/stac-writer. - Tests: FS commit (5), IDB commit (4), saveSession routing (4), saveSession integration via fault-injection helper (2). All green; typecheck clean.
Phase 4 (honest reporting): - saveSession reporting-order test proves markClean + 'Plot saved' fire only after commitPlotSave resolves; a rejected commit shows failure, keeps dirty, shows no success (SC-003). Phase 5 (coherent plot after an interrupted save): - FS reconcilePlotSave: journal present → roll forward (apply pending renames idempotently, drop journal); stray temps + no journal → roll back to last-good; clean → no-op; malformed journal → safe roll-back. + listSaveTemps. - IDB reconcilePlotSave: clean no-op (IndexedDB transactions are atomic). - VS Code open path runs reconcileBeforeOpen (extracted module) before the read; non-blocking warning on recovery. - web-shell getPlotData reconciles before the read (no-op, symmetric). - Tests: FS reconcile (6), IDB reconcile (3), open-path integration (4). Green; both apps typecheck clean.
IanMayo
changed the title
…i-key false positive)
Contributor
🚀 Preview DeploymentsCode Server (Full VS Code Extension)Browser-based VS Code with the Debrief extension and sample data pre-installed. Web Shell (Standalone App)Use this for Playwright testing and demos - runs outside Storybook. Storybook (Component Library)Browse all components, stories, and documentation. Briefing Renderer (Air-gapped Storyboard Player — spec #264)Standalone SPA that plays a Storyboard from a All Links
|
…-bzLml # Conflicts: # apps/vscode/src/commands/saveSession.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements backlog item #268 — make a plot save all-or-nothing. Spec, plan, and tasks were authored earlier on this branch; this PR now carries the full implementation + evidence.
The problem
A "Save" is several writes —
features.geojson, the STACitem.json, and the thumbnail PNGs. They landed independently and "Plot saved" appeared before the last of them committed. The feature-collection write was a rawfs.writeFileSyncthat bypassed theStacWriterboundary. So a failure or interruption mid-save could leave a half-updated plot (new tracks + stale thumbnail, a torn feature file, or a "saved" plot that never fully wrote).The approach (ADR-039)
One host-agnostic boundary operation,
commitPlotSave, commits the whole save unit atomically, plusreconcilePlotSave, called before the read on open, to heal an interrupted save. Enforced at the shared@debrief/stac-writerboundary so neither host can regress it (FR-009), and the FC write moves back onto the boundary (FR-004)..save-journal.json→ apply renames → clear. Reconcile rolls back before the journal exists (discard temps, keep originals = last-good) and forward after it (finish the renames = new version). Every interruption point resolves to one coherent plot.readwritetransaction (items+payloads) — atomic for free; a killed tab discards uncommitted work. The only gap was two writes in two transactions; now they share one. Reconcile is a clean no-op.markClean()+ "Plot saved" moved to after the commit resolves; a rejected commit shows a failure, keeps the plot dirty, and leaves the previous version intact.What changed
shared/stac-writer/src/interface.ts+commitPlotSave/+reconcilePlotSave+ DTOs;thumbnailsisPick<WritePlotThumbnailPairInput,…>;featureCollectionreusesRawGeoJSONFeatureCollectionapps/vscode/src/services/stacWriterFs.tscommitPlotSave(stage→journal→apply→clear) +reconcilePlotSave(roll back/forward, idempotent)apps/vscode/src/services/saveJournal.tsSaveJournaltype + typedparseSaveJournal()(noany)apps/vscode/src/commands/saveSession.tscommitPlotSave; success only after commitapps/vscode/src/commands/openPlot.ts+reconcileOnOpen.tsapps/web-shell/src/services/stacWriterIdb.tscommitPlotSave(one transaction) +reconcilePlotSave(clean)apps/web-shell/src/mocks/stacService.tscreateStandaloneItem+getPlotDataroute through commit / reconciledocs/project_notes/decisions.mdScene-capture's eager
features.geojsonwrite is a documented, deliberate deferral (best-effort single-file write, superseded by Save Session) — see ADR-039.Evidence (
specs/268-save-atomicity/evidence/)fault-injection-matrix.md— the credibility artifact: 8 filesystem + 4 IndexedDB interruption points each driven to a coherent outcome (SC-001/002/005).test-summary.md,usage-example.md, and the feature postmedia/shipped-post.md.Tests: 32 new (31 Vitest unit/integration + 1 web-shell Playwright smoke) + a mutation-verified compile-time
Pickguard, all green. The IndexedDB save is proven to use exactly one transaction; "Plot saved" fires for 0% of saves that did not fully commit (SC-003).Full-suite regression (same SHA, all green):
@debrief/stac-writer22,debrief-vscode841 (+1 pre-existing skip),@debrief/web-shell135 Vitest;pnpm -r typecheck+ vscodetsc --noEmit+ ESLint + ruff + pyright clean. No Python changed. (One timing-flakyshared/schemasperf-budget test passes on re-run.)Constitution
Serves Article I.3 (silent partial save → explicit success/failure), Article IV.2/IV.4 (FC write back onto the boundary; one implementation per host), and Article IV.5 (boundary save-unit type structurally derived with a compile-time guard). No schema change. PASS.
https://claude.ai/code/session_01Q6WZYZRVLVmHdwpWLSNV5x