schema: put the references block in the schema and the model - #199
Conversation
|
❔ Contributor Check: UNKNOWN
Automated check by AgenTrust Contributor Check. |
|
Both rulings go the same way, toward the text. Thank you for surfacing them as questions rather than shipping them as decisions, because I would have missed the second one on a read of the diff.
On the array shape you did not ask about: it is right. "a Everything else stands as written. Two things worth naming because they are the kind of decision that usually goes the other way and then costs someone a day:
Rules 3 and 4 belong in the conformance suite, yes. I cannot push to your fork, so this needs the two edits from you. Nothing else blocks it. |
2920caa to
efcc479
Compare
|
Rebased onto The only conflict was Ordering, since it is not visible from the PR page: this touches four of the ten files #201 normalizes. It merges into
|
|
Both edits are in,
The enum was doing a second job I had not separated from the first: it made the registry mechanically checkable. What replaces it is a test that the three values stay named in both the schema description and One question rather than a decision, because dropping the enum is what created it. Rules 3 and 4 to 714 passed now rather than 713, from the two case-table rows that changed direction. |
trace-spec had no .gitattributes, so line endings were whatever each contributor's editor produced. #184 landed a one-line CHANGELOG entry as +265-264, with all 259 removed lines ending in CR and none of the added ones, rewriting the blame on a file nobody had edited. The same class cost two listing PRs in awesome-ai-governance before it. Pins *.md, *.json, *.py, *.yml and *.yaml to LF and normalises the ten text files still stored with CRLF. The normalisation is line endings only: git diff --cached --ignore-all-space reports no content change on any of them. Doing it here rather than in a contributor's PR keeps the blame churn in one commit that is obviously mechanical, instead of spread across whoever next edits docs/schema.md, schema/trace-claim.json or src/agentrust_trace/models.py. Those three are exactly the files #199 is currently touching. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Spec section 3.1.2 landed in agentrust-io#198 as text. Text alone did not make the field usable: schema/trace-claim.json sets additionalProperties: false at the top level and TrustRecord is extra="forbid", so a record carrying references was rejected by both artifacts a producer validates against. examples/tpm2.json validates as committed and fails with "Additional properties are not allowed ('references' was unexpected)" the moment an entry is added. The block is an array. That is the reading the text supports rather than a decision made here: 3.1.2 says "a references entry", "an entry in references", and "omit the entry", and its field table describes the fields of an entry rather than of the block. Two of the four rules in 3.1.2 are properties of a record and are tested: rule 1, a record carrying references and no origin keeps the hardware runtime.platform it earned; rule 2, the signature covers the block, so rewriting any member of any entry in transit fails verification. Rules 3 and 4 bind verifiers rather than records — no schema can express them — and belong to the conformance suite. The schema description says so rather than implying it enforces them. resolver is constrained on presence and not on value. Rule 4 requires a producer that cannot name a resolver to omit the entry, and whether an identifier is self-asserted is not decidable from the record. retention is validated as an ISO 8601 duration by a pattern written out in alternation rather than with a negative lookahead, so the same string serves both files: pydantic's default regex engine has no look-around, and a lookahead form would build in JSON Schema and raise in the model, forcing the two apart. A test asserts the two pattern strings are byte-equal, and that the rel sets match, because a case table only catches a divergence it happens to sample. Both schema copies change together; tests/test_validate.py already fails on drift between them. docs/schema.md gets the field and a table, guarded the same way as the build_provenance table. 713 tests pass on this rebase onto aa4e3b5 (604 when first written; agentrust-io#184 added the delegation-link suite). Each guard was checked by reverting what it guards. Signed-off-by: Louielunz <48041247+lywinged@users.noreply.github.com>
Two constraints the schema asserted that the normative text does not. `rel` was a closed enum, matching `origin.kind`. Section 3.1.1 says of `kind` that it is closed "because the value of the field is that a verifier can key on it"; section 3.1.2 says only "Registered rel values" and does not make that claim, which is a distinction the two sections draw on purpose. Closing it here would make every new relation a schema change and a spec change at once. It is now a plain string, with the three registered values named in the schema description and in docs/schema.md. `minItems: 1` is gone. Rule 4 requires a producer that cannot name a resolver to omit the entry, not the block, and nothing in 3.1.2 says the array must be non-empty. An empty array does look like a record claiming to point somewhere and pointing nowhere, but that belongs in the spec if it belongs anywhere, and a schema asserting what the text does not is this PR's own argument reversed. The enum was also a guard: it made the registry mechanically checkable. What replaces it is a test that the three values stay named in both the schema description and docs/schema.md, since documentation is now all there is. Both new guards were checked by putting the constraint back and confirming they fail. The two case-table rows that asserted the old behaviour now assert the new: an unregistered `rel` is accepted, and so is an empty array. 714 passed, 1 skipped. ruff and mypy clean. Signed-off-by: Louielunz <48041247+lywinged@users.noreply.github.com>
bad5f4f to
3f4f05e
Compare
imran-siddique
left a comment
There was a problem hiding this comment.
The schema/model synchronization, array shape, open rel registry, absence of minItems, signature-tamper coverage, and schema/model drift guards are sound. One remaining blocker from the latest review note: rel is required but currently accepts the empty string, while id and resolver both require non-empty values. An empty rel is not a future registered relation; it carries no relation at all. Please add minLength/min_length 1 to rel in both schema copies and the model, plus an attributable rejection case and the existing parity coverage. No enum should return, and the array should remain allowed to be empty.
`rel` was open but not required to be present in any meaningful sense: with the enum gone it accepted the empty string, while `id` and `resolver` were both held to `minLength: 1`. An empty `rel` is not a future registered relation. It carries no relation at all, so it is now rejected the way an empty `id` or `resolver` is. `minLength: 1` in both schema copies and `min_length=1` on the model. The enum does not return, and the array stays allowed to be empty: open about which relation is named, strict that one is. The rejection case goes in the case table rather than in a test of its own, because the table runs every case against both the schema and the model and asserts the error path starts at `references`. That makes it the attributable rejection and the parity coverage in one, which was checked in both directions: removing `minLength` from the schema copies fails the schema half and leaves the model half green, and removing `min_length` from the model does the reverse. `docs/schema.md` said the schema does not constrain `rel`, which stopped being true with this commit. It now says the schema does not restrict which relation is named, only that one is. 716 passed, 1 skipped. ruff and mypy clean. Signed-off-by: Louielunz <48041247+lywinged@users.noreply.github.com>
The entry was written when `rel` was an enum, and its parenthetical list still reads as the permitted values. It is now the registry's current contents, and a reader who took the CHANGELOG at its word would expect the schema to reject a fourth relation. It does not. Restated: the three are what 3.1.2 registers today, the schema holds `rel` to being non-empty rather than to that list, and the array may be empty because rule 4 tells a producer to omit the entry rather than the block. No behaviour change. Found by re-reading the prose written before the enum was dropped rather than by a test, since nothing checks a changelog. Signed-off-by: Louielunz <48041247+lywinged@users.noreply.github.com>
|
@imran-siddique Done, The rejection case is in the case table rather than in a test of its own, because that table runs every case against both the schema and the model and asserts the error path starts at A differential over 29 cases between the previous revision and this one changes exactly one verdict — an empty Two things I did not decide.
The other is a question. 716 passed, 1 skipped. I have also rewritten the PR description. It still argued for |
imran-siddique
left a comment
There was a problem hiding this comment.
Reviewed schema/model parity, strict field constraints, documentation, and negative coverage. The references block is optional at the record level and strict when present.
Spec section 3.1.2 landed in #198 as text. The schema was not part of it, and text alone did not make the field usable:
schema/trace-claim.jsonsetsadditionalProperties: falseat the top level, soreferencesis rejected outright.TrustRecordisextra="forbid", so the reference model rejects it too.examples/tpm2.jsonvalidates as committed. Add onereferencesentry and it failed against the base withAdditional properties are not allowed ('references' was unexpected). Both artifacts a producer would validate against rejected a record the specification permits, and nothing failed, because nothing connects the field table in §3.1 to the schema.This adds the block to both, plus the reference model,
docs/schema.md, and a test file. Additions only — +493 −0 across seven files.Shape, after review
referencesis an array of entries. §3.1.2 says "Areferencesentry is a pointer, not evidence", "an entry inreferences", and "MUST omit the entry", and its field table describes the fields of an entry rather than of the block.Two constraints were in the first revision and are not here, both removed on review because the schema was asserting what the normative text does not:
relis not an enum. §3.1.1 says of the neighbouringkindthat it is closed "because the value of the field is that a verifier can key on it"; §3.1.2 says only "Registeredrelvalues" and declines to make that claim. Closing it would make every new relation a schema change and a spec change at once. The three registered values live in the schema description and indocs/schema.md, documented rather than enforced.minItems. Rule 4 requires a producer that cannot name a resolver to omit the entry, not the block, and nothing in §3.1.2 says the array must be non-empty.reldoes carryminLength: 1, added on review: open about which relation is named, strict that one is. An emptyrelis not a future registered relation, andidandresolverwere already held to the same.What is tested, and what cannot be
Of the four rules in §3.1.2, two are properties of a record:
referencesMUST NOT affectruntime.platform. A hardware record that addsreferencesand nooriginkeeps the platform it earned. The converse is tested too: addingreferencesmust not relax theoriginif/then.references. Nine tamper cases — each member, an entry appended, an entry removed, order reversed, the block removed — each fail verification.Rules 3 and 4 bind verifiers rather than records, and no schema can express either. The schema description says that rather than implying it enforces them. They belong in
agentrust-trace-tests.resolveris constrained on presence and not on value: rule 4 requires a producer that cannot name one to omit the entry, and whether an identifier is self-asserted is not decidable from the record.retentionAn ISO 8601 duration, written out in alternation rather than with a negative lookahead. That is not style: pydantic's default regex engine is Rust's, which has no look-around, so a lookahead form builds in JSON Schema and raises at import in the model — the two files would have to disagree. One string serves both, and a test asserts they are byte-equal.
Accepts
P30D,P1Y6M15D,P1Y15D,P2W,PT1H30M15S,P1DT12H. RejectsPandPT,P1DT,P1H,P1D1Y,P1Y2W,P2WT12H. It also rejects fractional components such asPT0.5H, which ISO 8601 permits on the smallest component."format": "duration"is deliberately absent.iter_errorsruns with aFormatCheckerattached, andformat: durationasserts only when an optional dependency is installed — the same record would validate differently on two machines, and the field would look checked where it was not.Drift
Both schema copies change together;
test_validate.pyalready fails when the packaged copy drifts from the normative file. Three guards in the same spirit:retentionanddigestpattern strings must be byte-equal between the model and the schema;relvalues must stay named in both the schema description anddocs/schema.md, since with the enum gone documentation is the whole of the registry;docs/schema.md's table must match the schema's field set and required set.Verification
716 passed, 1 skipped.
ruff check src tests scriptsandmypy src/agentrust_traceclean. Every guard was checked by reverting the thing it guards:docs/schema.mdminLengthremoved from the schema copiesrelcase failed, the model half stayed greenminLengthremoved from the modeldocs/schema.mdrow edited three waysreferences, not to something else going wrongA differential across 29 cases between the pre-review revision and this one changes exactly one verdict: an empty
relmoves from accepted to rejected. The other 28 are unchanged.Contribution basis
No sponsoring organization: under CONTRIBUTING this is a schema change tracking an already-merged spec change, plus tests and docs. It adds no normative rule of its own. The uppercase keywords in the schema description are quotations of §3.1.2 and name it in the same sentence.