feat(actions): durable node:sqlite action store — Phase 1 dual-write mirror#359
Merged
Conversation
…design) Approved brainstorming design for durable, structured action storage: YAML stays the git-tracked source of truth; a derived, gitignored node:sqlite DB (.rn-agent/state/actions.db) holds the corpus index + run/repair history, replacing the per-action JSON sidecars. Two phases: P1 DB layer + migration + graceful degradation; P2 reconcile() loud diagnostics + #348/#357 resolution guard (subsumes #357). All source TypeScript; learned-actions.mjs migrated to TS. Non-goals: cross-project library, team/remote sync. Refs #357, #348, #300 (item 3); #356 unblocked as a separate spec. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
9-task TDD plan for the node:sqlite store: action-db (open/schema/degrade, state load-save, sidecar migration), backend-selecting facade, wire run/repair/save-as-action call-sites, cdp_status.actionStore, supervisor --experimental-sqlite + engines>=22.5 + gitignore, learned-actions.mjs→TS, changeset+docs. YAML stays source of truth; DB derived/gitignored/rebuildable. Refs spec 2026-06-19-action-storage-persistence-design.md. Phase 2 (reconcile + #348/#357 resolution guard) is a separate plan. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude Opus + Codex review returned no-ship-as-is; applied findings to spec + plan
before any code (CLAUDE.md workflow step 2):
- A1/P0: ESM build → require('node:sqlite') ships dead; use createRequire(import.meta.url);
bump @types/node ^20→^22.5/^24; CI asserts loadSqlite() non-null on Node 24.
- A2/P0: real seam is domain/action-store.ts (loadAction/saveAction/saveActionWithCAS),
not the 3 tool files — wiring only tools = split-brain (stale-sidecar reads, DB writes).
- A3/P0: Phase 1 is additive — sidecars stay authoritative, DB is a mirror; flip authority
in Phase 2 with reconcile() so deleting the gitignored DB can't silently lose history.
- A4/P1: keep engines >=22 (not >=22.5); version-gated sqliteFlagForNode (22.5–23.5 only);
verify RN_BRIDGE_SUPERVISOR=0 in-process path degrades.
- A5/P1: append-and-trim in BEGIN IMMEDIATE + busy_timeout + WAL; explicit stats_json
(don't recompute cumulative totalRuns from capped history); preserve failure_detail; COALESCE.
- A6/P1: compile learned-actions to dist/ (strip-types needs 22.6 + won't resolve .ts specifiers).
- A7/P2: read-only 3-way storeMode; ESM tests; side-effect-free workerSpawnArgs; dbCache
close/reset lifecycle; drop redundant state/*.db gitignore (templates already ignore state/).
Gemini was unavailable (Code Assist individual tier retired → use antigravity,codex next time).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…degradation (Task 1)
Adds `scripts/cdp-bridge/src/domain/action-db.ts`:
- `loadSqlite()` — ESM-safe dynamic loader via `createRequire(import.meta.url)`;
returns `DatabaseSync` ctor or null (never throws)
- `openActionDb(projectRoot, opts?)` — opens `.rn-agent/state/actions.db`,
creates the three-table schema (actions_index + stats_json, run_records with
failure_detail, repair_records), sets PRAGMA busy_timeout=5000 + journal_mode=WAL,
and returns `{ db, close }` or null on any failure
- Graceful degradation path tested via `opts.sqliteCtor: null`
- CI-loud test asserts `loadSqlite()` is non-null on Node 24 so a future
ESM-loader regression fails visibly
- Bumps @types/node devDependency from ^20 to ^24 for Node 22.5+ API types
TDD: 4/4 unit tests pass; full 2367-test suite remains green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…insertRepairRecord, upsertIndex (COALESCE), loadState (stats from stats_json), recentRepairCount Extends ActionDb handle with five read/write primitives: - insertRunRecord: IMMEDIATE tx INSERT + cap-trim to RUN_HISTORY_MAX=50 - insertRepairRecord: IMMEDIATE tx INSERT + cap-trim to REPAIR_HISTORY_MAX=25 - upsertIndex: INSERT…ON CONFLICT DO UPDATE with COALESCE so partial (stats-only) updates never null out app_id/path/content_hash/status - loadState: reconstructs ActionRuntimeState from DB; stats read from stored stats_json — NOT recomputed from capped row history (so totalRuns correctly exceeds runHistory.length) - recentRepairCount: COUNT repair_records WHERE ts >= sinceIso (repair budget gate) Tests (TDD, 8 new cases): round-trip, no-recompute proof (totalRuns=70 with 50 rows), trim-at-cap for both tables, COALESCE preservation, repair history round-trip, recentRepairCount windowing, transport field fidelity. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds migrateSidecars() to the ActionDb handle: scans <projectRoot>/.rn-agent/state/<id>.state.json files, skips any id already present in actions_index, imports runHistory/repairHistory via the Task-2 append primitives (preserves chronological order), upserts the index with revision/stats/mtimeBaseline. Skips corrupt JSON and schemaVersion !== 1 files without throwing. Returns { migrated: count }; second call returns 0 (idempotent by design).
Extends the test suite with 4 new cases: happy-path import + idempotency, missing state dir, corrupt/wrong-version skip, and a multi-record roundtrip covering both runHistory and repairHistory.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Phase 1 additive dual-write facade between the action-store/tool layer and action-db.ts. Sidecars stay authoritative (read source); the SQLite DB is a best-effort populated mirror + read-only surface. - loadOrInitState: reads from the authoritative sidecar (DB read flips in Phase 2) - persist: sidecar-first (never swallowed), then best-effort DB mirror that appends only the new run/repair record(s) and COALESCE-upserts the index; a mirror failure never throws and never corrupts the sidecar - storeMode: READ-ONLY 3-way detector (sqlite | degraded:sqlite-unavailable | degraded:open-failed) that never opens/migrates the DB - dbFor: lazy per-root open + one-time migrateSidecars, cached with a failed-open marker - resetActionStore / closeActionStoresForTest lifecycle; __setSqliteCtorForTest seam Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dist/ is tracked; Tasks 1-3 committed action-db.ts source but not the rebuilt dist/domain/action-db.js artifact, and Task 1's @types/node bump left package-lock.json unstaged. Reconcile so a fresh checkout has the compiled module the facade imports. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Wire the authoritative write paths to also mirror to the node:sqlite DB (A2/A3/A5). Sidecars stay authoritative + the read source; the DB is a best-effort, sidecar-less mirror that never throws into the authoritative path. - action-state-store: add sidecar-less mirrorToDb() (derives projectRoot from the .rn-agent/actions/<id>.yaml path, fails open for non-conventional paths); refactor persist() to compose saveSidecar + mirrorToDb (DRY, behavior intact). - action-store: saveAction + acknowledgeExternalEdit mirror the index/mtime AFTER the #101 atomic pair-write (no double sidecar write, no row). - run-action persistRun: append the RunRecord row only on saveActionWithCAS ok:true (never on the #117 CAS-conflict path). - repair-action: append the RepairRecord row after saveAction. - save-as-action: seed the DB index row after the initial pair-write. #101 atomic pair-write + #117 CAS semantics preserved (62 regression tests green; full suite 2390/2390). New TDD test pins the mirror, the path derivation, the never-throw guarantee, and the CAS conflict. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds `actionStore` field to the cdp_status response — a read-only
3-way detector (`'sqlite'` / `'legacy-files'` / `'degraded:<reason>'`)
that calls `storeMode()` without opening or migrating the DB.
Also narrows `storeMode`'s return type from `string` to the
discriminated `ActionStoreMode = 'sqlite' | 'legacy-files' | \`degraded:\${string}\``
so the 3-way contract is encoded in the type system.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…wn (Task 7) Add pure supervisor-args.ts with sqliteFlagForNode() and workerSpawnArgs(): flag is passed only for 22.5 ≤ node < 23.6 (where node:sqlite needs it); v < 22.5 degrades gracefully, v ≥ 23.6 already on by default. Wire supervisor.ts to use workerSpawnArgs() instead of inline array. Engines kept at >=22 (not bumped) so older Node degrades rather than rejects. gitignore unchanged — templates/rn-agent/.gitignore state/ already covers *.db. 12/12 new tests; 2405/2405 full suite green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Port scripts/learned-actions.mjs (480-line CLI inventory tool) to scripts/cdp-bridge/src/learned-actions.ts, compiled by tsc into dist/learned-actions.js via the existing tsconfig (rootDir=src, outDir=dist). Output is byte-identical to the golden baseline (verified diff empty). - Add explicit TypeScript types (Flags, MemoryItem, FlowItem, FlowMeta, ProducesMap, SkeletonItem, CommandItem, *Result interfaces) - Use `import type` where type-only - No logic changes — faithful port of all 4 sections (A/B/C/D), all flags, exit codes, and human/JSON output modes - Delete scripts/learned-actions.mjs (git rm) - Update all 7 invocation sites to scripts/cdp-bridge/dist/learned-actions.js: commands/list-learned-actions.md, commands/run-action.md, agents/rn-tester.md, agents/rn-debugger.md, skills/creating-actions/SKILL.md, skills/creating-actions/references/m7-header-reference.md, plus source comments in domain/reusable-action.ts and tools/test-recorder-generators.ts - Add parity test (learned-actions-parity.test.js, 8 assertions): fixture corpus → spawn compiled CLI → assert count/ids/fields/exit codes Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds .changeset/action-storage-phase1.md (minor bump for rn-dev-agent-cdp) describing the node:sqlite action history store, dual-write mirror, graceful degradation, version-gated --experimental-sqlite flag, and learned-actions TS migration (Tasks 1–8). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ndary double-count The dual-write flow writes the authoritative sidecar FIRST, then mirrorToDb runs. The first mirror per project triggers dbFor() -> migrateSidecars(), which imports the entire current sidecar history (already including the just-appended record); mirrorToDb then appended that same record AGAIN, duplicating the newest run/repair row for any migrated action. Make insertRunRecord / insertRepairRecord idempotent: within the same BEGIN IMMEDIATE transaction, skip the INSERT when a row for (action_id, ts) already exists. This absorbs the migration-boundary overlap and any other accidental re-append while preserving full pre-migration history (A5 append semantics, best-effort mirror, authoritative sidecar/#101/#117 untouched). Tests: - I1 regression (action-store-mirror): seed a legacy sidecar with run1+run2 / repair1+repair2, drive a record-bearing mirror -> DB == authoritative count (2, not 3), no duplicate ts; a distinct run3 then appends to 3, each once. - Fresh-action control: no legacy sidecar -> appends work normally. - T2 (action-db): loadState round-trips a fail RunRecord with both failureCode AND failureDetail. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b19bbb44ae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… test cleanup, CAS assertion Finding 1 (F1, MED): migrateSidecars now validates Array.isArray(runHistory/repairHistory) before importing — a malformed sidecar is skipped cleanly with no index row written (retryable). History rows are inserted FIRST, upsertIndex LAST, so "index row present" reliably means "fully migrated"; any mid-import throw leaves no partial row. Finding 2 (F2, MED): all test bodies that open action-store/sqlite singletons or probe handles now wrap in try/finally so closeActionStoresForTest()/probe.close() run even when an assertion throws — preventing state leakage into later tests. Finding 3 (F3, MED): the CAS #117 test now captures and asserts ok===true on the conflict-creating write before asserting the stale bWriter gets EXTERNAL_WRITE, so the assertion cannot pass for the wrong reason. Regression test added: F1 regression — malformed sidecar (non-array histories) leaves no actions_index row; valid sibling migrates fully; idempotent re-run adds no duplicate rows. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Codex PR review (P2): cdp_record_test_save_as_action seeded the DB mirror from the pre-write initialState (lastSeenMtimeMs=0), but pairWrite rewrites the sidecar with its finalMtimeMs. A DB-backed load (Phase 2) would then see mtime_baseline=0 and treat the just-written YAML as externally edited. Mirror a state carrying writeResult.finalMtimeMs, matching saveAction's existing handling. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Owner
Author
Lykhoyda
added a commit
that referenced
this pull request
Jun 19, 2026
…lls (#361) Add a rn-dev-agent-plugin changeset so the next Version Packages PR bumps plugin.json + marketplace.json (0.55.5 → 0.56.0). Past changesets only versioned rn-dev-agent-cdp, so the plugin manifest stayed pinned and the merged #351/#353/#359 never reached installed users via /plugin update. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Lykhoyda
added a commit
that referenced
this pull request
Jun 19, 2026
…hanges (#364) * fix(ci): require a rn-dev-agent-plugin changeset for cdp-bridge/src changes Close the #361/#363 delivery gap at the source. cdp-bridge ships to users only via the plugin manifest (plugin.json/marketplace.json), versioned by the synthetic rn-dev-agent-plugin package. A rn-dev-agent-cdp-only changeset bumps the internal package but leaves the manifest pinned — so the change reaches main but never reaches installs via /plugin update (cdp-bridge advanced 0.48→0.49 while the plugin sat at 0.55.5, stranding #351/#353/#359). require-changeset.sh now requires a rn-dev-agent-plugin changeset entry whenever scripts/cdp-bridge/src/ changes, with an actionable error. Regression test updated: cdp-only changeset must now fail; plugin-only and cdp+plugin pass. Keeps the two packages on independent version lines (no changesets linked/fixed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(ci): parse changeset frontmatter, not whole file, for plugin bump Codex PR #364 P1: grepping the whole changeset file for "rn-dev-agent-plugin" let a cdp-only changeset falsely pass if its release-note body merely mentioned the plugin (e.g. a quoted format example), recreating the delivery gap. Now extract the frontmatter (lines between the first/second ---, same awk as validate-changeset-names.sh) and match only a rn-dev-agent-plugin package key. Added regression test: cdp-only frontmatter + body mentioning the quoted plugin name must still fail. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(ci): accept single-quoted rn-dev-agent-plugin changeset keys Codex PR #364 P2: the frontmatter key regex matched bare and double-quoted keys but not valid-YAML single-quoted ones ('rn-dev-agent-plugin': patch), so a correctly-authored changeset would falsely fail the guard. Widen the quote class to ["'] (both quote styles). Added a single-quoted regression test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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.
Summary
Phase 1 of action corpus persistence (brainstormed → specced → planned → multi-LLM-reviewed → SDD-implemented this session). The action corpus is the plugin's compounding asset, but its run/repair history lived in fragile per-action JSON sidecars with three silent-loss surfaces (worktree, project-root mis-resolution, gitignore).
This PR adds a derived, gitignored
node:sqlitestore (.rn-agent/state/actions.db) holding the corpus index + run/repair history, as an additive dual-write mirror: the JSON sidecars stay authoritative, reads stay sidecar-sourced, and the DB is a rebuildable mirror that degrades gracefully to sidecar-only whennode:sqliteis unavailable. No data-loss surface is introduced; the DB is always reconstructable from the sidecars.Phase 2 (separate plan) will flip authority to the DB, add
reconcile()loud "never-lost" diagnostics, and the #348/#357 resolution guard (subsumes #357).What's in it
domain/action-db.ts—node:sqlitewrapper (ESMcreateRequireloader; schema with explicitstats_json+failure_detail;BEGIN IMMEDIATE+busy_timeout+ WAL; append-and-trim writes; idempotent(action_id, ts)guard; graceful degrade to null) + one-time sidecar→DB migration.domain/action-state-store.ts— backend-selecting facade: sidecar-authoritative reads, best-effort DB mirror that never throws, read-only 3-waystoreMode(sqlite | legacy-files | degraded:<reason>), handle lifecycle (resetActionStore).domain/action-store.tsmade store-aware (the real chokepoint) — mirrors strictly after the saveAction: YAML+sidecar non-atomic write can cause false "external edit" alarm #101 atomic pair-write; cdp_run_action: per-actionId concurrency lost-update on RunRecord history #117 CAS short-circuit preserved (a refusal can never become a mirror success).cdp_status.actionStorereports the active backend.--experimental-sqlitevia a version-gatedsqliteFlagForNode(Node 22.5–23.5 only); engines floor stays>=22so older Node degrades, not breaks.learned-actions.mjs→ TypeScript (compiled todist/); all command/agent invocations updated; byte-identical output (parity-tested).Invariants preserved
EXTERNAL_WRITE+SaveActionPreconditionError) short-circuits before any mirror.Verification
dist/rebuilt & tracked.(action_id, ts)append guard + a RED→GREEN regression test.cdp-client-lifecycle.test.js) fails onmaintoo (build-state dependent) — not introduced here.Docs
docs/superpowers/specs/2026-06-19-action-storage-persistence-design.mddocs/superpowers/plans/2026-06-19-action-storage-persistence-phase1.mdChangeset included (
rn-dev-agent-cdpminor).🤖 Generated with Claude Code