fix(fixtures): give the Acta set a policy digest that is actually current - #137
Conversation
…rent All six fixtures carried the same `policy_digest`, and none of them equalled `current_policy_digest` in `expected.json`. By the comparison the README states for `05` -- "`policy_digest` names a policy bundle that is no longer the one in force" -- `01`, `02`, `04` and `06` fail it too, while the manifest declares them `pass`. So the freshness axis `05` exists to isolate was the one axis it did not isolate, and a second implementation running `expected.json` as the portable contract disagrees with it on four of six. The cause is `decisionPayload()`, which hardcoded `POLICY_V1` with no override, so `r05` got V1 like everything else -- even though the comment above it says V1 is what makes `05` stale. The intent was in the comment; the code did not carry it out. Two lines: a `policyDigest` parameter defaulting to V2, and V1 passed explicitly at `r05`. Everything downstream follows on its own, because `envelopeHash(r01)` is computed rather than pinned: signatures change, the chain head moves from `99c36f17` to `ddf7efb0`, and `02` and `04` relink automatically, with `04`'s deliberately wrong pointer staying deliberately wrong. `05` is unchanged. Verified before touching anything that the unmodified generator reproduces all committed files byte for byte, so the regenerated output can be trusted. Why nothing caught it, and what does now: - `gen.mjs`'s self-check asserted only that `05` fails freshness and `06` fails binding. Asserting the failure alone passes whether one fixture is stale or all six are. It now checks both axes for every fixture against the manifest. - No Python test read the `policy_freshness` or `session_binding` declarations at all; the two that existed were hardcoded to one fixture each and asserted inequality, which also passes either way. Replaced with parametrized tests over every fixture, which fail on the pre-fix data for exactly 01, 02, 04 and 06. - The crosswalk quotes 01 in full and states its `chain_head` "is real". Both went stale on regeneration and nothing would have noticed. Resynced, and a test now pins the document against the fixtures. The generator note explaining that freshness for 01/02/04/06 was "pass relative to their v1-era chain context in the docs narrative" is gone, since the manifest no longer needs it: the value is now correct on its own terms. It was never written to `expected.json` in any case, so the portable contract carried the declarations with nothing qualifying them. 214 passed, 1 skipped. ruff and mypy clean. No spec text, schema or record field touched; this is fixture data and the checks over it.
|
❔ Contributor Check: UNKNOWN
Automated check by AgenTrust Contributor Check. |
imran-siddique
left a comment
There was a problem hiding this comment.
Checked the fixtures directly rather than reading the table: on this branch, 01, 02, 03, 04 and 06 now equal current_policy_digest, and 05-stale-policy-digest.json keeps the old value, which is the whole point of that fixture. Suite passes, 214.
The defect is worth naming precisely, because it is the kind that makes a conformance set actively misleading rather than merely incomplete: five fixtures declared pass while failing the exact comparison the README defines for 05. A set where the negative case is indistinguishable from the positives does not test what it says it tests, and a downstream implementer calibrating against it would have concluded the stale-policy check was unimplementable.
Generating the digest from the same source as expected.json in gen.mjs rather than a POLICY_V1 constant is the part that stops it recurring.
Merging both this and #136.
Upstream merged agentrust-io#148, the last large piece this fork was carrying, which the maintainer rebased and opened himself under this fork's authorship. With agentrust-io#122, agentrust-io#125, agentrust-io#126, agentrust-io#136 and agentrust-io#137 already merged, most of the thirty-three commits here described work that now lives upstream with better provenance than this fork can give it: a PR number and a maintainer's review. `git rebase upstream/main` was tried first and abandoned. It stopped on the oldest commit in the set, a schema-and-version alignment whose content upstream has since taken, superseded and released three times over. Replaying thirty-three commits against fifteen of upstream's resolves early commits into shapes that no longer mean anything. So: main reset to upstream/main, fork-only material re-applied. The old history is tagged `archive/pre-576507b` and pushed rather than discarded. What is held here, and why it is held: - agentrust-io#117 gap disclosure: design note, two normative drafts, 18 vectors, generator - agentrust-io#116 verifier compatibility: 8 vectors, generator, normative draft - the normative crosswalk, mapping every RFC 2119 statement to whom it binds - `docs/conformance-method.md`, `coverage-report/` (historical), DECISIONS.md - the independent signature path and the package-consistency test Each is an unaccepted proposal or a method write-up, not unfinished work. Four files needed a real merge, and not in the same direction. `models.py` and `__init__.py` are upstream's plus this fork's two profile constants, because upstream had moved on with `origin` (agentrust-io#135) and the `declared` enforcement mode (agentrust-io#143) and a wholesale copy would have dropped both. `sign.py` and `test_sign.py` are this fork's `accepted_profiles` version, which supersedes the minimal cutover check upstream took from agentrust-io#125 -- a supersession CLAUDE.md predicted when agentrust-io#125 was offered. All four merged cleanly three-way against a817621, the last commit the two histories agree on. The crosswalk guard earned its place in the same run: upstream added two normative statements this fork had never seen, and `test_normative_crosswalk.py` failed until both had rows. A source-derived inventory noticing its subject moved is the property that document exists to have. 432 passed, 1 skipped. ruff and mypy clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: lywinged <48041247+lywinged@users.noreply.github.com>
Follows up the second half of my review on #98, which nobody had a chance to answer before it merged. Small, and it is fixture data plus the checks over it: no spec text, no schema, no record field.
cc @tomjwxf, since this touches your set.
What is wrong
All six fixtures carry the same
policy_digest, and none of them equalscurrent_policy_digestinexpected.json:By the comparison the README states for
05— "policy_digestnames a policy bundle that is no longer the one in force (current_policy_digestinexpected.json)" —01,02,04and06fail it too while being declaredpass.issued_atdoes not rescue them: there is no validity window in the payload, and the stated check is a digest comparison.That lands on what the set is for. The README says
04,05and06"make one point three ways", but on the freshness axis05was indistinguishable from every other fixture, and a second implementation runningexpected.jsonas the portable contract disagrees with it on four of six.The cause, and the fix
decisionPayload()hardcodedPOLICY_V1with no override, sor05got V1 like everything else — even though the comment directly above it says V1 is what makes05stale. The intent was in the comment; the code did not carry it out.Everything downstream follows on its own, because
envelopeHash(r01)is computed rather than pinned: signatures change, the chain head moves from99c36f17toddf7efb0, and02and04relink automatically, with04's deliberately wrong pointer staying deliberately wrong.05is byte-identical to before.Before touching anything I ran the generator unmodified and confirmed it reproduces every committed file byte for byte, so the regenerated output can be trusted rather than assumed.
Why nothing caught it, and what does now
The generator's self-check asserted that
05fails freshness and06fails binding, and nothing about the other four. Asserting the failure alone passes whether one fixture is stale or all six are. That asymmetry is the whole bug.gen.mjsnow checks both axes for every fixture against the manifest, skipping onlyn/a.tests/test_acta_fixtures.pyread thepolicy_freshnessandsession_bindingdeclarations nowhere at all. The two tests that existed were hardcoded to one fixture each and asserted inequality, which passes either way. Replaced with parametrized tests over every fixture; on the pre-fix data they fail for exactly 01, 02, 04 and 06, which I checked rather than assumed.01in full and says itschain_head"is real". Both went stale the moment the fixtures were regenerated, and nothing would have noticed. Resynced, and a test now pins the document against the fixtures.Also dropped: the generator note saying freshness for 01/02/04/06 was "pass relative to their v1-era chain context in the docs narrative". The manifest no longer needs it, and it was never written into
expected.json, so the portable contract carried those four declarations with nothing qualifying them.Checks
214 passed, 1 skipped.
ruff check src testsandmypy src/agentrust_traceclean.Unchanged and worth repeating from the review: the hand-rolled
canonicalize()ingen.mjsis correct. I differential-tested it againstrfc8785over ASCII, non-ASCII values, supplementary-plane values, supplementary-plane keys, nested arrays, large integers, floats, exponent-form floats, empty containers and escape sequences — identical bytes in every case. That is by construction rather than luck, since JCS is defined in terms of ECMAScript.