Implement the full ADR-0006 plan: three-layer separation of transaction
correctness, retry policy, and cross-process writer coordination.
Layer 1 — scripts/tx.ts (transaction correctness):
- runWriteTransaction executes work exactly once; no internal retry.
- BEGIN IMMEDIATE takes the write lock up front (avoids SQLITE_BUSY_SNAPSHOT).
- Guarded rollback: checks inTransaction() via adapter before attempting
ROLLBACK; never masks the primary exception.
- WriteTxDiagnostics attached to errors: phase, code, label,
rollbackSucceeded, rollbackError, transactionActive.
- Binding adapters (betterSqliteTransactionAdapter, nodeSqliteTransactionAdapter)
mapping better-sqlite3's `.inTransaction` and node:sqlite's `.isTransaction`.
- configureConnection centralizes WAL + synchronous + busy_timeout.
Layer 2 — scripts/write-coordinator.ts (retry policy):
- runRetryableWriteTransaction: bounded retry with total time budget.
- Only retries when the transaction confirmed ended (transactionActive=false)
and the error is SQLITE_BUSY during work/commit phase.
- BEGIN-phase BUSY = abort entire build (isBeginBusyFailure); the caller
returns `{ deferred: true, reason: 'writer_busy' }` instead of waiting.
- hasUnusableTransaction detects a still-active transaction after failure;
aborts the build immediately, never retries.
Layer 3 — scripts/writer-lease.ts (cross-process coordination):
- acquireWriterLease: dedicated writer.lock.sqlite with busy_timeout=0 +
BEGIN IMMEDIATE. Non-blocking attempt; bounded wait with retryDelayMs.
- writerLockPathFor derives lock path from the target DB path.
- Lease held for the entire build; released on completion or failure.
- Lock DB uses DELETE journal (not WAL); crash/close auto-releases.
- All consumers obey: skill acquires at build start (returns deferred if
unavailable); app daemon (via worker) acquires for its build cycle.
Build semantics changes:
- affectedSessionIds updated only after successful commit.
- BuildIndexResult gains skipped/skippedFiles for observability.
- Skill finalize failure now fails the build (was silently warned).
- Checkpoint changed to PASSIVE (TRUNCATE reserved for maintenance/exit).
- Skill buildIndex returns { deferred, reason } on lease contention;
indexer-service reschedules the build (deferredRetryMs) without publishing
a heartbeat (so the build-deferred state is visible to cross-process
arbitration).
- Service publishes heartbeat immediately on start() for correct arbitration.
Tests:
- tests/write-transaction.test.mjs: single-shot execution, diagnostics
propagation, auto-rolled-back transaction detected, rollback failure
captured as metadata, BEGIN IMMEDIATE semantics.
- tests/writer-lease.test.mjs: acquire/release, contention returns null,
bounded wait with release during budget.
- tests/app-writer-lease.test.mjs: better-sqlite3 adapter integration.
- tests/app-rollback-guard.test.mjs: rewritten — transient BUSY recovered
by coordinator, persistent BUSY skips file, begin-busy aborts build,
live-transaction aborts build, phantom affectedSessionIds prevented.
- tests/daemon-arbitration.test.mjs: skill defers to fresh app heartbeat,
builds when heartbeat is stale.
- tests/app-indexer-service.test.mjs: new cases for deferred-retry
scheduling and immediate heartbeat on start.
- app/tests/electron-concurrency.mjs + child: dual-child IPC structure for
real better-sqlite3 contention (holder acquires lock → build child starts
→ delayed release → result collected; persistent contention bounded).
ADR-0006 updated to reflect the implemented design.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reported as "cannot rollback - no transaction is active" — a secondary error.
SQLite auto-rolls back certain failures (SQLITE_BUSY / SQLITE_BUSY_SNAPSHOT,
disk full), after which the per-file loop's unguarded ROLLBACK in its catch
threw over the real error and aborted the whole build instead of skipping just
the bad file.
Stopgap only (see docs/adr/0006 for the planned full fix):
- Add safeRollback(db) in both indexers: it swallows only the rollback's own
error, so the true cause surfaces. Per-file failures are logged and the build
continues; the finalize failure still propagates.
- Give the skill's node:sqlite connection an explicit PRAGMA busy_timeout=5000
(it has no default). The app adds none: better-sqlite3 already defaults to
5000ms, so busy_timeout is NOT the root-cause fix and is not treated as one.
Add tests/app-rollback-guard.test.mjs: injects a DB that faithfully reproduces
"a write auto-rolls back the txn, then ROLLBACK errors" and asserts the build
survives (bad file skipped, other file indexed). Revert-checked: without the
guard the test fails with the exact "cannot rollback - no transaction is active".
docs/adr/0006 records the real fix (shared runWriteTransaction, single-writer
coordination, BEGIN IMMEDIATE, whole-transaction retry, PASSIVE checkpointing)
as deferred, two-phase work — and why bumping busy_timeout is not it.
Verified: suite 124/124, typecheck clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`npm run build:skill` emits dist/obelisk-skill/: the whole scripts/ Core
compiled to readable JS (module structure + comments preserved, ~1:1 with
source, no bundling/minifying per ADR-0004), plus schema.sql, SKILL.md,
references/, and a package.json. It excludes app/, tests/, and release/, which
is what keeps the artifact small.
- tsconfig.skill.json compiles scripts/**/*.{ts,mjs} (allowJs) with
rewriteRelativeImportExtensions, so the .ts specifiers inside the .mjs shells
(runtime.mjs -> ./core.js, indexer.mjs -> ./providers/*.js, ./persist.js) are
rewritten to .js. declaration:false — the skill needs no .d.ts.
- packaging/skill-package.json provides the artifact's package.json; it sets
"type":"module" because the emitted .ts->.js files are ESM.
- build:skill runs tsc then copies schema.sql/SKILL.md/references/package.json.
Add tests/build-skill.test.mjs: runs the real build:skill, asserts the artifact
structure, that no emitted .js/.mjs still imports a .ts module, and that the
compiled artifact builds an index and answers a search end-to-end under plain
Node (no type-stripping) against a temp HOME.
Verified: suite 123/123, typecheck clean, artifact smoke run green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The skill's `--build` (always a force build) cleared `index_state` and
re-indexed existing files, but never removed rows for transcripts that no
longer exist on disk, so deleted sessions accumulated forever (the dogfood
showed 328 indexed sessions vs 283 current files). A force build is meant to
be a clean rebuild, matching what the app already does.
Drop every derived table (messages, tool_calls, tool_results, sessions,
summaries, subagents, workflows, workflow_agents) in the force path, then
re-index from the current files. `memories` is the durable, human-approved
layer and is never cleared; messages_fts is repopulated by the existing
'rebuild' command in finalize.
Add a test that builds two sessions, deletes one transcript, force-rebuilds,
and asserts the stale session is purged while a seeded memory survives.
Verified the test is discriminating: without the fix it reports
['gone','keep'] instead of ['keep'].
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Convert the app's main and preload source from .js to .ts (git mv preserves
history), adding types where they carry value: the core-consumption seam
(BuildIndexOptions/BuildIndexResult, FileInfo), the indexer service/worker
factories, and the preload IPC bridge. Module-to-module specifiers use the
real .ts extension (mirroring scripts/, since Node type-stripping does not
rewrite .js->.ts); the worker's runtime path stays indexer-worker.js because
that is the built output.
Toolchain:
- Add app/tsconfig.json: strict but noImplicitAny:false (the app orchestrates
the already-strict core; annotating every SQLite-handle helper is low-value
churn) + allowImportingTsExtensions (safe under noEmit).
- Add @types/better-sqlite3 for the injected binding.
- electron.vite.config.ts inputs -> .ts; refresh the stale CommonJS comment.
- typecheck script runs root + app projects. Root tsconfig excludes the
app-importing tests (app-*.test.mjs, recap-capture-query.test.mjs) so the
lenient app files are not dragged into the strict root program; the app
source is covered by app/tsconfig.json instead. See docs/adr/0005.
Verified: npm run typecheck (root + app) clean; suite 121/121; electron-vite
build emits all 6 main entries + preload with no .ts/node:sqlite residue in
the bundles.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The desktop app now indexes through the shared provider adapters + persist
layer (scripts/providers/{claude,codex}, scripts/persist, scripts/parsing)
instead of maintaining its own parallel indexer. buildIndex shrinks from
~1173 to ~592 lines, eliminating the skill<->app parse duplication that
Phase 5 set out to remove. electron-vite bundles the .ts core from source
with better-sqlite3 injected; the provider->parsing graph stays
node:sqlite-free so nothing drags node:sqlite into the app.
Also fix a misleading log: when a manual rebuild tears down the worker
mid-build, the cancelled background build is a deliberate stop, not a
failure. Guard the service's failure log with the stopped flag so it no
longer prints "Obelisk index build failed: Indexer worker stopped" on
every rebuild.
- CONTEXT.md: provider-adapter + single-persist + node:sqlite-free parsing.
- docs/adr/0005: app builds with electron-vite (TS+ESM), packages with
electron-builder; preload CJS for sandbox; app consumes core from source.
Verified: full suite 121/121; a node:sqlite-adapter dogfood of the rebuild
path over real data (969 files, 285 sessions, FTS rebuilt) runs clean; app
Rebuild confirmed in real Electron/better-sqlite3.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
App source (main/preload/worker/renderer) -> ESM; app is now type: module;
__dirname via import.meta.url; worker spawned with type module. Preload is built
as CJS (electron-vite output format) because the sandboxed renderer does not
support ESM preload; main loads ../preload/index.js. Removed dead imports
(nativeImage, readline) and the obsolete scripts/dev.js.
Tests: 4 app tests require->import; app-main-settings rewritten with node:test
mock.module + dynamic import (replacing CJS Module._load mocking); test script
adds --experimental-test-module-mocks. electron-vite build clean, 119/119, and
npm run dev verified: app launches, preload bridges IPC, data loads.
Restructure app into src/{main,preload,renderer}; electron.vite.config.ts builds
all three (each main module its own input so CJS requires + the indexer worker
resolve; better-sqlite3 externalized; Vue plugin for renderer). main/index.js
paths updated for the out/ layout + ELECTRON_RENDERER_URL. Still JS/CJS — TS+ESM
and core consumption are the next stages. Verified: npm run dev launches clean;
electron-vite build succeeds; root suite 119/119.
Complete the skill-side provider migration: codex now goes through a pure adapter
and the shared persist layer, and the two original monolithic indexers are gone.
New:
- scripts/providers/codex.ts — pure codex adapter. Full-reparse (buffers the whole
file) because the event_msg↔response_item dedup needs whole-file, bidirectional
knowledge; emits SessionRecord with countMode 'total'. Handles guardian threads
(→ delete-session), agent spawns/tool calls (→ tool_call/subagent), token_count
(patched onto the message record) and task_complete (→ message-turn-duration).
Contract:
- SessionRecord.countMode ('total' | 'delta') tells persist whether to replace or
accumulate message_count — claude is line-incremental (delta), codex full-reparse
(total). SubagentRecord non-key fields are optional; persist merges them
column-wise with COALESCE. MessageTurnDurationRecord.turn_duration_ms is nullable.
Orchestration:
- buildIndex's codex branch parses via the adapter and writes via persist. An
unchanged file is skipped but still swept for stale guardian rows (routed through
persist as a delete-session), preserving prior behavior.
Cleanup:
- Remove the now-unused indexJsonl, indexCodexJsonl, deleteCodexThreadRows and
upsertCodexSubagent — their semantics now live in the adapters + persist.
indexer.mjs drops from ~840 to 428 lines. Codex pure helpers stay exported for
codex.ts and the guardian sweep (physical move deferred to the app-side reorg).
- Migrate the upsert drift test off indexJsonl to the claude.parse + persist path,
keeping the rowid-stability and count-replace regression guards.
Tests: tests/codex-parse.test.mjs (record-stream golden: dedup, tools, token patch,
turn-duration, guardian→delete) and tests/codex-index.test.mjs (full buildIndex
path: fresh build + incremental full-reparse, total-count replace, no duplicates).
Verified equivalent on the real ~/.obelisk index: codex messages 82476 and
subagents 522 identical before/after, zero guardian leakage; real incremental
confirmed (touch a codex file → reparsed idempotently, unchanged files skipped).
lint + typecheck clean, 119/119.
buildIndex's claude branch now parses via providers/claude.ts and writes via
persist.ts instead of the inlined indexJsonl. Behavior is equivalent — the full
buildIndex integration suite (runtime.test.mjs) stays green, 116/116 — and a
force rebuild of the real ~/.obelisk index (327 sessions, 119k messages)
reproduced identical session counts with project_path fully populated.
codex, indexSubagentMeta, workflows, history and the project_path pass are
untouched. indexJsonl is now unused by buildIndex (kept for its drift test;
removed once codex is migrated).
Adds tests/incremental-index.test.mjs: verifies resume/accumulate through the
full buildIndex path (append new lines to an indexed session → incremental
build resumes from the cursor, message_count accumulates, no duplicates). The
30s shouldSkipBuild debounce is cleared in-test so the incremental run fires.
Introduce scripts/persist.ts — the single, provider- and binding-agnostic
layer that consumes an adapter's IndexRecord stream and writes rows into an
injected SQLite handle (node:sqlite for skill/CLI, better-sqlite3 for the app).
It is the only layer that touches the database.
Write semantics are the canonical ones reconciled from the earlier drift:
- messages upsert via ON CONFLICT (turn_duration_ms not in the column list, so
it is never clobbered)
- sessions merge with the existing row: started_at MIN, ended_at MAX,
message_count reset-or-accumulate by resume state, fill-if-null for the rest;
project_path is preserved and left to refreshSessionProjectPaths
- message-turn-duration applies as a targeted UPDATE
- delete-session cascades across all tables
- the generator's return cursor is written back to index_state (mtime:lines →
the two existing columns; no schema migration yet)
Purely additive — buildIndex still uses the old indexJsonl path, so existing
behavior is unchanged. Rewiring happens in 5b-2b.
Adds tests/persist.test.mjs: all record kinds written, resume does not
double-count message_count, fresh re-scan resets it, delete-session cascades.
Full suite 115/115, lint + typecheck green.
claude.parse mirrors indexJsonl line-for-line but yields IndexRecords (no db).
Adds MessageTurnDurationRecord op. Purely additive — buildIndex still uses the
old path. 111/111 green.
scripts/indexer.mjs indexJsonl used INSERT OR REPLACE (rowid/FTS churn) and
carried message_count forward on full re-scan; align to app's ON CONFLICT upsert
+ count reset so the two indexers no longer silently diverge. Regression test added.
search() falls back to safe per-token quoting on malformed FTS input instead of
crashing (documented in api-reference.md). Adds raw() to the doc-synced shape
contract. Full suite 107/107.
tommy0103
