Skip to content

FEAT-065 rework: matching needs content corroboration — 6 wrong-verdict paths found pre-ship #122

Description

@avrabe

Adversarial review of the FEAT-065 adjudicator (PR #120, converted to draft) found six wrong-verdict paths. Nothing shipped — it is library-only with no CLI/WIT surface — but the feature is not fit for purpose as designed. All findings are reproduced and recorded in DD-021's limitations; DD-021's rationale has been retracted in place because the review falsified its central claim.

The design defect

DD-021 justified gating discharged on the ordinal domain's site set being unchanged, asserting aliasing "necessarily" changes that membership. False. A deletion paired with a same-kind insertion leaves the set byte-identical ⇒ false discharged. The group-set check is necessary but not sufficient.

Direction: stop matching on a positional ordinal plus a population check. Add content corroboration — fold a hash of the operator's local context (e.g. the preceding k opcode kinds within the region) into site_key, so a replaced operator cannot inherit its predecessor's identity. DD-020 already named this as the deferred option; the review shows it is required, not optional.

Work items

  1. Content-corroborated site_key (the core fix; kills findings 1 and the reordering residual).
  2. Never claim discharged when the function identity came from body_shape_hash — degrade to uncertain. Fixes the cross-function collision, and honestly handles stripped modules where any opcode edit moves every key.
  3. Report NEW obligations. Today an obligation introduced at a site with no prior advisory yields zero verdicts — an agent that fixes A while breaking B passes clean. Add a NewObligation outcome (or fold into regressed) covering every after-open site absent from before.
  4. Fix regressed. It fires on a byte-identical module, emitting contradictory still-open + regressed under one id with a fabricated before_code. Underlying data gap: the proven-safe advisory does not record which trap kind it proves, so an open div-by-zero and a proven signed-overflow at one i32.div_s are indistinguishable by site. Either record the trap kind on the advisory, or key regressions per (site, trap kind).
  5. Treat laundering as uncertain — see the companion issue on havoc_region emitting no gap.
  6. Make obligation_id actually unique (two ProvenSafe kinds at one pc collapse; shape-hash collides across functions), or stop claiming uniqueness in DD-021.
  7. Moved should not fire on raw pc inequality — one earlier instruction turns every still-open in the function into moved, so a gate keying on still-open misses almost every real diff.
  8. Fix the removed-with-code detail string — it asserts "the site no longer exists" in cases where nothing was deleted, actively misleading a consumer.
  9. Empty site_key should degrade to uncertain, not be filtered out. Vacuous today (no path reaches it), but silent omission is the wrong default in the one place where "cannot be keyed" is known.

Test-quality note

feat065_regression_is_reported passes vacuously — it would pass on a self-comparison. Rewrite it to assert the self-comparison produces NO regressed, which is the property that actually matters.

Standard for re-review

An adjudicator that can wrongly report discharged is worse than none: an agent optimises straight into it, and the cheapest exploit is deleting code. Before this leaves draft, the acceptance bar should include an explicit adversarial suite — self-comparison yields only still-open; deletion never yields discharged; cardinality-preserving delete+insert yields uncertain; a newly introduced fault always yields a verdict.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions