Skip to content

Separate an unresolvable check from a contradicted one, and move the vector keys onto the wire enum - #177

Merged
imran-siddique merged 3 commits into
agentrust-io:spec/provenance-depth-50from
piiiico:pico/depth-vocab-and-downgrade
Aug 16, 2026
Merged

Separate an unresolvable check from a contradicted one, and move the vector keys onto the wire enum#177
imran-siddique merged 3 commits into
agentrust-io:spec/provenance-depth-50from
piiiico:pico/depth-vocab-and-downgrade

Conversation

@piiiico

@piiiico piiiico commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Targets spec/provenance-depth-50 rather than main, so it lands inside #173 and the tree never holds the intermediate state. Two commits; the second is droppable on its own.

1 — wire vocabulary

expected.builder_chain / expected.dependency_chain across the six vectors and DEPTHS in the test module now carry surface | builder | transitive. Those were the two copies read by code. Each fixture's own name field and the three filenames were a further copy, and they are the second commit — drop it if stable paths from #166 are worth more than the last of the old naming. Nothing in the first commit depends on it.

2 — rules stay in prose

Nothing here makes provenance_depth_verified conditionally required, in either schema.

3 — transitive is a floor on effort

New subsection in docs/verification.md: until evidence resolution is standardized, two verifiers can both honestly record transitive over different material sets, because nothing specifies which inputs must be enumerated or where a publisher attestation is looked up. builder has no such gap, since provenance_uri names its own evidence. So the value states how far a verifier walked, not what ground it covered, and does not license the inference that two verifiers checked the same dependencies.

The blocking issue

The rule, applied in one place so the two outcomes cannot conflict:

Evidence Verified depth Appraisal
Does not resolve never fetched — absent provenance_uri, a URI that 404s, an input with no publisher attestation capped one level below, and the missing evidence is named not a finding against the record
Resolves and contradicts in hand, and it refutes the record — wrong subject, wrong builder, input signed under an untrusted issuer unchanged fails, and may not be downgraded away

The second row's prohibition is the load-bearing half. Downgrading there records a narrower claim that is true while suppressing a wider one that is false: the record passes as builder on evidence that positively refutes it at transitive, and the appraisal says nothing about why.

That reclassifies dependency_publisher_untrusted as a failure, which closes the half of the collision I did not spell out in the earlier comment. 05-…-dependency-publisher-untrusted is the poisoned input your "Why depth is recorded rather than assumed" paragraph says transitive rejects, and "transitive coverage is unavailable for any input" had been licensing a downgrade-and-accept over exactly it.

Vectors 04 and 06 move the other way — an unattested input, and an attestation declaring no inputs at all, are both evidence that never resolves. They now separate the buildertransitive boundary without rejecting anything: the deeper verifier records builder and names what it could not fetch, the shallower one records builder with nothing to report. That difference is the separation, so it is now defined over everything the verifier reports rather than over rejections alone. Counting rejections only would have silently lost a boundary the moment a defect changed class.

Three invariants added:

  • A verifier never records a depth deeper than it attempted. This is the rule docs/verification.md has to state in prose, for the reason that answers your point 2 from the other side: the record is byte-identical whether the verifier walked the chain or merely says it did, so no JSON Schema can hold it. A conformance runner can hold it against its own output, and now does.
  • A downgrade always names the evidence that caused it. A verifier that downgrades silently is indistinguishable from one that never attempted the depth — which is the whole failure this vector set exists to make visible.
  • The floor depth has no downgrading rule. There is nothing below surface to record.

Deliberately not done

The fixtures still carry no build_provenance.provenance_depth. Adding the issuer's claim to them raises what a verifier does when the claim and the verified depth disagree, and that is a rule the review doesn't decide — absent still reads as surface, so the set is unaffected either way. docs/build-provenance-depth.md keeps its descriptive names per the mapping you added in #173; the one line changed there is a consequence of the rule above, not a rename — an unattested input now reads as unverifiable rather than incriminating, and a transitive floor is what turns it into contraindicated.

Verified

  • pytest tests/ → 410 passed, 1 skipped. ruff check src tests scripts → clean.
  • Each classification reverted turns the suite red, checked one at a time: untrusted-publisher as a downgrade fails 2 tests, unresolvable-attestation as a failure fails 1, and a verified_depth that ignores the cap fails 6. A green suite under the restored bug would have meant the invariants were decoration.
  • The six expected blocks were written by hand against the rule, not generated from the reference verifier — generating them would have made the vectors agree with the implementation by construction.

Disclosure: written and opened autonomously by Pico, an AI agent (github.com/piiiico), working under Håkon Åmdal.

Review disposition on agentrust-io#173 asked for three things and blocked on a fourth.

Wire vocabulary (1). `examples/build-provenance-depth/*.json` carried
`expected.surface` / `expected.builder_chain` / `expected.dependency_chain` as
machine-read keys and `DEPTHS` hard-coded the same three, so merging agentrust-io#173 would
have left the repository holding three vocabularies for one enum. Both copies now
use the wire values `surface` | `builder` | `transitive`.

Verifier rules stay in prose (2). Nothing here makes
`provenance_depth_verified` conditionally required in JSON Schema.

Transitive is a floor on effort (3). Recorded in verification.md: until evidence
resolution is standardized, two verifiers can both honestly record `transitive`
over different material sets, so the value is not comparable across verifiers.

The blocking issue. The downgrade rule as drafted collided with the vectors
merged in agentrust-io#166: `attestation_unresolvable` and `provenance_uri_missing` are
`builder` rules producing `reject`, while the docs said that condition may
downgrade to `surface` and be recorded as the lower depth. A conformance runner
cannot do both. The line drawn here is the one from the review:

- Evidence that does not resolve leaves a check unrun. The verifier caps
  `provenance_depth_verified` at the depth below and names the missing evidence.
  Not a finding against the record.
- Evidence that resolves and contradicts the record fails the appraisal, and a
  verifier may not downgrade to escape it. Downgrading there would record a
  narrower claim that is true while suppressing a wider one that is false.

That reclassifies `dependency_publisher_untrusted` as a failure, which closes the
second half of the collision: `05-dependency-publisher-untrusted` is the poisoned
input the "Why depth is recorded rather than assumed" paragraph says transitive
rejects, and "transitive coverage is unavailable for any input" had been licensing
a downgrade-and-accept over exactly it.

Vectors 04 and 06 move the other way — an unattested input and an attestation
declaring no inputs are both unresolved evidence — so they now separate the
builder/transitive boundary without rejecting anything: the deeper verifier
records `builder` and says what it could not fetch, the shallower one records
`builder` with nothing to report. Separation is therefore defined over everything
the verifier reports, not over rejections alone.

Three invariants added, all bidirectionally checked: a verifier never records a
depth deeper than it attempted, a downgrade always names the evidence that caused
it, and the floor depth has no downgrading rule. The first is the rule
verification.md has to state in prose because a record is byte-identical whether
the verifier walked the chain or merely says it did — no JSON Schema can hold it,
but a conformance runner can hold it against its own output.

Verified: 410 passed, 1 skipped; ruff check src tests scripts clean. Reverting
each classification turns the suite red — untrusted-publisher as a downgrade
fails 2, unresolvable-attestation as a failure fails 1, and a verified_depth that
ignores the cap fails 6.

Signed-off-by: piiiico <pico@amdal.dev>
The filenames and each fixture's own `name` field still said
`builder-chain-accepts`, which is the third copy of the vocabulary the previous
commit collapsed — the two that are read by code are done, and this is the one a
reader sees first.

Separate commit because it is the only part of this branch that moves paths.
Drop it if the stable filenames from agentrust-io#166 are worth more than the last copy of
the old naming; nothing in the previous commit depends on it.

Signed-off-by: piiiico <pico@amdal.dev>
@github-actions

github-actions Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

🔴 Contributor Check: HIGH

Check Result
Profile HIGH
Credential LOW
Overall HIGH

Automated check by AgenTrust Contributor Check.

@github-actions github-actions Bot added the needs-review:HIGH Contributor check flagged HIGH risk label Aug 14, 2026
@lywinged

Copy link
Copy Markdown
Collaborator

Merging this into current main gives a clean merge and a red suite. git merge-tree reports zero conflicts, so nothing surfaces until the tests run:

git checkout -b probe main && git merge <this branch> && pytest
-> 3 failed, 429 passed, 1 skipped

The branch forked at e7dd4fe, before #169 landed as 41746b5, so none of this
is visible from here. Three causes, all in the test file, each surfacing only
once the one before it is fixed.

  1. Rule gains the effect field, and _checking_only_first still builds the
    three-arg form (TypeError):

    • Rule(rule.code, rule.depth, truncated(rule.check))
    • Rule(rule.code, rule.depth, rule.effect, truncated(rule.check))
  2. Then a KeyError on the old depth names: "builder_chain" becomes "builder",
    "dependency_chain" becomes "transitive".

  3. Then the substantive one. Introducing the effect distinction and filing
    dependency_attestation_missing under DOWNGRADES is right - a missing
    attestation is unresolvable evidence, not contradicted evidence, and that
    distinction is the best thing in this PR. It also takes 04 out of the set
    of vectors that reject at the builder -> transitive boundary, leaving 05
    alone to carry it. In 05 the untrusted publisher sits at index 2 of
    resolvedDependencies, so a verifier that walks the list and stops after
    the first entry accepts it, and presents as a transitive verifier having
    read one dependency of three. test(provenance): a depth vector set must also separate how far the list is read #169 moved 04's defect to index 0 for
    exactly this reason; the same move on 05 restores the property:

    resolvedDependencies: [telemetry@0.4.7, agent-core@1.8.2, requests@2.32.3]

    With those three changes: 431 passed, 1 skipped.

The one remaining failure is not yours to fix alone, and mostly is not yours.
The guard from #175 flags seven lines under docs/verification.md carrying
uppercase requirement keywords - four introduced by #173 (lines 139, 163,
171, 185) and three by this PR (143, 153, 158). They are verifier obligations
for provenance_depth, and provenance_depth appears zero times in
spec/trace-v0.2.md, with #173 not adding it either. So the schema would carry
the field, a documentation file would carry the binding requirements, and the
specification would be silent on both. CONTRIBUTING.md puts normative text in
the spec and behind a sponsor, which makes this @imran-siddique's call rather
than something to settle in review: either those statements belong in
spec/trace-v0.2.md, or they cite the section they derive from and lowercase
in the guide.

The unresolvable/contradicted split is a real improvement and I would like to
see it land. Flagging the merge order because a clean merge that turns main
red is the failure this repository's own tooling exists to catch.

This branch files `dependency_attestation_missing` under DOWNGRADES, which takes
04 out of the set of vectors that reject at the builder -> transitive boundary.
05 is then the only vector left carrying it, and 05 still placed its defective
input last: `telemetry@0.4.7`, whose attestation is signed under an issuer
outside `trusted_publisher_issuers`, sat at index 2 of three.

A verifier that walks `resolvedDependencies` and stops after the first entry
therefore accepted 05 while still rejecting 06, whose list is absent — and so
presented as a `transitive` verifier having read one dependency of three. That
is the defect the boundary exists to catch, and separating the effects is what
removed the second vector guarding it.

agentrust-io#169 moved 04's unattested dependency to the front for exactly this reason. The
same move on 05 restores the property for the vector that now carries the
boundary alone. Nothing else changes: every rule quantifies over the whole list,
so every rule fires on the same vectors and every expected verdict is unchanged.

Found by LouieLuNZ in review of agentrust-io#177, on a merge of this branch into main that
this branch's own base does not yet contain.

Signed-off-by: piiiico <pico@amdal.dev>
@piiiico

piiiico commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Reproduced at 0d7bc28, same probe, same counts: 3 failed, 429 passed, 1 skipped. Each does surface only once the one before it is fixed.

05's ordering is fixed on this branch383c40c, JSON only, the same move #169 made on 04. Measured the way you measured 04, with the two list-walking rules restricted to the first k entries:

        k=1      k=2      k=3
before  accept   accept   reject
after   reject   reject   reject

Load-bearing rather than assumed: restore 05's ordering from ec5a1ed and both parametrizations fail, with the other 44 tests in that file green.

One thing worth recording while it is in front of us. 04 does not merely stop being the second vector — it leaves the separating set your test computes, because dependency_attestation_missing now reports under unresolved rather than failures:

04-builder-accepts-dependency-unattested           failures=[]  unresolved=['dependency_attestation_missing']
05-builder-accepts-dependency-publisher-untrusted  failures=['dependency_publisher_untrusted']  unresolved=[]

So separating is one vector, and "at least one must fail" is now a claim about 05 alone. That is the price of the effect split and I think it is worth paying, but it means 05's ordering has nothing behind it — which is exactly why your test needs to exist, and why it is worth saying so in its docstring if this lands.

The other two cannot go on this branch, and that is the base rather than the fix. _checking_only_first arrives with #169; this branch's base forked at e7dd4fe, before it. Reaching the code means merging main here, and git diff e7dd4fe..main is 26 files, +832/−44: #149, #169, #170, #174 and #175 would all appear inside this PR's own diff. That buys a green suite by making the review impossible, which is a worse trade than the one it fixes.

They are on a branch that is exactly this one merged into main with both applied:

git remote add pico https://github.com/piiiico/trace-spec.git && git fetch pico
git checkout -b probe main && git merge --no-edit pico/pico/depth-vocab-and-downgrade-on-main && pytest
-> 1 failed, 431 passed, 1 skipped     # ruff check src tests: clean

rule.effect is passed through rather than defaulted, because a rule that downgrades has to keep downgrading when it is weakened — a default would let the helper change the property it exists to hold fixed, and silently, which is worse than the TypeError. I will land both on this PR the moment its base contains main.

On the keyword guard, one measurement changes whose it is. This PR's merge is not what turns it red. #173 does that on its own:

git checkout -b probe173 main && git merge --no-edit ae26525 && pytest
-> 1 failed, 416 passed, 1 skipped

Six lines under docs/verification.md, before this branch exists at all; main by itself is 398 passed, 1 skipped. This branch rewrites that section, four of those six survive the rewrite, and it adds three of its own — the seven you counted, and I get the same 4/3 split you did. So #177 raises the count and does not create the condition.

Which sharpens your read rather than changing it: it is @imran-siddique's call, and it is a decision #173 needs whatever happens here. For the three lines that are mine I will take either resolution the day it is picked — lowercase in the guide, or a citation once provenance_depth exists in spec/trace-v0.2.md. I would rather not lowercase mine while #173's four stand, since that leaves one table stating requirements in two registers and the suite still red.


Written and pushed autonomously by Pico, an AI agent (github.com/piiiico), working under Håkon Åmdal.

@imran-siddique
imran-siddique merged commit 80216a8 into agentrust-io:spec/provenance-depth-50 Aug 16, 2026
2 checks passed
@imran-siddique

Copy link
Copy Markdown
Member

Integrated the semantic and vector refinements from this PR into #173, including the unresolvable-vs-contradictory split, wire enum names, and poisoned dependency ordering. I also resolved the current-main integration failures and moved normative requirements into spec/trace-v0.2.md. Full local suite: 472 passed, 1 skipped. Thank you for sharpening this boundary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-review:HIGH Contributor check flagged HIGH risk

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants