Persist plans, ledger, and review revisions in SQLite

[mchwang]

The plan/linking library previously relied on caller-supplied state that disappeared on restart. Add the next planned storage slice in runner/store using Node SQLite: immutable plan revisions and base/head snapshots, trusted ownership/rebase mappings, persisted approvals/choices, and audited execution checkpoints.

Suggestion requests are bound to saved identity/revision before a response arrives. Apply atomically allocates the next revision, consumes the request, and invalidates siblings; competing processes cannot both apply stale suggestions. Rewritten missing/foreign commits retain null ownership. Checkpoint continuation records preserve original out-of-scope evidence.

Validation: 157 tests pass (baseline 136), including two-process Apply contention, abrupt-exit recovery, transaction rollback, real-Git linking from remapped ownership, stale approval writes, and warning-free SQLite startup. npm run typecheck and git diff --check pass.

Decisions and integration boundaries: docs/implementation/persistent-review-store.md. The API is for the trusted runner; actual filesystem audits, prefix reconciliation/suffix validation, and execution state-machine enforcement remain in #6. No UI or agent execution is included. #3 is next, and the go/no-go experiment still gates later build steps.

Closes #2.

Review round 1: reproduced and fixed both summary findings (ledger comparison must ignore property order; checkpoint item must end the completed prefix). No findings declined. Regressions failed before the fixes and pass afterward.

Review round 2: reproduced and fixed historical-owner retries after plan amendment, while still rejecting new entries for removed owners.

Review round 3: fixed repeated/new-revision continuation approval with failing regressions. Declined duplicate-source mappings as a correctness issue: an added test already passed, proving the existing primary key and transaction reject and fully roll back the batch.

Review round 4: reproduced and fixed orphaned segment assignments after item removal; expiration is atomic with revision creation.

Review round 5: reproduced and fixed approval revival, unknown identity-mapping provenance, and linking of historical owners missing from the selected revision. Raw ledger ownership remains immutable; its revision-specific linking view maps absent owners to Unplanned.

Review complete: round 6 on 600ef0d returned no findings and recommends approval. All threads are resolved; both CI runs pass on that head. Six rounds total. The only declined finding was duplicate source mappings, already rejected atomically and now covered by a regression.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Two moderate unresolved findings remain in runner/store.ts.

Review effort: Lite
Findings: None

What changed in this PR

Adds durable SQLite-backed persistence for plans, revisions, ledger mappings, approvals, suggestions, and execution checkpoints.

Changes:

• Implements transactional storage and concurrency control.
• Adds persistence, recovery, rollback, and contention tests.
• Includes runner sources in typechecking.
• Documents storage contracts and integration boundaries.

File	Summary
tsconfig.json	Includes runner sources in typechecking.
test/​store.test.ts	Adds persistence, recovery, rollback, and concurrency coverage.
runner/​store.ts	Implements SQLite persistence and atomic state transitions. Two moderate findings remain (1 vote each): canonicalize ledger comparisons and require the checkpoint item to be the last completed item.
docs/​implementation/​persistent-review-store.md	Documents storage decisions, contracts, and integration boundaries.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[mchwang]

Addressed both findings from round 1's summary in the latest commit:

• Ledger retries compare SHA, owner, origin, and source SHA directly, so JSON property order cannot cause a false ownership conflict.
• A checkpoint's item must be the final item in its executed prefix.

Both regressions failed before the fixes and pass afterward. Full validation: 151 tests, typecheck, and diff formatting pass. There were no inline review threads to resolve and no declined findings. Requesting a fresh review of the updated head.

[Copilot]

Copilot review overview

🟡 Changes recommended

Unresolved critical and moderate findings remain in runner/store.ts.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Skip current-owner validation for existing ledger entries · New

[Copilot]

Copilot review overview

🔵 Needs a closer look

Address the two moderate issues in runner/store.ts involving continuation reapproval and duplicate source SHA mappings.

Review effort: Lite
Findings: None

Resolved since last review (1)

• Skip current-owner validation for existing ledger entries

[mchwang]

Round 3 response:

• Fixed continuation reapproval. Each approved revision gets an immutable record; the getter returns the latest approved revision. Repeating the same approval is idempotent. Both cases were reproduced as failing regressions before the fixes.
• Declined duplicate source SHA mappings as a correctness finding. The existing (key, snapshot_id, old_sha) primary key rejects a repeated source, and the enclosing transaction rolls back the entire snapshot/ledger/mapping batch. A new regression passed before any implementation change and observes the original snapshot, an empty ledger, and no mappings after the rejection. No silent partial mapping is accepted.

Final checks: 154 tests, typecheck, and diff formatting pass. Requesting a fresh assessment of the updated head.

[Copilot]

Copilot review overview

🔵 Needs a closer look

A moderate unresolved issue affects assignments targeting removed items and can cause applyChoices() to fail.

Review effort: Lite
Findings: None

[mchwang]

Fixed the round 4 summary finding: saving an amended revision now expires assignments targeting removed items within the same transaction. Unrelated choices remain. Reintroducing an item ID cannot revive the old assignment. The regression reproduced Assigned item does not exist before the fix and passes afterward.

Full validation on this head: 155 tests, typecheck, and diff formatting pass. Requesting another review; no finding declined in this round.

[Copilot]

Copilot review overview

🟡 Changes recommended

Two critical persistence issues and one moderate ownership integration issue remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2

Open (2)

• Remove stale approvals when plan items are removed · New
• Persist foreign entries for missing identity sources · New

[mchwang]

All round 5 findings are addressed with failing-before/passing-after regressions. In addition to the two inline fixes, the ownership integration case is fixed: ownership(identity, revision) maps owners absent from that revision to null for linking, while getLedger retains the immutable original owner. The real-Git integration test now removes the owner from the plan and successfully links its changes as Unplanned. Pass the same revision to ownership and plan retrieval.

Final validation: 157 tests, typecheck, and diff formatting pass. No findings declined in this round. Requesting a fresh review.

[Copilot]

Copilot review overview

🟢 Approval recommended

No unresolved blocking issues remain, and all readiness assessments approve.

Review effort: Lite
Findings: None

Resolved since last review (2)

• Persist foreign entries for missing identity sources
• Remove stale approvals when plan items are removed