h-dv/code-index

Watch

Fork 0

test/doc: repeated `plugin rollback` is a two-state toggle and nothing pins or documents it #130

New issue

Closed

opened 2026-09-04 20:54:48 +02:00 by buildagent · 1 comment

buildagent commented

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

Member

Split out of #78. Verified 2026-09-04. The behaviour is defensible; what is missing is a test and a sentence. Filed rather than fixed inside the residual sweep because it is coverage, not correctness.

What actually happens

crates/indexer/src/promotion.rs:552-565:

pub fn rollback(conn: &mut Connection) -> Result<Option<Promoted>, IndexerError> {
    let target: Option<i64> = { … conn.prepare_cached(
        "SELECT id FROM plugin_generations WHERE state = ?1 ORDER BY id DESC LIMIT 1")?
        .query_row([generations::SUPERSEDED], |r| r.get(0)).optional()? };
    let Some(target) = target else { return Ok(None) };
    promote(conn, target).map(Some)
}

promote is documented as "ROLLBACK as well as promotion, because they are THE SAME TRANSACTION WITH THE GENERATIONS SWAPPED", and it moves the incumbent to superseded. SUPERSEDED_RETENTION_LIMIT = 1, enforced inside the same transaction (everything in superseded outside ORDER BY id DESC LIMIT 1 is moved to deleting).

So: promote 2 → {1 superseded, 2 active}; rollback → {1 active, 2 superseded}; second rollback → back to {1 superseded, 2 active}. It toggles between exactly two generations and can never reach a third.

Why the audit's word "silently" is wrong

cmd_rollback (crates/cli/src/plugin.rs:2624-2704) reads the target before acting and prints package generation {t}, from its own retained rows plus a domain naming the generation being replaced. plugin status prints the whole generation inventory with states and rollback: available — n superseded generation(s) retained. And with retention 1 there is nothing older to reach — the older generation is already in deleting — so "roll back further" is not an available semantic. A second rollback is "the same epoch transition in reverse" applied to the new state, which is literally what #78 calls rollback; acceptance 8 says only "rollback works while retention permits".

What is missing

1. No test drives a repeated rollback. Every rollback test drives exactly one call:

crates/cli/tests/plugin_activation_cli.rs:704 rollback_restores_the_retained_generation
:781 rollback_with_nothing_retained_refuses_and_exits_non_zero
:1450, :1833 capability-grant carriage across a rollback
crates/indexer/tests/generation_promotion.rs:788 a_rollback_restores_the_old_projection_with_the_producer_gone
crates/indexer/tests/generation_collect.rs:905, generation_crash_matrix.rs:462 (crash during one rollback), bench_promotion_lock.rs:211
crates/cli/tests/plugin_duplicate_id_cli.rs:459 (disclosure only)

So a future change to the target-selection query — ORDER BY id DESC → ASC, or a retention bump that makes a third generation reachable — is ungraded.

2. No prose anywhere says rollback is a two-state toggle rather than a stack pop. An operator reading rollback: available — 1 superseded generation(s) retained twice has no way to learn that the second call returns them to where they started.

Split out of #78. Verified 2026-09-04. **The behaviour is defensible; what is missing is a test and a sentence.** Filed rather than fixed inside the residual sweep because it is coverage, not correctness. ## What actually happens `crates/indexer/src/promotion.rs:552-565`: ```rust pub fn rollback(conn: &mut Connection) -> Result<Option<Promoted>, IndexerError> { let target: Option<i64> = { … conn.prepare_cached( "SELECT id FROM plugin_generations WHERE state = ?1 ORDER BY id DESC LIMIT 1")? .query_row([generations::SUPERSEDED], |r| r.get(0)).optional()? }; let Some(target) = target else { return Ok(None) }; promote(conn, target).map(Some) } ``` `promote` is documented as "ROLLBACK as well as promotion, because they are THE SAME TRANSACTION WITH THE GENERATIONS SWAPPED", and it moves the incumbent to `superseded`. `SUPERSEDED_RETENTION_LIMIT = 1`, enforced inside the same transaction (everything in `superseded` outside `ORDER BY id DESC LIMIT 1` is moved to `deleting`). So: promote 2 → `{1 superseded, 2 active}`; rollback → `{1 active, 2 superseded}`; **second rollback → back to `{1 superseded, 2 active}`**. It toggles between exactly two generations and can never reach a third. ## Why the audit's word "silently" is wrong `cmd_rollback` (`crates/cli/src/plugin.rs:2624-2704`) reads the target *before* acting and prints `package generation {t}, from its own retained rows` plus a domain naming the generation being replaced. `plugin status` prints the whole generation inventory with states and `rollback: available — n superseded generation(s) retained`. And with retention 1 there is nothing older to reach — the older generation is already in `deleting` — so "roll back further" is not an available semantic. A second rollback *is* "the same epoch transition in reverse" applied to the new state, which is literally what #78 calls rollback; acceptance 8 says only "rollback works while retention permits". ## What is missing **1. No test drives a repeated rollback.** Every rollback test drives exactly one call: * `crates/cli/tests/plugin_activation_cli.rs:704` `rollback_restores_the_retained_generation` * `:781` `rollback_with_nothing_retained_refuses_and_exits_non_zero` * `:1450`, `:1833` capability-grant carriage across a rollback * `crates/indexer/tests/generation_promotion.rs:788` `a_rollback_restores_the_old_projection_with_the_producer_gone` * `crates/indexer/tests/generation_collect.rs:905`, `generation_crash_matrix.rs:462` (crash *during* one rollback), `bench_promotion_lock.rs:211` * `crates/cli/tests/plugin_duplicate_id_cli.rs:459` (disclosure only) So a future change to the target-selection query — `ORDER BY id DESC` → `ASC`, or a retention bump that makes a third generation reachable — is ungraded. **2. No prose anywhere says rollback is a two-state toggle rather than a stack pop.** An operator reading `rollback: available — 1 superseded generation(s) retained` twice has no way to learn that the second call returns them to where they started.

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 commented

2026-09-06 09:34:00 +02:00

Author

Member

Triage 2026-09-06 at `f6a878a`: CLOSING. Both halves — a test that drives more than one rollback, and operator-facing prose — are in. The interesting part is that this issue's own proposed mutation was run and survived, and that is recorded rather than papered over.

Verified against master and by re-running the test myself.

The test

a_second_rollback_swaps_back_rather_than_stepping_further_out — crates/indexer/tests/generation_promotion.rs:877. It drives three rollbacks, compares the projection at every step, and asserts periodicity, which is the property "two-state toggle" actually names.

$ export CARGO_INCREMENTAL=0
$ cargo test -p code-index-indexer --test generation_promotion -- --exact \
    a_second_rollback_swaps_back_rather_than_stepping_further_out
test a_second_rollback_swaps_back_rather_than_stepping_further_out ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 16 filtered out; finished in 1.29s
EXIT=0

The prose, and it is in the payload rather than only in doc comments

crates/cli/src/activation.rs:1135-1139 — plugin status tells the operator outright that "it SWAPS rather than pops, so running it twice returns you here".
crates/cli/src/plugin.rs:2732-2737 — the plugin rollback notes.
crates/indexer/src/promotion.rs:553 — the API doc.

That ordering matters for this repo's own rule that disclosures belong in the payload, not in prose an operator never reads. The first of those three is the one that will actually reach someone mid-incident.

The filing was wrong about the mutation, and the tree says so

This issue proposed grading the behaviour by flipping ORDER BY id DESC → ASC. That mutation was run and SURVIVED, and it is recorded as a survivor with its reason at generation_promotion.rs:851-864: under SUPERSEDED_RETENTION_LIMIT = 1 the two queries are identical, so the mutation is structurally ungradable — not a weak test, an impossible one.

The killing mutation used instead (rollback refuses a second call) is recorded as RED and isolating. Recording a survivor with its reason, rather than inventing a test that looks like coverage, is the right outcome and is why I am comfortable closing this rather than asking for the originally-specified mutation.

Residual

None. The only thing carried forward is the corrected understanding of what is gradable here, which now lives in the test file.

🤖 Triage lane, 2026-09-06, master f6a878a

## Triage 2026-09-06 at `f6a878a`: CLOSING. Both halves — a test that drives more than one rollback, and operator-facing prose — are in. The interesting part is that this issue's own proposed mutation was **run and survived**, and that is recorded rather than papered over. Verified against master and by re-running the test myself. ### The test `a_second_rollback_swaps_back_rather_than_stepping_further_out` — `crates/indexer/tests/generation_promotion.rs:877`. It drives **three** rollbacks, compares the projection at every step, and asserts periodicity, which is the property "two-state toggle" actually names. ``` $ export CARGO_INCREMENTAL=0 $ cargo test -p code-index-indexer --test generation_promotion -- --exact \ a_second_rollback_swaps_back_rather_than_stepping_further_out test a_second_rollback_swaps_back_rather_than_stepping_further_out ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 16 filtered out; finished in 1.29s EXIT=0 ``` ### The prose, and it is in the payload rather than only in doc comments - `crates/cli/src/activation.rs:1135-1139` — `plugin status` tells the operator outright that **"it SWAPS rather than pops, so running it twice returns you here"**. - `crates/cli/src/plugin.rs:2732-2737` — the `plugin rollback` notes. - `crates/indexer/src/promotion.rs:553` — the API doc. That ordering matters for this repo's own rule that disclosures belong in the payload, not in prose an operator never reads. The first of those three is the one that will actually reach someone mid-incident. ### The filing was wrong about the mutation, and the tree says so This issue proposed grading the behaviour by flipping `ORDER BY id DESC` → `ASC`. That mutation was **run and SURVIVED**, and it is recorded as a survivor with its reason at `generation_promotion.rs:851-864`: under `SUPERSEDED_RETENTION_LIMIT = 1` the two queries are **identical**, so the mutation is structurally ungradable — not a weak test, an impossible one. The killing mutation used instead (rollback refuses a second call) is recorded as RED and isolating. Recording a survivor with its reason, rather than inventing a test that looks like coverage, is the right outcome and is why I am comfortable closing this rather than asking for the originally-specified mutation. ### Residual None. The only thing carried forward is the corrected understanding of what is gradable here, which now lives in the test file. 🤖 Triage lane, 2026-09-06, master `f6a878a`

buildagent closed this issue

2026-09-06 09:34:05 +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#130

No description provided.

Rows
Columns

test/doc: repeated plugin rollback is a two-state toggle and nothing pins or documents it #130

What actually happens

Why the audit's word "silently" is wrong

What is missing

Triage 2026-09-06 at f6a878a: CLOSING. Both halves — a test that drives more than one rollback, and operator-facing prose — are in. The interesting part is that this issue's own proposed mutation was run and survived, and that is recorded rather than papered over.

The test

The prose, and it is in the payload rather than only in doc comments

The filing was wrong about the mutation, and the tree says so

Residual

test/doc: repeated `plugin rollback` is a two-state toggle and nothing pins or documents it #130

Triage 2026-09-06 at `f6a878a`: CLOSING. Both halves — a test that drives more than one rollback, and operator-facing prose — are in. The interesting part is that this issue's own proposed mutation was run and survived, and that is recorded rather than papered over.