refactor(storage): establish canonical Bun schema and DuckDB dialect

[mariusvniekerk]

SQLite, PostgreSQL, and DuckDB previously evolved independent schema and query foundations, so parity depended on duplicated declarations and backend-specific plumbing. This first stack layer establishes one Bun model registry, guarded backend handles, and a dedicated DuckDB dialect while converging SQLite and PostgreSQL in place under their existing serialization boundaries.

Stamped schemas now validate structural, index, and trigger drift without repair; expensive row invariants remain limited to one-time convergence. Shared timestamps preserve the shipped SQLite empty sentinel and RFC3339Nano text contract, and Quack rejects direct driver arguments that would otherwise be discarded.

Direct database/sql access remains limited to lifecycle, connection-local, transport, and compatibility seams. DuckDB remains a disposable mirror, while narrowly scoped FTS and vector behavior stays adapter-specific. Stack 1 of 6.

^{generated by a clanker}

[roborev-ci]

roborev: Combined Review (ce3270c)

Medium-severity schema compatibility issues remain in the storage refactor.

Medium

• internal/db/bunmodel/identity.go:72 — The canonical worktree mapping model generates TIMESTAMP DEFAULT CURRENT_TIMESTAMP columns in DuckDB, while Quack attach rejects catalogs containing dynamic timestamp defaults. Schema v10 mirrors therefore break remote Quack access.

  • Fix: Remove these defaults for DuckDB and explicitly supply created_at and updated_at in mirror writers, or strip the defaults in the DuckDB schema adapter.

• internal/postgres/schema.go:1414 — PostgreSQL convergence rejects sessions with empty source provenance without first migrating legacy rows. Databases upgraded from versions predating these columns receive empty defaults and fail EnsureSchema, blocking the push that would populate them.

  • Fix: Backfill legacy provenance where ownership is known, or allow legacy rows through convergence and require a full push before stamping compatibility.

---

Reviewers: 2 done | Synthesis: codex, 13s | Total: 19m21s

[roborev-ci]

roborev: Combined Review (7899144)

The refactor has two medium-severity data-integrity issues; no security regressions were identified.

Medium

• internal/duckdb/push.go:829 — Pin ID reuse can abort curation refreshes. Curation refresh upserts pins before deleting stale rows. SQLite may reuse a deleted highest pin ID, causing a new pin to collide with a stale DuckDB primary key despite having a different (session_id, ordinal). Delete stale pins before inserting current pins, or avoid using source pin IDs as DuckDB primary keys. Add a regression test for SQLite ID reuse.

• internal/postgres/schema.go:1349 — Schema convergence omits canonical foreign keys. Legacy tables already exist when CreateCommonSchema runs, so CREATE TABLE IF NOT EXISTS cannot add constraints such as the composite message/tool-call relationships. CheckCommonSchema can nevertheless stamp the schema as compatible. Add idempotent ALTER TABLE ... ADD CONSTRAINT migrations for all canonical foreign keys and verify them through pg_constraint before writing the compatibility stamp.

---

Reviewers: 2 done | Synthesis: codex, 12s | Total: 20m2s

[roborev-ci]

roborev: Combined Review (0a688f7)

High-severity PostgreSQL schema convergence issue and two medium-severity migration gaps must be addressed before merge.

High

• internal/postgres/schema.go:2655 — pushSchemaCurrent accepts pre-change PostgreSQL schemas without validating the common-schema stamp, newly required columns, or native pricing timestamp types. A normal pg push can therefore skip convergence and leave updated_at as TEXT, causing later pricing updates that use timestamp/interval operators to fail.
  • Fix: Require bun_common_schema_v1, then validate the common schema and pricing column types before taking the fast path.

Medium

• internal/postgres/schema.go:1337, internal/postgres/push.go:2253 — PostgreSQL push, scan, compatibility, and fingerprint paths do not fully handle the newly canonical session fields: file_size, file_mtime, file_inode, file_device, file_hash, and local_modified_at. Values remain NULL, and changes to most fields can be silently skipped.

  • Fix: Add these fields to insert/update arguments, the conflict predicate, sessionPushFingerprint, pgSessionCols, scanPGSession, and the compatibility probe.

• internal/postgres/schema.go:1404 — CreateCommonSchema relies on CREATE TABLE IF NOT EXISTS, which cannot add canonical composite foreign keys to tables previously created by coreDDL. Required message-to-tool-call, tool-call-to-result, and message-to-pin relationships may remain absent even though CheckCommonSchema stamps the migration complete.

  • Fix: Add explicit idempotent PostgreSQL constraint migrations and validate the required foreign keys before writing the compatibility stamp.

---

Reviewers: 2 done | Synthesis: codex, 12s | Total: 13m20s

[roborev-ci]

roborev: Combined Review (95a5c2b)

Changes requested: four medium-severity backend persistence and schema-convergence issues remain.

Medium

• internal/postgres/schema.go:1337 — Six new session file-metadata columns are omitted from PostgreSQL pushes, reads, compatibility checks, and mostly from sessionPushFingerprint. Values remain NULL, and metadata-only updates may be skipped. Add all six columns throughout these paths and add round-trip tests.

• internal/duckdb/push.go:1038 — parser_parent_session_id is omitted from DuckDB insert, update, argument, and fingerprint paths, losing parser-derived parent relationships. Include ParserParentSessionID and add a round-trip test where it differs from ParentSessionID.

• internal/postgres/worktree_mappings_push.go:157 — Mapping publication omits created_at, replacing source creation timestamps with migration or insertion time. Publish CreatedAt, update compatibility checks, and republish existing mappings to correct defaulted values.

• internal/postgres/schema.go:1349 — Canonical foreign keys are not migrated into existing PostgreSQL tables. Add idempotent constraint migrations, validate them against the PostgreSQL catalog, and test that orphaned relationships are rejected.

---

Reviewers: 2 done | Synthesis: codex, 9s | Total: 24m27s

[roborev-ci]

roborev: Combined Review (6d0f950)

Storage convergence has two medium-severity correctness issues; no critical or high-severity findings were identified.

Medium

• DuckDB pin ID collision — internal/db/bunmodel/curation.go:15, internal/duckdb/push.go:829
  DuckDB treats copied SQLite pin IDs as primary keys. Because SQLite can reuse an INTEGER PRIMARY KEY after unpinning, inserting a replacement before deleting the stale mirror row can fail on id, despite the upsert targeting (session_id, ordinal). Let DuckDB generate mirror-local IDs, make id non-key data, or clear existing pins before reinsertion.

• Missing PostgreSQL foreign-key convergence — internal/postgres/schema.go:1349
  Convergence stamps the schema without adding most registered canonical foreign keys. CREATE TABLE IF NOT EXISTS cannot retrofit constraints onto existing coreDDL tables, leaving relationships such as sessions-to-archives, pins-to-messages, and results-to-tool-calls unenforced. Add idempotent constraint migrations, update fresh DDL, and validate constraints before writing the compatibility stamp.

---

Reviewers: 2 done | Synthesis: codex, 8s | Total: 14m47s

[roborev-ci]

roborev: Combined Review (6791f37)

The storage refactor has four medium-severity schema-parity and data-integrity issues across PostgreSQL and DuckDB.

Medium

• PostgreSQL provenance fields are not returned — internal/postgres/sessions.go:93
  Session queries neither select nor scan source_archive_id and source_database_generation, so PostgreSQL-backed reads silently return empty provenance even though push code stores it. Add both columns to pgSessionCols and destinations to scanPGSession, plus a PostgreSQL round-trip test.

• DuckDB drops parser lineage — internal/duckdb/push.go:1026, internal/duckdb/sync.go:1041, internal/duckdb/store.go:344
  Although parser_parent_session_id is created, it is omitted from writes, fingerprints, reads, and scans. Lineage is lost, and changes cannot trigger incremental updates. Wire the field through each path and test differing parser and effective parent IDs.

