feat(db,cli): 2.H — durable run history via @relavium/db (RunStore + read API)#44
Merged
Conversation
… reached PR #43 merged: the engine regression harness (2.K) is live in the required CI gate. With 2.F + 2.K both Done, global milestone M3 is reached. - phase-2-cli.md: status line + the §2.K header marked ✅ Done (PR #43, 2026-06-23); the "Remaining build order" status note now shows 2.K done / M3 reached / next: 2.H (its gate-resume + agent-replay halves stay deferred, landing with 2.G / later). - current.md: 2.K ✅ Done, M3 reached; "Next pickup: 2.H" pointing at the build-order queue. - CLAUDE.md + README.md: Phase-2 status sentence now records 2.K + M3 reached. Next pickup is 2.H (durable run history) — the highest-leverage feeder. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…concile canonical docs The Phase-2 CLI's ~/.relavium/history.db (better-sqlite3, ADR-0021) needed an at-rest posture: ADR-0005/0008's "encrypted SQLite" framing is the DESKTOP's SQLCipher path, and ADR-0021 chose better-sqlite3 (no SQLCipher) for the Node side but decided "only the driver," leaving encryption open. ADR-0050 (Accepted): the CLI's history.db is UNENCRYPTED at rest, guarded by 0600/0700 OS permissions applied via explicit chmod (umask-independent; Windows falls back to the %USERPROFILE% NTFS ACL). Safe because the file holds no credentials — keys stay in the keychain (ADR-0006) and the engine masks secrets at the bus (ADR-0036); so the content is run data, not secrets. App-layer AES via the keychain is recorded as a named future upgrade, not silent debt. Cross-surface coexistence with the desktop's SQLCipher at the same path (they cannot share one file) is a named Phase-3 follow-on; no live collision in Phase 2. Reconcile the corpus in lockstep so no canonical home contradicts the ADR: - ADR-0005 + ADR-0008: dated "Amended 2026-06-23" notes scoping "encrypted" to the desktop, pointing to ADR-0050; README index row added. - database-schema.md: per-surface encryption (intro, Mermaid, storage table, agent-session note, session-message at-rest line, the cross-host callout, and §"Encryption at rest"); new §"Secrets at the write boundary" recording the writer's pass-through-plus-assert obligation (the four unsafe columns). - config-spec.md + keychain-and-secrets.md: history.db at-rest note now per-surface. Refs: ADR-0050, phase-2-cli.md 2.H Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…read API) The tables already existed (schema.ts + migration 0000); 2.H is the missing SQLite writer + the CLI wiring that injects it as the engine's RunStore. No core change — the RunStore port + createCliHost(store) seam were already in place. @relavium/db: - run-history-store.ts: createRunHistoryStore(db, deps) — the three RunStore methods (resolveWorkflowId slug→UUID upsert per ADR-0022; persistEvent; listInterruptedRuns) plus the 2.I/2.G read API (listRuns / loadRun / loadRunEvents). persistEvent folds each durable event into run_events (full event, lossless) + derived runs/step_executions/ run_costs, all in one transaction (derived-first so run:started's runs row precedes its FK-referencing run_events row). Per-node cost = the delta of node:completed .cumulativeCostMicrocents (cost:updated is streamed, not persisted), so sum(run_costs) == runs.total_cost_microcents; token/cost totals fold incrementally so status shows live partials. A fault rejects the promise (ADR-0050 fatal posture). - Pass-through for secrets: persists the already bus-masked event verbatim; a secrets fixture asserts no raw secret reaches the unsafe columns (ADR-0036/0006/0050). - time.ts: extracted isoToEpochMs/epochMsToIso (shared with session-store). apps/cli: - history/open.ts: open ~/.relavium/history.db (better-sqlite3), migrate-on-first-use, chmod 0600 the db + WAL/SHM sidecars; ensureGlobalConfigDir now chmod 0700 the home dir (explicit, umask-independent; Windows → %USERPROFILE% ACL) — the ADR-0050 at-rest posture. - run.ts: inject the SQLite store via an optional openRunStore dep + close it in finally; specs.ts wires it in production. Tests/2.K harness omit it → the in-memory store, so they never touch the user's home. load.ts now exposes homeDir. Tests: 9 store unit tests (fold, seq/UNIQUE integrity, status transitions, interrupted runs, secrets) + 2 CLI e2e (real run → history.db at a temp home; 0600/0700 perms; a gate-paused run's events reconstruct a checkpoint via reconstructCheckpointState — the 2.G substrate). Full gate green; engine purity + seam intact; Leakwatch 0. Refs: ADR-0050, phase-2-cli.md 2.H Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address the relavium-reviewer findings on the 2.H implementation (1 High, 4 Medium, 2 Low; 0 blockers): - [High] history/open.ts: the main `history.db` chmod 0600 is now a hard failure (it is guaranteed to exist; ADR-0050's at-rest guarantee rests on it) — only the WAL/SHM sidecars stay best-effort. - [Med] run.ts: a pre-run history-open fault (cannot create/open/migrate history.db) now maps to a CliError(invalid_invocation) → exit 2, not exit 1 — a CI/--json consumer can tell "db couldn't open" from "a node failed mid-run". - [Med] run-history-store.ts: handle node:retrying — mark the retried attempt's step_executions row `failed` so it can't linger as a ghost `running` row for 2.I status. - [Med] run-history-store.test.ts: extend the secrets fixture to every unsafe column (step_executions output/error JSON + runs.workflow_definition_snapshot), and add a node:retrying test. - [Med] session-store.ts: the "encrypted history.db" comment is now per-surface (desktop SQLCipher / CLI 0600/0700, ADR-0050). - [Low] run-history-store.ts: listInterruptedRuns is one GROUP BY query, not N+1. - [Low] database-schema.md: intro now says "same path" (not "same file") — consistent with the cross-host callout that the two surfaces cannot share one file. The reviewer independently verified clean: transaction ordering, the cost-delta invariant under serial delivery, engine purity, the @relavium/llm seam, in-memory test isolation, and RunHistoryStore↔RunStore assignability. Full gate green; Leakwatch 0. Refs: ADR-0050, phase-2-cli.md 2.H Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…PASS)
All three reviews verdict PASS/ship-it; findings were test-coverage + doc-precision
(0 blockers, 2 Medium, several Low). The consensus actionable item (flagged by all three)
was the WAL/SHM chmod catch.
- [Med · consensus] open.ts: the WAL/SHM sidecar chmod catch is now ENOENT-narrow — a real
failure (EPERM/EIO) on an existing sidecar throws instead of silently leaving it
world-readable; only genuine absence (no checkpoint yet) is tolerated.
- [Med] run-history-store.test.ts: add the fan-out cost-invariant test (interleaved
node:completed cumulatives still telescope to runs.total) — closes the explicit 2.H
parallel acceptance criterion that was only covered serially.
- [Low] add a backward-compat test (node:completed without cumulativeCostMicrocents → 0).
- [Low] run-history-store.ts: comment that per-node run_costs attribution is approximate
under a fan-out (the run-level SUM stays exact); note the known limitation that a
failed/cancelled run's total can undercount (no total on run:failed in the shared schema
— out of 2.H scope).
- [Low] clarify the secrets fixture's step_executions.input_json check (always '{}' by
design — vacuous-but-complete), so a reader doesn't assume node inputs are captured there.
- [Low] database-schema.md + ADR-0050: reword "asserts at the write boundary" → the writer
is pass-through, the no-raw-secret invariant is the upstream masking guarantee
regression-guarded by the package secrets fixture (a runtime secret scan on opaque JSON is
infeasible) — so a future desktop/cloud writer isn't expected to implement one.
db 96 tests, cli 125; full gate green; Leakwatch 0; packages/core untouched.
Refs: ADR-0050, phase-2-cli.md 2.H
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @cemililik, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughImplements milestone 2.H (durable run history): adds a SQLite-backed ChangesDurable Run History (2.H)
Sequence Diagram(s)sequenceDiagram
participant User
participant specs as specs.ts<br/>(relavium run)
participant runCommand
participant openHistoryStore
participant ensureGlobalConfigDir
participant SQLiteClient
participant RunHistoryStore
participant Engine
User->>specs: relavium run <workflow>
specs->>runCommand: openRunStore: openHistoryStore
runCommand->>runCommand: loadResolvedConfig → {homeDir}
runCommand->>openHistoryStore: (workflowDef, homeDir)
openHistoryStore->>ensureGlobalConfigDir: homeDir
ensureGlobalConfigDir->>ensureGlobalConfigDir: mkdir ~/.relavium/tmp<br/>chmod 0o700
openHistoryStore->>SQLiteClient: open ~/.relavium/history.db
openHistoryStore->>SQLiteClient: runMigrations
openHistoryStore->>openHistoryStore: chmod 0600 DB, sidecars
openHistoryStore->>RunHistoryStore: createRunHistoryStore(client, deps)
openHistoryStore-->>runCommand: OpenedHistory{store, close}
runCommand->>Engine: createCliHost(store)
Engine->>RunHistoryStore: persistEvent(run:started)
Engine->>RunHistoryStore: persistEvent(node:started/completed)
Engine->>RunHistoryStore: persistEvent(run:completed/paused)
Engine-->>runCommand: outcome
runCommand->>RunHistoryStore: finally close()
runCommand-->>User: exit code
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements workstream 2.H, wiring durable CLI run history to a local SQLite database (~/.relavium/history.db) with owner-only permissions (0700/0600) as specified in ADR-0050. It introduces the SQLite-backed RunHistoryStore in @relavium/db to persist run events, step executions, and costs, and integrates it into the CLI's run command. Feedback on the changes suggests defensive programming improvements to prevent potential runtime crashes by safely accessing tokensUsed and totalTokensUsed via optional chaining, as well as a performance optimization to combine two separate database queries in listInterruptedRuns into a single query using a LEFT JOIN.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+176
to
+208
| db.insert(runCosts) | ||
| .values({ | ||
| id: deps.uuid(), | ||
| runId, | ||
| nodeId: event.nodeId, | ||
| inputTokens: event.tokensUsed.input, | ||
| outputTokens: event.tokensUsed.output, | ||
| costMicrocents: nodeCost, | ||
| createdAt: ts, | ||
| } satisfies NewRunCostRow) | ||
| .run(); | ||
| db.update(stepExecutions) | ||
| .set({ | ||
| status: 'completed', | ||
| outputJson: JSON.stringify(event.output), | ||
| inputTokens: event.tokensUsed.input, | ||
| outputTokens: event.tokensUsed.output, | ||
| costMicrocents: nodeCost, | ||
| durationMs: event.durationMs, | ||
| completedAt: ts, | ||
| updatedAt: ts, | ||
| }) | ||
| .where(stepMatch(runId, event.nodeId, event.attemptNumber)) | ||
| .run(); | ||
| db.update(runs) | ||
| .set({ | ||
| totalInputTokens: sql`${runs.totalInputTokens} + ${event.tokensUsed.input}`, | ||
| totalOutputTokens: sql`${runs.totalOutputTokens} + ${event.tokensUsed.output}`, | ||
| totalCostMicrocents: cumulative, | ||
| updatedAt: ts, | ||
| }) | ||
| .where(eq(runs.id, runId)) | ||
| .run(); |
There was a problem hiding this comment.
Defensive Programming: Non-agent/non-LLM nodes (such as transform or condition nodes) do not consume tokens, meaning event.tokensUsed can be undefined or null at runtime. Accessing event.tokensUsed.input directly will cause a runtime TypeError. Extracting these values safely using optional chaining and nullish coalescing prevents potential crashes and simplifies the database update queries.
const inputTokens = event.tokensUsed?.input ?? 0;
const outputTokens = event.tokensUsed?.output ?? 0;
db.insert(runCosts)
.values({
id: deps.uuid(),
runId,
nodeId: event.nodeId,
inputTokens,
outputTokens,
costMicrocents: nodeCost,
createdAt: ts,
} satisfies NewRunCostRow)
.run();
db.update(stepExecutions)
.set({
status: 'completed',
outputJson: JSON.stringify(event.output),
inputTokens,
outputTokens,
costMicrocents: nodeCost,
durationMs: event.durationMs,
completedAt: ts,
updatedAt: ts,
})
.where(stepMatch(runId, event.nodeId, event.attemptNumber))
.run();
db.update(runs)
.set({
totalInputTokens: sql.raw("total_input_tokens + " + inputTokens),
totalOutputTokens: sql.raw("total_output_tokens + " + outputTokens),
totalCostMicrocents: cumulative,
updatedAt: ts,
})
.where(eq(runs.id, runId))
.run();
Contributor
Author
There was a problem hiding this comment.
Skipping — this defensive path is unreachable.
tokensUsed is a required field on NodeCompletedEventSchema (tokensUsed: TokensUsedSchema, not .optional()), and persistEvent runs RunEventSchema.parse(event) before the fold — so a node:completed missing tokensUsed is rejected at the boundary and never reaches this code. A non-LLM node (transform/condition/merge) completes with tokensUsed: { input: 0, output: 0 }, never undefined.
So event.tokensUsed.input cannot throw here; optional chaining + nullish coalescing would be dead code guarding a schema-impossible state, and it would silently mask a real upstream contract break instead of failing loud. The Zod schema is the single source of truth — if the field ever became optional there, the type would force this update first. Leaving the direct access.
Comment on lines
+234
to
+245
| db.update(runs) | ||
| .set({ | ||
| status: 'completed', | ||
| outputJson: JSON.stringify(event.outputs), | ||
| totalInputTokens: event.totalTokensUsed.input, | ||
| totalOutputTokens: event.totalTokensUsed.output, | ||
| totalCostMicrocents: event.totalCostMicrocents, | ||
| completedAt: ts, | ||
| updatedAt: ts, | ||
| }) | ||
| .where(eq(runs.id, runId)) | ||
| .run(); |
There was a problem hiding this comment.
Defensive Programming: If a workflow does not execute any LLM-based nodes, event.totalTokensUsed may be undefined or null on the run:completed event. Using optional chaining prevents a potential runtime crash when updating the final run record.
Suggested change
| db.update(runs) | |
| .set({ | |
| status: 'completed', | |
| outputJson: JSON.stringify(event.outputs), | |
| totalInputTokens: event.totalTokensUsed.input, | |
| totalOutputTokens: event.totalTokensUsed.output, | |
| totalCostMicrocents: event.totalCostMicrocents, | |
| completedAt: ts, | |
| updatedAt: ts, | |
| }) | |
| .where(eq(runs.id, runId)) | |
| .run(); | |
| db.update(runs) | |
| .set({ | |
| status: 'completed', | |
| outputJson: JSON.stringify(event.outputs), | |
| totalInputTokens: event.totalTokensUsed?.input ?? 0, | |
| totalOutputTokens: event.totalTokensUsed?.output ?? 0, | |
| totalCostMicrocents: event.totalCostMicrocents, | |
| completedAt: ts, | |
| updatedAt: ts, | |
| }) | |
| .where(eq(runs.id, runId)) | |
| .run(); |
Contributor
Author
There was a problem hiding this comment.
Skipping — same reasoning as the tokensUsed comment above.
totalTokensUsed is a required field on RunCompletedEventSchema (z.object({ input, output })), and persistEvent validates every event with RunEventSchema.parse(event) before the fold. A run:completed without totalTokensUsed is rejected at the boundary, so it can't be undefined/null by the time this update runs — even a workflow with no LLM nodes emits { input: 0, output: 0 }.
Optional chaining here would be unreachable defensive code; leaving the direct access so a schema-contract break would fail loud rather than be silently masked.
Comment on lines
+365
to
+399
| listInterruptedRuns: () => { | ||
| const rows = db | ||
| .select() | ||
| .from(runs) | ||
| .where(and(inArray(runs.status, [...NON_TERMINAL_STATUSES]), isNull(runs.deletedAt))) | ||
| .all(); | ||
| if (rows.length === 0) { | ||
| return Promise.resolve([]); | ||
| } | ||
| // One aggregating query for the per-run last seq (a single GROUP BY, not N+1) — this is a RunStore | ||
| // port method the desktop/cloud surfaces also implement, so it must scale past a single-user CLI. | ||
| const lastByRun = new Map( | ||
| db | ||
| .select({ runId: runEvents.runId, m: sql<number>`max(${runEvents.seq})` }) | ||
| .from(runEvents) | ||
| .where( | ||
| inArray( | ||
| runEvents.runId, | ||
| rows.map((r) => r.id), | ||
| ), | ||
| ) | ||
| .groupBy(runEvents.runId) | ||
| .all() | ||
| .map((r) => [r.runId, r.m]), | ||
| ); | ||
| const interrupted = rows.map( | ||
| (row): InterruptedRunInfo => ({ | ||
| runId: row.id, | ||
| workflowId: row.workflowId, | ||
| resumable: row.status === 'paused', | ||
| lastSequenceNumber: lastByRun.get(row.id) ?? 0, | ||
| }), | ||
| ); | ||
| return Promise.resolve(interrupted); | ||
| }, |
There was a problem hiding this comment.
Performance & Scalability Optimization: The current implementation performs two separate database queries: one to fetch all interrupted runs, and another to fetch the maximum sequence number for those runs using inArray with mapped IDs. This approach has several drawbacks:
- It executes two round-trips to the database.
- It pulls all columns of all matching runs into memory.
- It is susceptible to SQLite's parameter limit (historically 999 host parameters) when there are many interrupted runs.
By rewriting this into a single query using a LEFT JOIN and coalesce(max(...), 0), we can fetch all the required data in a single, highly efficient database pass that scales perfectly and avoids parameter limits.
listInterruptedRuns: () => {
const interrupted = db
.select({
runId: runs.id,
workflowId: runs.workflowId,
status: runs.status,
lastSequenceNumber: sql.raw("coalesce(max(run_events.seq), 0)") as any,
})
.from(runs)
.leftJoin(runEvents, eq(runs.id, runEvents.runId))
.where(and(inArray(runs.status, [...NON_TERMINAL_STATUSES]), isNull(runs.deletedAt)))
.groupBy(runs.id)
.all()
.map((row) => ({
runId: row.runId,
workflowId: row.workflowId,
resumable: row.status === "paused",
lastSequenceNumber: row.lastSequenceNumber as number,
}));
return Promise.resolve(interrupted);
},![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
packages/db/src/run-history-store.test.ts (1)
201-213: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winThe “gap-free” test does not currently assert a missing-sequence gap rejection.
This case validates duplicate
(run_id, seq)only; add a true gap case so the acceptance invariant is explicitly guarded.Suggested assertion extension
await expect( store.persistEvent(ev('node:started', 1, { nodeId: 'y', nodeType: 'input' })), ).rejects.toThrow(); // UNIQUE(run_id, seq) + + await expect( + store.persistEvent(ev('node:started', 3, { nodeId: 'z', nodeType: 'input' })), + ).rejects.toThrow(); // gap: seq=2 missing });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/db/src/run-history-store.test.ts` around lines 201 - 213, The test `persists a gap-free seq stream and rejects a duplicate (run_id, seq)` currently only validates duplicate rejection but does not test the gap-free invariant. Add an additional assertion that attempts to persist an event with a sequence number that creates a gap (for example, after persisting seq 0 and 1, try to persist seq 3 which skips seq 2) and verify that this also throws an error, ensuring the store enforces gap-free sequence acceptance as the test name implies.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/reference/desktop/database-schema.md`:
- Around line 404-421: The blockquote in the database-schema.md file that begins
with "Cross-host access (CLI / VS Code)" contains a blank line that lacks the
required `>` marker, which breaks the blockquote formatting in some Markdown
parsers. Add the `> ` prefix to the blank line inside this blockquote to
maintain proper Markdown formatting and ensure the blockquote continues
uninterrupted.
In `@packages/db/src/run-history-store.test.ts`:
- Around line 144-146: The test assertion on line 145 expects the results from
the select().from(runCosts).all() query to be in a specific order [300, 700]
without guaranteeing the order via an ORDER BY clause or explicit sorting. This
makes the test flaky across different database environments. Add an ORDER BY
clause to the select query on the runCosts table to ensure deterministic
ordering (such as ordering by a timestamp or ID field), or alternatively sort
the costs array before the assertion using .sort() to make the expectation
order-independent while still validating that the correct cost values are
present.
- Around line 28-34: Remove the unsafe `as RunEvent` type cast from the return
statement of the `ev` fixture helper function. Instead of bypassing type
checking with a cast, refactor the function to properly construct and return a
valid RunEvent object that satisfies TypeScript's type system without relying on
unsafe type assertions, ensuring the fixture helper maintains strict type
safety.
In `@packages/db/src/run-history-store.ts`:
- Around line 318-339: The resolveWorkflowId function has a race condition where
concurrent invocations can both execute the SELECT query and then both execute
the INSERT, causing duplicate entries or constraint violations. Replace the
separate select and insert operations with an atomic upsert operation that
inserts the workflow record or retrieves the existing one in a single
transaction. Use your database library's upsert functionality (such as an INSERT
... ON CONFLICT pattern) to ensure that only one workflow record with the given
slug is created, regardless of concurrent access.
- Around line 173-176: The issue is that while nodeCost is clamped to be
non-negative at line 175 using Math.max(0, cumulative - prev), the raw
cumulative value being written to the database (around line 204) is not clamped.
This allows the cumulative cost to regress, causing runs.totalCostMicrocents to
decrease while run_costs remains non-decreasing, breaking invariants. Ensure the
cumulative value written to the database is also monotonic by clamping it to be
at least the previous value, similar to how nodeCost is already guarded, so that
both the individual node cost and the cumulative total cost remain
non-decreasing.
---
Nitpick comments:
In `@packages/db/src/run-history-store.test.ts`:
- Around line 201-213: The test `persists a gap-free seq stream and rejects a
duplicate (run_id, seq)` currently only validates duplicate rejection but does
not test the gap-free invariant. Add an additional assertion that attempts to
persist an event with a sequence number that creates a gap (for example, after
persisting seq 0 and 1, try to persist seq 3 which skips seq 2) and verify that
this also throws an error, ensuring the store enforces gap-free sequence
acceptance as the test name implies.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5c672dd1-cfab-4694-9fb5-d9244b7b50b2
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
CLAUDE.mdREADME.mdapps/cli/package.jsonapps/cli/src/commands/run.tsapps/cli/src/commands/specs.tsapps/cli/src/config/load.tsapps/cli/src/config/paths.tsapps/cli/src/history/open.e2e.test.tsapps/cli/src/history/open.tsdocs/decisions/0005-sqlite-drizzle-local-postgres-cloud.mddocs/decisions/0008-local-first-phase-1-cloud-phase-2.mddocs/decisions/0050-cli-history-db-at-rest-posture.mddocs/decisions/README.mddocs/reference/contracts/config-spec.mddocs/reference/desktop/database-schema.mddocs/reference/desktop/keychain-and-secrets.mddocs/roadmap/current.mddocs/roadmap/phases/phase-2-cli.mdpackages/db/src/index.tspackages/db/src/run-history-store.test.tspackages/db/src/run-history-store.tspackages/db/src/session-store.tspackages/db/src/time.ts
Comment on lines
+28
to
+34
| function ev<T extends RunEvent['type']>( | ||
| type: T, | ||
| seq: number, | ||
| rest: Record<string, unknown>, | ||
| ): RunEvent { | ||
| return { type, runId: 'run-1', timestamp: TS, sequenceNumber: seq, ...rest } as RunEvent; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify remaining unsafe cast hotspots in TS/TSX sources.
rg -n --type=ts --type=tsx '\bas\s+RunEvent\b|\bas\s+any\b|\bas\s+unknown\s+as\b' packages appsRepository: HodeTech/Relavium
Length of output: 183
🏁 Script executed:
# First, read the actual file to confirm the unsafe cast exists
head -50 packages/db/src/run-history-store.test.ts | cat -nRepository: HodeTech/Relavium
Length of output: 2079
🏁 Script executed:
# Search for unsafe casts with correct rg syntax (ts files only)
rg -n 'as (RunEvent|any|unknown)' --type ts packages apps 2>/dev/null | head -20Repository: HodeTech/Relavium
Length of output: 2162
🏁 Script executed:
# Find the RunEvent type definition in the shared package
fd -e ts -e tsx -path '*shared*' | head -20Repository: HodeTech/Relavium
Length of output: 324
🏁 Script executed:
# Search for RunEvent type definition
rg -n 'type RunEvent|interface RunEvent' --type ts packages/shared/srcRepository: HodeTech/Relavium
Length of output: 344
🏁 Script executed:
# Read the RunEventSchema definition to understand the structure
head -100 packages/shared/src/run-event.ts | cat -nRepository: HodeTech/Relavium
Length of output: 5419
🏁 Script executed:
# Search for the schema discriminator pattern
rg -n 'discriminatedUnion|z\.union' packages/shared/src/run-event.ts -A 3 | head -30Repository: HodeTech/Relavium
Length of output: 1558
🏁 Script executed:
# Get more details on the RunEventSchema and see the structure of specific events
sed -n '392,430p' packages/shared/src/run-event.ts | cat -nRepository: HodeTech/Relavium
Length of output: 2004
🏁 Script executed:
# Check one specific event schema to understand the structure
sed -n '100,150p' packages/shared/src/run-event.ts | cat -nRepository: HodeTech/Relavium
Length of output: 2825
🏁 Script executed:
# Verify the specific event schemas to understand if Extract pattern would work correctly
sed -n '32,50p' packages/shared/src/run-event.ts | cat -nRepository: HodeTech/Relavium
Length of output: 1208
Remove the unsafe as RunEvent cast in the event fixture helper.
Line 33 bypasses union safety with an unsafe cast, allowing malformed fixture payloads to compile and weaken test guarantees.
Suggested typed helper (no unsafe cast)
function ev<T extends RunEvent['type']>(
type: T,
seq: number,
- rest: Record<string, unknown>,
-): RunEvent {
- return { type, runId: 'run-1', timestamp: TS, sequenceNumber: seq, ...rest } as RunEvent;
+ rest: Omit<Extract<RunEvent, { type: T }>, 'type' | 'runId' | 'timestamp' | 'sequenceNumber'>,
+): Extract<RunEvent, { type: T }> {
+ return { type, runId: 'run-1', timestamp: TS, sequenceNumber: seq, ...rest };
}Per coding guidelines, **/*.{ts,tsx}: "All source code must be TypeScript in strict mode; no any, no unsafe as. Prefer type guards."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function ev<T extends RunEvent['type']>( | |
| type: T, | |
| seq: number, | |
| rest: Record<string, unknown>, | |
| ): RunEvent { | |
| return { type, runId: 'run-1', timestamp: TS, sequenceNumber: seq, ...rest } as RunEvent; | |
| } | |
| function ev<T extends RunEvent['type']>( | |
| type: T, | |
| seq: number, | |
| rest: Omit<Extract<RunEvent, { type: T }>, 'type' | 'runId' | 'timestamp' | 'sequenceNumber'>, | |
| ): Extract<RunEvent, { type: T }> { | |
| return { type, runId: 'run-1', timestamp: TS, sequenceNumber: seq, ...rest }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/db/src/run-history-store.test.ts` around lines 28 - 34, Remove the
unsafe `as RunEvent` type cast from the return statement of the `ev` fixture
helper function. Instead of bypassing type checking with a cast, refactor the
function to properly construct and return a valid RunEvent object that satisfies
TypeScript's type system without relying on unsafe type assertions, ensuring the
fixture helper maintains strict type safety.
Source: Coding guidelines
…gle-query interrupted, dedup Address the still-valid findings from the latest review pass (skips noted below): - runs.total_cost_microcents now writes `prev + nodeCost` (= max(prev, cumulative)), not the raw cumulative snapshot — so the run total stays monotonic and always equals sum(run_costs) even if a snapshot regressed (a deeper engine bug). - resolveWorkflowId: insert-or-ignore (ON CONFLICT DO NOTHING) + read-back, atomic against a concurrent `relavium run` on the same slug (the old SELECT-then-INSERT could race two processes onto the active-slug UNIQUE index). - listInterruptedRuns: one LEFT JOIN + coalesce(max(seq),0) grouped by the run PK — drops the second round-trip and the inArray(ids) that would hit SQLite's host-parameter limit at scale. - Merge the duplicate node:failed / node:retrying case blocks (Sonar S-dup) — same step-failed write. - test: the multi-node run_costs assertion now sorts the deltas (SQLite gives no row order without ORDER BY); the `ev` fixture helper strongly types `rest` per variant (a wrong/missing field is now a compile error — the assembly cast remains but widens nothing). Skipped, with reason: - "blockquote blank line" (database-schema.md): the blank is BETWEEN two distinct blockquotes (the session-message at-rest note and the cross-host note), not inside one — both render correctly. - "gap-free seq rejection test": the store persists the bus-assigned seq; a gap is not write-rejected (only UNIQUE(run_id,seq) rejects duplicates). Gap-freeness is the RunEventBus guarantee (ADR-0036), so the suggested seq-3-after-0,1 test would not throw. - tokensUsed / totalTokensUsed optional-chaining: both are REQUIRED on their schemas; RunEventSchema .parse (before the fold) rejects a missing one — the optional chaining would be dead code. db 96 tests; full gate green; Leakwatch 0; packages/core untouched. Refs: ADR-0050, phase-2-cli.md 2.H Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
cemililik
added a commit
that referenced
this pull request
Jun 23, 2026
…xt pickup 2.C PR #44 merged: durable local run history via @relavium/db (the SQLite RunStore writer + read API + ADR-0050 at-rest posture) is live. - phase-2-cli.md: §2.H header ✅ Done (PR #44); status line records 2.H Done (ADR-0050); the "Remaining build order" queue drops the completed 2.H row, renumbers (2.C is now #1), and the gate-closing backbone is 2.C → 2.E → 2.G → 2.I → 2.L (2.K + 2.H done). - current.md: 2.H ✅ Done; "Next pickup: 2.C" (provider/keys). - CLAUDE.md + README.md: Phase-2 status sentence records durable run history landed. Next pickup is 2.C (provider/keys) — independent, unblocks 2.R + 2.M + live runs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 23, 2026
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.
What & why
Phase-2 workstream 2.H — durable local run history for the
relaviumCLI via@relavium/db.The CLI now persists every run (events, steps, costs) to
~/.relavium/history.db, the substratethat powers
list/logs/status(2.I), the cross-processgateresume (2.G), and chat sessionpersistence (2.M).
Key shape: the four tables (
runs/step_executions/run_events/run_costs) alreadyexisted in
@relavium/db, and the engine'sRunStoreport +createCliHost(store)seam werealready in place — so 2.H is the missing SQLite writer + the CLI wiring that injects it.
packages/coreis unchanged.What's in it
@relavium/db—run-history-store.ts(the heart, mirrorssession-store.ts):createRunHistoryStore(db, deps): the threeRunStoremethods (resolveWorkflowIdslug→UUIDupsert per ADR-0022;
persistEvent;listInterruptedRuns) + the 2.I/2.G read API(
listRuns/loadRun/loadRunEvents).persistEventfolds each durable event intorun_events(the full event, lossless) plusderived
runs/step_executions/run_costs, all in one transaction (derived-first, sorun:started'srunsrow precedes its FK-referencingrun_eventsrow). A fault rejects thepromise (ADR-0050 fatal posture), rolling the transaction back.
agent:*/cost:updatedevents are not persisted (they go through#bus.emit),so per-node cost is the delta of
node:completed.cumulativeCostMicrocents— givingsum(run_costs) == runs.total_cost_microcents(exact run total even under fan-out; per-nodeattribution is approximate under parallel, documented).
verbatim and adds no runtime scan (infeasible on opaque JSON). The no-raw-secret invariant is
regression-guarded by a secrets fixture across every unsafe column.
apps/cli:history/open.ts: openhistory.db(better-sqlite3), migrate-on-first-use,chmod 0600the db(hard-fail) + best-effort the WAL/SHM sidecars;
ensureGlobalConfigDirnowchmod 0700the homedir — the ADR-0050 at-rest posture.
run.ts: inject the SQLite store via an optionalopenRunStoredep, close it infinally; apre-run open fault →
exit 2(invalid invocation), notexit 1.specs.tswires it in production.Tests + the 2.K harness omit it → the in-memory store, so they never touch the real
~/.relavium/.ADR + docs: ADR-0050 — the CLI
history.dbis unencrypted at rest, guarded by
0600/0700OS permissions (no credentials in the file:keychain + bus-masking); app-layer AES is a named future upgrade; cross-surface coexistence with the
desktop's SQLCipher is a named Phase-3 follow-on. Reconciled in lockstep: ADR-0005/0008 amendment
notes, and
database-schema.md/config-spec.md/keychain-and-secrets.mdnow state theper-surface at-rest posture (no canonical home contradicts the ADR).
Also in this PR
e3aff46) — marks 2.K (PR feat(cli): 2.K — engine regression harness over relavium run --json (M3) #43) Done + M3 reached across the status surfaces.Tests
@relavium/db: 12 store unit tests — fold correctness,seq/UNIQUEintegrity, statustransitions,
node:retryingghost-row prevention, interrupted-runs, the fan-out cost invariant,backward-compat, and the secrets fixture (every unsafe column).
apps/cli: 2 e2e — a real run →history.dbat a temp home (migrate-on-first-use,0600/0700perms), and a gate-paused run whose persisted events reconstruct a checkpoint via
reconstructCheckpointStatein a fresh connection (the 2.G resume substrate).Review trail
A
relavium-reviewerpass (8 findings, all fixed —4c7fa9b) + three independent external reviews(all PASS / ship-it; their findings fixed —
9e95363). Reviewers independently verified clean:transaction ordering, the cost-delta invariant, the async-reject contract, in-memory test isolation,
the chmod posture, secrets pass-through, engine purity, and the seam.
Verification
pnpm turbo run lint typecheck test buildgreen;@relavium/db96 tests,apps/cli125 tests;format clean; engine-deps within allowlists; Leakwatch 0;
packages/coreuntouched; no newexternal runtime dependency (
@relavium/dbis aworkspace:*dep).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
~/.relavium/history.db, enabling resumption of paused runs and persistent execution event logs.Documentation
Tests