Skip to content

fix(fixtures): give the Acta set a policy digest that is actually current - #137

Merged
imran-siddique merged 1 commit into
agentrust-io:mainfrom
lywinged:fix/acta-policy-digest
Aug 9, 2026
Merged

imran-siddique merged 1 commit into
agentrust-io:mainfrom
lywinged:fix/acta-policy-digest

Conversation

@lywinged

@lywinged lywinged commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator

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 equals current_policy_digest in expected.json:

current_policy_digest              sha256:bda8d8f6...
01-valid-accepted.json             sha256:b5af974a...  != current   declared=pass
02-valid-denied.json               sha256:b5af974a...  != current   declared=pass
03-signature-key-mismatch.json     sha256:b5af974a...  != current   declared=n/a
04-broken-chain.json               sha256:b5af974a...  != current   declared=pass
05-stale-policy-digest.json        sha256:b5af974a...  != current   declared=fail
06-session-binding-mismatch.json   sha256:b5af974a...  != current   declared=pass

distinct policy_digest values across all six: 1

By the comparison the README states for 05 — "policy_digest names a policy bundle that is no longer the one in force (current_policy_digest in expected.json)" — 01, 02, 04 and 06 fail it too while being declared pass. issued_at does 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, 05 and 06 "make one point three ways", but on the freshness axis 05 was indistinguishable from every other fixture, and a second implementation running expected.json as the portable contract disagrees with it on four of six.

The cause, and the fix

decisionPayload() hardcoded POLICY_V1 with no override, so r05 got V1 like everything else — even though the comment directly above it says V1 is what makes 05 stale. The intent was in the comment; the code did not carry it out.

-function decisionPayload({ tool, decision, reason, sessionId, issuedAt, prevHash }) {
+function decisionPayload({ tool, decision, reason, sessionId, issuedAt, prevHash, policyDigest = POLICY_V2 }) {
     ...
-    policy_digest: POLICY_V1,
+    policy_digest: policyDigest,

 const r05 = envelope(decisionPayload({
   tool: 'run_shell', decision: 'allow', sessionId: SESSION,
   issuedAt: '2026-07-08T09:00:12.000Z',
+  policyDigest: POLICY_V1, // the only fixture carrying the superseded policy
 }), SIGNER_PRIV);

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 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 05 fails freshness and 06 fails 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.mjs now checks both axes for every fixture against the manifest, skipping only n/a.
  • tests/test_acta_fixtures.py read the policy_freshness and session_binding declarations 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.
  • The crosswalk quotes 01 in full and says its chain_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 tests and mypy src/agentrust_trace clean.

Unchanged and worth repeating from the review: the hand-rolled canonicalize() in gen.mjs is correct. I differential-tested it against rfc8785 over 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.

…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.
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Contributor Check: UNKNOWN

Check Result
Profile UNKNOWN
Credential LOW
Overall UNKNOWN

Automated check by AgenTrust Contributor Check.

@github-actions github-actions Bot added the needs-review:UNKNOWN Contributor check flagged UNKNOWN risk label Aug 9, 2026

@imran-siddique imran-siddique left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@imran-siddique
imran-siddique merged commit 618f544 into agentrust-io:main Aug 9, 2026
6 of 7 checks passed
lywinged added a commit to lywinged/trace-spec that referenced this pull request Aug 10, 2026
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>
@lywinged
lywinged deleted the fix/acta-policy-digest branch August 18, 2026 05:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-review:UNKNOWN Contributor check flagged UNKNOWN risk

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants