fix(#1901): detectSchema fails loud instead of caching wrong schema mode

[SaarMesh-Bot]

Fixes #1901.

Thanks to @MarekWo for the exceptionally thorough report — root cause, repro, and a prioritised fix checklist in one. This implements it.

Problem

detectSchema() swallowed any probe-query error with a bare return, so a single transient failure of the first PRAGMA table_info(observations) at startup left isV3 (and the feature flags) at their zero value for the entire process lifetime. The server then ran v2 SQL against a v3 DB: Packets page empty, /api/channels/<name>/messages → 500, logs full of no such column: o.observer_id, while the database was perfectly healthy. Nothing re-checked the flag, so only a manual restart recovered it.

Fix

Works through the report's checklist:

• Don't swallow the error. detectSchema now returns error and OpenDB aborts on it. main.go already log.Fatalfs on an OpenDB failure, so the supervisord/Docker restart policy retries and a transient cause clears on the next attempt — strictly better than serving a broken read API.
• Log the mode unconditionally — [db] schema mode: v3 (observer_idx) / v2 (observer_id). A clean startup log is now positive evidence detection ran, not just an absence of errors.
• Run detection on a single pinned connection (conn.Conn(ctx)) rather than an arbitrary pooled one, so the startup race in the report's hypothesis can't quietly hand detection a fresh, not-yet-openable handle — if the connection can't be acquired, we fail loud.
• Close() no longer checkpoints the read-only handle. PRAGMA wal_checkpoint(TRUNCATE) on a mode=ro connection always failed with disk I/O error (778) and looked like a storage fault on every shutdown (the report's aside). The ingestor (the writer) owns WAL checkpointing.

The three near-identical PRAGMA scan loops are consolidated into one schemaColumns() helper that returns errors instead of ignoring Scan failures.

On the "single source of truth" item

The report suggests deriving isV3 from dbschema.TableHasColumn(...). I kept the PRAGMA-scan structure here because detectSchema sets six flags from three tables in a single pass; swapping to TableHasColumn would mean six separate probe calls and wouldn't actually be cleaner. The goal it was aimed at — never cache a false negative — is met by making the existing scan fail loud. Happy to switch to the single-probe-per-column shape if you'd prefer it.

Honest note on the connection

conn.Conn(ctx) pins a single connection for all four probes and fails loud if it can't be acquired; it does not guarantee the literal connection Ping() validated (database/sql doesn't expose that). The fail-fast is what actually closes the bug — a mis-detected schema aborts startup instead of persisting for the process lifetime.

Tests

• TestDetectSchemaFailsLoudOnProbeError — injects a probe failure through a rowQuerier and asserts the error propagates and isV3 stays unset (the invariant the old bare-return violated).
• TestDetectSchemaV3AndV2 — covers both schema shapes through OpenDB.

go vet ./cmd/server and go build are clean; targeted go test -run 'DetectSchema|OpenDB' is green.

Heads-up on the full go test ./cmd/server run: a handful of TestHandleNodePaths_* / TestHandleAnalytics* tests return 503 index loading, plus one intentional panic test — these fail identically on pristine master (a06ac8ac) with this branch stashed, i.e. they're pre-existing/timing-related and untouched by this change.

Out of scope (per the issue): frontend behaviour when the API 500s.

🤖 Authored with Claude · Co-Authored-By trailer on the commit.

[efiten]

Review from the queue triage. Written 2026-08-30 against the tree at that time; posting now that the maintenance window in #1922 has opened.

Verdict: approve with two notes, and one merge-order finding that matters more than the
review itself.

The core change is right: a swallowed PRAGMA failure previously left isV3=false against a v3
database for the whole process lifetime. Failing startup lets the supervisor retry. The
rowQuerier interface makes the failure path unit-testable, and the injected-probe-failure test
is a proper regression guard. fmt.Sprintf into the PRAGMA is fine: PRAGMA takes no bound
parameters and the table name is a caller-supplied literal, which the comment states.

1. Scope creep worth flagging. Close() also loses its PRAGMA wal_checkpoint(TRUNCATE).
   The reasoning is sound (the handle is mode=ro, so the checkpoint can only ever fail) but it
   is an unrelated change bundled into a schema-detection PR, and it removes the
   [db] WAL checkpoint complete line from shutdown logs.
2. Behaviour on a persistent probe failure changes from "run wrong" to "crash loop under
   restart: unless-stopped". That is the correct trade and exactly what bug(db): detectSchema() silently swallows PRAGMA failure — server runs v2 SQL against a v3 DB until restart (empty Packets, /channels/*/messages 500) #1901 asks for, but
   say it out loud in the PR.

Merge-order finding: #1909 and #1878 conflict, in two places. Both rewrite OpenDB right
after d := &DB{conn: conn, path: path} (one inserts prepareStatements(), the other replaces
d.detectSchema()), and both rewrite func (db *DB) Close() (one prepends statement closing,
the other deletes the checkpoint block). Both report MERGEABLE against master because neither
has merged yet. Whichever lands first forces a rebase on the other. Decide the order
deliberately; I would take #1909 first, since it is the correctness fix and #1878 is an
optimisation.

[efiten]

Recycling this again, and the reason is my mistake rather than anything about your PR.

Earlier today I approved the pending workflow run on this PR. That was the wrong order: approving an action_required run makes it execute against the merge commit from when the run was created, not against current master. This one was created weeks ago, so it tested a base that predates the #1923 fix and five merges that have landed since. The result it produced says nothing useful.

Closing and reopening now gets a fresh merge commit against current master, which is what the run should have been all along. No action needed from you, and apologies for the second round of noise.