• DuckDB schema version was not incremented — internal/duckdb/schema.go:25
SchemaVersion remains 10 despite an incompatible layout change, allowing materially different mirror schemas to appear compatible. Increment the version and update metadata expectations, fixtures, and documentation.

• PostgreSQL convergence omits canonical foreign keys — internal/postgres/schema.go:1501
  Convergence creates existing tables with IF NOT EXISTS and stamps the canonical schema without installing or validating required foreign keys for source provenance, tool calls/results, and pinned messages. Add idempotent constraint migrations and validate the PostgreSQL catalog before writing the compatibility stamp.

---

Reviewers: 2 done | Synthesis: codex, 26s | Total: 23m48s

[roborev-ci]

roborev: Combined Review (f2df2ed)

Changes requested: two medium-severity compatibility issues could break CockroachDB initialization and leave stale DuckDB mirrors unreconstructed.

Medium

• internal/postgres/schema.go:1514 — CockroachDB does not support PostgreSQL advisory locks. Common-schema convergence now unconditionally calls pg_advisory_xact_lock, causing CockroachDB schema checks to fail before compatibility validation, even for current databases. Detect CockroachDB and use a compatible migration-serialization mechanism while retaining advisory locks for PostgreSQL. Add integration coverage for both initialization paths.

• internal/duckdb/schema.go:25 — DuckDB schema version was not incremented. The schema remains at version 10 despite material changes to columns, types, keys, indexes, and tables. Existing version-10 mirrors may pass compatibility checks despite having an incompatible physical schema. Increment SchemaVersion and update related expectations so existing mirrors are rebuilt.

---

Reviewers: 2 done | Synthesis: codex, 18s | Total: 18m31s

[roborev-ci]

roborev: Combined Review (6110445)

Schema convergence introduces a medium-severity CockroachDB compatibility regression.

Medium

• internal/postgres/schema.go:1514 — EnsureSchema now unconditionally calls pg_advisory_xact_lock, which CockroachDB does not support. This causes every CockroachDB startup to fail, even when the compatibility stamp already exists. Detect CockroachDB and use a supported serialization mechanism, such as locking a known metadata row with transaction retries, while retaining advisory locking for PostgreSQL.

---

Reviewers: 2 done | Synthesis: codex, 16s | Total: 19m56s

[roborev-ci]

roborev: Combined Review (59df744)

Medium-severity schema migration issues need resolution before merge.

Medium

• internal/db/bun_schema.go:316 — Canonical convergence creates the new session identity trigger without removing legacy session, observation, snapshot, and worktree-mapping triggers. New sessions continue populating legacy storage and cause duplicate publication-revision/journal updates. Drop the legacy triggers during convergence and do not reinstall them after activating the canonical schema.

• internal/postgres/schema.go:1561 — CreateCommonSchema runs CREATE TABLE IF NOT EXISTS after coreDDL has already created every table, preventing canonical foreign keys from being installed even on fresh databases. This leaves relationships such as ordinal pins and tool calls to messages, and source records to archives, without expected integrity and cascade behavior. Add idempotent constraint migrations or create fresh common tables through the Bun registry first, and validate required constraints in stamped-schema checks.

---

Reviewers: 2 done | Synthesis: codex, 9s | Total: 26m0s

[roborev-ci]

roborev: Combined Review (b5871d1)

Medium-severity provenance issue found; no security vulnerabilities were identified.

Medium

• internal/db/orphaned.go:1550 — Rekeying fresh project-identity observations changes source_archive_id but retains the old source_archive_salt. During CopySessionMetadataFrom, conflicting source observations may then be ignored, preserving inconsistent provenance and producing unstable project keys. Update source_archive_salt from the target source_archives row when rekeying source_project_identity_observations.

---

Reviewers: 2 done | Synthesis: codex, 10s | Total: 22m21s

[roborev-ci]

roborev: Combined Review (cf03ad7)

Changes requested: one medium-severity DuckDB synchronization issue was found.

Medium

• internal/duckdb/push.go:829 — New pins are inserted before stale pins are removed. If SQLite reuses a deleted pin ID, the stale mirrored row causes a DuckDB primary-key collision and aborts synchronization. Clear existing mirror pins for resident sessions before inserting the new snapshot, and test ID reuse across different pins.

---

Reviewers: 2 done | Synthesis: codex, 6s | Total: 21m49s

[roborev-ci]

roborev: Combined Review (81c9b39)

Verdict: High-severity fresh-archive initialization failure detected.

• High — internal/db/project_identity_backfill.go:57-68: EnsureProjectIdentityBackfillQueued queries source_session_project_identity_snapshots before OpenFreshIsolatedContext runs the migration that creates canonical source_* tables. Capture and fresh-resync paths therefore fail with “no such table.” Initialize the canonical SQLite schema before queueing the backfill, or create these tables earlier during fresh-archive setup.

---

Reviewers: 2 done | Synthesis: codex, 7s | Total: 25m15s

[roborev-ci]

roborev: Combined Review (88a2f4d)

Verdict: Two high-severity issues require resolution before merging.

High

• internal/db/bun_schema.go:638-640 — PRAGMA index_info returns NULL names for expression-index columns, causing valid canonical expression indexes to be reported as schema drift. Archives containing these indexes may fail schema convergence and cannot reopen. Validate expression indexes using sqlite_schema.sql or expression-aware pragma_index_xinfo, and add reopening coverage.

• internal/db/db.go:1038-1053 — OpenFreshIsolatedContext skips schema convergence, so fresh capture archives lack canonical tables such as source_archives and identity-mapping tables. Subsequent capture writes or read-only opens can fail. Initialize the required IDs and apply the current common schema before returning the database.

---

Reviewers: 2 done | Synthesis: codex, 6s | Total: 14m10s

[roborev-ci]

roborev: Combined Review (23cbd2c)

Verdict: High-severity fresh database initialization failure identified; mirrored session provenance is also lost.

High

• internal/db/project_identity_backfill.go:57-67, internal/db/db.go:1054: OpenFreshIsolatedContext queues project-identity backfill before creating canonical source_* tables. Fresh capture databases fail initialization because source_session_project_identity_snapshots does not exist.
  • Fix: Create/converge the canonical schema before calling EnsureProjectIdentityBackfillQueued.

Medium

• internal/postgres/sessions.go:92-124,255-315; internal/duckdb/store.go:344-412: PostgreSQL and DuckDB session queries omit source_archive_id and source_database_generation, causing mirrored sessions to lose provenance when read.
  • Fix: Include and scan both columns into db.Session, with a round-trip test.

No concrete security vulnerability or privileged attack path was identified.

---

Reviewers: 2 done | Synthesis: codex, 8s | Total: 34m14s

[roborev-ci]

roborev: Combined Review (d003e4f)

Verdict: Three medium-severity data-isolation issues were identified; another reviewer found no security vulnerabilities.

Medium

• internal/db/project_identity.go:1605-1610 — Credential scrubbing omits source_archive_id from its deletion predicate, potentially deleting matching observations from another source archive. Include and bind the observation’s archive ID.

• internal/db/project_identity.go:1014-1016 — Empty snapshot cleanup filters only by source_session_id, so colliding IDs can delete snapshots from another source. Scope the deletion by archive ID, database generation, and session ID.

• internal/db/project_identity.go:1164-1191 — Aggregate reconciliation does not consistently preserve source archive and database-generation provenance, risking selection and deletion of another source’s snapshots or aggregates. Propagate provenance through lookup, deletion, and reinsertion queries.

---

Reviewers: 2 done | Synthesis: codex, 7s | Total: 36m21s

[roborev-ci]

roborev: Combined Review (899c482)

Verdict: Changes require fixes for 6 findings.

High

• internal/db/bunmodel/message.go:24-30; internal/duckdb/schema.go:111-117; internal/duckdb/messages.go:26-34: The canonical Message model omits reasoning_effort, so fresh DuckDB schemas do not create that column while DuckDB queries still select and scan it. Add reasoning_effort to the shared model/schema, or remove it consistently from DuckDB reads and writes.

  Reported by: codex

