index: generation-safe plugin activation, invalidation, promotion and rollback #78
Labels
No labels
code-review
correctness
dos
performance
security
severity/high
severity/low
severity/medium
tech-debt
Kind/Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Blocks
Depends on
#42 test: remaining corpus metamorphic gates — permutation, project split and plugin generations
h-dv/code-index
#73 perf: project_overview needs generation-scoped O(1) aggregates, not repeated refs scans
h-dv/code-index
Reference
h-dv/code-index#78
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Child of #75. Depends on #76, #77, #79, the refusal-state audit #72 and freshness repair #82.
Problem
Plugin package identity changes the meaning of unchanged source bytes. Content-only stat/hash reconciliation therefore cannot make plugin activation, upgrade or removal correct.
The previous proposal forced reparsing by extension and then recorded a rules hash. That does not cover a real plugin architecture:
Outcome
Plugin activation is a generation build followed by an atomic visibility switch. Queries see the complete old generation or the complete new generation, never a partially reparsed mixture. Upgrade, disable, rollback and removal are first-class state transitions with kill -9 recovery.
Identity
The activation input digest is a canonical length-prefixed hash over:
Package semantic version is not sufficient. Editing bytes without bumping a version still changes identity.
The digest is project-specific because the same installed package may receive different capabilities or coexist with different packages in another linked project.
State model
Use explicit durable states, not absent-key conventions:
Only one active generation exists per project plugin-set identity. A failed/pending generation never becomes visible through ordinary symbol/ref/graph queries.
State transitions are transactions with monotonic sequence numbers. Recovery derives the next action from durable state; it does not guess from file mtimes.
Storage strategy
Generation membership must be explicit on contributions and all derived resolver state.
Two acceptable implementation families:
Choose by measurement, but required properties are fixed:
A structural gate must enumerate every table/view that contains generation-derived data. Adding a new derived table without generation policy fails CI.
Source text FTS that is independent of extraction may remain shared, but plugin-generated semantic rows may not.
Claim-domain reconciliation
A package activation computes the affected domain from old and new ordered claims:
The dirty set is the union of:
This handles text to code, code to text, package A to B, and contribution removal. It is not restricted to kind='code'.
Path eligibility itself becomes project/plugin-set dependent. Global plugin_set/is_code_path statics cannot answer it. Coverage and freshness APIs must route through the project’s active/requested plugin state and distinguish:
Build pipeline
No query-visible state changes before step 11.
Source changes during build
A generation build may overlap watcher changes. Define a high-water protocol:
Do not hold the normal writer lock for plugin parsing. Promotion is short; heavy parse/resolve work is generation-scoped.
Resolution and ids
Pending generation symbol ids must not leak through normal tools or stable handles.
At promotion:
Rollback performs the same epoch transition in reverse.
Failure and rollback
Failures preserve the prior active generation:
Failed generation diagnostics remain inspectable but bounded. A retry creates a new attempt identity; it does not mutate a failed record into success.
Rollback requires no package execution while the superseded generation is retained. Removal activates a generation without that package, proves the old contribution absent, then garbage-collects it.
Disk-space admission is checked before build. If old+pending generations exceed a configured ceiling, activation refuses before damaging the active generation.
Daemon and multi-process coordination
Cold/incremental equivalence
For activation identity G:
Compare canonical projections of:
Positive controls first prove that changing the package actually changes the expected projection.
Crash matrix
Inject process death after every durable transition and meaningful batch:
After restart, the active query projection is exactly old or new and doctor explains/resumes/cleans pending state. No manual database deletion is an accepted repair.
Performance
Measure on 100k-file and high-ref fixtures:
A plugin profile/bridge change that requires full resolution must use the existing long-operation busy timeout/cancellation discipline and a hard work budget. It degrades/fails activation with disclosure instead of wedging the daemon.
Tests
Four original transitions remain mandatory, generalized beyond extensions:
Also:
Acceptance
Existing reclassification fix to preserve
Comments 5353–5354 confirm the former StatTouch corruption experimentally and record its immediate fix in
4463d60: a code/text classification mismatch cannot take the hash-tier touch shortcut and instead flows through full upsert extraction. That fix is a permanent lower-level invariant, not superseded by generation activation.Generation tests must retain both positive controls: unchanged content+classification may touch cheaply; changed classification must fully re-extract and purge the prior contribution. Package rule changes with unchanged classification still require the generation identity/build path above.
Both experiments RUN. Both CONFIRMED.
Executed against
Writer::commit_batchwith a real in-memory schema (db::open_in_memory), drivingWriterMsg::StatTouchdirectly — the same pathrun_tasktakes atindex.rs:5425/:5498. Temporary tests were injected intowriter.rs's test module, run, then the file was restored from snapshot and md5-verified (crates/indexer/src/writer.rs: OK,git statusclean).Experiment 1 — text → code produces a confident EMPTY row
The row now claims to be code, reports zero symbols, and still carries no
outline_hash. Downstream that reads asoverlap.is_code = true,index_coverage: indexed / indexed_as: code, freshness not-stale — andchanged_symbolsreporting a symbol-bearing file as symbol-free. Indistinguishable from a genuinely empty file.The missing
outline_hashis a second-order finding worth noting on its own:writer.rs:317-320keeps files without one out of the resolve scope entirely, so such a row is not merely empty — it is invisible to resolution as well.Experiment 2 — removal orphans every symbol
write_stat_touchis a bareUPDATE files SET mtime_ns, ctime_ns, size, lang, kind(writer.rs:359-364). Symbols cascade onDELETE FROM files(writer.rs:238-241), not on UPDATE. So the file is reclassified as text while its symbol rows stay live and reachable — searchable, resolvable, and attributable to a file that no longer claims to have any.The test that should have caught this already exists
touch_restamps_lang_and_kind_when_classification_changed(writer.rs:869+) performs exactly the Experiment 2 flip —("rust", Code)→("md", Text)— and asserts onlylangandkind. It never looks atsymbols. Its own doc anticipates the trigger verbatim: "a plugin can be added/removed". The mechanism was tested; its consequence was not.That is the cheapest available fix for the near term, independent of the rest of this issue: extend that test to assert the symbol population, and it will fail today.
Consequence for the contract
T3's invalidation must clear the hash tier, not just
mtime_ns— only a forced full re-parse routes throughwrite_upsert's DELETE+INSERT and purges the orphans. Anmtime_ns = -1-only invalidation lands back on the StatTouch path and reproduces both outcomes above.Fixed on master —
4463d60Both measured failures are closed. The rest of this issue (the rule-set fingerprint, T1–T6, the metamorphic suite) is still open — this only removes the corruption that would have fired the moment anything flipped a classification.
The fix
One condition, both directions, at the decision point rather than in the writer.
FileSnapshotnow carriesis_code(fromfiles.kind), and both hash-tier shortcuts —run_taskandrun_text_only_task— require it to match the classification the task is claiming. A mismatch falls through to the full parse, wherewrite_upsert's DELETE+INSERT purges the stale rows and writes whatever the new classification actually produces.The writer is deliberately left alone.
StatTouchmeans "content unchanged, do not re-extract"; it cannot purge what it never re-derives, and making it try would put extraction logic in the writer.Consequence for T3
The experiments settle a detail of the proposed contract: T3 must clear the hash tier, not just
mtime_ns. Anmtime_ns = -1-only invalidation lands back on the StatTouch path — which, with this fix in place, now correctly refuses and re-parses on a classification change, but still touches when the classification is unchanged and only the rules moved. The fingerprint remains necessary.Tests
reclassification_is_not_a_touch(index.rs) asserts!= StatTouch— the contract is "do not take the shortcut", not any particular message — with a control asserting the unchanged case still does take it, so the test cannot pass by disabling the hash tier outright. Mutation-checked: removing the guard fails it and prints theStatTouchit should have refused.touch_restamps_lang_and_kind_when_classification_changednow asserts the symbol population. Worth recording why the obvious version of that assertion would have been worthless: its fixture usedoutcome(), which produces zero symbols, soassert_eq!(symbols, 0)would have been vacuously true. It now writes two symbols, asserts them as a precondition, and pins that the writer does not purge — with a comment namingrun_taskas where the invariant actually lives, so the two cannot drift.Gate: 1405 tests (+1), 93 suites, 0 failures; fmt and clippy clean at
-D warnings. Not yet pushed.buildagent referenced this issue2026-08-26 13:32:13 +02:00
index: a rule-set edit never invalidates anything — corruption-class blocker for runtime formatsto index: generation-safe plugin activation, invalidation, promotion and rollbackinvalid_fact, blaming the package for the disk #128DAEMON_READERSis a hand-list with a summed floor, so an unregistered daemon reader is invisible to the generation-gate scan #129plugin rollbackis a two-state toggle and nothing pins or documents it #130Residual sweep, 2026-09-04 — two closed, one refuted, four filed
Seven residuals were put to this session. Each was verified against the tree before being acted on, and two of the seven were not what they said they were.
1. Engine flags outside the activation identity — CLOSED
The audit's framing was "a stale
.cwasmis silently reused". That premise is false for the shipped path, and the tree already says so:PackageSetholds aStagingDircreated per set under the system temp dir (code-index-plugin-stage.<pid>.<seq>.<nanos>),load_oneprecompiles into it, andDropisremove_dir_all.packages.rs's module doc puts it plainly — "a.cwasmis rebuilt every time a host starts, so there is no stale artifact for a generation to inherit". Nothing caches an artifact across a process.The real half was the identity, and it was live.
HostIdentity::enginecarriedWASM_ENGINE="wasmtime 36.0.14"and nothing else, so editingengine_config()moved neither the identity nor — measured, for three of the eight flags —Module::deserialize's verdict.max_wasm_stackin particular decides whether a deeply nested file traps or extracts, so two indexes with genuinely different content claimed one activation identity. Same class as ledger C10/H3, andengine.rs's own module doc had already drawn the conclusion: "Whatever key #78 uses has to CARRY THE FLAGS ITSELF."It does now, by one mechanism serving both sides:
engine::EngineFlags+ENGINE_FLAGSis the single list every flag is applied from (apply) and spelled from (render), both destructuringselfwith no..— the compiler-is-the-test ruleactivation_digestalready uses.EngineFlags::confighandsWASM_ENGINE+ the rendered flags toConfig::module_version(ModuleVersionStrategy::Custom(..)), whichModule::deserializecompares byte for byte. The engine version is kept inside that string becauseCustomreplaces wasmtime's own version check rather than adding to it; dropping it would have been a relaxation.protocol::WASM_ENGINE_FLAGScarries the same string parent-side (that crate links no runtime), reported by the worker asHostBuild::engine_flagsand composed intoidentity_line— so what enters the identity is the flags of the binary that will actually compile and run, not the parent's guess, exactly as S32 did for the version.REPORT_MAJOR1 → 2, because a missing known key is a fault and the addition is therefore not additive.conform::host_engine()was the same defect one site over — a stored conformance verdict was "current" under changed flags, on the same stated reasoning — and now returns version + flags.cranelift_opt_levelstays outside the key, with an argument rather than a shrug: this host never sets it (the setter is behind wasmtime'scraneliftfeature, which the shipped dependency line does not ask for), so it is wasmtime's default and moves only with the wasmtime version — which the key carries.flags_gate::the_engine_config_sets_nothing_the_flags_do_not_carryfails the build if that stops being true.Mutation (run). In
EngineFlags::config, compute the key and do not bake it — the pre-fix world exactly. RED:Those are precisely the two flags the original measurement listed as silently accepted. Positive control runs first in the same body: the unchanged flags must compile, load and run, and an anti-vacuity assert requires one bend per rendered field.
The partition test
a_foreign_cwasm_is_refused_for_exactly_the_flags_that_are_baked_inis unchanged and still 5/3 — it now measures what stock wasmtime gives for free, with the key held constant, and its doc says why the two tests are kept apart.2. Cursors carry no active-generation fingerprint — VERIFIED, filed as #127
Real, and the recorded deferral rationale in
epoch.rswas stale on both its premises: the production promoter shipped in v0.23.0 (cli/src/activation.rs, two sites, commit1a6a431), and the cost objection ("only reachable throughstats, a full-table count sweep") is wrong by three orders of magnitude —index_coveragehas carried the epoch since S49 andReadEpoch::probeis measured at 8.5-10.2 µs. The doc is corrected in the tree. Not fixed here because what remains is a wire change to two paginated replies with both skew directions, which is its own piece of work. #127 carries the full measurement, including that the hazard is cross-session (both promote paths refuse behind a live daemon) and that it is not benign when reached.3.
statsis not snapshot-read — REFRAMED and CLOSED at the real defectAs stated it is not actionable: no daemon RPC arm is snapshot-read, there is no read-transaction helper anywhere on that path, and
stats' opening comment argues the choice explicitly ("far too expensive here… pointed the safe way").The defect underneath is smaller, real, and a violation of a rule this codebase already states.
statsprobes aReadEpoch, gates a dozen counts with it, and then — thirteen statements later, in autocommit — calledread_activation, which re-readWHERE state = 'active'itself.local_index::read_file_claimstates the rule verbatim: "active_generationmust be the SAME id the rest of this reply's rows were gated to. Re-reading it would let a promotion land between the two and produce a reply whose symbols came from one generation and whose claim named another."index_coverageobeyed it;statsdid not. Promotion NULLstarget_idon the outgoing generation's refs, so a straddling reply can report zero resolved references beside a generation that had thousands.read_identity/read_activationnow takepinned_active: Option<i64>—Some= "the caller has already gated other rows to this id",None= "the caller gates nothing else" (that ishealth, whose only generation-derived content is this block, and the two CLI readers). Both arms keep the no-FROMscalar-subquery shape so a missing table and a missing row stay distinguishable. The three coverage counts that also selected the active generation by state follow the same pin.Mutation (run). Take the unpinned arm unconditionally — the pre-fix body. RED:
the block re-derived its own active generation: pinned to 5, reported Some(3). Positive control: pinning to the generation that is active must reproduce the unpinned read exactly, so the gate cannot be one that changes the unchanged case.4. Daemon RPC health exposes no activation identity — REFUTED, already shipped
Healthis not{root, schema_version}. It gainedactivation: Option<ActivationIdentity>(all five of #78's identity fields) andplugin_host_abi: Option<String>at #80 S52, commitd66552c, 2026-09-01 — three days before this session. The claim describes the S49 tree. It is covered byhealth_probe_e2e.rs(value-pinned againststats, plus a planner-based cost guard over both bodies), four skew legs inwire_skew_e2e.rs, and theRPC_METHODSgate.5-7. Filed
invalid_factfires on rusqlite errors. Verified and worse than stated: the bareErr(e) =>atbuild.rs:745sits over adispatch_roundwhose body is?on a dozen rusqlite calls, andreason_code_registry.rsregistersstorage_exhaustedasno_producerwith a justification that code contradicts. No test drives the arm; #78's own test list names "disk-full/busy cancellation" and nothing implements it.plugin rollback. Verified as a toggle, but the word "silently" is wrong:cmd_rollbackprints the target generation before acting andplugin statusprints the whole inventory. WithSUPERSEDED_RETENTION_LIMIT = 1there is nothing older to reach, so a second rollback is "the same epoch transition in reverse" applied to the new state. What is missing is a test pinning the toggle and one sentence of prose.DAEMON_READERSenumeration backstop. Verified, and confirmed that no escapee exists today. The floor is a>=on a sum over the four registered files only, so an unregistered reader contributes 0 ungated and 0 sites. Theread_dir-and-set-equality pattern already exists twice in this repo (bounding_site_registry,reason_code_registry/RPC_METHODS).agent_task_benchmark_runtime_pluginis a coin flip — the unconsulted-package-set prose lands inside a 1.05x token band #131Residuals audited. One was a false premise hiding a real defect next door, one was already shipped, one was not actionable as written.
Following #115 (a package refusal silently promoted over the last good generation), the remaining seven were worked in order. Each was verified first-hand before acting, which mattered: three of the seven were not what the audit described.
1. Engine flags — the serious one, and the audit's premise was FALSE
The claim was "a stale
.cwasmis silently reused". That does not happen:PackageSetholds a per-setStagingDirwhoseDropisremove_dir_all, andpackages.rssays so. Nothing caches an artifact across a process.The real defect was one field over, and it is worse than the framing suggested.
HostIdentity::enginewas the wasmtime version alone, so anengine_config()edit moved neither the identity nor — for 3 of 8 flags, measured —deserialize's verdict.max_wasm_stackdecides whether a deeply nested file traps or extracts. So two indexes holding different content could claim one identity. That is an activation-identity defect, not a caching one.Fixed as one mechanism with both sides spelled from the same list:
EngineFlags/ENGINE_FLAGS, with no..rest pattern, so the compiler is the test.EngineFlags::configbakes engine + flags into the artifact viamodule_version(Custom(..)); the same value travelsprotocol::WASM_ENGINE_FLAGS→HostBuild::engine_flags→identity_lineinto the digest, reported by the worker as S32 did for the version.REPORT_MAJOR1→2.conform::host_engine()was the same defect one site over.cranelift_opt_levelstays out, with an argument and a gate enforcing the argument: it is never set here, and its default moves only with the wasmtime version, which the key already carries.The mutation is red on the reuse, not on a recomputed hash: strip
module_versionand a.cwasmcompiled under different settings of["stack", "backtrace"]LOADS — naming the exact two flags independently measured as silently accepted. Positive control (unchanged flags compile, load and run) executes first in the same body.2. Cursors — verified, filed as #127
Real. Its recorded deferral rationale was stale on both premises: the production promoter shipped in v0.23.0, and "only reachable through
stats, a count sweep" is wrong by three orders of magnitude (ReadEpoch::probe= 8.5–10.2 µs). Doc corrected in tree; what remains is a wire change to two paginated replies.3.
statssnapshot-read — not actionable as written, real defect found by reframingNo daemon RPC arm is snapshot-read, and
statsargues that choice explicitly — so "makestatssnapshot-read" is a change to a deliberate design, not a fix.Reframing found the actual violation:
read_activationre-readstate='active'thirteen statements after the epoch gated the counts — breaking a ruleread_file_claimstates verbatim. Now pinned (pinned_active: Option<i64>, three-state, counts included). Mutation red:pinned to 5, reported Some(3), with a positive control.4. Health activation identity — REFUTED
Already shipped at #80 S52, commit
d66552c, three days ago.Healthcarriesactivation(all five fields) andplugin_host_abi. The audit and this issue both describe pre-fix code.5–7 — filed rather than done thinly
invalid_factfires on rusqlite errors. Worse than stated:storage_exhaustedis registeredno_producerwith a justification the code contradicts, so a disk failure is reported to an operator as a malformed fact — pointing them at the package instead of the disk.DAEMON_READERSenumeration backstop — verified, no live escapee today, and the pattern exists twice in-repo.Plus #131, found running the gates, now fixed
agent_task_benchmark_runtime_pluginwas a coin flip: 13 isolated runs gave 7275 ten times and 7424/7455/7478 three times against a 7425.6 ceiling. The delta ispackage_set_unconsulted_semantics, ~190 tokens rendered only when the client beats package discovery — introduced by78e2963two days ago, so pre-existing.The block is real when it appears, so the fix lets discovery settle (
Phase::startwaits forpackage_set_consulted) rather than widening the band or excluding the field. Now 6790 tokens, eight consecutive isolated runs, identical — and below the old 7072, because the recorded value had itself been taken from a raced payload.Worth recording: the first version of that wait went green while being useless — it polled with
response_format: "concise", which dropsplugin_activation, so it readfalseforever and every phase paid the full 60s deadline. 181s for a 2s suite. The green result hid it; only the runtime gave it away.All eight criteria met. Closing on
01a478b, CI run 584 green.Criterion 7 was the live defect and it was the serious one; the rest were already met or were met by the residual sweep. Walking them:
01a478b. See below.dirty_domain, graded by the claim-domain suitesread_activationre-readstate='active'thirteen statements after the epoch gated the counts, breaking a ruleread_file_claimstates verbatim. Now pinned three-state.kill -9at every injected phase recovers without database deletiongeneration_equivalence.rs:113-121), so this is equivalence of what the generations hold, not of what they are called.Criterion 1 was quietly false until the last commit
HostIdentity::enginewas the wasmtime version alone. So anengine_config()edit moved neither the activation identity nor, for 3 of 8 flags,deserialize's verdict — andmax_wasm_stackdecides whether a deeply nested file traps or extracts. Two indexes holding different content could claim one identity, which is exactly what this criterion forbids.Fixed as one mechanism with both sides spelled from a single list (
EngineFlags/ENGINE_FLAGS, no..rest pattern, so the compiler is the test), baked into the artifact viamodule_versionand carried into the digest. The mutation goes red on the reuse, not on a recomputed hash.The audit that surfaced it had the premise wrong — it claimed a stale
.cwasmwas cached across processes, which does not happen (PackageSetholds aStagingDirwhoseDropisremove_dir_all). Checking the false premise is what found the real defect one field over.Criterion 7, and why the gate asks what it asks
A package refusing on a subset of files still produced a generation that replaced the last good one, dropping those files' facts on all three channels — because the refusal path writes a contribution row, so build gates that count contributions saw a healthy count, and
dispatch_roundreturnedAcceptedregardless.The gate now asks about the loss, not the refusal: does promoting delete facts the active generation is serving for that file? A threshold was rejected (arbitrary, and at any N>0 it licenses exactly the silent loss); "any refusal fails" was rejected too (one permanently-broken file would make every future activation unreachable). Asking about the consequence avoids both.
m0059records it durably becauseRecovery::Revalidatere-runs the gates with no extraction at all — an in-memory count reads zero there.The residuals, and what two of them turned out to be
dispatch_round's entire error universe isIndexerError::Sqliteplus one contract violation, so for nearly every error it could see the label was a lie. The registry caught the fix itself.DAEMON_READERSset equality plus per-file floors. It went green on its first run with an empty exception list: the four registered files were already exactly right.ORDER BY id DESC → ASCchange it named is structurally ungradable atSUPERSEDED_RETENTION_LIMIT = 1, where the two queries are identical. Recorded as a survivor with that reason rather than a test that would have looked like coverage.d66552c.Carried forward, not silently dropped
Response::Unclaimedcollapses three causes (routes-to-nobody, vanished file, read failure) into one count that reads as the designed case. Same silent-loss family as #115; code-verified, not driven end to end, so deliberately not widened into that fix.claim::Keyhas no bounded path globs, and a fixture row namedbounded_glob_claimtests no glob #135change_impactcannot be surfaced at edit time by any hook #282