From bb6e744d238ebda074746ad2b18808a744871b7b Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 6 Aug 2026 07:03:20 -0400 Subject: [PATCH 1/6] fix(evidence-gate): require artifacts to be credible, not merely present MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../skills/evidence/hooks/pr-evidence-gate.py | 81 ++++++++++++++++++- 1 file changed, 78 insertions(+), 3 deletions(-) diff --git a/domains/pr-workflow/skills/evidence/hooks/pr-evidence-gate.py b/domains/pr-workflow/skills/evidence/hooks/pr-evidence-gate.py index b79d0e2..f819998 100755 --- a/domains/pr-workflow/skills/evidence/hooks/pr-evidence-gate.py +++ b/domains/pr-workflow/skills/evidence/hooks/pr-evidence-gate.py @@ -473,6 +473,81 @@ def _add(violations, kind, token, unit): }) +# ── artifact credibility ─────────────────────────────────────────────────── +# Matching a URL shape only proves someone typed a URL, and the same generator +# writes the claim and the string that satisfies the check — so presence alone +# carries no information. An artifact counts only if the author could not have +# authored its contents: a namespace where the bytes are written by CI, by the +# upload endpoint, or by an observability backend; or a local path that is +# actually on disk. Presence stays necessary and stops being sufficient. +ARTIFACT_HOST_ALLOWLIST = ( + "github.com/", # narrowed by ARTIFACT_PATH_ALLOWLIST below + "user-images.githubusercontent.com/", + "gist.github.com/", + "sentry.io/", + "grafana.net/", + "grafana.com/", +) +# github.com is author-writable in general (a branch, a wiki, a comment anchor), +# so only the sub-namespaces whose bytes CI or the upload endpoint produce count. +ARTIFACT_PATH_ALLOWLIST = ( + "/actions/runs/", + "/user-attachments/", + "/blob/", + "/commit/", + "/pull/", +) +# Escape hatch for hosting the author does control — an artifact bucket, an +# internal dashboard. Registering one is a deliberate, visible downgrade: the +# artifact becomes fetchable rather than independent, and a reader who trusts +# it is trusting the author. Comma-separated substrings. +# EVIDENCE_GATE_ARTIFACT_HOSTS=my-bucket.s3.amazonaws.com,dash.internal +_EXTRA_HOSTS = tuple( + h.strip().lower() + for h in (os.environ.get("EVIDENCE_GATE_ARTIFACT_HOSTS") or "").split(",") + if h.strip() +) +_URL_RE = re.compile(r"https?://[^\s)>\]\"']+") +_LOCAL_REF_RE = re.compile( + r"`?([\w./-]+\.(?:test|spec)\.[tj]sx?)(?::\d+)?`?" + r"|`?([\w./-]+\.(?:png|jpe?g|gif|mp4|webm|har|log|json))`?" +) + + +def _url_is_credible(url): + low = url.lower() + if _EXTRA_HOSTS and any(h in low for h in _EXTRA_HOSTS): + return True + if not any(h in low for h in ARTIFACT_HOST_ALLOWLIST): + return False + if "github.com/" in low and "githubusercontent" not in low and "gist." not in low: + return any(p in low for p in ARTIFACT_PATH_ALLOWLIST) + return True + + +def _local_ref_exists(ref): + if os.path.isabs(ref): + return os.path.exists(ref) + for root in (os.getcwd(), os.environ.get("CLAUDE_PROJECT_DIR") or ""): + if root and os.path.exists(os.path.join(root, ref)): + return True + return False + + +def _has_credible_artifact(unit, pattern): + """True only if this unit carries an artifact the author could not fabricate.""" + if not pattern.search(unit): + return False + for url in _URL_RE.findall(unit): + if _url_is_credible(url): + return True + for m in _LOCAL_REF_RE.finditer(unit): + ref = m.group(1) or m.group(2) + if ref and _local_ref_exists(ref): + return True + return False + + def _positive_verdict(unit): """A non-negated verdict token in this unit, or None.""" for m in VERDICT.finditer(unit): @@ -483,14 +558,14 @@ def _positive_verdict(unit): def _scan_unit(unit, violations): # ── VERDICT: excused by a co-located inspectable artifact. - if not ARTIFACT.search(unit): + if not _has_credible_artifact(unit, ARTIFACT): tok = _positive_verdict(unit) if tok: _add(violations, "verdict", tok, unit) # ── OBSERVATION: needs an observation-class artifact. A /blob/ code # permalink does NOT excuse it. - if not OBS_ARTIFACT.search(unit): + if not _has_credible_artifact(unit, OBS_ARTIFACT): for m in OBSERVATION.finditer(unit): if _negated(unit, m.start()): continue @@ -533,7 +608,7 @@ def _scan_unit(unit, violations): # ── BARE IDENTIFIER (item 12): an id with no resolving link and no # re-hosted capture is a digging assignment. - if not RESOLVER.search(unit) and not OBS_ARTIFACT.search(unit): + if not RESOLVER.search(unit) and not _has_credible_artifact(unit, OBS_ARTIFACT): bm = BARE_ID.search(unit) if bm: _add(violations, "bare-identifier", bm.group(0), unit) From f401caa0525da5d9acb97e74c116c775613a186d Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 6 Aug 2026 07:09:00 -0400 Subject: [PATCH 2/6] fix(evidence-gate): chain the publish to the gate's verdict for real MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../skills/evidence/hooks/pr-evidence-gate.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/domains/pr-workflow/skills/evidence/hooks/pr-evidence-gate.py b/domains/pr-workflow/skills/evidence/hooks/pr-evidence-gate.py index f819998..4586ce6 100755 --- a/domains/pr-workflow/skills/evidence/hooks/pr-evidence-gate.py +++ b/domains/pr-workflow/skills/evidence/hooks/pr-evidence-gate.py @@ -323,10 +323,14 @@ def _repo_pr_from_cmd(cmd): def _find_gate(): here = os.path.dirname(os.path.abspath(__file__)) for cand in ( + # An explicit override that loses to a default is not an override. This + # ranked last, so a control run pointing ATTEST_GATE at a stand-in gate + # silently exercised the installed one instead and reported on it — the + # test looked like it passed and measured the wrong binary. + os.environ.get("ATTEST_GATE", ""), os.path.join(here, "..", "scripts", "attest-gate.sh"), os.path.join(here, "attest-gate.sh"), os.path.expanduser("~/.claude/skills/mms-evidence/scripts/attest-gate.sh"), - os.environ.get("ATTEST_GATE", ""), ): if cand and os.path.isfile(cand): return os.path.abspath(cand) @@ -345,10 +349,19 @@ def _find_gate(): ) +# Where the bold stops is not a fact about the claim. `**Verdict:** proven` +# ran all thirteen checks and `**Verdict: proven**` ran none — the same +# sentence, rendered the same way, one of them silently unenforced. Match the +# bolded Verdict lead however the emphasis falls, while staying anchored to a +# line-leading bold run so that mentioning the word in prose still does not +# drag a normal comment into the gate. +_VERDICT_LEAD = re.compile(r"^\s*\*\*\s*Verdict\b[^*\n]*\*\*", re.M | re.I) + + def _is_evidence_artifact(body): if any(m in body for m in ARTIFACT_MARKERS): return True - return bool(re.search(r"^\*\*Verdict:\*\*", body, re.M)) + return bool(_VERDICT_LEAD.search(body)) def _run_attest_gate(body, cmd): From 966170f226fb8f7496d9561157cfad0e9e7a4ef1 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 6 Aug 2026 07:40:47 -0400 Subject: [PATCH 3/6] fix(evidence): make the determinism check, the exit codes and the frontmatter mean something MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- domains/pr-workflow/skills/attest/skill.md | 3 +- .../skills/evidence/assets/evidence-run.yml | 15 +++- .../skills/evidence/hooks/session-audit.mjs | 74 +++++++++++++++++++ .../evidence/scripts/selector-recompute.sh | 25 ++++++- domains/pr-workflow/skills/evidence/skill.md | 3 +- 5 files changed, 115 insertions(+), 5 deletions(-) create mode 100755 domains/pr-workflow/skills/evidence/hooks/session-audit.mjs diff --git a/domains/pr-workflow/skills/attest/skill.md b/domains/pr-workflow/skills/attest/skill.md index e5af6d8..b6dc460 100644 --- a/domains/pr-workflow/skills/attest/skill.md +++ b/domains/pr-workflow/skills/attest/skill.md @@ -1,6 +1,7 @@ --- name: attest -description: The gate an evidence artifact passes before it is published to a pull request, issue or shared tracker. Two halves that do not substitute for each other — a mechanical pass that greps for the properties a reader needs (marker pair, pinned environment, a captured artifact rather than typed prose, a destination that is still open) and a dispatched pass sent to fresh instances that contest the framing, the coverage, and how it reads to a stranger. The author is the wrong checker: they remember running the check, and the memory supplies the provenance the text lacks. Verdicts are attested, attested with named caveats, blocked, or not a run — the last being common and legitimate, because a run that could not execute has produced nothing to publish. Triggers on mms-attest, or before posting any evidence, validation or diligence output to a public surface. +description: >- + The gate an evidence artifact passes before it is published to a pull request, issue or shared tracker. Two halves that do not substitute for each other — a mechanical pass that greps for the properties a reader needs (marker pair, pinned environment, a captured artifact rather than typed prose, a destination that is still open) and a dispatched pass sent to fresh instances that contest the framing, the coverage, and how it reads to a stranger. The author is the wrong checker: they remember running the check, and the memory supplies the provenance the text lacks. Verdicts are attested, attested with named caveats, blocked, or not a run — the last being common and legitimate, because a run that could not execute has produced nothing to publish. Triggers on mms-attest, or before posting any evidence, validation or diligence output to a public surface. maturity: experimental --- diff --git a/domains/pr-workflow/skills/evidence/assets/evidence-run.yml b/domains/pr-workflow/skills/evidence/assets/evidence-run.yml index 1a65b24..770dfa0 100644 --- a/domains/pr-workflow/skills/evidence/assets/evidence-run.yml +++ b/domains/pr-workflow/skills/evidence/assets/evidence-run.yml @@ -224,7 +224,12 @@ jobs: # Contention produced numbers that were published and then retracted. Running the # head arm twice and diffing costs one repeat and turns that into a pre-publish # signal rather than a correction. - continue-on-error: true + # + # Deliberately NOT continue-on-error, unlike the two runner steps above. There the + # exit code is the verdict and a finding is not a failure. Here a difference is not + # a finding about the code — it says the instrument did not return the same answer + # twice, so neither answer can be published. With continue-on-error the step's own + # `exit 1` was swallowed and the run went green anyway. env: RUNNER: ${{ inputs.runner }} ARGS: ${{ inputs.args }} @@ -246,8 +251,14 @@ jobs: <(jq -S 'del(.env, .label, .log, .logs)' "$B") > determinism.diff; then echo "deterministic across two runs" | tee -a "$GITHUB_STEP_SUMMARY" else - echo "::warning::runner is NOT deterministic at this ref — do not publish these numbers" + # Hard fail, not a warning. The instruction this step emits is "do not + # publish these numbers", and a warning cannot enforce it — the run goes + # green, the artifact is produced, and the numbers publish anyway. A + # control whose only effect is advisory text is the failure this whole + # package is about, restated one level up. + echo "::error::runner is NOT deterministic at this ref — these numbers are not publishable" cat determinism.diff >> "$GITHUB_STEP_SUMMARY" + exit 1 fi fi diff --git a/domains/pr-workflow/skills/evidence/hooks/session-audit.mjs b/domains/pr-workflow/skills/evidence/hooks/session-audit.mjs new file mode 100755 index 0000000..52f84b9 --- /dev/null +++ b/domains/pr-workflow/skills/evidence/hooks/session-audit.mjs @@ -0,0 +1,74 @@ +#!/usr/bin/env node +// +// Stop hook: say, at the end of a session, whether anything published without a gate. +// +// The corpus's own diagnostic for a rule that keeps being violated is "count repeats within +// a session — three fires means the rule is inert". Nothing was counting. The canonical file +// for the largest failure cluster failed to maintain its own recurrence count, which is the +// same class of failure it documents: a number that depends on someone remembering to +// increment it is not a measurement. A script does not forget. +// +// It reports UNGATED publishes — an outward-facing write with no gate invocation anywhere +// earlier in the session. That is the real signal and it is deterministic. +// +// It deliberately does NOT report "unchained" publishes, which skill-audit also emits. A +// publish is unchained when the gate is not part of the same shell command. Since the gate +// is wired as a PreToolUse hook it fires out-of-band on every write by construction, so it +// is never in the command, so every publish is unchained. Counting those produces a large +// number that measures the enforcement mechanism rather than any defect — which is exactly +// how an audit comes to report thousands of violations of a rule that is being enforced. +// +// Contract: reads Stop-hook JSON on stdin, writes a note to stderr, always exits 0. It is a +// report, not a gate; blocking the end of a session teaches nothing the note does not. +import { spawnSync } from 'node:child_process'; +import { existsSync } from 'node:fs'; + +const AUDIT_CANDIDATES = [ + process.env.SKILL_AUDIT, + `${process.env.HOME}/Code/metamask/skills/tools/skill-audit.mjs`, + `${process.env.HOME}/.claude/skills/mms-evidence/hooks/skill-audit.mjs`, +].filter(Boolean); + +let stdin = ''; +process.stdin.setEncoding('utf8'); +for await (const chunk of process.stdin) stdin += chunk; + +let transcript; +try { + transcript = JSON.parse(stdin || '{}').transcript_path; +} catch { + process.exit(0); +} +if (!transcript || !existsSync(transcript)) process.exit(0); + +const audit = AUDIT_CANDIDATES.find((p) => existsSync(p)); +if (!audit) process.exit(0); + +const proc = spawnSync(process.execPath, [audit, transcript, '--json'], { + encoding: 'utf8', + timeout: 30_000, +}); +// skill-audit exits 1 when it FINDS something — the exit code is the verdict, not an +// error. Treating non-zero as failure made this hook bail on precisely the sessions it +// exists to report on, and report nothing on all the others, so it would have read as +// "clean" forever. Parse whatever it printed and let the payload decide. +if (!proc.stdout) process.exit(0); + +let report; +try { + report = JSON.parse(proc.stdout); +} catch { + process.exit(0); +} + +const ungated = report.ungatedPublishLines ?? []; +if (ungated.length === 0) process.exit(0); + +process.stderr.write( + `evidence audit: ${ungated.length} outward-facing publish(es) ran with no gate ` + + `anywhere earlier in this session.\n` + + ` transcript lines: ${ungated.slice(0, 12).join(', ')}` + + `${ungated.length > 12 ? `, … +${ungated.length - 12} more` : ''}\n` + + ` ${report.publishes} publish(es), ${report.gateRuns} gate run(s) total.\n`, +); +process.exit(0); diff --git a/domains/pr-workflow/skills/evidence/scripts/selector-recompute.sh b/domains/pr-workflow/skills/evidence/scripts/selector-recompute.sh index 029d540..1f8359f 100755 --- a/domains/pr-workflow/skills/evidence/scripts/selector-recompute.sh +++ b/domains/pr-workflow/skills/evidence/scripts/selector-recompute.sh @@ -26,6 +26,13 @@ # --module ui/selectors/multichain-accounts/account-tree \ # --export getWalletsWithAccounts \ # --fixture test/data/mock-state.json --slice metamask --perturb pinnedAccountList +# +# Exit codes — the code is the verdict, so a finding and a failure to measure differ: +# 0 measured: "narrowed" or "recomputes on unrelated writes" +# 2 no reading extracted from the probe output +# 3 usage error +# 4 VALUE UNSTABLE — correctness precondition failed, the count is not meaningful +# 5 probe-failed — the instrument produced no reading set -uo pipefail # A run's artifact has to say whether a reader can verify it. In CI the run URL is that @@ -53,7 +60,7 @@ while [ $# -gt 0 ]; do --n) N="${2:-}"; shift 2 ;; --label) LABEL="${2:-}"; shift 2 ;; --out) OUT_DIR="${2:-}"; shift 2 ;; - -h|--help) sed -n '2,27p' "$0"; exit 0 ;; + -h|--help) sed -n '2,35p' "$0"; exit 0 ;; *) die "unknown argument: $1" ;; esac done @@ -202,4 +209,20 @@ printf 'limits: one fixture, one perturbed key. A selector unmoved here can stil under state this fixture does not reach, and the count says nothing about the cost of each recomputation.\n' >&2 [ -n "$A" ] || exit 2 + +# The exit code is the verdict, and it has to distinguish a finding from a failure to +# measure. Both of the cases below were exit 0, so a caller gating on the exit code saw +# green on a run whose own artifact says the number is not meaningful — and the only thing +# standing between that and publication was attest-gate happening to grep the verdict +# string out of the prose. +# +# 0 a real measurement: narrowed, or recomputes on unrelated writes. A finding is not +# a failure, and a selector that recomputes is a result, not an error. +# 4 VALUE UNSTABLE — the correctness precondition failed. The count is not meaningful, +# so there is no measurement here to gate on. +# 5 probe-failed — the instrument did not produce a reading at all. +case "$VERDICT" in + "VALUE UNSTABLE"*) exit 4 ;; + "probe-failed") exit 5 ;; +esac exit 0 diff --git a/domains/pr-workflow/skills/evidence/skill.md b/domains/pr-workflow/skills/evidence/skill.md index 78ae160..d7b6547 100644 --- a/domains/pr-workflow/skills/evidence/skill.md +++ b/domains/pr-workflow/skills/evidence/skill.md @@ -1,6 +1,7 @@ --- name: evidence -description: Produce reviewer-grade evidence that a claim is true — or that it is not. Matches the evidence to the specific falsifiable claim rather than running a fixed checklist, across a catalog of 41 lanes: before/after screenshots, falsifying regression tests, render and selector proofs, bundle and LavaMoat diffs, Sentry and Tempo links, state-migration and vault checks, plus the Autonomous Engineering Platform (AEP) harness for autonomous visual and perf capture. Assembles an evidence bundle and publishes it, images re-hosted and local paths scrubbed. Runs three ways: on a PR whose claim someone else made, in the inner loop against uncommitted changes before a reviewer sees them, and on a symptom with no claim yet, where the hypothesis to kill is your own. Triggers on the evidence command and its subcommands (visual, perf, preflight, status, plan, lane, compare) — installed as mms-evidence — or when the user mentions validating or proving a PR, capturing evidence, before/after screenshots, a screen recording for a PR, attaching Sentry or DevTools output as proof, AEP or visual/perf validation, or publishing an evidence bundle. +description: >- + Produce reviewer-grade evidence that a claim is true — or that it is not. Matches the evidence to the specific falsifiable claim rather than running a fixed checklist, across a catalog of 41 lanes: before/after screenshots, falsifying regression tests, render and selector proofs, bundle and LavaMoat diffs, Sentry and Tempo links, state-migration and vault checks, plus the Autonomous Engineering Platform (AEP) harness for autonomous visual and perf capture. Assembles an evidence bundle and publishes it, images re-hosted and local paths scrubbed. Runs three ways: on a PR whose claim someone else made, in the inner loop against uncommitted changes before a reviewer sees them, and on a symptom with no claim yet, where the hypothesis to kill is your own. Triggers on the evidence command and its subcommands (visual, perf, preflight, status, plan, lane, compare) — installed as mms-evidence — or when the user mentions validating or proving a PR, capturing evidence, before/after screenshots, a screen recording for a PR, attaching Sentry or DevTools output as proof, AEP or visual/perf validation, or publishing an evidence bundle. --- # /evidence From f8c9e51ccfb1d4921432c4e0ba4e0a89a0628a0f Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 6 Aug 2026 07:45:41 -0400 Subject: [PATCH 4/6] fix(evidence-gate): a verdict inside a code fence is a quotation, not a claim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../skills/evidence/hooks/pr-evidence-gate.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/domains/pr-workflow/skills/evidence/hooks/pr-evidence-gate.py b/domains/pr-workflow/skills/evidence/hooks/pr-evidence-gate.py index 4586ce6..f7331b4 100755 --- a/domains/pr-workflow/skills/evidence/hooks/pr-evidence-gate.py +++ b/domains/pr-workflow/skills/evidence/hooks/pr-evidence-gate.py @@ -358,10 +358,18 @@ def _find_gate(): _VERDICT_LEAD = re.compile(r"^\s*\*\*\s*Verdict\b[^*\n]*\*\*", re.M | re.I) +_FENCE = re.compile(r"^```.*?^```", re.M | re.S) + + def _is_evidence_artifact(body): if any(m in body for m in ARTIFACT_MARKERS): return True - return bool(_VERDICT_LEAD.search(body)) + # A verdict line inside a fenced block is an example of one, not one. Writing about + # this gate — a PR that quotes `**Verdict:** proven` to show what triggers it — was + # otherwise classified as a validation run and asked for the whole envelope. The + # trigger has to be able to tell a claim from a quotation of a claim, or documenting + # the rule becomes a violation of it. + return bool(_VERDICT_LEAD.search(_FENCE.sub("", body))) def _run_attest_gate(body, cmd): From cf462ba5c121f2f828135be0bbc91a73dcfb236c Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 6 Aug 2026 08:16:54 -0400 Subject: [PATCH 5/6] feat(evidence): measure which non-negotiables the gate actually enforces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../scripts/principle-coverage-baseline.md | 13 +++ .../evidence/scripts/principle-coverage.py | 82 +++++++++++++++++++ domains/pr-workflow/skills/evidence/skill.md | 26 ++++++ 3 files changed, 121 insertions(+) create mode 100644 domains/pr-workflow/skills/evidence/scripts/principle-coverage-baseline.md create mode 100755 domains/pr-workflow/skills/evidence/scripts/principle-coverage.py diff --git a/domains/pr-workflow/skills/evidence/scripts/principle-coverage-baseline.md b/domains/pr-workflow/skills/evidence/scripts/principle-coverage-baseline.md new file mode 100644 index 0000000..0a030bc --- /dev/null +++ b/domains/pr-workflow/skills/evidence/scripts/principle-coverage-baseline.md @@ -0,0 +1,13 @@ + +## 🧪 Validation Run + +> Trial run of an experimental evidence skill — feedback via MetaMask/skills. + +**Verdict:** proven — **Claim:** the selector stops recomputing on unrelated writes. +Measured 3 recomputations before and 1 after: ![capture](https://github.com/user-attachments/assets/1f2e3d4c-aaaa-bbbb-cccc-ddddeeeeffff) — `evidence-artifacts/recompute.json` + +Environment: head `7bfc16c`, node `v20.11.0`. + +Not covered by this run: one fixture, one perturbed key; a selector unmoved here can still +recompute under state this fixture does not reach. + diff --git a/domains/pr-workflow/skills/evidence/scripts/principle-coverage.py b/domains/pr-workflow/skills/evidence/scripts/principle-coverage.py new file mode 100755 index 0000000..e3ce05e --- /dev/null +++ b/domains/pr-workflow/skills/evidence/scripts/principle-coverage.py @@ -0,0 +1,82 @@ +"""Which of the skill's non-negotiables does the machinery actually enforce? + +The proposal was "every principle with a hook class becomes one line pointing at the +class". 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, so this asks the only question that +decides what is safe to delete: construct a body that violates the principle and nothing +else, and see whether the gate blocks it. + +A principle that survives here is enforced, and its prose can shrink to a pointer. +A principle that does not is the ONLY place the rule exists, and deleting it deletes +the rule. +""" +import json +import os +import subprocess +import sys +import tempfile + +GATE = sys.argv[1] if len(sys.argv) > 1 else os.path.join( + os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "hooks", "pr-evidence-gate.py") +D = os.path.dirname(os.path.abspath(__file__)) +PATH = os.path.join(tempfile.mkdtemp(), "case.md") +PUBLISH = " ".join(["gh", "pr", "comment", "45249", "--repo", + "MetaMask/metamask-extension", "--body-file"]) + +# A body that PASSES everything, so each case below differs by one violation only. +CLEAN = open(os.path.join(D, "principle-coverage-baseline.md")).read() +_A = "Measured 3 recomputations before and 1 after" +_L = "Not covered by this run: one fixture, one perturbed key; a selector unmoved here can still" + +# Each: (principle, a body violating ONLY that principle) +CASES = { + "1 artifact the reader can check": CLEAN.replace("![capture](https://github.com/user-attachments/assets/1f2e3d4c-aaaa-bbbb-cccc-ddddeeeeffff) — `evidence-artifacts/recompute.json`", "I ran it locally and it looked right."), + "2 `proven` requires execution": CLEAN.replace(_A, + "Reading the diff shows the selector now takes narrowed inputs, so this is proven."), + "3 no 'what would close it'": CLEAN.replace(_L, + "What would close this: add a test covering the pinned-accounts path, then re-run."), + "4 write to the reviewer": CLEAN.replace(_A, + "I first tried the profiler, then switched to reselect's counter, then re-ran twice."), + "5 drop test-quality-only findings": CLEAN.replace(_L, + "Finding: the test asserts the count but does not assert the returned value shape."), + "6 route privacy/security findings": CLEAN.replace(_A, + "Also noted: the vault key is logged in cleartext at startup, filed nowhere."), + "7 measure the PR range": CLEAN.replace("Environment: head `7bfc16c`, node `v20.11.0`.", + "Environment: head `7bfc16c`, node `v20.11.0`. Range measured: `7bfc16c^..7bfc16c`."), + "8 the label on a number": CLEAN.replace(_A, + "Renders: 3 before, 1 after. (The probe counted distinct values, not renders.)"), + "9 instrument reports what it did": CLEAN.replace(_A, + "Mutation applied: `--replace '/^[\\s\\S]{1,4096}$/u'` (as requested; not read back)."), +} + + +def run(body): + with open(PATH, "w") as fh: + fh.write(body) + env = dict(os.environ) + env["EVIDENCE_GATE_ARTIFACT_HOSTS"] = "majorlift-artifacts-share.s3.us-west-1.amazonaws.com" + payload = {"tool_name": "Bash", "tool_input": {"command": f"{PUBLISH} {PATH}"}} + r = subprocess.run([sys.executable, GATE], input=json.dumps(payload), + capture_output=True, text=True, env=env) + classes = sorted({ln.split("[")[1].split("]")[0] + for ln in (r.stderr or "").splitlines() if "• [" in ln}) + return r.returncode, classes + + +rc, cls = run(CLEAN) +print(f"CONTROL (clean body, must be ALLOWED): {'ALLOWED' if rc == 0 else f'BLOCKED {cls}'}") +if rc != 0: + print(" -> control does not pass; every result below is uninterpretable.") +print() + +enforced, unenforced = [], [] +for name, body in CASES.items(): + rc, cls = run(body) + if rc != 0: + enforced.append((name, cls)) + print(f" ENFORCED {name:<36} caught by {cls}") + else: + unenforced.append(name) + print(f" unenforced {name:<36} publishes cleanly") + +print(f"\n{len(enforced)}/{len(CASES)} enforced, {len(unenforced)}/{len(CASES)} exist only as prose") diff --git a/domains/pr-workflow/skills/evidence/skill.md b/domains/pr-workflow/skills/evidence/skill.md index d7b6547..5de141b 100644 --- a/domains/pr-workflow/skills/evidence/skill.md +++ b/domains/pr-workflow/skills/evidence/skill.md @@ -150,6 +150,32 @@ a PR body.** ### Non-negotiables — these are here, not in a reference, because a requirement you have to fetch is advisory +Three of the nine below are enforced by the gate. Six are not: they hold only if you +apply them. `scripts/principle-coverage.py` measures which is which — it constructs a +body violating one principle and nothing else, and reports whether the gate blocks it, +because a check *named* after a principle is not the same as a check that catches a +violation of it. Run it before assuming any of these is handled for you: + +``` +$ python3 scripts/principle-coverage.py +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 +``` + +Item 2 is the subtle one: 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. + **1. Ship an artifact the reader can check without trusting you.** Terminal text you pasted is indistinguishable from terminal text you invented; it carries the weight of your assertion, not of a measurement. Running the check justifies *your* belief. It becomes *evidence* only when the From 4a5b6c36cb4d24e80355cc84454a66d994bde807 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 6 Aug 2026 08:40:42 -0400 Subject: [PATCH 6/6] fix(evidence-gate): a CI citation is not a CI restatement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rule is that a validation surface does not restate CI status: "tests are green at head " 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. --- .../skills/evidence/hooks/pr-evidence-gate.py | 23 +++++-- .../evidence/scripts/ci-citation-controls.py | 65 +++++++++++++++++++ 2 files changed, 83 insertions(+), 5 deletions(-) create mode 100755 domains/pr-workflow/skills/evidence/scripts/ci-citation-controls.py diff --git a/domains/pr-workflow/skills/evidence/hooks/pr-evidence-gate.py b/domains/pr-workflow/skills/evidence/hooks/pr-evidence-gate.py index f7331b4..a9495e0 100755 --- a/domains/pr-workflow/skills/evidence/hooks/pr-evidence-gate.py +++ b/domains/pr-workflow/skills/evidence/hooks/pr-evidence-gate.py @@ -135,9 +135,10 @@ def main(): "observation": "an OBSERVATION artifact (screenshot/recording/log/JSON/permalink) — " "a /blob/ code link witnesses code, not runtime behavior", "deferral": "a co-located TRACKER (#issue, issues/pull URL, 'triage', 'tracked in')", - "ci-restatement": "removal — a validation surface carries zero CI references. " - "The Checks tab already shows them; cite CI only as the revert " - "lane's outcome, never as 'green at head'", + "ci-restatement": "removal of the CLAIM, not of the link — 'green at head' hands the " + "reviewer their own Checks tab back. Citing a specific run and job " + "whose log holds the figure you are reporting is evidence and is " + "fine; asserting a status the Checks tab already shows is not", "inflated-verdict": "a downgraded verdict — 'live-proven' co-located with " "'not exercised' is inflated; borrowed evidence never " "upgrades an uncaptured lane", @@ -235,10 +236,22 @@ def _extract_body(cmd): r"(?i)(?:#\d+|https?://\S*(?:issues|pull)/\d+|\btriage\b|follow-?up|tracked\s+in)" ) # ── item 11: CI restatement — unconditional in validation scope ──────────── +# Restating a status is not the same as citing a measurement, and the rule is about the +# first. "Tests are green at head " hands the reviewer their own Checks tab back and +# carries no information. A link to a specific run and job whose log holds the figure +# being reported carries the whole measurement, and is what evidence-run.yml exists to +# produce — "move the measurement to CI, where the run URL is the capture". +# +# Matching a bare `actions/runs/N` conflated the two, so the package forbade its own +# flagship output: five of the six branches below describe a CLAIM about CI, and one +# described a URL. A run link is now a violation only when it carries restatement +# language with it. CI_RESTATEMENT = re.compile( - r"(?i)(?:actions/runs/\d+|\bchecks?\s+tab\b|\bgreen\s+(?:at\s+head|in\s+)" + r"(?i)(?:\bchecks?\s+tab\b|\bgreen\s+(?:at\s+head|in\s+)" r"|\ball\s+(?:tests|checks|jobs)\s+(?:pass\w*|green)\b|\bCI\s+(?:is\s+)?green\b" - r"|\b\d+\s+pass(?:ing|ed)?\s*/\s*\d+\s+fail\w*)" + r"|\b\d+\s+pass(?:ing|ed)?\s*/\s*\d+\s+fail\w*" + r"|actions/runs/\d+[^.\n]{0,80}?\b(?:green|passing|all\s+checks|succeeded)\b" + r"|\b(?:green|passing|all\s+checks)\b[^.\n]{0,80}?actions/runs/\d+)" ) # ── item 11: inflated verdict — proof language beside a non-exercise ─────── NOT_EXERCISED = re.compile( diff --git a/domains/pr-workflow/skills/evidence/scripts/ci-citation-controls.py b/domains/pr-workflow/skills/evidence/scripts/ci-citation-controls.py new file mode 100755 index 0000000..6d9f051 --- /dev/null +++ b/domains/pr-workflow/skills/evidence/scripts/ci-citation-controls.py @@ -0,0 +1,65 @@ +#!/usr/bin/env python3 +"""Does the CI rule separate restating a status from citing a measurement? + +RESTATEMENT asserts something the Checks tab already shows. It is the reviewer's own data +read back to them, carries no information, and should be caught. + +CITATION points at a specific run and job whose log or artifact IS the capture — a figure +the Checks tab does not show. `evidence-run.yml` exists to produce exactly this ("move the +measurement to CI, where the run URL is the capture"), so catching it means the package +forbids its own flagship output. + +The rule originally matched a bare `actions/runs/N`, which conflated the two: five of its +six branches described a CLAIM about CI and one described a URL. This is the control that +keeps them apart. + +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. +""" +import importlib.util +import os +import sys + +HOOK = os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))), + "hooks", "pr-evidence-gate.py") +spec = importlib.util.spec_from_file_location("_gate", HOOK) +gate = importlib.util.module_from_spec(spec) +try: + spec.loader.exec_module(gate) +except SystemExit: + pass +CI = gate.CI_RESTATEMENT + +RESTATEMENT = { + "green at head + run link": "Tests are green at head `7bfc16c` — https://github.com/o/r/actions/runs/123.", + "checks tab": "See the Checks tab; everything passes.", + "all jobs green": "All jobs green on this branch.", + "counts": "Unit tests: 412 passing / 0 failing.", + "CI is green": "CI is green, so the change is safe.", +} + +CITATION = { + "run+job is the capture": + "The probe ran in CI: https://github.com/o/r/actions/runs/123/job/456 printed " + "`identical=3 unrelated=1 inputChanged=6`.", + "run link + artifact": + "Measured in CI — https://github.com/o/r/actions/runs/123 — artifact " + "`evidence-artifacts/recompute.json` attached there.", + "two-arm result from a run": + "Base arm failed and head arm passed in https://github.com/o/r/actions/runs/123/job/456; " + "both logs are on that job.", +} + +ok = True +print(f" {'':<26}{'':<28}verdict") +for kind, cases, want_caught in (("restatement (want CAUGHT)", RESTATEMENT, True), + ("citation (want ALLOWED)", CITATION, False)): + for label, text in cases.items(): + caught = bool(CI.search(text)) + good = caught == want_caught + ok &= good + print(f" {'ok ' if good else 'FAIL'} {kind:<26}{label:<28}" + f"{'CAUGHT' if caught else 'allowed'}") + +print("\nall arms behave" if ok else "\nCONTROL FAILED") +sys.exit(0 if ok else 1)