Skip to content

fix(readiness): surface unrepaired state.db corruption on the state_db probe (OOF-106) - #87052

Open
shannonsands wants to merge 2 commits into
NousResearch:mainfrom
shannonsands:fix/oof-106-readiness-corruption-signal
Open

shannonsands wants to merge 2 commits into
NousResearch:mainfrom
shannonsands:fix/oof-106-readiness-corruption-signal

Conversation

@shannonsands

Copy link
Copy Markdown
Contributor

Problem

The state_db readiness probe only reads sqlite_master (page 1). Page-level corruption in the sessions table b-tree sails past it, so /api/status stays green while session persistence is broken. In the OOF-106 incident (ashriel-fox-cloud-5148), a page-corrupt state.db reported "ok" for 10+ days while repair attempts churned in the background and accumulated 505 malformed backups.

#86747 fixed the repair/backup side (bounded attempt ledger + backup dedup/retention) — this PR closes the remaining gap: the readiness surface.

Changes (both bounded, read-only)

  1. Consult the persistent repair-attempt ledger (state.db.repair-attempts.json from state.db corruption cascade: repair path loops forever, accumulating 89GB of dead backups #86747): when its fingerprint (size:mtime_ns) still matches the current file bytes, automatic repair has already failed on this exact database and nothing has changed since. The probe reports degraded / "unrepaired corruption" without opening the DB. Import-light — reads the sidecar JSON directly, no hermes_state import. Malformed/stale/absent ledgers read as "no signal" (never degrade a healthy DB, never crash the probe).

  2. Deepen the read probe one step past page 1: fetch a single row from sessions (SELECT * deliberately, so the table b-tree is walked rather than an index). Still O(1) pages, still read-only under PRAGMA query_only, catches root-page damage of the one table every session write depends on. Guarded for pre-schema databases.

Tests

  • Ledger matching current bytes → degraded / unrepaired corruption
  • Stale ledger (fingerprint mismatch — file repaired/replaced) → ok
  • Garbage ledger file → ok, no crash
  • Real corruption fixture: zeroed sessions root page (schema page intact — passes the old probe, fails the new one) → degraded

tests/gateway/test_readiness.py: 5 passed + 1 pre-existing failure (test_collect_runtime_readiness_reports_healthy_local_runtime) confirmed failing identically on clean main. ruff clean.

Relationship to #82940

Supersedes the readiness portion of #82940, rebuilt against #86747's ledger format. The rest of #82940 (backup dedup, retention cap, persistent repair guard) was independently landed by #86747, so #82940 is being closed.

Linear: OOF-106

…b probe (OOF-106)

The state_db readiness probe only read sqlite_master (page 1), so
page-level corruption in the sessions table b-tree kept /api/status
green — in the OOF-106 incident a page-corrupt state.db reported "ok"
for 10+ days while sessions silently failed to persist and repair
attempts churned in the background.

Two additions, both bounded and read-only:

* Consult the persistent repair-attempt ledger (NousResearch#86747's
  state.db.repair-attempts.json): when its fingerprint still matches
  the current file bytes, automatic repair has already failed on this
  exact database and the probe reports "degraded / unrepaired
  corruption" without opening the DB at all. Import-light (reads the
  sidecar JSON directly); malformed/stale ledgers read as "no signal".

* Deepen the read probe one step past page 1: fetch a single row from
  the sessions table (SELECT * so the table b-tree, not an index, is
  walked). Still O(1) pages, still read-only, catches root-page damage
  of the table every session write depends on.

Tests: ledger-match degrades, stale ledger ignored, garbage ledger
ignored, and a real zeroed-root-page corruption fixture that passes the
schema-only probe but fails the deepened one.

Complements NousResearch#86747 (bounded repair loop + backup dedup), which covers
the repair/backup side of OOF-106 but leaves the readiness surface
falsely green. Supersedes the readiness part of NousResearch#82940.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery area/sessions Session lifecycle, resume, persistence, history sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 15, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(readiness): surface unrepaired state.db corruption on the state_db probe (OOF-106)

Good fix closing a real false-green gap, with strong tests (ledger match / stale / malformed / zeroed root page). Points:

  1. Ledger format contract duplicated: _unrepaired_corruption_marker (gateway/readiness.py lines 17-47) reads the sidecar JSON with a hand-rolled schema (fingerprint, failed_attempts) that must match what hermes_state writes (state.db corruption cascade: repair path loops forever, accumulating 89GB of dead backups #86747). Import-light is deliberate and fine, but the key names are now a cross-module contract with no enforcement — if hermes_state renames a key, the probe silently stops detecting (false green again, the exact failure this fixes). Consider a shared constant for the ledger filename/key names, or a parity test that writes the ledger via hermes_state's real writer and asserts the probe reads it.
  2. SELECT * FROM sessions LIMIT 1 on every probe: SELECT * (line 86) pulls the full first row — if session rows carry large payloads, every readiness probe allocates/transfers that blob just to descend the table b-tree. A narrower projection that still forces table-page access (e.g. a small non-indexed column, or WHERE rowid IS NOT NULL) would give the same corruption coverage with less churn. The comment explains why id alone is insufficient (index-only satisfaction) — the reasoning is sound, just optimize the projection.
  3. Fingerprint granularity: size:mtime_ns matching (line 47) is cheap but on coarse-mtime filesystems a repair that rewrites same-size bytes within the same timestamp tick could leave the marker "matching" a now-healthy DB → permanent degraded until next write. Rare, and self-heals on any later write; worth a sentence in the docstring.
  4. Minor: stray blank lines added at the top of the test file (lines 93-95 region) — cosmetic.

…l writer

Review follow-up (NousResearch#87052): the probe hand-parses the sidecar ledger
that hermes_state writes — an unenforced cross-module contract. New
parity test drives hermes_state._record_repair_outcome() directly and
asserts the probe degrades on a recorded failure and recovers when the
writer clears the ledger, so any schema drift (filename, fingerprint
format, failed_attempts key) fails CI instead of silently re-opening
the false-green gap.

Also: docstring note on size:mtime_ns granularity (coarse-mtime
filesystems can hold a stale match until the next write — pessimistic,
never falsely green) and a stray blank line in the test file.
@shannonsands

Copy link
Copy Markdown
Contributor Author

Thanks — addressed in 2c2f0b8de:

1. Ledger contract (taken, highest value): added test_corruption_ledger_contract_parity_with_hermes_state — it drives hermes_state._record_repair_outcome() (the real writer) and asserts the probe degrades on a recorded failure and returns to ok when the writer clears the ledger on success. Any drift in the filename, fingerprint format, or failed_attempts key now fails CI instead of silently re-opening the false-green gap. I kept the probe itself import-light (no hermes_state import at runtime) — the test is where the coupling gets enforced.

2. SELECT * (declining, deliberate): the wide projection is the point — SQLite satisfies narrow projections from index b-trees without touching table pages (the docstring calls this out for id, but it applies to any indexed column). SELECT * is the only projection guaranteed to walk the table b-tree regardless of future index additions. Cost-wise it's one row, one page read, on a probe that runs at health-poll cadence; the row is fetched and discarded. A narrower-but-still-table-forcing alternative would need knowledge of which columns are unindexed — a contract that rots.

3. Fingerprint granularity (taken): docstring now notes the coarse-mtime false-match case and why it's acceptable — the failure mode is a briefly pessimistic signal until the next successful write, never a false green.

4. Blank lines (taken): fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants