Skip to content

fix(evidence): make the gate's checks, triggers and exit codes mean something - #112

Merged
MajorLift merged 6 commits into
MetaMask:jongsun/add/pr-validate-falsifying-testfrom
MajorLift:fix/evidence-gate-credibility
Aug 7, 2026
Merged

fix(evidence): make the gate's checks, triggers and exit codes mean something#112
MajorLift merged 6 commits into
MetaMask:jongsun/add/pr-validate-falsifying-testfrom
MajorLift:fix/evidence-gate-credibility

Conversation

@MajorLift

Copy link
Copy Markdown
Contributor

Stacked on #84 — targets jongsun/add/pr-validate-falsifying-test, not main.

Five defects in the evidence gate and its runners. Each was found by running the thing, not by reading it, and each control is stated below so the claim is checkable.

1. Artifacts were matched, not resolved

ARTIFACT and OBS_ARTIFACT matched https?://\S+ and a bare filename pattern, so any URL-shaped string discharged them. The same generator writes the claim and the string that satisfies the check, so presence carried no information about whether the evidence exists.

Measured against the unpatched hook, claim and artifact in the same block (the checks are unit-scoped, so proximity matters):

body before
https://example.invalid/nope/capture.png ALLOWED
`src/totally/made-up.test.ts:42` ALLOWED
`evidence/never-captured.png` ALLOWED

An artifact now counts only if the author could not have authored its bytes: a namespace where CI, the upload endpoint, or an observability backend writes them, or a local path that is actually on disk. github.com is author-writable in general, so only /actions/runs/, /user-attachments/, /blob/, /commit/ and /pull/ under it qualify.

Controls, before → after: fabricated 1/3 → 3/3 blocked; genuine 2/2 → 2/2 allowed.

Replaying real published bodies surfaced the interesting case: evidence hosted in an author-controlled S3 bucket is now refused, correctly — fetching it unauthenticated proves it is fetchable, not that the author did not write it. Rather than decide that silently, EVIDENCE_GATE_ARTIFACT_HOSTS registers such hosts explicitly. That is a visible downgrade from independent to merely fetchable, which is the point: it should be a choice someone made, not a property of a regex.

This is still not resolution — nothing fetches the URL and checks for a 200. Presence stays necessary and stops being sufficient.

2. Where the bold stopped decided whether the gate ran

_is_evidence_artifact matched ^\*\*Verdict:\*\*, so:

**Verdict:** proven     -> all 13 checks ran
**Verdict: proven**     -> none ran

The same sentence, rendered identically, one of them silently unenforced. Now matches a line-leading bolded Verdict however the emphasis falls, still anchored so prose mentioning the word does not drag an ordinary reply into the gate.

3. $ATTEST_GATE ranked last in its own resolver

_find_gate consulted $ATTEST_GATE after three default paths. An override that loses to a default is not an override: a control run pointing it at a stand-in silently exercised the installed gate and reported on that instead.

This is the failure the skill's own non-negotiable 9 describes — an instrument reporting the instruction it was given rather than the effect it had — sitting in the resolver for the gate itself.

Verified by substituting a stand-in gate whose exit code is controlled, with the hook copied to a directory with no sibling scripts/ (as gate-controls.sh does, for exactly this reason):

before after
stand-in exits 0 ALLOWED ALLOWED
stand-in exits 1 ALLOWED BLOCKED
gate path nonexistent ALLOWED BLOCKED

gate-controls.sh: all eleven arms behave, including the three negative arms that catch a gate which has started blocking everything.

4. The determinism check warned and continued

evidence-run.yml's determinism step emitted ::warning:: and carried continue-on-error, so the step whose own message reads "do not publish these numbers" went green and the numbers published.

Now ::error:: plus exit 1, with continue-on-error removed from that step only. The two runner steps keep it, because there the exit code is the verdict and a finding is not a failure. Here a difference means the instrument did not return the same answer twice, so neither answer is publishable.

Verified by extracting the step and running it: identical arms exit 0 — including the label/log/logs/env fields it deletes by design, which is the false positive that taught operators to publish through it — and differing arms exit 1.

5. selector-recompute.sh returned 0 for "not meaningful"

It exited 0 for every outcome including VALUE UNSTABLE and probe-failed, so a caller gating on the exit code saw green on a run whose own artifact says the number is not meaningful. The only thing between that and publication was attest-gate happening to grep the verdict string out of the prose.

Now 4 for VALUE UNSTABLE, 5 for probe-failed, 0 kept for both real measurements — a selector that recomputes is a result, not an error. Codes documented in the header, --help range extended to show them.

Also

  • hooks/session-audit.mjs, wired as a Stop hook: a session that published without a gate says so at the end. It reports ungated publishes only. skill-audit also emits unchained — publishes where the gate is not part of the same shell command — but the gate is a PreToolUse hook and fires out-of-band on every write by construction, so every publish is unchained and that count measures the enforcement mechanism rather than any defect.

    Its own first version treated skill-audit's exit 1 as an error, when the exit code is the verdict — so it bailed on exactly the sessions it exists to report on and would have read as clean forever. Caught by the positive control.

  • evidence and attest frontmatter folded to block scalars: description was a plain YAML scalar containing ": ", so it did not parse. tools/install masked it by folding on the way out, so the installed copy parsed and the source did not. The performance skill has the same defect but lives on main, so it is fixed in fix(install): don't destroy local changes on sync #111.

    The folding has to preserve the description exactly, since the installer reads it back out. Reproduce the round-trip on this branch:

    tools/install --target /tmp/fm-check --maturity experimental --include pr-workflow/evidence
    python3 -c "import yaml,sys; a=yaml.safe_load(open('domains/pr-workflow/skills/evidence/skill.md').read().split('---')[1])['description']; b=yaml.safe_load(open('/tmp/fm-check/.claude/skills/mms-evidence/SKILL.md').read().split('---')[1])['description']; print(' '.join(a.split())==' '.join(b.split()))"
    

    Prints True.

  • A verdict inside a fenced code block is now treated as a quotation, not a claim. Broadening the trigger made it fire on text about the trigger — this very PR, quoting the two verdict forms to show what matches, was classified as a validation run and asked for the whole envelope. Documenting the rule became a violation of it.

node --test test/*.test.mjs — 61/61.


⚠️ Open question this surfaced — needs a decision, not from me

Once the trigger actually fires, attest-gate runs against bodies it never saw before. One real, already-published body is now blocked:

[ci-restatement] "actions/runs/31036153698"
  needs: removal — a validation surface carries zero CI references
  in: **Same result, from GitHub's own CI** (not a local run) — [Unit tests (3)](...)

That body cites a CI run as the measurement, not as "tests are green at head". And two rules in this package disagree about whether that is allowed:

  • evidence-run.yml exists precisely to move measurement into CI — "the run URL is the capture" — and its whole design is that a local run is self-attested and a CI run is not.
  • attest-gate check 6 says a validation surface carries zero CI references.

Under the second rule, evidence produced by the first rule cannot be published. That was invisible while the trigger was broken, because the check never ran.

I have deliberately not resolved this by weakening either rule — the distinction between "CI ran my measurement, here is the run" and "CI is green, therefore correct" is real, and where to draw it belongs to whoever owns check 6. Flagging rather than closing, per non-negotiable 10.

The ARTIFACT and OBS_ARTIFACT classes matched `https?://\S+` and a bare
filename pattern, so any URL-shaped string discharged them. The same
generator writes the claim and the string that satisfies the check, which
makes presence carry no information about whether the evidence exists.

Measured against the unpatched hook, with the claim and the artifact in the
same block (proximity matters — the checks are unit-scoped):

  https://example.invalid/nope/capture.png        ALLOWED
  `src/totally/made-up.test.ts:42`                ALLOWED
  `evidence/never-captured.png`                   ALLOWED

An artifact now counts only if the author could not have authored its bytes:
a namespace where CI, the upload endpoint or an observability backend writes
them, or a local path that is actually on disk. github.com is author-writable
in general, so only /actions/runs/, /user-attachments/, /blob/, /commit/ and
/pull/ under it qualify.

Controls, before -> after:
  fabricated (want blocked)   1/3 -> 3/3
  genuine    (want allowed)   2/2 -> 2/2

Replaying real published bodies found the interesting case: evidence hosted
in an author-controlled S3 bucket is now refused, correctly — fetching it
unauthenticated proves it is fetchable, not that the author did not write it.
Rather than decide that silently, EVIDENCE_GATE_ARTIFACT_HOSTS registers such
hosts explicitly. Doing so is a visible downgrade from independent to merely
fetchable, which is the point: it should be a choice someone made, not a
property of the regex.

  strict default        -> those bodies block
  bucket registered     -> those bodies pass, fabrications still block

The rule is presence stays necessary and stops being sufficient. Note this is
still not resolution: nothing yet fetches the URL and checks for a 200. An
allowlisted-but-nonexistent /actions/runs/99999999999 link is refused by the
separate CI-restatement rule rather than by this one.
The hook already ran attest-gate.sh in-process on the exact bytes about to be
published, which is a stronger binding than recording a verdict against a
hash — there is no window in which the artifact can be edited after the gate
passes. Two defects meant it often did not run, or ran the wrong gate.

1. _is_evidence_artifact matched `^\*\*Verdict:\*\*`, so where the bold
   stopped decided whether thirteen checks ran:

     **Verdict:** proven     -> all 13 checks
     **Verdict: proven**     -> none

   The same sentence, rendered identically, one of them silently unenforced.
   Now matches a line-leading bolded Verdict however the emphasis falls, still
   anchored so prose mentioning the word does not drag an ordinary reply in.

2. _find_gate ranked $ATTEST_GATE last, behind three default paths. An
   override that loses to a default is not an override: a control run pointing
   it at a stand-in silently exercised the installed gate and reported on that
   instead. This is the failure the skill's own non-negotiable 9 describes —
   an instrument reporting the instruction it was given rather than the effect
   it had — and it was in the resolver for the gate itself.

Verified by substituting a stand-in gate whose exit code is controlled, with
the hook copied to a directory with no sibling scripts/ (as gate-controls.sh
does, for exactly this reason):

                          before   after
  stand-in exits 0        ALLOWED  ALLOWED
  stand-in exits 1        ALLOWED  BLOCKED
  gate path nonexistent   ALLOWED  BLOCKED

gate-controls.sh: all eleven arms behave, including the three negative arms
that catch a gate which has started blocking everything.
…ntmatter mean something

Three controls that were present and inert.

1. evidence-run.yml's determinism check emitted ::warning:: and carried
   continue-on-error, so the step whose own message reads "do not publish
   these numbers" went green and the numbers published. Now ::error:: plus
   exit 1, with continue-on-error removed from that step only. The two runner
   steps keep it, because there the exit code is the verdict and a finding is
   not a failure. Here a difference means the instrument did not return the
   same answer twice, so neither answer is publishable.

   Verified by extracting the step and running it: identical arms exit 0 —
   including the label/log/logs/env fields it deletes by design, which is the
   false positive that taught operators to publish through it — and differing
   arms exit 1.

2. selector-recompute.sh returned 0 for every outcome including
   "VALUE UNSTABLE" and "probe-failed", so a caller gating on the exit code
   saw green on a run whose own artifact says the number is not meaningful.
   The only thing between that and publication was attest-gate happening to
   grep the verdict string out of the prose.

   Now 4 for VALUE UNSTABLE and 5 for probe-failed, with 0 kept for both real
   measurements — a selector that recomputes is a result, not an error. Codes
   documented in the header, and the --help range extended so it shows them.

3. Three of fifty skill.md frontmatters were invalid YAML: `description` was a
   plain scalar containing ": ", which YAML reads as a nested mapping. Any
   tool parsing source frontmatter fails on them; tools/install masked it by
   folding the value to a block scalar on the way out, so the installed copy
   parsed and the source did not.

   Folds evidence and attest to block scalars at rest. The performance skill
   has the same defect but lives on main, so it is fixed in the tools/install
   branch rather than here.

   Round-trip checked: the installed description is byte-identical to the
   source description after folding.

`node --test test/*.test.mjs` 61/61.
… a claim

Broadening the artifact trigger made it fire on text ABOUT the trigger. A pull
request quoting `**Verdict:** proven` to show what the gate matches was
classified as a validation run and asked for the whole envelope, so
documenting the rule became a violation of it. Fenced blocks are stripped
before the trigger is tested.

  real verdict in prose                    -> artifact
  real verdict, bold-wrapped               -> artifact
  verdict ONLY inside a fence              -> not an artifact
  fenced example alongside a real one      -> artifact
  ordinary reply                           -> not an artifact
  explicit VALIDATION_RUN marker           -> artifact

gate-controls.sh 11/11; chaining controls still verified; fabricated-artifact
controls still 3/3 blocked.

Surfaced by the trigger now working: a real published body is blocked by the
CI-restatement rule, which had never run against it because the old trigger
did not match its verdict line. That rule and this package's own direction
disagree, and the conflict is left for the owner rather than resolved here —
see the pull request description.
The proposal was to cut the prose: every principle with a hook class becomes
one line pointing at the class, leaving only irreducible judgement in text.
That assumes the class exists and fires, and a check NAMED after a principle
is not the same as a check that catches a violation of it.

scripts/principle-coverage.py decides it by measurement. It builds a body
violating one principle and nothing else, publishes it past the gate, and
reports whether the gate blocked it. The clean baseline is a positive control:
if it does not pass, every result is uninterpretable and the script says so —
the first version of this measurement reported 9/9 enforced on a baseline that
was itself blocked.

Result: 3 of 9 enforced, 6 exist only as prose, including items 7, 8 and 9,
which are the three most recently learned. Cutting those would delete the only
place the rule exists, so the cut is not made.

Item 2 is the subtle one and is recorded as such: check 8 keys on a provenance
marker being present anywhere in the body, so a verdict reached by reading
passes as long as an artifact sits nearby. The check is a proxy for the rule.

What the measurement is actually good for is the inverse of the proposal — it
names the six checks worth building, in priority order, and it can be re-run
after each one to show the number move.

`node --test` 61/61; gate-controls 11/11; the output quoted in skill.md is
reproduced by running the script.
@MajorLift

Copy link
Copy Markdown
Contributor Author

Added scripts/principle-coverage.py (cf462ba) — and with it, the reason the prose cut is not in this PR.

What was proposed

Cut the skill text to what no gate can check: every principle with a hook class becomes one line pointing at the class, leaving only irreducible judgement in prose. The argument being that 671 lines of principle are measurably not what produces compliance.

Why it isn't done

That assumes the class exists and fires. A check named after a principle is not the same as a check that catches a violation of it, and nothing had ever tested the difference.

The script decides it by measurement: build a body that violates one principle and nothing else, push it at the gate, record whether it blocks. The clean baseline is a positive control — if it doesn't pass, every result below it is uninterpretable.

CONTROL (clean body, must be ALLOWED): ALLOWED

  ENFORCED    1 artifact the reader can check      caught by ['attest-gate', 'verdict']
  unenforced  2 `proven` requires execution        publishes cleanly
  ENFORCED    3 no 'what would close it'           caught by ['attest-gate']
  unenforced  4 write to the reviewer              publishes cleanly
  ENFORCED    5 drop test-quality-only findings    caught by ['attest-gate']
  unenforced  6 route privacy/security findings    publishes cleanly
  unenforced  7 measure the PR range               publishes cleanly
  unenforced  8 the label on a number              publishes cleanly
  unenforced  9 instrument reports what it did     publishes cleanly

3/9 enforced, 6/9 exist only as prose

3 of 9. The six unenforced ones include items 7, 8 and 9 — the three most recently learned, each written after a specific run shipped wrong. For those, the prose is not a restatement of a check. It is the only place the rule exists, and cutting it deletes the rule rather than the redundancy.

Item 2 is worth naming separately: check 8 keys on a provenance marker being present anywhere in the body, so a verdict reached by reading passes as long as an artifact sits nearby. The check is a proxy for the rule, not the rule.

The instrument's own control caught it

The first version of this measurement reported 9/9 enforced. That was wrong in the most flattering possible direction: the clean baseline was itself blocked, so every case was blocked for the baseline's reason and every one read as a success. It now refuses to interpret results when the control fails.

That is the same failure as non-negotiable 9 — an instrument reporting the instruction it was given rather than the effect it had — which is the third time that shape has turned up in this package's own tooling this week, after $ATTEST_GATE ranking last in its own resolver and session-audit.mjs treating "found something" as "errored".

What it's actually for

The inverse of the proposal. The measurement names the six checks worth building, and re-running it after each one shows the number move. A coverage figure that a script maintains does not depend on anyone remembering to keep it honest — which was the original complaint about the prose.

The rule is that a validation surface does not restate CI status: "tests are
green at head <sha>" hands the reviewer their own Checks tab back and carries
no information. The implementation matched a bare `actions/runs/N`, which is a
different thing — five of its six branches described a CLAIM about CI, and one
described a URL.

So a link to a specific run and job whose log holds the figure being reported
was refused, and that is precisely what evidence-run.yml exists to produce:
"move the measurement to CI, where the run URL is the capture". The package
forbade its own flagship output, and a real published body was blocked by it.

A run link is now a violation only when it carries restatement language with
it. The distinction is whether the sentence asserts a status the Checks tab
already shows, or points at an execution whose output the Checks tab does not.

scripts/ci-citation-controls.py holds the line, with five restatements that
must be caught and three citations that must be allowed. It imports
CI_RESTATEMENT from the hook rather than restating it, because a control that
tests its own copy of a pattern passes forever while the real one drifts.

  before: 3 of 8 misclassified — every citation a false positive
  after:  0 of 8
  the same controls against the old pattern still catch the citations, so the
  control has power rather than passing by construction

gate-controls 11/11; replaying real published bodies now shows zero
regressions, where the CI rule previously blocked one; principle-coverage
unchanged at 3/9; `node --test` 61/61.
@MajorLift

Copy link
Copy Markdown
Contributor Author

Resolved in 4a5b6c3 — the open question above was the wrong question. The rule and the workflow don't conflict; the implementation was broader than the rule.

The rule vs. what was matched

The rule is that a validation surface does not restate CI status. CI_RESTATEMENT had six branches:

actions/runs/\d+                                  <- a URL
checks? tab
green (at head|in )
all (tests|checks|jobs) (pass\w*|green)           <- claims about CI
CI (is )?green
\d+ pass(ing|ed)? / \d+ fail\w*

Five describe a claim. One describes a link. Those are different things, and only the claim is what the rule is about.

  • "Tests are green at head 7bfc16c" — hands the reviewer their own Checks tab back. Zero information.
  • "The probe ran in CI: .../actions/runs/123/job/456 printed identical=3 unrelated=1" — carries the entire measurement. The Checks tab shows pass/fail; it does not show that.

The second is exactly what evidence-run.yml is designed to produce — "move the measurement to CI, where the run URL is the capture". Matching the bare URL meant the package refused its own flagship output, which is why a real published body was blocked.

Fix

A run link is a violation only when it carries restatement language with it. Measured over five restatements that must be caught and three citations that must be allowed:

current proposed
misclassified 3 / 8 — every citation a false positive 0 / 8

scripts/ci-citation-controls.py holds the line. It imports CI_RESTATEMENT from the hook rather than restating it, because a control that tests its own copy of a pattern passes forever while the real one drifts. Run against the old pattern the same controls still catch the citations, so the control has power rather than passing by construction.

Replaying real published bodies: zero regressions, where the CI rule previously blocked one. gate-controls 11/11, principle-coverage unchanged at 3/9, node --test 61/61.

What generalises

This is the fourth instance this week of the same shape — a check whose name matches a principle while its implementation matches something adjacent and cheaper to detect. ARTIFACT matched "a URL is present" for "the reader can verify this". _is_evidence_artifact matched one bolded spelling for "this is a validation run". $ATTEST_GATE resolved to whichever gate came first rather than the one named. Each passed review because the name reads correctly.

principle-coverage.py and this script are both instances of the only thing that catches it: state the rule as inputs that must be caught and inputs that must be allowed, and check the implementation against both halves. The negative half is where all four hid.

@MajorLift
MajorLift merged commit 4a5b6c3 into MetaMask:jongsun/add/pr-validate-falsifying-test Aug 7, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant