h-dv/code-index

Watch

Fork 0

A package worker's refusal is silently promoted into the active generation (#78 criterion 7) #115

New issue

Closed

opened 2026-09-04 16:12:30 +02:00 by buildagent · 1 comment

buildagent commented

2026-09-04 16:12:30 +02:00

Member

Summary

A package that traps, hangs or refuses on a subset of a project's files still produces a generation that replaces the last good one. The refused files' facts — symbols, refs, imports — are dropped on all three channels, and nothing anywhere says so. There is no roll-up from "this dispatch was refused" to "this generation failed", and no per-file disclosure that a promoted generation is partial.

This violates #78 criterion 7 (generation-safe activation, promotion and rollback).

The chain, verified link by link on the working tree

crates/indexer/src/index.rs:8279-8286 — extract_for's Parse::Err arm maps a refusal to an empty extraction_diagnostics, with a comment claiming "The refusal itself rides in files.parse_error".
The claim is false on the generation-build path. writer::write_pending_contribution (crates/indexer/src/writer.rs:445-529) writes zero parse_error — only write_upsert does (writer.rs:268-291), and write_upsert is the ACTIVE path a generation build must never take. write_pending_contribution writes the file_contributions row unconditionally via insert_contribution_measured, then gates symbols/refs/imports behind if let ParseResult::Ok(extract). A refused file therefore leaves a contribution row that is indistinguishable from a legitimately fact-free one.

It cannot simply write files.parse_error either: files is single-valued and query-visible, and touching it is the "no query-visible state changes before step 11" constraint the function exists to honour.
build::extract_one (build.rs:1199-1250) returns Response::Accepted { outcome, hash } regardless of ParseResult::Error — run_task hands back a WriterMsg::Upsert carrying the error and the acceptance check is only the stat-identity high-water protocol. dispatch_round (build.rs:1004, called at :684 and :794) then settles the queue row STATE_DONE.
The build gates count contributions, not errors. Gate 5 (build.rs:1444) asserts every reparse file that settled done produced exactly one contribution — and a refusal wrote one at step 2, so the count looks healthy. Gate 6's projection check is contributions-based too.
crates/cli/src/activation.rs:740 (and the resume path at :905) promotes Outcome::Ready unconditionally.

Net effect: promotion supersedes the active contribution for that file (the partition excludes re-extracted files from the carry) and installs an empty one in its place. The facts are gone, Ready's counts look healthy, and the operator is told generation N ACTIVE.

The tree already expects the fix

crates/mcp-server/tests/reason_code_registry.rs:819-836 registers worker_timeout and worker_crash as no_producer in these words:

"The dispatch verdict ships as Reason::HostDeadlineExceeded; what does not exist is the roll-up that turns one refused dispatch into a failed generation — dispatch_round records the attempt and redispatches."
"the same roll-up for host.worker_trapped / host.worker_unavailable / abi.frame_truncated. Those three are produced; nothing maps them onto a generation."

Scope beyond packages

ParseResult::Error is also produced by the builtin arm (parse_with_plugin: set_language failure, parser.parse returning None, and catch_unwind catching a plugin panic). The defect and its fix are generic over both producers — a refusal is a refusal whoever refused.

What a fix has to do

Make a refused dispatch durably visible to the build gates — durably, because the Recovery::Revalidate path re-runs the gates over an already-settled queue with no extraction at all, so an in-memory count would read zero.
Decide the promotion policy explicitly, between "any refusal fails the generation" (a single hostile file becomes a denial of service on every future activation) and "promote and disclose" (silent loss).
Keep three states apart: absent = did not report, empty = measured and nothing was refused, non-empty = the finding.
Disclose a promoted partial generation per file, in the payload.

Found by two independent audits from opposite ends of the chain, converging on the same defect; verified end to end.

Refs #78 (criterion 7).

## Summary A package that traps, hangs or refuses on a **subset** of a project's files still produces a generation that **replaces** the last good one. The refused files' facts — symbols, refs, imports — are dropped on all three channels, and **nothing anywhere says so**. There is no roll-up from "this dispatch was refused" to "this generation failed", and no per-file disclosure that a promoted generation is partial. This violates #78 criterion 7 (generation-safe activation, promotion and rollback). ## The chain, verified link by link on the working tree 1. **`crates/indexer/src/index.rs:8279-8286`** — `extract_for`'s `Parse::Err` arm maps a refusal to an **empty** `extraction_diagnostics`, with a comment claiming *"The refusal itself rides in `files.parse_error`"*. 2. **The claim is false on the generation-build path.** `writer::write_pending_contribution` (`crates/indexer/src/writer.rs:445-529`) writes **zero** `parse_error` — only `write_upsert` does (`writer.rs:268-291`), and `write_upsert` is the ACTIVE path a generation build must never take. `write_pending_contribution` writes the `file_contributions` row **unconditionally** via `insert_contribution_measured`, then gates symbols/refs/imports behind `if let ParseResult::Ok(extract)`. A refused file therefore leaves a contribution row that is indistinguishable from a legitimately fact-free one. It cannot simply write `files.parse_error` either: `files` is single-valued and query-visible, and touching it is the "no query-visible state changes before step 11" constraint the function exists to honour. 3. **`build::extract_one` (`build.rs:1199-1250`)** returns `Response::Accepted { outcome, hash }` **regardless of `ParseResult::Error`** — `run_task` hands back a `WriterMsg::Upsert` carrying the error and the acceptance check is only the stat-identity high-water protocol. `dispatch_round` (`build.rs:1004`, called at `:684` and `:794`) then settles the queue row `STATE_DONE`. 4. **The build gates count contributions, not errors.** Gate 5 (`build.rs:1444`) asserts every `reparse` file that settled `done` produced *exactly one contribution* — and a refusal wrote one at step 2, so the count looks healthy. Gate 6's projection check is contributions-based too. 5. **`crates/cli/src/activation.rs:740`** (and the resume path at `:905`) promotes `Outcome::Ready` unconditionally. Net effect: promotion supersedes the active contribution for that file (the partition excludes re-extracted files from the carry) and installs an empty one in its place. The facts are gone, `Ready`'s counts look healthy, and the operator is told `generation N ACTIVE`. ## The tree already expects the fix `crates/mcp-server/tests/reason_code_registry.rs:819-836` registers `worker_timeout` and `worker_crash` as `no_producer` in these words: > *"The dispatch verdict ships as `Reason::HostDeadlineExceeded`; what does not exist is **the roll-up that turns one refused dispatch into a failed generation** — `dispatch_round` records the attempt and redispatches."* > *"the same roll-up for `host.worker_trapped` / `host.worker_unavailable` / `abi.frame_truncated`. Those three are produced; nothing maps them onto a generation."* ## Scope beyond packages `ParseResult::Error` is also produced by the **builtin** arm (`parse_with_plugin`: `set_language` failure, `parser.parse` returning `None`, and `catch_unwind` catching a plugin panic). The defect and its fix are generic over both producers — a refusal is a refusal whoever refused. ## What a fix has to do 1. Make a refused dispatch **durably** visible to the build gates — durably, because the `Recovery::Revalidate` path re-runs the gates over an already-settled queue with no extraction at all, so an in-memory count would read zero. 2. Decide the promotion policy explicitly, between "any refusal fails the generation" (a single hostile file becomes a denial of service on every future activation) and "promote and disclose" (silent loss). 3. Keep three states apart: absent = did not report, empty = measured and nothing was refused, non-empty = the finding. 4. Disclose a promoted partial generation **per file, in the payload**. Found by two independent audits from opposite ends of the chain, converging on the same defect; verified end to end. Refs #78 (criterion 7).

buildagent referenced this issue

2026-09-04 17:20:37 +02:00

Response::Unclaimed is the same silent-loss door as #115, still open: three causes collapse into one count that reads as the designed case #123

buildagent commented

2026-09-04 17:21:07 +02:00

Author

Member

Fixed. The gate asks about the loss, not the refusal — which is why it fails neither way.

Policy: regression-gated, not a threshold

The discriminator is structural: does promoting this generation delete facts the active one is serving for that file?

No — the active generation holds no symbol/ref/import for it (new file, newly claimed, fact-free, or already refused): promote, and disclose per file.
Yes — gate_failed, naming the paths. The last good generation keeps answering, which is #78's criterion 4.

Both alternatives were considered and rejected with reasons, written into build.rs:

A threshold is arbitrary, needs tuning, and at any N>0 licenses exactly the silent loss this issue was filed about.
"Any refusal fails" would let one hostile file that never parsed anyway make every future activation unreachable, while costing the index nothing.

The predicate avoids both because it asks about the consequence rather than the event.

One detail that matters: the denominator is facts that exist, not the active row's own refusal column. Reading that column would treat a pre-migration NULL as "did not refuse", collapsing two states the schema is being changed to separate.

The chain — all five links held, with three corrections

Verified with search_symbols / find_callers / read_code, not text grep. None of the corrections changes the substance:

Link 2 was half true, and interestingly so. The comment at index.rs:8279 claims the refusal "rides in files.parse_error". write_upsert really does write it (:269/275/287) — so the claim is correct for the ordinary path and false for the build path sitting next to it. And it cannot be fixed there: files is single-valued and query-visible, and not touching it is the entire reason write_pending_contribution exists.
Link 3: Response::Accepted is returned by extract_one at build.rs:1249, not by dispatch_round. dispatch_round consumes it (find_callers confirms exactly the two call sites).
Extra link, and it is the load-bearing one: build.rs:1386 in finish is the only production transition to ready. promotion::resume and rollback only promote already-ready/superseded generations. So this is a single chokepoint — there is no second door.

What was built

m0059 adds file_contributions.refusal TEXT (m0058 was taken by another lane; CURRENT_VERSION 58→59). Three states: NULL = not measured, '' = measured and did not refuse, otherwise the producer's own words.

No backfill, deliberately: unlike m0057 there is no provable population, and deriving one from files.parse_error is a cross-table correlation that would keep answering confidently after the shape moved.

It is durable rather than in-memory because Recovery::Revalidate re-runs the gates with no extraction at all — an in-memory count reads zero there, and the gate would pass on a resumed build.

ProducerVerdict { diagnostics, refusal } carries it (a struct, not an eighth argument — that trips clippy::too_many_arguments). Gate 5b in build.rs; Ready gains refusals (complete) plus refused_files (8-row evidence).

Mutations — all RUN, all RED, against the shipped code

mutation	result
delete the roll-up (this defect, reintroduced)	RED, isolated — `must FAIL, not promote: Ready { … refusals: 1, refused_files: [RefusedFile { path: "loses.rs", refusal: "plugin panicked during extract" }] }` — the defect printing itself
`refusals: 0, refused_files: vec![]` unconditionally	RED, isolated
gate spelled `if refusals > 0` (the rejected policy)	RED — `expected a ready generation, got Failed { reason: "gate_failed" }`
`refusals = len + 1`	RED — `refusals: 1, refused_files: []`: a count contradicted by its own evidence
drop `AND refusal <> ''`	RED, 5 tests
m0059 backfilled from `files.parse_error`	RED — `left: Some("plugin panicked…") right: None`
m0059 as `TEXT NOT NULL DEFAULT ''`	RED — `left: Some("") right: None`

A mutation found a real defect in the fix itself. The count and the evidence list were two statements over "the same" predicate; dropping <> '' made them disagree — refusals: 7, refused_files: [], which reads as truncated. They are now one query with the discriminator on the same row, and refusals is that vector's length.

Two honest records rather than re-aimed tests: the rejected-policy mutation survived at the positive control, and the positive control (if true) is recorded as not isolated — a control asserting "ordinary builds still promote" necessarily shares that assertion with every test that wants a Ready.

No test seam: a LanguagePlugin that panics produces a genuine ParseResult::Error through parse_with_plugin's real catch_unwind, and the tests drive build::build → gate → promotion::promote through activate's own match arm.

What an operator sees

Promoted-but-partial:

rows       contributions=812 … imports=1902 refusals=1
refused    1 file(s) — this generation is PARTIAL. … Nothing was lost by promoting: the
           active generation held no facts for any of them either …
refused_file  vendor/weird.rb  —  package refused: host.worker_trapped (extract)

refusals= is unconditional, so 0 is a printed measurement, not an absence.

Fact-losing: activation FAILED (gate_failed), with generation_failures.detail = "1 of 1 refused file(s) would have LOST facts the active generation is serving: app/models/user.rb".

Corrected in passing

The worker_timeout / worker_crash prose in reason_code_registry.rs was stale: the roll-up now exists, but those codes stay unwritten because extract_for flattens the structured Reason into a string — so the gate cannot honestly claim "timeout" over "crash". Said plainly rather than left implying coverage.

Split out rather than absorbed

#123 — Response::Unclaimed is the same silent-loss door: three causes (routes-to-nobody, vanished file, non-UTF-8/read failure) collapse into one count that reads as the designed case. Code-verified, not driven end to end, so it was not widened into this change.

Also recorded and not fixed here: files.parse_error is not restamped by promotion (m0054 names that omission deliberately), so it can disagree with file_contributions.refusal until the next ordinary pass; and plugin_activation has no MCP surface for refusals.

Gates: fmt, clippy -D warnings, rustdoc, cargo test --workspace 2891/0 twice, daemon leg 571/0, precision_gate 7/7, baseline.json md5 unmoved.

## Fixed. The gate asks about the **loss**, not the refusal — which is why it fails neither way. ### Policy: regression-gated, not a threshold The discriminator is structural: **does promoting this generation delete facts the active one is serving for that file?** - **No** — the active generation holds no symbol/ref/import for it (new file, newly claimed, fact-free, or already refused): **promote**, and disclose per file. - **Yes** — `gate_failed`, naming the paths. The last good generation keeps answering, which is #78's criterion 4. Both alternatives were considered and rejected with reasons, written into `build.rs`: - **A threshold** is arbitrary, needs tuning, and at any N>0 licenses exactly the silent loss this issue was filed about. - **"Any refusal fails"** would let one hostile file that never parsed anyway make every future activation unreachable, while costing the index nothing. The predicate avoids both because it asks about the consequence rather than the event. One detail that matters: the denominator is **facts that exist**, not the active row's own `refusal` column. Reading that column would treat a pre-migration `NULL` as "did not refuse", collapsing two states the schema is being changed to separate. ### The chain — all five links held, with three corrections Verified with `search_symbols` / `find_callers` / `read_code`, not text grep. None of the corrections changes the substance: - **Link 2 was half true, and interestingly so.** The comment at `index.rs:8279` claims the refusal "rides in `files.parse_error`". `write_upsert` really does write it (`:269/275/287`) — so the claim is correct for the *ordinary* path and false for the *build* path sitting next to it. And it cannot be fixed there: `files` is single-valued and query-visible, and not touching it is the entire reason `write_pending_contribution` exists. - **Link 3**: `Response::Accepted` is returned by **`extract_one` at `build.rs:1249`**, not by `dispatch_round`. `dispatch_round` consumes it (`find_callers` confirms exactly the two call sites). - **Extra link, and it is the load-bearing one**: `build.rs:1386` in `finish` is the **only** production transition to `ready`. `promotion::resume` and `rollback` only promote already-`ready`/`superseded` generations. So this is a single chokepoint — there is no second door. ### What was built **m0059** adds `file_contributions.refusal TEXT` (m0058 was taken by another lane; `CURRENT_VERSION` 58→59). Three states: `NULL` = not measured, `''` = measured and did not refuse, otherwise the producer's own words. **No backfill**, deliberately: unlike m0057 there is no provable population, and deriving one from `files.parse_error` is a cross-table correlation that would keep answering confidently after the shape moved. It is **durable rather than in-memory because `Recovery::Revalidate` re-runs the gates with no extraction at all** — an in-memory count reads zero there, and the gate would pass on a resumed build. `ProducerVerdict { diagnostics, refusal }` carries it (a struct, not an eighth argument — that trips `clippy::too_many_arguments`). Gate 5b in `build.rs`; `Ready` gains `refusals` (complete) plus `refused_files` (8-row evidence). ### Mutations — all RUN, all RED, against the shipped code | mutation | result | |---|---| | delete the roll-up (this defect, reintroduced) | **RED, isolated** — `must FAIL, not promote: Ready { … refusals: 1, refused_files: [RefusedFile { path: "loses.rs", refusal: "plugin panicked during extract" }] }` — the defect printing itself | | `refusals: 0, refused_files: vec![]` unconditionally | **RED, isolated** | | gate spelled `if refusals > 0` (the rejected policy) | **RED** — `expected a ready generation, got Failed { reason: "gate_failed" }` | | `refusals = len + 1` | **RED** — `refusals: 1, refused_files: []`: a count contradicted by its own evidence | | drop `AND refusal <> ''` | **RED, 5 tests** | | m0059 backfilled from `files.parse_error` | **RED** — `left: Some("plugin panicked…") right: None` | | m0059 as `TEXT NOT NULL DEFAULT ''` | **RED** — `left: Some("") right: None` | **A mutation found a real defect in the fix itself.** The count and the evidence list were two statements over "the same" predicate; dropping `<> ''` made them disagree — `refusals: 7, refused_files: []`, which reads as *truncated*. They are now one query with the discriminator on the same row, and `refusals` is that vector's length. **Two honest records rather than re-aimed tests:** the rejected-policy mutation **survived** at the positive control, and the positive control (`if true`) is recorded as **not isolated** — a control asserting "ordinary builds still promote" necessarily shares that assertion with every test that wants a `Ready`. No test seam: a `LanguagePlugin` that panics produces a genuine `ParseResult::Error` through `parse_with_plugin`'s real `catch_unwind`, and the tests drive `build::build` → `gate` → `promotion::promote` through `activate`'s own match arm. ### What an operator sees Promoted-but-partial: ``` rows contributions=812 … imports=1902 refusals=1 refused 1 file(s) — this generation is PARTIAL. … Nothing was lost by promoting: the active generation held no facts for any of them either … refused_file vendor/weird.rb — package refused: host.worker_trapped (extract) ``` `refusals=` is unconditional, so **`0` is a printed measurement**, not an absence. Fact-losing: `activation FAILED (gate_failed)`, with `generation_failures.detail` = *"1 of 1 refused file(s) would have LOST facts the active generation is serving: app/models/user.rb"*. ### Corrected in passing The `worker_timeout` / `worker_crash` prose in `reason_code_registry.rs` was stale: the roll-up now exists, but those codes stay unwritten because `extract_for` flattens the structured `Reason` into a string — so the gate cannot honestly claim "timeout" over "crash". Said plainly rather than left implying coverage. ### Split out rather than absorbed **#123** — `Response::Unclaimed` is the same silent-loss door: three causes (routes-to-nobody, vanished file, non-UTF-8/read failure) collapse into one count that reads as the designed case. Code-verified, not driven end to end, so it was not widened into this change. Also recorded and not fixed here: `files.parse_error` is **not restamped by promotion** (m0054 names that omission deliberately), so it can disagree with `file_contributions.refusal` until the next ordinary pass; and `plugin_activation` has **no MCP surface** for refusals. Gates: fmt, clippy `-D warnings`, rustdoc, `cargo test --workspace` **2891/0 twice**, daemon leg **571/0**, `precision_gate` 7/7, `baseline.json` md5 unmoved.

buildagent closed this issue

2026-09-04 17:21:10 +02:00

buildagent referenced this issue

2026-09-04 18:33:55 +02:00

A refused package is invisible on the MCP surface: archive_refused reaches no coverage_reasons code, so an agent's answer is qualified by nothing #124