spec: clarify conformance, verifier profiles and silent mode - #408
Conversation
Signed-off-by: Imran Siddique <imran.siddique@opaque.co>
lywinged
left a comment
There was a problem hiding this comment.
Checked at 3f85de8 against 1adbe20. The precedence rule and artifact identifiers make sense. One scope correction is needed before approval.
In spec/trace-v0.2.md, lines 36 to 37 call a schema “too broad” whenever it admits a record prohibited by the specification. Please limit that statement to structural validation constraints.
At this head, a signed control passes validate_json and returns from verify_record with a caller-supplied trusted key and fixed now. Changing one signature byte, while retaining valid base64url encoding, still passes validate_json but raises InvalidSignature. Keeping the original record unchanged and advancing now by 86401 seconds also passes schema validation but raises the stale-record ValueError.
Sections 3.2.2 and 3.3 require those verification failures. They do not show a schema defect: these checks are outside structural validation. The control establishes this boundary, not complete conformance; its revocation result reports no_check_performed.
This wording carries forward the 2026-09-11 ruling in #247. The new section’s first bullet already separates schema validation from semantic and verification checks. Making the same boundary explicit in the narrower/broader sentence preserves that ruling’s precedence order and avoids describing this intended separation as a defect.
Suggested replacement:
A schema is too narrow or too broad when it rejects or admits records contrary to the specification’s structural validation constraints. Requirements enforced by verification, including signature and freshness checks, remain separate.
Locally, the 18 documentation-coverage tests, staged strict MkDocs build, house-style check and diff whitespace check pass. I inspected the generated section’s HTML structure and text. I did not run the full Python suite locally; the hosted Python 3.11 and 3.12 test jobs report success. The PR body already identifies the added reporting obligations as normative and leaves adoption to the specification process.
Signed-off-by: Imran Siddique <imran.siddique@opaque.co>
|
@lywinged Addressed in 70f0dfe. The narrower/broader sentence now applies to structural validation constraints and explicitly separates signature and freshness verification requirements. Please re-review. Validation: 23 documentation tests and the staged strict MkDocs build passed locally. GitHub CI passed for Python 3.11 and 3.12, and CodeQL passed on the updated head. |
Signed-off-by: Imran Siddique <imran.siddique@opaque.co>
Signed-off-by: Imran Siddique <imran.siddique@opaque.co>
Signed-off-by: Imran Siddique <imran.siddique@opaque.co>
|
@lywinged could you re-review the current head, 17e419c? The narrower/broader wording now applies to structural validation constraints and explicitly separates signature and freshness verification. Your change request is still open. Please flag anything remaining; the normative additions still follow the specification process. |
lywinged
left a comment
There was a problem hiding this comment.
Re-checked at 17e419c. main has moved to 9fd79b6 since the merge in d4e2f9b, and the head
still merges cleanly onto it.
The scope point from my earlier review is resolved: the narrower/broader sentence is now the
suggested replacement, word for word. Nothing else I would hold the #247 section for.
On what the pull request has added since:
Verifier profiles (#116). I cannot review this part independently. I opened #116, and the
accepted_profiles implementation, its vectors and its tests are mine, from #347. So the fact that
the new section 3.3 text matches verify_record and the three verifier-compatibility modules pass,
69 passed and 5 skipped, checks the text against my own implementation and says nothing about
whether the requirement is right. It needs another maintainer's review. One change I would make to
my own proposal as carried here: the text puts a MUST on the contents of a "verification
statement", which the specification does not otherwise define, while the same paragraph already
calls these "verifier-result fields". Using that term, or defining the other, would close it.
Silent mode. The CHANGELOG calls this a description correction, but section 4.3 gains a
MUST NOT, "a policy deny MUST NOT block the action", and "the audit chain still records" becomes
"MUST still record". The base text did not state whether a deny blocks, though it called the
recorded decisions "would-have-denied"; the allow behavior from #28 was stated only in schema
descriptions. The opposite description, "evaluated and enforced", shipped in v0.9.0 and v0.10.0
under the same eat_profile, so a silent record does not say which meaning it was written under.
For those reasons I would put this in the non-breaking or breaking class of GOVERNANCE.md rather
than editorial. I can find no issue for it yet, only the comment on #143.
docs/integration/openshell.md:67, which is in the published navigation, maps a layer that
"enforces while suppressing operational logs" to silent. That layer blocks on deny, and silent
now MUST NOT block, so the guide would produce records that contradict the new requirement.
Deleting the line removes the contradiction, but a genuinely silent layer then falls to line 66's
advisory, which is right only if advisory is not weaker than silent. Neither the guide nor
the specification orders the two. Which do you intend?
A follow-up for whenever the text is adopted: src/agentrust_trace/revocation.py:114 and lines 133
to 134 still call profile and accepted_profiles "not accepted normative text" and the proposal
under review.
Section 4.3 adds a MUST NOT that v0.9.0 and v0.10.0 described the opposite of, under the same eat_profile, so the CHANGELOG now classifies it as breaking rather than a description correction. The section also states that advisory is not weaker than silent, and the OpenShell guide maps an enforcing layer that suppresses logs to advisory. Section 3.3 uses "verifier-result fields" for what a successful verification reports, instead of the undefined "verification statement". Raised by @lywinged in review. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Imran Siddique <imran.siddique@opaque.co>
Signed-off-by: Imran Siddique <imran.siddique@opaque.co>
|
Thanks @lywinged. At 185d328, section 3.3 and docs/verification.md say "verifier-result fields" instead of "verification statement", and the CHANGELOG classifies silent mode as breaking for the reasons you gave. #421 opens the 30-day comment period. Section 4.3 now orders the modes, advisory not weaker than silent, so openshell.md maps an enforcing layer that suppresses logs to advisory. The revocation.py wording waits for adoption, as you suggested. |
lywinged
left a comment
There was a problem hiding this comment.
Approving at e7911b7.
The three points from my last review are resolved in 185d328. Section 3.3 and docs/verification.md now say "verifier-result fields". The CHANGELOG classifies the silent-mode change as breaking, with #421 open for the comment period. Section 4.3 orders the modes, so mapping an OpenShell layer that enforces while suppressing logs to advisory understates it rather than contradicting the new MUST NOT. The merge in e7911b7 brings in main and nothing else.
As before, I am not an independent reviewer of the #116 text, which carries my own proposal. Qiang-Xu approved it at 17e419c, and the only change to it since is the wording I asked for.
One wording point, not blocking: "enforce is the strongest and silent the weakest" sits directly above the declared paragraph, which says declared asserts less than all three. "the weakest of the three evaluating modes" would keep the two consistent.
What this changes
Consolidates three maintainer-carried specification follow-ups in one PR:
Carried by Imran Siddique under the rulings in the linked threads. Contributor credit and normative change markers are recorded in CHANGELOG.md and the specification. Addresses #247 and #116 without closing either coordination issue; follows up on merged #143.
Type of change
Normative proposal for review, not adoption by publication. The precedence and profile sections add reporting obligations. Existing conformance statements lacking the required artifact identifiers or profile declarations need those details. The silent-mode correction follows the maintainer's recorded behavior ruling. Trust Record fields, wire format, schema constraints and verifier runtime behavior are unchanged.
Final classification and adoption remain subject to CONTRIBUTING.md and GOVERNANCE.md, including the 30-day comment period if classified as breaking. Maintainer sponsorship does not bypass independent review.
Spec sections
Opening authority and conformance section, section 3.3 verification, and section 4.3 policy claim.
Validation
Checklist
AI-assisted preparation and validation; independent review remains required.