h-dv/code-index

Watch

Fork 0

bug: two versions of one package id can both be approved — the normal upgrade path creates it, and the diagnosis blames the wrong package #91

New issue

Closed

opened 2026-09-03 13:05:51 +02:00 by buildagent · 1 comment

buildagent commented

2026-09-03 13:05:51 +02:00

Member

Split out of #90's adversarial review, which reported it as "both generations of one package id run at once". That framing is too strong for the common case, and it misses the sharper problem. Verified by reading the code paths.

1. Creating the duplicate is the ORDINARY UPGRADE PATH

ApprovalRecord::set de-duplicates by digest:

pub fn set(&mut self, approval: Approval) {
    self.approvals.retain(|a| a.digest != approval.digest);   // <- digest, not package_id
    self.approvals.push(approval);
    self.approvals.sort_by(|a, b| a.digest.cmp(&b.digest));
}

So plugin enable <v2-digest> after plugin enable <v1-digest> leaves both approved. An operator upgrading a package is in this state immediately, and nothing says so. Not an exotic input — it is what upgrading looks like.

discover_with_store then activates every approval with no id guard:

for approval in &record.approvals {
    out.activate(store, approval, &staging_path, host_bin);
}

2. What happens next splits on claims, and NEITHER branch is right

Claims intersect — the normal case for two versions of one package, since they claim the same extensions. The second is refused by the real check:

for other in accepted {
    claims.check_against(&other.claims, Scope::PackageVsPackage)?;
}

So it does not silently run both. But the refusal is package.claim_conflict at scope PackageVsPackage, which tells the operator "another package's claims collide with yours". They go looking for a third-party package. The truth is "you approved two versions of your own", and the repair is plugin disable <old digest>. Right refusal, wrong diagnosis, wrong repair.

Claims do NOT intersect — v2 drops a claim and adds another (v1 claims .xaml, v2 claims only .axaml). Then both activate, and that is where the review's stronger claim is correct.

3. The sharp part: two live owners MINT THE SAME LANGUAGE ID

The wire language id is derived <package id>/<local id>, so two digests of one package id produce the same files.lang / symbols.lang.

That falsifies a premise stated in the claim machinery's own doc:

ClaimTable::conflicts_with — "the languages are irrelevant here because two OWNERS can never share one"

With duplicate package ids, two owners share one. #77 deliberately made provenance row-level and said files.lang cannot be the source of truth for mixed-language resolution — but a consumer reading the language id now cannot tell which of two live digests produced a row, and the claim algebra is reasoning under an assumption that no longer holds.

4. #90 can create this WITHOUT a human

Activation-on-demand writes approvals. After #90's F2 fix an auto-enable only writes a digest the basis project approved — but if this project already has v2 approved manually and the basis approved v1, v1 is "not approved here", so it is offered and can be silently enabled. The duplicate then arrives with nobody having chosen it.

What to decide, not just what to fix

The fix is not obviously "supersede by package id". Silently dropping a grant an operator explicitly made is its own defect, and pinning an exact digest is the epic's stated model ("Projects activate an exact digest", #75). Candidate directions:

enable refuses when another digest of the same id is approved, naming it and the one command that resolves it. Preserves "an operator's grant is never dropped by a machine".
set supersedes by package id, with the superseded digest disclosed in the reply. Matches upgrade intuition; violates 1.
Keep both approved but activate one deterministically, with a real reason code for the other. Closest to today, minus the misleading diagnosis.

Whichever is chosen, two things are needed regardless:

A reason code that names THIS state rather than reusing package.claim_conflict, so the witness sends the operator to the right repair.
A disclosure: plugin_activation / plugin status should say "two versions of <id> are approved here" while it is true.

Acceptance

Enabling a second digest of an already-approved package id reaches a state the payload NAMES, on both the CLI and the MCP surface.
The e2e that today asserts a package.claim_conflict for this case asserts the new code instead — and a mutation restoring the old code goes red.
The non-intersecting-claims case (both activate, same language id) has a test, whichever direction is chosen.
#90's auto-enable cannot create the state without disclosing it.

Split out of #90's adversarial review, which reported it as *"both generations of one package id run at once"*. **That framing is too strong for the common case, and it misses the sharper problem.** Verified by reading the code paths. ## 1. Creating the duplicate is the ORDINARY UPGRADE PATH `ApprovalRecord::set` de-duplicates by **digest**: ```rust pub fn set(&mut self, approval: Approval) { self.approvals.retain(|a| a.digest != approval.digest); // <- digest, not package_id self.approvals.push(approval); self.approvals.sort_by(|a, b| a.digest.cmp(&b.digest)); } ``` So `plugin enable <v2-digest>` after `plugin enable <v1-digest>` leaves **both** approved. An operator upgrading a package is in this state immediately, and nothing says so. Not an exotic input — it is what upgrading looks like. `discover_with_store` then activates every approval with no id guard: ```rust for approval in &record.approvals { out.activate(store, approval, &staging_path, host_bin); } ``` ## 2. What happens next splits on claims, and NEITHER branch is right **Claims intersect** — the normal case for two versions of one package, since they claim the same extensions. The second is refused by the real check: ```rust for other in accepted { claims.check_against(&other.claims, Scope::PackageVsPackage)?; } ``` So it does **not** silently run both. But the refusal is `package.claim_conflict` at scope `PackageVsPackage`, which tells the operator *"another package's claims collide with yours"*. They go looking for a third-party package. **The truth is "you approved two versions of your own", and the repair is `plugin disable <old digest>`.** Right refusal, wrong diagnosis, wrong repair. **Claims do NOT intersect** — v2 drops a claim and adds another (v1 claims `.xaml`, v2 claims only `.axaml`). Then **both activate**, and that is where the review's stronger claim is correct. ## 3. The sharp part: two live owners MINT THE SAME LANGUAGE ID The wire language id is derived `<package id>/<local id>`, so two digests of one package id produce the **same** `files.lang` / `symbols.lang`. That falsifies a premise stated in the claim machinery's own doc: > `ClaimTable::conflicts_with` — *"the languages are irrelevant here because **two OWNERS can never share one**"* With duplicate package ids, two owners share one. #77 deliberately made provenance row-level and said `files.lang` cannot be the source of truth for mixed-language resolution — but a consumer reading the language id now cannot tell which of two live digests produced a row, and the claim algebra is reasoning under an assumption that no longer holds. ## 4. #90 can create this WITHOUT a human Activation-on-demand writes approvals. After #90's F2 fix an auto-enable only writes a digest the basis project approved — but if this project already has v2 approved manually and the basis approved v1, v1 is "not approved here", so it is offered and can be silently enabled. **The duplicate then arrives with nobody having chosen it.** ## What to decide, not just what to fix The fix is not obviously "supersede by package id". Silently dropping a grant an operator explicitly made is its own defect, and pinning an exact digest is the epic's stated model (*"Projects activate an exact digest"*, #75). Candidate directions: 1. **`enable` refuses when another digest of the same id is approved**, naming it and the one command that resolves it. Preserves "an operator's grant is never dropped by a machine". 2. **`set` supersedes by package id**, with the superseded digest disclosed in the reply. Matches upgrade intuition; violates 1. 3. **Keep both approved but activate one deterministically**, with a real reason code for the other. Closest to today, minus the misleading diagnosis. Whichever is chosen, two things are needed regardless: - **A reason code that names THIS state** rather than reusing `package.claim_conflict`, so the witness sends the operator to the right repair. - **A disclosure**: `plugin_activation` / `plugin status` should say "two versions of `<id>` are approved here" while it is true. ## Acceptance - Enabling a second digest of an already-approved package id reaches a state the payload NAMES, on both the CLI and the MCP surface. - The e2e that today asserts a `package.claim_conflict` for this case asserts the new code instead — and a mutation restoring the old code goes red. - The non-intersecting-claims case (both activate, same language id) has a test, whichever direction is chosen. - #90's auto-enable cannot create the state without disclosing it.

dhoyer referenced this issue from a commit

2026-09-03 20:34:01 +02:00

feat: the infrastructure a language plugin needs, and a daemon that can be diagnosed

buildagent commented

2026-09-04 09:44:00 +02:00

Author

Member

Shipped in v0.26.0

When a store holds more than one approved, resolvable digest of the same package id, only one can be live. Which one that is, and which digests it displaced, are now reported — previously the others simply did not run and nothing said so.

DuplicatePackageId { package_id, active, shadowed }: the id as the stored bytes declare it, the digest that activates, and every other approved resolvable digest of the same id. It is a view over the existing unapproved list, not a fifth list — these rows belong in the same place as every other "you asked, and here is what happened to it" answer.

The daemon-side type is separate from the indexer's by design: the indexer type is the live set's own vocabulary, the wire type's field names are a contract a client matches on, and the mapping between them is the identity — so there is nothing to re-derive and therefore nothing that can drift.

Live on the installed build

project_overview -> "package_duplicate_ids": {"availability":"reported","duplicates":[]}

An empty measurement, not an absent field.

Ordering

The set is applied in digest-sorted order, so which copy activates is deterministic rather than filesystem-dependent. Worth recording because the decision record for this originally rejected "lowest digest" on the grounds that it would change which version runs on machines working today — that claim was wrong, set already sorted by digest, the two rules are identical, and the record was corrected in place rather than left standing.

## Shipped in v0.26.0 When a store holds more than one approved, resolvable digest of the same package id, only one can be live. Which one that is, and which digests it displaced, are now reported — previously the others simply did not run and nothing said so. `DuplicatePackageId { package_id, active, shadowed }`: the id as the **stored bytes** declare it, the digest that activates, and every other approved resolvable digest of the same id. It is a **view over the existing `unapproved` list**, not a fifth list — these rows belong in the same place as every other "you asked, and here is what happened to it" answer. The daemon-side type is separate from the indexer's by design: the indexer type is the live set's own vocabulary, the wire type's field names are a contract a client matches on, and the mapping between them is the identity — so there is nothing to re-derive and therefore nothing that can drift. ## Live on the installed build ``` project_overview -> "package_duplicate_ids": {"availability":"reported","duplicates":[]} ``` An empty **measurement**, not an absent field. ## Ordering The set is applied in digest-sorted order, so which copy activates is deterministic rather than filesystem-dependent. Worth recording because the decision record for this originally rejected "lowest digest" on the grounds that it *would change which version runs on machines working today* — that claim was **wrong**, `set` already sorted by digest, the two rules are identical, and the record was corrected in place rather than left standing.

buildagent closed this issue

2026-09-04 09:44:05 +02:00

buildagent referenced this issue

2026-09-04 09:45:00 +02:00

plugin: a package cannot replace a builtin, and a guest cannot name a node kind #87

buildagent referenced this issue

2026-09-05 19:55:36 +02:00

Embedded-region dispatch: EMBEDDED_DISPATCH_SEMANTICS_VERSION = 0, so no file can carry two producers and #77's criterion 2 is unexercisable #119

buildagent referenced this issue

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

overview_payload_budget_e2e has under 1% headroom against a load-dependent disclosure, so a correctness gate turns on machine load #98

buildagent referenced this issue