• internal/db/bun_schema.go:630-650; internal/db/bunmodel/registry.go:91-100: SQLite convergence rejects its own expression indexes: the registry creates non-partial CASE-expression indexes, but validation requires partial=true and compares expression terms against NULL PRAGMA column names. Validate the canonical expression SQL/metadata consistently with creation, while handling any legacy partial-index form explicitly.

  Reported by: codex

• internal/db/staged_content.go:605-618: The staged-content insertion path does not populate MessageOrdinal, so new tool-call rows receive ordinal 0 and the NULL-only repair trigger cannot correct them. Set MessageOrdinal to m.Ordinal when constructing staged ToolCall values, or derive and validate it during insertion.

  Reported by: codex

• internal/db/db.go:2988-3008; internal/db/bunmodel/usage.go:5-25: Common-schema validation runs before ensureUsageEventsSchemaLocked adds provider_id, so older archives whose usage_events table lacks that column fail migration before the repair executes. Run usage-event migrations before common-schema convergence, or include provider_id repair in the convergence transaction.

  Reported by: codex

Medium

• internal/db/worktree_mappings.go:140-175: SQLite worktree mappings scan an INTEGER enabled column directly into bool; the SQLite driver returns int64, which database/sql cannot convert to bool. Scan the value as an integer or generic value and normalize it to bool.

  Reported by: codex

• internal/db/pricing.go:541-562: CopyModelPricingFrom checks for genai_pricing but unconditionally reads old_db.pricing_metadata. Older source archives without that table roll back the entire pricing copy. Probe pricing_metadata existence and skip that copy when absent, preserving the model-pricing copy.

  Reported by: codex

---

Reviewers: 2 done | Synthesis: codex, 12s | Total: 14m41s

[roborev-ci]

roborev: Combined Review (b0ef87d)

Verdict: Changes require fixes for 3 findings.

High

• internal/db/db.go:1127-1145: Fresh isolated databases queue the project identity backfill before creating the canonical source identity tables, causing initialization to fail on an empty database. Create the current canonical common schema, including source identity tables, before queuing the backfill while preserving the no-historical-migration behavior.

  Reported by: codex

• internal/db/bunmodel/message.go:25-28: The canonical Bun Message model omits reasoning_effort, although fresh DuckDB schemas and push/read paths require the column, causing fresh mirror writes to fail and losing the value during row conversion. Add and map reasoning_effort in the canonical model and row converters, and assert the column in generated-schema tests.

  Reported by: codex

Medium

• internal/db/bun_schema.go:694-716; internal/db/db.go:3361-3369; internal/db/project_identity.go:1532-1543: Migration copies git_remote verbatim into canonical observations, while the scrub exits when its existing completion marker is set, allowing credential-bearing legacy URLs to migrate unsanitized. Version or invalidate the old scrub marker and run an atomic scrub during migration regardless of its prior value. Scrub both canonical and legacy tables, with a regression test covering an already-set marker.

  Reported by: codex (security)

---

Reviewers: 2 done | Synthesis: codex, 12s | Total: 42m22s

[roborev-ci]

roborev: Combined Review (a6ee520)

Verdict: Changes require fixes for 2 findings.

High

• internal/db/bunmodel/message.go:14-40; internal/duckdb/schema.go:111-118; internal/duckdb/messages.go:26-34; internal/duckdb/push.go:1383-1397: The canonical Bun Message model omits reasoning_effort, so the new DuckDB schema does not create that column while existing DuckDB read and push paths still select and insert it. Fresh mirrors will fail message reads and pushes. Add ReasoningEffort to the canonical model and row conversions, then cover the column in schema compatibility tests.

  Reported by: codex

• internal/db/bun_schema.go:288-313; internal/postgres/schema.go:1644-1671: Pricing metadata migration handles only three sentinel keys and omits the persisted _openrouter_models key. SQLite upgrades lose OpenRouter ownership state, while PostgreSQL upgrades leave JSON in model_pricing.updated_at before attempting to cast it to TIMESTAMPTZ, causing schema convergence to fail. Migrate every legacy metadata sentinel, including _openrouter_models, into the dedicated metadata representation before changing updated_at; update PostgreSQL writers and add upgrade coverage for the OpenRouter key.

  Reported by: codex

---

Reviewers: 2 done | Synthesis: codex, 9s | Total: 18m22s

[roborev-ci]

roborev: Combined Review (734782a)

Verdict: Changes require fixes for 1 finding.

High

• internal/db/bunmodel/message.go:25: The canonical Bun Message model omits the existing reasoning_effort column. Fresh DuckDB schema creation therefore lacks the column, while DuckDB reads and inserts still select and write it; populating a fresh mirror fails and schema compatibility checks incorrectly accept the incomplete schema. Add ReasoningEffort to bunmodel.Message, preserve it in both Bun row conversion functions, and add a fresh-mirror regression covering message insertion and reads.

  Reported by: codex

---

Reviewers: 2 done | Synthesis: codex, 4s | Total: 33m5s

[roborev-ci]

roborev: Combined Review (50c09eb)

Verdict: Changes require fixes for 6 findings.

Critical

• internal/db/db.go:3079: Database initialization runs common-schema convergence before creating or loading database_id, archive_id, and archive_salt. Fresh and legacy archives therefore fail to open because the required archive_metadata rows do not yet exist. Run convergence after initializing the three identity values in both normal and isolated-open paths, or have convergence bootstrap the identity atomically.

  Reported by: codex

• internal/db/bunmodel/message.go:25-27; internal/db/bun_rows.go:237-313: Fresh DuckDB schemas generated from bunmodel.Message omit reasoning_effort, while DuckDB reads and writes that column. Rebuilding or syncing a non-empty mirror can fail with a missing-column error and conversions can drop the value. Add ReasoningEffort to the canonical Bun model and both row conversion functions, and cover it in schema column assertions.

  Reported by: codex

High

• internal/postgres/schema.go:1644-1671: The PostgreSQL pricing migration casts every remaining model_pricing.updated_at value to TIMESTAMPTZ, but the _openrouter_models JSON sentinel remains in that column and causes an invalid timestamp cast. Move the sentinel to dedicated metadata before the type change or explicitly handle it during migration, and stop storing opaque metadata in the timestamp column.

  Reported by: codex

• internal/db/pricing_list.go:18-90; internal/duckdb/push.go:25-45; internal/postgres/pricing.go:894-928: Pricing metadata moved to SQLite pricing_metadata, but ListModelPricing and both mirror sync paths transfer only model_pricing rows. As a result, _openrouter_models is lost in DuckDB and PostgreSQL while reconciliation still depends on the sentinel. Propagate pricing_metadata through each sync plan into dedicated backend metadata storage, and update reconciliation to use that representation instead of inserting sentinel values into model_pricing.

  Reported by: codex

• internal/db/pins.go:9-74; internal/db/orphaned.go:1283-1407; internal/duckdb/push.go:1648-1658: SQLite adds source_uuid to pinned_messages, but PinnedMessage, pin scans, archive-resync pin copying, and DuckDB pin insertion do not read or populate it. Pins created or restored through SQLite lose durable message identity across resyncs and mirrors. Add SourceUUID to the public pin type and scans, populate it from the pinned message, and preserve it in resync and DuckDB synchronization paths.

  Reported by: codex

Medium

• internal/db/pricing.go:557-562: CopyModelPricingFrom unconditionally selects old_db.pricing_metadata, a table absent from pre-cutover archives. Resyncing such an archive fails the pricing copy and can leave the replacement archive without pricing metadata. Probe the attached source schema and translate legacy model_pricing sentinel rows into pricing_metadata when the new table is absent.

  Reported by: codex

---

Reviewers: 2 done | Synthesis: codex, 11s | Total: 19m31s