h-dv/code-index

Watch

Fork 0

index: a SQLite failure during a generation build is recorded as `invalid_fact`, blaming the package for the disk #128

New issue

Closed

opened 2026-09-04 20:54:18 +02:00 by buildagent · 2 comments

buildagent commented

2026-09-04 20:54:18 +02:00

Member

Split out of #78. Verified first-hand 2026-09-04.

The defect

crates/indexer/src/build.rs:745-754, the Err arm of the dispatch loop:

Err(e) => {
    generations::fail(conn, generation, "invalid_fact", &e.to_string())?;
    return Ok(Outcome::Failed { generation, reason: "invalid_fact" });
}

It is not a map_err — it is a bare Err(e) => over dispatch_round(..) -> Result<Round, IndexerError>. IndexerError is Sqlite(#[from] rusqlite::Error) (crates/indexer/src/db.rs), and dispatch_round's body is ? on rusqlite calls throughout: conn.prepare(…)?, rows.collect::<rusqlite::Result<_>>()?, conn.transaction()?, write_pending_contribution(…)?, four .execute(params![…])?, tx.commit()?, owed_count(conn, generation)?.

So a SQLITE_FULL, SQLITE_BUSY or SQLITE_IOERR on any of them arrives at that line indistinguishable from a plugin contract violation, and is recorded under a stable reason code that tells the operator the package emitted bad facts when the truth is that the disk filled up.

#78 enumerates these as two separate classes under "Failure and rollback": "invalid fact response" and "SQLite busy/space/resource exhaustion".

`storage_exhausted` exists, is registered as having no producer, and its registered justification is contradicted by that code

crates/mcp-server/tests/reason_code_registry.rs:

("storage_exhausted", "no_producer",
 "a write that ran out of disk. `admission::admit` refuses on a RETAINED-ROW ceiling \
  before the first durable write, which is a different thing measured earlier; no \
  `SQLITE_FULL` path turns into a stable reason."),

A SQLITE_FULL path does turn into a stable reason today — the wrong one.

No test drives the arm

Every occurrence of the literal in the tree: build.rs:749, build.rs:752, generations.rs:266 (doc), generations.rs:308 (declaration), generations.rs:1476, plus prose in reason_code_registry.rs and two .md files. generations.rs:1476 is inside retained_diagnostics_are_bounded, which uses "invalid_fact" merely as a valid reason string to test clamp_detail against the DIAGNOSTIC_BYTES CHECK — it never reaches build.rs.

By contrast gate_failed and source_churn_deadline each have a real driving test (generation_promotion.rs:2644, generation_build.rs:812). #78's own test list names "disk-full/busy cancellation"; nothing implements it.

A second, smaller finding at the same site

generations::fail(...)? at line 749 is itself a database write. Under genuine disk exhaustion it fails too and the ? propagates — defeating the promise stated four lines above it (build.rs:733: "? here would leave a generation in building with no failure row, so every error is turned into a recorded failure"). Recovery is not permanent (next_action resumes from building), but the stated invariant is conditional on the disk.

Shape of the fix

Classify rather than collapse: match IndexerError::Sqlite(rusqlite::Error::SqliteFailure(e, _)) on DiskFull/IoErr → storage_exhausted, Busy/Locked → its own class, everything else → invalid_fact. Remove the no_producer row from reason_code_registry.rs. Add a fault-injection test that actually drives each arm — the reason code is what an operator repairs from, so a test that only asserts the string is in a table is not evidence.

Split out of #78. Verified first-hand 2026-09-04. ## The defect `crates/indexer/src/build.rs:745-754`, the `Err` arm of the dispatch loop: ```rust Err(e) => { generations::fail(conn, generation, "invalid_fact", &e.to_string())?; return Ok(Outcome::Failed { generation, reason: "invalid_fact" }); } ``` It is not a `map_err` — it is a bare `Err(e) =>` over `dispatch_round(..) -> Result<Round, IndexerError>`. `IndexerError` is `Sqlite(#[from] rusqlite::Error)` (`crates/indexer/src/db.rs`), and `dispatch_round`'s body is `?` on rusqlite calls throughout: `conn.prepare(…)?`, `rows.collect::<rusqlite::Result<_>>()?`, `conn.transaction()?`, `write_pending_contribution(…)?`, four `.execute(params![…])?`, `tx.commit()?`, `owed_count(conn, generation)?`. So a `SQLITE_FULL`, `SQLITE_BUSY` or `SQLITE_IOERR` on any of them arrives at that line indistinguishable from a plugin contract violation, and is recorded under a **stable reason code that tells the operator the package emitted bad facts** when the truth is that the disk filled up. #78 enumerates these as two separate classes under "Failure and rollback": *"invalid fact response"* and *"SQLite busy/space/resource exhaustion"*. ## `storage_exhausted` exists, is registered as having no producer, and its registered justification is contradicted by that code `crates/mcp-server/tests/reason_code_registry.rs`: ``` ("storage_exhausted", "no_producer", "a write that ran out of disk. `admission::admit` refuses on a RETAINED-ROW ceiling \ before the first durable write, which is a different thing measured earlier; no \ `SQLITE_FULL` path turns into a stable reason."), ``` A `SQLITE_FULL` path does turn into a stable reason today — the wrong one. ## No test drives the arm Every occurrence of the literal in the tree: `build.rs:749`, `build.rs:752`, `generations.rs:266` (doc), `generations.rs:308` (declaration), `generations.rs:1476`, plus prose in `reason_code_registry.rs` and two `.md` files. `generations.rs:1476` is inside `retained_diagnostics_are_bounded`, which uses `"invalid_fact"` merely as *a valid reason string* to test `clamp_detail` against the `DIAGNOSTIC_BYTES` CHECK — it never reaches `build.rs`. By contrast `gate_failed` and `source_churn_deadline` each have a real driving test (`generation_promotion.rs:2644`, `generation_build.rs:812`). #78's own test list names "disk-full/busy cancellation"; nothing implements it. ## A second, smaller finding at the same site `generations::fail(...)?` at line 749 is itself a database write. Under genuine disk exhaustion it fails too and the `?` propagates — defeating the promise stated four lines above it (`build.rs:733`: *"`?` here would leave a generation in `building` with no failure row, so every error is turned into a recorded failure"*). Recovery is not permanent (`next_action` resumes from `building`), but the stated invariant is conditional on the disk. ## Shape of the fix Classify rather than collapse: match `IndexerError::Sqlite(rusqlite::Error::SqliteFailure(e, _))` on `DiskFull`/`IoErr` → `storage_exhausted`, `Busy`/`Locked` → its own class, everything else → `invalid_fact`. Remove the `no_producer` row from `reason_code_registry.rs`. Add a fault-injection test that actually drives each arm — the reason code is what an operator repairs from, so a test that only asserts the string is in a table is not evidence.

buildagent referenced this issue

2026-09-04 20:55:54 +02:00

index: generation-safe plugin activation, invalidation, promotion and rollback #78

buildagent referenced this issue

2026-09-04 21:51:22 +02:00

index: generation-safe plugin activation, invalidation, promotion and rollback #78

dhoyer referenced this issue from a commit

2026-09-05 00:58:23 +02:00

fix: reproducible guests, honest storage failures, and cursors that cannot span a promotion

buildagent referenced this issue

2026-09-05 08:12:05 +02:00

index: generation-safe plugin activation, invalidation, promotion and rollback #78

buildagent referenced this issue

2026-09-06 09:33:43 +02:00

test: DAEMON_READERS is a hand-list with a summed floor, so an unregistered daemon reader is invisible to the generation-gate scan #129

buildagent commented

2026-09-06 09:47:14 +02:00

Author

Member

Triage 2026-09-06: LEFT OPEN by a narrow margin — the headline defect is fixed and graded, but the tree still says a decision is "left to #128", and closing would leave that pointer dangling.

What is fixed, and it is the substance of the issue

Classification instead of collapse: generations::failure_class — crates/indexer/src/generations.rs:378, with the structural partition doc from :327 and the helper store_refused_the_write at :400 covering DiskFull / SystemIoFailure / CantOpen / ReadOnly / NoMem / Busy / Locked, and excluding Constraint / Mismatch / Misuse.
The Err arm now calls it — crates/indexer/src/build.rs:828, let reason = generations::failure_class(&e);.
The no_producer row is removed — crates/mcp-server/tests/reason_code_registry.rs:1195-1203 (floor moved 19 → 18, with the reason) and :1796-1804 (the old storage_exhausted mutation recipe retired, recording that adding the producer turned the gate red). That last detail is the good kind: the registry noticed its own change.
The second finding is answered rather than fixed — build.rs:733-745 now states that the promise is conditional on the store, and that the resumable building state is the honest outcome. Correct: under genuine disk exhaustion generations::fail cannot write either, and pretending otherwise would be the overclaim.

And there is a real fault-injection test, which is what this issue asked for:

$ cargo test -p code-index-indexer --test generation_build -- --exact \
    a_store_that_refuses_the_write_fails_as_storage_and_not_as_a_bad_fact
test result: ok. 2 passed; 0 failed
EXIT=0

crates/indexer/tests/generation_build.rs:882 — it uses PRAGMA max_page_count so the error is SQLite's own, asserts the durable generation_failures row, and carries both a kill mutation and a positive control recorded as RUN.

Two deviations from the stated fix shape, both reasoned

Busy/Locked did not get "its own class" as the issue proposed; they fold into storage_exhausted. The reason is written at generations.rs:363-372: splitting contention into its own code is representable only with a migration, and SQLite's own message rides in the failure's detail (bounded by DIAGNOSTIC_BYTES), which is where "database or disk is full" and "database is locked" are told apart. Defensible.
IndexerError::Io has no arm because no path into dispatch_round produces one — an arm would be a claim with no site and no test behind it.

Why it stays open

generations.rs:355-363 explicitly leaves a decision to this issue:

The residual itself. Everything that is not a SQLite store verdict keeps invalid_fact. At the one site that uses this that residual is PluginContract (which IS an invalid fact) plus SQLite verdicts that are neither storage nor contention — a constraint violation from one of m0050's own triggers, say. Those are OUR defect and not the package's, and naming them honestly needs either a new code (a migration…) or a producer for UNCLASSIFIED… Both are a decision of their own and are left to #128.

That is the same defect this issue is named for — blaming the package for something that is not the package — narrowed from "the disk" to "our own trigger". It is much rarer and much less damaging, and it may well be the right call to accept it. But the tree currently points at this issue for that decision.

So: close it and re-point that sentence, or leave it open until the decision is taken. I have left it open because a close would silently orphan the pointer, and this issue's whole subject is a wrong attribution surviving because nobody looked. If the preference is to close and file a successor, say so — it is a one-line change to generations.rs:362.

🤖 Triage lane, 2026-09-06, master 45cf6e4

## Triage 2026-09-06: LEFT OPEN by a narrow margin — the headline defect is fixed and graded, but **the tree still says a decision is "left to #128"**, and closing would leave that pointer dangling. ### What is fixed, and it is the substance of the issue - **Classification instead of collapse**: `generations::failure_class` — `crates/indexer/src/generations.rs:378`, with the structural partition doc from `:327` and the helper `store_refused_the_write` at `:400` covering `DiskFull` / `SystemIoFailure` / `CantOpen` / `ReadOnly` / `NoMem` / `Busy` / `Locked`, and excluding `Constraint` / `Mismatch` / `Misuse`. - The `Err` arm now calls it — `crates/indexer/src/build.rs:828`, `let reason = generations::failure_class(&e);`. - **The `no_producer` row is removed** — `crates/mcp-server/tests/reason_code_registry.rs:1195-1203` (floor moved 19 → 18, with the reason) and `:1796-1804` (the old `storage_exhausted` mutation recipe retired, recording that adding the producer turned the gate red). That last detail is the good kind: the registry noticed its own change. - **The second finding is answered rather than fixed** — `build.rs:733-745` now states that the promise is conditional on the store, and that the resumable `building` state is the honest outcome. Correct: under genuine disk exhaustion `generations::fail` cannot write either, and pretending otherwise would be the overclaim. And there is a real fault-injection test, which is what this issue asked for: ``` $ cargo test -p code-index-indexer --test generation_build -- --exact \ a_store_that_refuses_the_write_fails_as_storage_and_not_as_a_bad_fact test result: ok. 2 passed; 0 failed EXIT=0 ``` `crates/indexer/tests/generation_build.rs:882` — it uses `PRAGMA max_page_count` so the error is **SQLite's own**, asserts the durable `generation_failures` row, and carries both a kill mutation and a positive control recorded as RUN. ### Two deviations from the stated fix shape, both reasoned 1. **`Busy`/`Locked` did not get "its own class"** as the issue proposed; they fold into `storage_exhausted`. The reason is written at `generations.rs:363-372`: splitting contention into its own code is representable only with a migration, and SQLite's own message rides in the failure's `detail` (bounded by `DIAGNOSTIC_BYTES`), which is where *"database or disk is full"* and *"database is locked"* are told apart. Defensible. 2. `IndexerError::Io` has no arm because no path into `dispatch_round` produces one — an arm would be a claim with no site and no test behind it. ### Why it stays open `generations.rs:355-363` explicitly leaves a decision to **this issue**: > **The residual itself.** Everything that is not a SQLite store verdict keeps `invalid_fact`. At the one site that uses this that residual is `PluginContract` (which IS an invalid fact) plus SQLite verdicts that are neither storage nor contention — a constraint violation from one of m0050's own triggers, say. **Those are OUR defect and not the package's**, and naming them honestly needs either a new code (a migration…) or a producer for `UNCLASSIFIED`… **Both are a decision of their own and are left to #128.** That is the same defect this issue is named for — *blaming the package for something that is not the package* — narrowed from "the disk" to "our own trigger". It is much rarer and much less damaging, and it may well be the right call to accept it. But the tree currently points at this issue for that decision. **So: close it and re-point that sentence, or leave it open until the decision is taken.** I have left it open because a close would silently orphan the pointer, and this issue's whole subject is a wrong attribution surviving because nobody looked. If the preference is to close and file a successor, say so — it is a one-line change to `generations.rs:362`. 🤖 Triage lane, 2026-09-06, master `45cf6e4`

buildagent referenced this issue

2026-09-06 09:54:11 +02:00

code-index://docs/reason-codes is at 3,979 of its 4,000-token cap, so the next reason code this project mints cannot be documented #184

dhoyer referenced this issue from a commit

2026-09-06 21:15:46 +02:00

fix: four facts the process held and the reader could not recover (#159, #128, #93, #137)

dhoyer referenced this issue from a commit

2026-09-06 21:15:46 +02:00

merge lane/identity-disclosure: one path-identity rule, and a residual that stops blaming the package (#159 #128 #93 #137)

buildagent commented

2026-09-06 23:33:36 +02:00

Author

Member

FIXED, merged as a80eb61. Verified at 1d81180: generations.rs no longer ends _ => "invalid_fact" — the line survives only in a comment recording what it used to be, and in the mutation that restores it.

failure_class had a residual arm that contradicted the structural partition written two paragraphs above it in the same file. PluginContract is the only variant a package can cause; everything else is ours. Three arms now, with UNCLASSIFIED taking the residual — it was already in FAILURE_REASONS and already accepted by m0050's trigger, so no migration was needed.

Its UNWRITTEN row is gone too. That row's justification said the vocabulary covered "every failure path this build CLASSIFIES", which was false in its first clause and therefore backwards in its second.

Mutation run: restore _ => "invalid_fact" → left: "invalid_fact" / right: "unclassified". Second mutation: restore the UNWRITTEN row verbatim → "registered UNWRITTEN (reserved) and 1 production source(s) name it".

Closing.

FIXED, merged as `a80eb61`. Verified at `1d81180`: `generations.rs` no longer ends `_ => "invalid_fact"` — the line survives only in a comment recording what it used to be, and in the mutation that restores it. `failure_class` had a residual arm that contradicted the structural partition written two paragraphs above it in the same file. `PluginContract` is the only variant a package can cause; everything else is ours. Three arms now, with **`UNCLASSIFIED`** taking the residual — it was already in `FAILURE_REASONS` and already accepted by m0050's trigger, so **no migration was needed**. Its `UNWRITTEN` row is gone too. That row's justification said the vocabulary covered "every failure path this build CLASSIFIES", which was false in its first clause and therefore backwards in its second. Mutation run: restore `_ => "invalid_fact"` → `left: "invalid_fact" / right: "unclassified"`. Second mutation: restore the `UNWRITTEN` row verbatim → "registered UNWRITTEN (reserved) and 1 production source(s) name it". Closing.

buildagent closed this issue

2026-09-06 23:33:40 +02:00

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

h-dv/code-index#128

No description provided.

Rows
Columns

index: a SQLite failure during a generation build is recorded as invalid_fact, blaming the package for the disk #128

The defect

storage_exhausted exists, is registered as having no producer, and its registered justification is contradicted by that code

No test drives the arm

A second, smaller finding at the same site

Shape of the fix

Triage 2026-09-06: LEFT OPEN by a narrow margin — the headline defect is fixed and graded, but the tree still says a decision is "left to #128", and closing would leave that pointer dangling.

What is fixed, and it is the substance of the issue

Two deviations from the stated fix shape, both reasoned

Why it stays open

index: a SQLite failure during a generation build is recorded as `invalid_fact`, blaming the package for the disk #128

`storage_exhausted` exists, is registered as having no producer, and its registered justification is contradicted by that code