Skip to content

Commit baf9181

Browse files
voidstackloopclaude
andcommitted
rust: inert SQLite scaffold for audit events (Phase 1 smallest slice)
Follow-up to the v1.3.0 release's Rust migration assessment (docs/RUST_MIGRATION_ASSESSMENT.md). Adds lib/src/store/audit.rs (rusqlite, bundled SQLite, WAL mode, schema-versioned, idempotent migration-from-JSON) with a TypeScript bridge (app/src/native-sqlite-store.ts) - built, tested through the real addon, and deliberately NOT wired into audit-log-store.ts's live path yet, per the assessment's own rollback plan (ship inert, cut over behind a flag later). Also fixes a real build issue this surfaced: adding rusqlite broke 'cargo test' linking for the whole crate (undefined references to napi's own C-ABI symbols, normally supplied by the Node process at runtime) because this crate had napi's 'dyn-symbols' feature off. Re-verified the built addon still loads correctly under Node after turning it on. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 68c8478 commit baf9181

8 files changed

Lines changed: 685 additions & 23 deletions

File tree

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
import * as fs from "node:fs";
2+
import * as os from "node:os";
3+
import * as path from "node:path";
4+
import { randomUUID } from "node:crypto";
5+
import { describe, it, expect } from "vitest";
6+
import {
7+
getSqliteStoreCapabilityReport,
8+
openAuditStore,
9+
migrateAuditLogFromJson,
10+
auditEventCount,
11+
verifyAuditStore,
12+
} from "./native-sqlite-store";
13+
14+
// Same addon-presence-varies-by-environment situation as
15+
// native-datastore.test.ts — see that file's comment. This module is inert
16+
// (see its own header comment): nothing in the running app calls it, so
17+
// these tests exist purely to prove the scaffold itself is correct ahead of
18+
// any future cutover, not to protect a live code path.
19+
const addonPresent = fs.existsSync(path.join(__dirname, "..", "native"));
20+
21+
function tempDbPath(): string {
22+
return path.join(os.tmpdir(), `native-sqlite-store-test-${randomUUID()}.sqlite3`);
23+
}
24+
25+
describe(`native-sqlite-store (addon ${addonPresent ? "present" : "unavailable"})`, () => {
26+
it("getSqliteStoreCapabilityReport reflects whether the addon actually loaded", () => {
27+
const report = getSqliteStoreCapabilityReport();
28+
expect(report.available).toBe(addonPresent);
29+
if (!addonPresent) expect(report.reason).toBeTruthy();
30+
});
31+
32+
(addonPresent ? it : it.skip)("opens a store, migrates JSON events, counts, and verifies — full round trip", () => {
33+
const dbPath = tempDbPath();
34+
try {
35+
openAuditStore(dbPath);
36+
expect(fs.existsSync(dbPath)).toBe(true);
37+
38+
const events = [
39+
{ id: "a", timestamp: "2026-01-01T00:00:00.000Z", actionCategory: "case-created" },
40+
{ id: "b", timestamp: "2026-01-01T00:00:01.000Z", actionCategory: "case-updated", previousEventHash: null, eventHash: "h2" },
41+
];
42+
const report = migrateAuditLogFromJson(dbPath, JSON.stringify(events));
43+
expect(report).toEqual({ migrated: 2, skippedExisting: 0, totalSourceEvents: 2 });
44+
expect(auditEventCount(dbPath)).toBe(2);
45+
46+
// Rerunning the same migration must not duplicate rows.
47+
const second = migrateAuditLogFromJson(dbPath, JSON.stringify(events));
48+
expect(second).toEqual({ migrated: 0, skippedExisting: 2, totalSourceEvents: 2 });
49+
expect(auditEventCount(dbPath)).toBe(2);
50+
51+
const integrity = verifyAuditStore(dbPath);
52+
expect(integrity).toEqual({ ok: true, eventCount: 2, detail: "ok" });
53+
} finally {
54+
fs.rmSync(dbPath, { force: true });
55+
fs.rmSync(`${dbPath}-wal`, { force: true });
56+
fs.rmSync(`${dbPath}-shm`, { force: true });
57+
}
58+
});
59+
60+
(addonPresent ? it : it.skip)("rejects a source event missing an id without partially migrating the batch", () => {
61+
const dbPath = tempDbPath();
62+
try {
63+
openAuditStore(dbPath);
64+
const events = [{ timestamp: "2026-01-01T00:00:00.000Z", actionCategory: "case-created" }];
65+
expect(() => migrateAuditLogFromJson(dbPath, JSON.stringify(events))).toThrow();
66+
expect(auditEventCount(dbPath)).toBe(0);
67+
} finally {
68+
fs.rmSync(dbPath, { force: true });
69+
fs.rmSync(`${dbPath}-wal`, { force: true });
70+
fs.rmSync(`${dbPath}-shm`, { force: true });
71+
}
72+
});
73+
74+
(!addonPresent ? it : it.skip)("throws (rather than silently proceeding) when the addon isn't available", () => {
75+
expect(() => openAuditStore(tempDbPath())).toThrow();
76+
});
77+
});

app/src/native-sqlite-store.ts

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
import * as path from "node:path";
2+
import { classifyLoadError, type NativeCapabilityReport } from "./native-capability";
3+
4+
// Phase-1 scaffold (see docs/RUST_MIGRATION_ASSESSMENT.md) for a
5+
// SQLite-backed audit store — lib/src/store/audit.rs. Deliberately INERT:
6+
// nothing in the running app calls into this yet. audit-log-store.ts's live
7+
// read/write path is unchanged and still the JSON file. This module exists
8+
// so the Rust store, its migration path, and this bridge are all built and
9+
// tested ahead of an explicitly flagged future cutover, rather than
10+
// designing that cutover blind.
11+
//
12+
// Unlike native-datastore.ts, there is no fallback here — a SQLite store
13+
// with no SQLite backing has nothing meaningful to fall back to. That's
14+
// fine precisely because nothing calls this yet; a real cutover would need
15+
// to decide what "the addon isn't available" means for a store that would
16+
// by then be load-bearing (almost certainly: refuse to switch over, keep
17+
// using JSON, and surface that clearly — not silently lose the fast path
18+
// the way json-store.ts's fallback safely can).
19+
20+
interface NativeAddon {
21+
openAuditStore(dbPath: string): void;
22+
migrateAuditLogFromJson(dbPath: string, jsonArray: string): MigrationReport;
23+
auditEventCount(dbPath: string): number;
24+
verifyAuditStore(dbPath: string): StoreIntegrityReport;
25+
}
26+
27+
export interface MigrationReport {
28+
migrated: number;
29+
skippedExisting: number;
30+
totalSourceEvents: number;
31+
}
32+
33+
export interface StoreIntegrityReport {
34+
ok: boolean;
35+
eventCount: number;
36+
detail: string;
37+
}
38+
39+
let nativeAddon: NativeAddon | undefined;
40+
let loadFailed = false;
41+
let capabilityReport: NativeCapabilityReport = { available: false };
42+
43+
function getNativeAddon(): NativeAddon {
44+
if (loadFailed) throw new Error("Native SQLite store addon is unavailable — see getSqliteStoreCapabilityReport() for why.");
45+
if (!nativeAddon) {
46+
try {
47+
nativeAddon = require(path.join(__dirname, "..", "native")) as NativeAddon;
48+
capabilityReport = { available: true };
49+
} catch (err) {
50+
loadFailed = true;
51+
capabilityReport = { available: false, reason: classifyLoadError(err), detail: err instanceof Error ? err.message : String(err) };
52+
throw err;
53+
}
54+
}
55+
return nativeAddon;
56+
}
57+
58+
export function getSqliteStoreCapabilityReport(): NativeCapabilityReport {
59+
try {
60+
getNativeAddon();
61+
} catch {
62+
// capabilityReport was already set by getNativeAddon() before it threw.
63+
}
64+
return capabilityReport;
65+
}
66+
67+
/** Opens (creating on first use) the audit SQLite store and applies its
68+
* schema. Idempotent. Throws if the native addon isn't available — callers
69+
* of this scaffold are expected to check getSqliteStoreCapabilityReport()
70+
* or catch, not silently proceed as if a store exists when it doesn't. */
71+
export function openAuditStore(dbPath: string): void {
72+
getNativeAddon().openAuditStore(dbPath);
73+
}
74+
75+
/** Imports events from `jsonArray` (the literal contents of audit-log.json)
76+
* into the SQLite store, skipping ids already present. Safe to call
77+
* repeatedly — a rerun only inserts what's new. */
78+
export function migrateAuditLogFromJson(dbPath: string, jsonArray: string): MigrationReport {
79+
return getNativeAddon().migrateAuditLogFromJson(dbPath, jsonArray);
80+
}
81+
82+
export function auditEventCount(dbPath: string): number {
83+
return getNativeAddon().auditEventCount(dbPath);
84+
}
85+
86+
export function verifyAuditStore(dbPath: string): StoreIntegrityReport {
87+
return getNativeAddon().verifyAuditStore(dbPath);
88+
}

docs/RUST_MIGRATION_ASSESSMENT.md

Lines changed: 63 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -194,15 +194,9 @@ and this assessment both get updated as each slice lands.
194194

195195
---
196196

197-
## What was actually changed in this pass (Phase 0, minimal slice)
197+
## What was actually changed
198198

199-
Per the brief's own closing instruction — "implement only Phase 0 and the
200-
smallest vertical slice of Phase 1 unless the evidence shows another first
201-
slice is safer" — the evidence above (no profiled bottleneck on any P0
202-
non-persistence item, and persistence's own Phase 1 being a multi-day,
203-
high-risk, schema-migrating undertaking that cannot be safely finished and
204-
verified in one pass) points to *not* starting Phase 1 at all yet. What
205-
shipped alongside this document:
199+
### Phase 0
206200

207201
1. **`docs/ARCHITECTURE.md` reconciled** — its native-addon section
208202
described download-only; corrected to include the datastore/audit
@@ -214,21 +208,68 @@ shipped alongside this document:
214208
into a single "unavailable, fall back" boolean. `getNativeCapabilityReport()`
215209
now inspects the thrown error and reports which of those it actually
216210
was, purely for diagnostics/logging — every existing fallback behavior
217-
is unchanged.
211+
is unchanged. New shared module: `app/src/native-capability.ts`.
212+
3. **CI's `rust` job now builds and load-verifies the addon on Windows and
213+
macOS, not just Linux** — previously flagged here as a real, unverified
214+
gap. `.github/workflows/ci.yml`'s `rust` job gained a 3-OS matrix; fmt
215+
and clippy still run once (Rust source is OS-independent), but `cargo
216+
build`, `cargo test`, a real `napi build`, and a `require()` + exports
217+
check now run on every platform. Verified locally on Linux (the one
218+
platform this sandbox can run); Windows/macOS are verified for real by
219+
GitHub's own runners on the next push, which is the actual point — this
220+
sandbox was never going to be able to confirm those two itself.
221+
222+
### Phase 1 — smallest vertical slice
223+
224+
An inert SQLite scaffold for audit events: `lib/src/store/audit.rs`
225+
(`rusqlite`, bundled SQLite, WAL mode, a `schema_version` table, an
226+
`audit_events` table, and `open`/`migrate-from-JSON`/`count`/`verify`
227+
functions), exposed via N-API, with a TypeScript bridge
228+
(`app/src/native-sqlite-store.ts`). **Deliberately not wired into
229+
`audit-log-store.ts`'s live read/write path** — nothing in the running app
230+
calls this yet; it exists to prove the pattern (schema versioning,
231+
idempotent migration-from-JSON safe to rerun, transactional batch inserts,
232+
`PRAGMA integrity_check`-based corruption detection) works end-to-end
233+
through the real built addon before committing to an actual cutover, per
234+
the brief's own rollback requirement (ship behind a flag, default off).
235+
236+
9 new Rust tests (idempotent open, schema-version-recorded-once, WAL-mode
237+
active, migration correctness/idempotency/partial-rerun/no-duplicate-rows,
238+
a rejected malformed batch leaving zero rows behind — one bad event fails
239+
the whole transaction rather than partially migrating), 5 new TypeScript
240+
tests (full round trip through the actual built `.node` addon, rejected
241+
malformed batch, and the addon-unavailable throw path), all verified via
242+
the real built binary (`npm run build:debug` + a `require()` smoke test),
243+
not just `cargo test`.
244+
245+
**A real build-infrastructure finding surfaced by adding this:** `cargo
246+
test` started failing to link at all (not just for the new module — for
247+
the *entire* crate, including previously-passing tests) once `rusqlite`
248+
was added, with undefined references to `napi_reference_unref` /
249+
`napi_delete_reference` / `napi_call_threadsafe_function`. These are napi's
250+
own C-ABI symbols, normally supplied by the Node process that loads a
251+
`.node` addon at runtime — a standalone `cargo test` binary has no Node
252+
process to supply them, and this crate's `napi` dependency had
253+
`default-features = false` without napi's own `dyn-symbols` feature (which
254+
resolves those symbols dynamically instead of requiring them at static
255+
link time, and is part of napi's *default* feature set — this crate had
256+
just never turned it on, and the existing code path apparently never
257+
triggered the linker into demanding those symbols before). Fix: added
258+
`dyn-symbols` to the `napi` dependency's feature list in `lib/Cargo.toml`.
259+
Re-verified after the fix that the real built addon still loads correctly
260+
under Node (it does — see the smoke test above); this is a supported,
261+
recommended napi-rs configuration, not a workaround.
218262

219263
**Explicitly not done, with reasons, per "stop and document the blocker"
220264
rather than half-finish:**
221265

222-
- Multi-platform CI build/load jobs — this sandbox has no Windows/macOS
223-
runner access; the CI YAML change itself would be easy, but "add a job"
224-
without being able to verify it actually builds and loads the addon
225-
there is exactly the kind of unverified change this brief says not to
226-
ship.
227-
- Any Phase 1+ code (SQLite store, crypto vault, RAG index, filesystem
228-
capability layer, process supervisor, system-inspection rewrite,
229-
ingestion, safety-scanning engine, recommender inference) — none of it
230-
was started. Each is a substantial, independently-scoped project per the
231-
brief's own phase breakdown; starting one without the characterization
232-
tests, benchmarks, and migration/rollback machinery the brief itself
233-
requires first would produce exactly the "half-migrated" state the brief
234-
says to avoid.
266+
- Actually cutting `audit-log-store.ts` over to the SQLite store, or
267+
migrating any other store (patient cases, sessions, evidence, model
268+
registry). The scaffold above is the foundation that cutover would use,
269+
not the cutover itself — flipping the live read/write path needs the
270+
three-way native/fallback/new-store comparison and feature-flag rollout
271+
described in §6, which is real, separately-scoped work.
272+
- Crypto vault, RAG index, filesystem capability layer, process supervisor,
273+
system-inspection rewrite, ingestion, safety-scanning engine, recommender
274+
inference — none started. Each is a substantial, independently-scoped
275+
project per the brief's own phase breakdown.

0 commit comments

Comments
 (0)