diff --git a/domains/pr-workflow/skills/attest/references/dispatched-passes.md b/domains/pr-workflow/skills/attest/references/dispatched-passes.md new file mode 100644 index 00000000..0583920e --- /dev/null +++ b/domains/pr-workflow/skills/attest/references/dispatched-passes.md @@ -0,0 +1,41 @@ +# Phase 1 — the three dispatched briefs + +Send each to a **fresh instance** with the artifact and nothing else: not the transcript, not +your reasoning, not what you expect it to find. Context is what you are testing for. An instance +that knows what you meant will read what you meant. + +Run them concurrently — they are independent, and sequencing lets the first one's findings frame +the others. + +## outframe — contest the frame + +> You are reading a finished set of findings you did not produce. Do not check whether the +> findings are correct. Ask what claim was chosen and what a different framing makes visible: +> what question would a reader with different priorities have asked of the same material, what +> does the chosen frame make it impossible to notice, and which of the findings only look +> significant because of how the problem was cut. Return findings the framing hid, not a +> critique of the writing. + +## missing — contest the coverage + +> You are auditing a completed run for what it did not do. Enumerate: a modality that was not +> run, a claim asserted but not verified, a source cited but not read, a case the method +> structurally cannot reach. For each, say what running it would cost and what it could change. +> Do not restate what the run found. Absence is the deliverable. + +## press — read it as the stranger + +> You are the reviewer this lands in front of, with no context and a decision to make. Read only +> the artifact. Say what you would have to take on trust, which number you could not check if you +> wanted to, what reads as a measurement but is a sentence, and anything that assumes you were +> present for work you were not. Flag register slips: hedging that reads as concealment, +> confidence that outruns the evidence, and any place the author's process shows through. + +## Reading the returns + +A finding from any pass that invalidates the claim is `BLOCKED`. A finding that qualifies it is +`ATTESTED WITH` — and the caveat goes **into the published artifact**, not just into the verdict, +or the reader never sees it. + +Disagreement between passes is signal, not noise: `press` clearing something `outframe` flagged +usually means the artifact reads well and is framed wrong, which is the more dangerous state. diff --git a/domains/pr-workflow/skills/attest/references/phase-0-checks.md b/domains/pr-workflow/skills/attest/references/phase-0-checks.md new file mode 100644 index 00000000..994ad33c --- /dev/null +++ b/domains/pr-workflow/skills/attest/references/phase-0-checks.md @@ -0,0 +1,35 @@ +# Phase 0 — what each check catches + +Generated from the checks in `mms-evidence/scripts/attest-gate.sh`; that script is the +authority. Each entry exists because a run shipped without it. + +| # | check | run mode | diligence mode | +|---|---|---|---| +| 1 | marker pair | ✓ | ✓ | +| 2 | canonical header | ✓ | ✓ | +| 3 | verdict line | ✓ | ✓ | +| 4 | citations pinned | ✓ | ✓ | +| 5 | captured artifact | ✓ | ✓ | +| 6 | no prescriptions | ✓ | ✓ | +| 7 | no process narration | ✓ | ✓ | +| 8 | verdict is earned | ✓ | ✓ | +| 9 | verdict matches artifact | ✓ | ✓ | +| 10 | floats something for review | ✓ | ✓ | +| 11 | disclaimer present and early | ✓ | ✓ | +| 12 | destination is open | ✓ | ✓ | +| 13 | figures trace to an exhibit | ✓ | ✓ | + +Checks 1–4 differ by mode: in `--diligence` they test that contract's own marker pair, its +header, and that citations are pinned to a tag or SHA rather than a branch head, and the +verdict-line check reports SKIP because a diligence artifact renders none. Checks 8 and 9 SKIP +for the same reason. Everything from 5 down is shared, because those defects are shared. + +**Check 5 is the one that matters, and it asks for a medium.** Every earlier version tested a +property of the plaintext — does it carry a marker, does the command contain a placeholder — and +each caught one defect and missed the next, because every property of plaintext is forgeable by +whatever emits the plaintext. Four runs shipped that way. A `/blob/` permalink is a citation and +does not satisfy it: it witnesses a line in a file, never a run. + +**Check 12 tests the destination**, which no property of the text reveals. Across one register of +published runs, 22 of 27 comments went to pull requests that had already merged — median 22 days +after the merge, gate-clean every time. diff --git a/domains/pr-workflow/skills/attest/skill.md b/domains/pr-workflow/skills/attest/skill.md new file mode 100644 index 00000000..e5af6d89 --- /dev/null +++ b/domains/pr-workflow/skills/attest/skill.md @@ -0,0 +1,103 @@ +--- +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. +maturity: experimental +--- + +# /mms-attest + +The gate an evidence artifact passes before it leaves your hands. Use before posting any +`/mms-evidence` or diligence output to a pull request, issue, or shared tracker. + +## The author is the wrong reader, and the wrong checker + +A validation run claims something was measured. Its characteristic failure is not a wrong number +— it is **prose that reads like a measurement**. An operator who ran the check cannot see this, +because they remember running it; the memory supplies the provenance the text lacks, before the +eye registers that it was missing. + +This is not hypothetical. A run in this workflow shipped a results section whose commands, exit +codes and "reached 100%" were typed by hand, while the real logs sat unpublished on disk. The +author had the skill installed that forbids exactly that. + +So the gate has two halves, and neither substitutes for the other. + +**The mechanical half is not advisory.** Marker presence, a pinned environment, whether any +fenced block is a tool's output rather than the author's transcription, whether the destination +is still open — all greppable. Anything checkable is checked before a model is asked for +judgement, because a model asked "is this good evidence?" answers from inside the frame that +produced it. + +**The dispatched half is positional.** Contesting the frame, the coverage, and the reading cannot +be self-run, for the same reason an author cannot proofread their own sentence for a word their +eye supplies. + +## Phase 0 — mechanical, no model + +``` +scripts/attest-gate.sh --target +scripts/attest-gate.sh --target --diligence +``` + +Thirteen checks; every one a hard fail. `--diligence` swaps the four Validation-Run envelope +checks for a no-verdict contract's own and shares everything downstream. See +[references/phase-0-checks.md](references/phase-0-checks.md) for what each check exists to catch +and the run that caused it to be written. + +**Run it as the same command that publishes, or it is a log line.** The gate and the write must +be one chain — `gate && publish`. Running both and reading the verdict afterwards is how a +blocked artifact reaches a public PR. The `hooks/pr-evidence-gate.py` PreToolUse hook enforces +this independently of your discipline, and fails closed; phase 0 is what you run to iterate +before it does. + +## Phase 1 — dispatched, three lenses + +| pass | reads for | returns | +|---|---|---| +| **outframe** | the frame — what claim was chosen, and what a different framing makes visible | findings the framing hid | +| **missing** | coverage — modality not run, claim unverified, source unread | the gap list | +| **press** | the text as it ships, as the stranger who has to act on it | leak and register findings | + +Dispatch to fresh instances is the mechanism, not an optimisation: a self-run frame check is +composed inside the frame it is meant to test. Briefs in +[references/dispatched-passes.md](references/dispatched-passes.md). + +Skipping a pass is allowed. Silently skipping it is not — name it as skipped in the verdict. + +## Phase 2 — shape + +Front-load the verdict, cut anything that does not change what the reader does, keep every +artifact and move only its placement. Shape only, after content is settled — a shape pass that +reaches content is how a capability table gets dissolved into paragraphs and the comment's +payload disappears. + +## Verdict + +``` +ATTESTED phase 0 clean, no blocking finding from phase 1 +ATTESTED WITH publishable, with named caveats carried INTO the artifact +BLOCKED phase 0 failure, or a phase 1 finding that invalidates the claim +NOT A RUN nothing was measured; there is no artifact to publish +``` + +`NOT A RUN` is legitimate and common. A run that could not execute its check produced no +evidence, and publishing the attempt with a disclaimer is worse than publishing nothing — the +disclaimer reads as hedging and the figure is kept anyway. + +## Anti-patterns + +| Bad | Good | +|---|---| +| Running phase 1 to decide phase 0 | Mechanical checks first; cheap and unarguable | +| Self-running the dispatched passes | Dispatch, or skip and say it was skipped | +| Attesting your own run | The gate is positional; an author attesting themselves attests nothing | +| Treating phase 0 items as advisory | Every one is a hard fail | +| `ATTESTED WITH` as a soft pass | The caveat goes *into the published artifact*, not just the verdict | +| Softening a check to fit the case in hand | If the new version could be satisfied by better prose alone, it is no longer the check | + +## Related + +- `mms-evidence` — produces the artifact this gates +- `mms-instrument-check` — prove the instrument fires before its output counts +- `mms-unmeasured-join` — audit the inference between the facts +- `mms-scope-of-search` — what a negative result is a fact about diff --git a/domains/pr-workflow/skills/evidence/assets/evidence-run.yml b/domains/pr-workflow/skills/evidence/assets/evidence-run.yml new file mode 100644 index 00000000..1a65b24d --- /dev/null +++ b/domains/pr-workflow/skills/evidence/assets/evidence-run.yml @@ -0,0 +1,273 @@ +# Evidence runner — lives in ONE repo and measures any other. +# +# It does not need to be installed in the repo under review, and there is no reason to +# fork that repo either: `target_repo` is an input and the job checks it out read-only. +# Put this in whatever repo you want the runs and artifacts to belong to. +# +# Why this exists rather than running the measurement locally: a validation run is a +# claim that a command produced an output, and the reader has to be able to check that +# without going through the author. A local run's only witness is the author. Every +# failure class this suite has shipped was a local-environment failure — a helper script +# in /tmp, a probe deleted after the run, an absolute path, a toolchain that drifted, a +# contended host producing numbers that had to be retracted. +# +# In CI none of those is expressible. The workflow file is the recipe, the workspace is +# the repo, the run records its own ref, and the run URL is itself the capture — +# `actions/runs/` is what the publish gate accepts. +# +# Trigger from the CLI: +# gh workflow run evidence-run.yml \ +# -f runner=falsify-probe \ +# -f ref= \ +# -f args='--test path/to.test.ts --source path/to.ts --line 9 --replace " return x;"' +# +# Then cite the run URL in the comment. The artifacts are attached to the run; the +# orchestrator reads them to write the finding. +name: Evidence run + +'on': + workflow_dispatch: + inputs: + runner: + description: Runner to execute + required: true + type: choice + options: + - falsify-probe + - selector-recompute + - render-count + - tsc-substitution + - capture + target_repo: + description: >- + Repository to measure. No default on purpose: a default here is a standing + decision about what every unthinking dispatch touches, and this workflow's whole + argument is that it should be proved somewhere harmless first. + required: true + type: string + ref: + description: Commit SHA to measure. Pin it — a branch name makes the run unrepeatable. + required: true + type: string + args: + description: Arguments passed to the runner, verbatim + required: true + type: string + needs_install: + description: >- + Install the target's dependencies. Required for the jest and tsc runners; a waste + of ten minutes for `capture` wrapping git or a policy audit, which read files only. + required: false + default: true + type: boolean + baseline: + description: >- + Second SHA to measure identically. Twice now a run reported "no finding" when it + had no comparison, so the baseline arm is offered here rather than left to memory. + required: false + type: string + skills_repo: + description: >- + Where to source the runners. Defaults to upstream. Overridable because a runner + fix and the run that needs it cannot both wait on a review: point this at a fork + branch, and say in the artifact that you did. + required: false + default: MetaMask/skills + type: string + skills_ref: + description: >- + Ref within skills_repo. Overrides the EVIDENCE_SKILLS_REF variable. + required: false + type: string + probe_path: + description: >- + Path, within skills_repo, of a probe file to copy into the target tree before the + runner executes. `render-count` takes a hand-written probe, and a probe that lives + only on the author's disk is the exact defect the run URL exists to remove. + required: false + type: string + probe_dest: + description: Where in the target tree to place probe_path. Required with probe_path. + required: false + type: string + +permissions: + contents: read + +jobs: + measure: + runs-on: ubuntu-latest + timeout-minutes: 45 + steps: + - name: Reject a moving ref + env: + REF: ${{ inputs.ref }} + BASE: ${{ inputs.baseline }} + run: | + # A branch name makes the artifact unrepeatable, which is the property this + # workflow exists to provide. Checked before anything is fetched. + for r in "$REF" ${BASE:+"$BASE"}; do + case "$r" in + *[!0-9a-f]* | "") echo "::error::'$r' is not a commit SHA"; exit 1 ;; + esac + [ ${#r} -eq 40 ] || { echo "::error::'$r' must be the full 40-char SHA"; exit 1; } + done + + - name: Checkout the target at the measured ref + uses: actions/checkout@v6 + with: + repository: ${{ inputs.target_repo }} + ref: ${{ inputs.ref }} + fetch-depth: 2 # the runners diff against the parent + + # Before setup-node, not after. A target that pins its package manager through + # `packageManager` in package.json makes setup-node's `cache: yarn` probe run + # `yarn cache dir` under the runner's global yarn 1.22, which refuses and fails + # the step — so every jest runner died at setup with nothing measured. The target + # repo's own workflows order it exactly this way. + - name: Enable corepack + if: inputs.needs_install + run: corepack enable + + - uses: actions/setup-node@v4 + if: inputs.needs_install + with: + node-version-file: .nvmrc + cache: yarn + + - name: Install + if: inputs.needs_install + run: yarn --immutable + + # Pinned to a commit, for the same reason the measured ref must be: a branch name + # makes the run unrepeatable, and "which version of the runner produced this" is + # exactly the question a reader asks. Override per-repo with the EVIDENCE_SKILLS_REF + # variable; bump the default when the runners land on the skills repo's main. + - name: Fetch the runners at a pinned version + uses: actions/checkout@v6 + with: + repository: ${{ inputs.skills_repo || 'MetaMask/skills' }} + ref: ${{ inputs.skills_ref || vars.EVIDENCE_SKILLS_REF || '56578cee0f679881e6f928177ef3cf6d45a5bfec' }} + path: .evidence-skills + # The security-domain analysis scripts (policy-audit.py and its siblings) are + # wrapped by the `capture` runner rather than being runners themselves, so they + # need no entry in the `runner` choice list — but they do need to be on disk. + # A sparse path absent from the chosen ref is silently empty, so listing it here + # costs nothing when it is not there. + sparse-checkout: | + domains/pr-workflow/skills/evidence/scripts + domains/pr-workflow/skills/evidence/probes + domains/security/skills + + - name: Verify the runners arrived + env: + RUNNER: ${{ inputs.runner }} + run: | + # A sparse checkout of a path that does not exist on the chosen ref succeeds and + # produces an empty directory, so the next step would fail with "No such file" + # and no indication that the REF was the problem. + F=".evidence-skills/domains/pr-workflow/skills/evidence/scripts/$RUNNER.sh" + [ -f "$F" ] || { + echo "::error::$RUNNER.sh not present at the pinned skills ref." + echo "::error::Set the EVIDENCE_SKILLS_REF repository variable to a commit that has it." + exit 1 + } + echo "runner $RUNNER.sh sourced from ${{ inputs.skills_repo || 'MetaMask/skills' }} @ ${{ inputs.skills_ref || vars.EVIDENCE_SKILLS_REF || 'pinned default' }}" \ + >> "$GITHUB_STEP_SUMMARY" + + - name: Place the probe + if: inputs.probe_path != '' + env: + SRC: .evidence-skills/${{ inputs.probe_path }} + DEST: ${{ inputs.probe_dest }} + run: | + # Copied from the runners checkout, so the probe has a permalink of its own and + # the reader can see the file that produced the count rather than taking the + # count on the author's word. + [ -n "$DEST" ] || { echo "::error::probe_dest is required with probe_path"; exit 1; } + [ -f "$SRC" ] || { echo "::error::probe not found at $SRC on the chosen skills ref"; exit 1; } + mkdir -p "$(dirname "$DEST")" + cp "$SRC" "$DEST" + echo "probe $SRC -> $DEST" >> "$GITHUB_STEP_SUMMARY" + + - name: Run + id: run + continue-on-error: true # the exit code IS the verdict; a finding is not a failure + env: + RUNNER: ${{ inputs.runner }} + ARGS: ${{ inputs.args }} + run: | + RUNNERS=.evidence-skills/domains/pr-workflow/skills/evidence/scripts + mkdir -p evidence-artifacts + set +e + eval "bash \"$RUNNERS/$RUNNER.sh\" --label \"$RUNNER-head\" --out evidence-artifacts $ARGS" + code=$? + set -e + echo "head_exit=$code" >> "$GITHUB_OUTPUT" + echo "runner exited $code — the exit code is the verdict, not a build failure" \ + >> "$GITHUB_STEP_SUMMARY" + + - name: Baseline arm + if: inputs.baseline != '' + continue-on-error: true + env: + RUNNER: ${{ inputs.runner }} + ARGS: ${{ inputs.args }} + BASE: ${{ inputs.baseline }} + run: | + git checkout --detach "$BASE" + if [ "${{ inputs.needs_install }}" = "true" ]; then yarn --immutable; fi + RUNNERS=.evidence-skills/domains/pr-workflow/skills/evidence/scripts + eval "bash \"$RUNNERS/$RUNNER.sh\" --label \"$RUNNER-base\" --out evidence-artifacts $ARGS" + + - name: Determinism check + # 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 + env: + RUNNER: ${{ inputs.runner }} + ARGS: ${{ inputs.args }} + REF: ${{ inputs.ref }} + run: | + RUNNERS=.evidence-skills/domains/pr-workflow/skills/evidence/scripts + git checkout --detach "$REF" + eval "bash \"$RUNNERS/$RUNNER.sh\" --label \"$RUNNER-repeat\" --out evidence-artifacts $ARGS" || true + A="evidence-artifacts/$RUNNER-head.json" + B="evidence-artifacts/$RUNNER-repeat.json" + if [ -f "$A" ] && [ -f "$B" ]; then + # `label`, `log` and `logs` name the arm, and `env` carries timing — all differ + # between the two runs by construction. Comparing them makes the check fire on + # every run, which is the same as not having it. `logs` was missing from this + # list, so `render-count` — the only runner that writes the plural key — failed + # the check on every run while reporting identical counts. A warning that is + # always wrong for one runner teaches the operator to publish through it. + if diff <(jq -S 'del(.env, .label, .log, .logs)' "$A") \ + <(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" + cat determinism.diff >> "$GITHUB_STEP_SUMMARY" + fi + fi + + - name: Publish the run summary + if: always() + run: | + for f in evidence-artifacts/*.md; do + [ -f "$f" ] || continue + { echo; cat "$f"; } >> "$GITHUB_STEP_SUMMARY" + done + + - name: Upload artifacts + if: always() + uses: actions/upload-artifact@v4 + with: + name: evidence-${{ inputs.runner }}-${{ inputs.ref }} + # the artifact name carries what was measured, so a downloaded zip is + # self-describing rather than needing the run page to interpret + path: | + evidence-artifacts/** + determinism.diff + retention-days: 90 + if-no-files-found: error diff --git a/domains/pr-workflow/skills/evidence/hooks/gate-controls.sh b/domains/pr-workflow/skills/evidence/hooks/gate-controls.sh new file mode 100755 index 00000000..286d067f --- /dev/null +++ b/domains/pr-workflow/skills/evidence/hooks/gate-controls.sh @@ -0,0 +1,120 @@ +#!/usr/bin/env bash +# +# Control matrix for the emit-time gate. Run it from anywhere; it copies the hook to a +# directory with no sibling scripts/ so `_find_gate()` resolves the way it does in +# production rather than the way it does in a checkout. +# +# It exists because three copies of this hook were on one machine, the oldest was the one +# wired into settings, and it had no `gh api` matcher — so every publish through that path +# went ungated for weeks while two newer copies sat unused. Nothing noticed, because a gate +# that blocks nothing is indistinguishable from a gate with nothing to block. +# +# Positives must block (exit 2). Negatives must pass (exit 0). Both halves matter: a gate +# that blocks everything is as broken as one that blocks nothing, and only the negative +# arm catches it. +set -uo pipefail +HOOK="${1:-$(cd "$(dirname "$0")" && pwd)/pr-evidence-gate.py}" +[ -f "$HOOK" ] || { echo "usage: gate-controls.sh [path/to/pr-evidence-gate.py]" >&2; exit 2; } + +tmp="$(mktemp -d)"; trap 'rm -rf "$tmp"' EXIT +cp "$HOOK" "$tmp/hook.py" + +printf '\n**LavaMoat grants — x**\n\n```\n$ yarn build\n exit 0\n```\n\n' > "$tmp/bad.md" +printf 'Addressed: see the linked run.\n' > "$tmp/reply.md" +# The enrichment rule needs the REPORT shape, not report vocabulary: three or more +# paragraphs, a cited link, and no reply-template opener. A one-line probe passes it for +# the wrong reason, which is how a mis-specified positive arm reads as a working rule. +cat > "$tmp/finding.md" <<'BODY' +The migration path is ground-truthed against the fixture set and rules out the ordering hazard. + +Two of the three cases resolve through the same upstream guard, so the remaining exposure is +the un-guarded third: https://github.com/o/r/blob/abc123/src/migrate.ts#L40 + +That leaves the rollback lane unaccounted for, which is worth its own pass before this lands. +BODY + +probe() { printf '{"tool_name":"Bash","tool_input":{"command":%s}}' "$(python3 -c 'import json,sys;print(json.dumps(sys.argv[1]))' "$1")"; } + +fails=0 +check() { # name expected command + local name="$1" want="$2" cmd="$3" got + probe "$cmd" | python3 "$tmp/hook.py" >/dev/null 2>&1; got=$? + if [ "$got" = "$want" ]; then printf ' ok %-34s exit=%s\n' "$name" "$got" + else printf ' FAIL %-34s exit=%s want=%s\n' "$name" "$got" "$want"; fails=$((fails+1)); fi +} + +# ── wiring ─────────────────────────────────────────────────────────────────────── +# The arms below prove the SCRIPT works. They say nothing about whether anything calls +# it, and those are different questions: a hook that is not wired, or wired to a path +# that no longer exists, is indistinguishable from a hook with nothing to block. One +# session ran to completion with every PreToolUse hook inert — 306 certification markers +# written, none of them enforcing anything — because nobody asked this question. +wiring() { + local found=0 + # $HOME is not necessarily the login home — an account-switching setup points it at a + # per-account directory, which is exactly the case this was first run in. Enumerating + # from $HOME alone found one config, reported it as "the" wiring, and never looked at + # the other. Derive the roots instead, and de-duplicate by realpath so a symlinked + # config is not counted twice or missed once. + local roots=() seen=() r + for r in "$HOME" "$(getent passwd "$(id -un)" | cut -d: -f6)" /home/*/ ; do + [ -d "$r" ] || continue + roots+=("$r/.claude/settings.json") + for a in "$r"/.claude-accts/*/.claude/settings.json; do [ -f "$a" ] && roots+=("$a"); done + done + for cfg in "${roots[@]}"; do + [ -f "$cfg" ] || continue + local rp; rp=$(readlink -f "$cfg") + case " ${seen[*]} " in *" $rp "*) continue ;; esac + seen+=("$rp") + local cmd + cmd=$(python3 -c ' +import json,sys +try: d=json.load(open(sys.argv[1])) +except Exception: sys.exit(0) +out=[] +def w(o): + if isinstance(o,dict): + for k,v in o.items(): + if k=="command" and isinstance(v,str) and "pr-evidence-gate" in v: out.append(v) + else: w(v) + elif isinstance(o,list): + [w(x) for x in o] +w(d.get("hooks",{})) +print(out[0] if out else "")' "$cfg") + [ -n "$cmd" ] || continue + found=1 + local path; path=$(printf '%s' "$cmd" | grep -oE '[^ "]*pr-evidence-gate\.py') + path="${path/\$HOME/$HOME}" + if [ -f "$path" ]; then printf ' ok wired: %s\n' "${cfg/#$HOME/~}" + else printf ' FAIL wired to a missing file: %s → %s\n' "${cfg/#$HOME/~}" "$path"; fails=$((fails+1)); fi + done + [ "$found" = 1 ] || { printf ' FAIL no settings file registers the gate as a PreToolUse hook\n'; fails=$((fails+1)); } +} + +echo "gate-controls: $HOOK" +wiring +check "positive: gh api body write" 2 "gh api repos/o/r/issues/comments/1 -X PATCH -F body=@$tmp/bad.md" +check "positive: gh pr comment" 2 "gh pr comment 1 --repo o/r --body-file $tmp/bad.md" +check "positive: finding via comment" 2 "gh issue comment 1 --repo o/r --body-file $tmp/finding.md" +check "negative: unrelated command" 0 "ls -la" +check "negative: gh read, no body" 0 "gh pr view 1 --repo o/r" +check "negative: a reply is a reply" 0 "gh issue comment 1 --repo o/r --body-file $tmp/reply.md" + +# The gate reads the command as text, so a body it cannot resolve is a body it cannot +# check. These three are how an entire session of publishes went ungated while every +# other arm above was green: the path was assembled from a shell variable each time. +check "positive: body path via \$VAR" 2 'gh pr comment 1 --repo o/r --body-file $D/c.md' +check "positive: body via \$(cat ...)" 2 'gh pr comment 1 --repo o/r --body "$(cat c.md)"' +check "positive: gh api body via \$VAR" 2 'gh api repos/o/r/issues/1/comments -F body=@$D/c.md' + +echo +if [ "$fails" -eq 0 ]; then + echo "gate-controls: all arms behave, and the gate is wired" + echo + echo "Wiring is not liveness. This proves a settings file names an existing file; it" + echo "cannot prove the running session loaded it. For that, run a command the gate must" + echo "block and confirm it is blocked — in a session, not here." + exit 0 +fi +echo "gate-controls: $fails arm(s) wrong — the gate is not doing what it claims"; exit 1 diff --git a/domains/pr-workflow/skills/evidence/hooks/pr-evidence-gate.py b/domains/pr-workflow/skills/evidence/hooks/pr-evidence-gate.py new file mode 100755 index 00000000..b79d0e20 --- /dev/null +++ b/domains/pr-workflow/skills/evidence/hooks/pr-evidence-gate.py @@ -0,0 +1,578 @@ +#!/usr/bin/env python3 +""" +Emit-time evidence gate (PreToolUse:Bash). + +Blocks outward-facing `gh pr|issue edit|create|comment` — and `gh api` body +writes, which bypass the porcelain — whose body contains, in a validation-scoped +paragraph, a claim that the trustworthiness gate would reject. Rationale: an +unbacked "confirmed / verified / proven / observed / ingested / ✅" launders an +unverified assertion as fact under the author's name, and an untracked "remains +pending" decays to never. + +The trustworthiness gate is the checklist; THIS is the trigger that runs it. +Each class below implements a numbered item of `references/evidence-trustworthiness.md`. + +Contract: reads PreToolUse JSON on stdin. Exit 0 = allow. Exit 2 = block +(stderr shown to the model). Fails OPEN on anything it cannot parse, so it never +bricks unrelated Bash commands — but once it has identified a body it is going to +publish, it fails CLOSED: if attest-gate.sh cannot be found or run, the write is +refused rather than waved through. +""" +import json +import os +import re +import subprocess +import sys +import tempfile + + +def _out_allow(): + sys.exit(0) + + +def _block(msg): + sys.stderr.write(msg) + sys.exit(2) + + +# Outward-facing gh write surfaces. The porcelain set is wider than +# `gh pr edit|create` because the same unbacked verdict launders identically +# through a PR comment or an issue body. `gh api` is included because a PATCH +# to .../comments/ is the same publish with a different spelling — a gate +# that cannot see the write it is meant to police is not a gate. +GH_PORCELAIN = re.compile(r"\bgh\s+(?:pr|issue)\s+(?:edit|create|comment)\b") +GH_API = re.compile(r"\bgh\s+api\b") + + +def main(): + try: + payload = json.load(sys.stdin) + except Exception: + _out_allow() + + if payload.get("tool_name") != "Bash": + _out_allow() + + cmd = (payload.get("tool_input") or {}).get("command", "") + + is_porcelain = bool(GH_PORCELAIN.search(cmd)) + is_api = bool(GH_API.search(cmd)) and re.search(r"(?:-F|-f|--field|--raw-field)\s+body=|--input\b", cmd) + if not (is_porcelain or is_api): + _out_allow() + if is_porcelain and "--body" not in cmd: # covers --body and --body-file + _out_allow() + + body = _extract_body(cmd) + # An unexpanded shell construct is not a body. `--body "$(cat f)"` extracts the literal + # characters `$(cat f)`, which scans clean and publishes whatever the shell substitutes + # later — the gate would be inspecting a string the reader never sees. + # + # Look at the ARGUMENT, not the body text. A first attempt scanned the body for `$` and + # backticks and rejected every evidence comment ever written, because markdown inline + # code is backticks and these artifacts are full of them. The shell metacharacters that + # matter are in the command; the body is just prose. + if _body_arg_is_unresolvable(cmd): + body = "" + if not body: + # FAIL CLOSED. The previous reasoning here was "can't read it -> nothing to scan", + # which inverts the situation: by this point the command has already been identified + # as an outward-facing write, so an unreadable body is not an absent risk, it is an + # unverifiable one. + # + # This was not theoretical. The extraction is textual, so a path assembled from a + # shell variable — `--body-file $S/comment.md` — or a body spliced in with + # `--body "$(cat f)"` yields nothing, and every such publish sailed through while + # the gate reported itself healthy. An entire session of publishes went ungated this + # way, including one the gate blocks when handed the same body by literal path. + _block( + "EVIDENCE GATE (PreToolUse) — blocked an outward-facing write whose body " + "could not be read.\n\n" + "The body path could not be resolved from the command. This hook reads the " + "command as text and cannot expand shell variables, command substitution, or " + "heredocs, so a body assembled that way is unverifiable rather than safe.\n\n" + "Pass a literal path:\n" + " gh pr comment --repo --body-file /abs/path/to/comment.md\n\n" + "If the body genuinely has no file, write it to one first. The gate has to see " + "what you are about to publish.\n" + ) + + violations = _scan(body) + if re.search(r"\bgh\s+issue\s+comment\b", cmd): + violations += _scan_enrichment_via_comment(body) + violations += _run_attest_gate(body, cmd) + if not violations: + _out_allow() + + lines = [ + "EVIDENCE GATE (PreToolUse) — blocked outward-facing GitHub write.", + "", + "Each finding names the trustworthiness-gate item it violates. Fix by", + "attaching the missing artifact in the SAME block, or by downgrading the", + "claim (⚠️ inconclusive / remove it). Do not rephrase around the check.", + "", + ] + for v in violations[:12]: + need = NEEDS.get(v.get("kind", "verdict"), "ARTIFACT") + lines.append(f' • [{v["kind"]}] "{v["token"]}"') + lines.append(f' needs: {need}') + lines.append(f' in: {v["snippet"]}') + if len(violations) > 12: + lines.append(f" … and {len(violations) - 12} more.") + lines += [ + "", + "If the evidence exists on disk, BIND it: every collected artifact the", + "claim rests on gets referenced or re-hosted before the write.", + ] + _block("\n".join(lines) + "\n") + + +NEEDS = { + "enrichment": "a BODY EDIT instead (`gh issue edit --body-file`) — this reads like a resolved finding, not a reply", + "attest-gate": "the check named above to pass — run scripts/attest-gate.sh yourself to iterate", + "gate-missing": "attest-gate.sh on disk; refusing to publish a body nothing verified", + "gate-error": "attest-gate.sh to run successfully; refusing to publish unverified", + "verdict": "an inspectable ARTIFACT (https:// permalink, /blob//, or a *.test.ts ref)", + "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'", + "inflated-verdict": "a downgraded verdict — 'live-proven' co-located with " + "'not exercised' is inflated; borrowed evidence never " + "upgrades an uncaptured lane", + "bare-identifier": "a resolving link for the id (permalink or absolute-windowed " + "query) OR the re-hosted capture showing it", + "truncated-identifier": "the FULL identifier, quoted verbatim — an ellipsized id " + "cannot be grepped against any artifact, and a co-located " + "resolver does not excuse it", + "mutable-ref": "a commit-pinned permalink (/blob//…#Lx-Ly) — a branch ref " + "can be rewritten after review", + "dump-resolver": "a reader-native exhibit — a live link or a visual. A raw " + "log/JSON/HAR dump is appendix-only, never the exhibit a claim rests on", + "link-only-exhibit": "an embedded visual of the linked view ALONGSIDE the permalink — " + "link-only defers validation behind click + auth + query rendering", + "data-only-exhibit": "an in-environment capture (the resolving UI with its query, " + "project/environment selectors and time window in-frame) — " + "quoted data alone carries no liveness provenance", + "step-waiver": "a per-step ⏳ + tracker whose blocker is that step's OWN unmet " + "precondition — an impossibility argument is not a discharge", +} + + +def _extract_body(cmd): + # 1) --body-file / --input + m = re.search(r"--(?:body-file|input)[=\s]+(?:'([^']+)'|\"([^\"]+)\"|(\S+))", cmd) + if m: + path = m.group(1) or m.group(2) or m.group(3) + try: + with open(os.path.expanduser(path), "r", encoding="utf-8") as fh: + raw = fh.read() + except Exception: + return "" + # `gh api --input` takes a JSON file; pull .body out of it. + try: + obj = json.loads(raw) + if isinstance(obj, dict) and isinstance(obj.get("body"), str): + return obj["body"] + except Exception: + pass + return raw + # 2) gh api -F body=@ / --field body=@ + m = re.search(r"(?:-F|--field|--raw-field)\s+body=@(?:'([^']+)'|\"([^\"]+)\"|(\S+))", cmd) + if m: + path = m.group(1) or m.group(2) or m.group(3) + try: + with open(os.path.expanduser(path), "r", encoding="utf-8") as fh: + return fh.read() + except Exception: + return "" + # 3) --body "$(cat <<'EOF' ... EOF)" heredoc + m = re.search(r"<<-?'?EOF'?\s*\n(.*?)\n\s*EOF", cmd, re.DOTALL) + if m: + return m.group(1) + # 4) --body '...' / --body "..." / gh api -f body='...' + m = re.search(r"(?:--body|(?:-f|--field|--raw-field)\s+body=)[=\s]*'((?:[^']|'\\'')*)'", cmd, re.DOTALL) + if m: + return m.group(1) + m = re.search(r'(?:--body|(?:-f|--field|--raw-field)\s+body=)[=\s]*"(.*?)"', cmd, re.DOTALL) + if m: + return m.group(1) + return "" + + +# ── item 1/5: verdict claims ──────────────────────────────────────────────── +VERDICT = re.compile( + r"(?i)(?:\bcapture[ds]?\s+confirm\w*|\bconfirm(?:s|ed)\b|\bverif(?:y|ies|ied)\b" + r"|\bproven\b|\bobserved\b|\bingested\b|\bdemonstrat(?:e|es|ed)\b" + r"|\blive-proven\b|\bsuccessful\b|\bvalidated\b" + r"|does not drop\b|✅)" +) +ARTIFACT = re.compile( + r"(?i)(?:https?://\S+|actions/runs/\d+|/blob/|\bjob/\d+" + r"|`?[\w./-]*\.(?:test|spec)\.[tj]sx?(?::\d+)?`?)" +) +# ── item 2: runtime observation claims ───────────────────────────────────── +OBSERVATION = re.compile( + r"(?i)(?:\brendered\b|byte-identical(?:ly)?|\bsnapshot\s+shows?\b" + r"|\bscreenshots?\s+show\w*|\breproduc(?:ed|es)\b" + r"|\bstill\s+(?:shown|shows|fails|failing|raises)\b" + r"|\bin\s+a\s+(?:real|live)\s+browser\b|\blive\s+test\s+build\b" + r"|\bin\s+two\s+independent\s+runs\b|\bworks\s+as\s+described\b)" +) +OBS_ARTIFACT = re.compile( + r"(?i)(?:!\[|/ instead of /blob// ───────── +MUTABLE_REF = re.compile( + r"(?i)https?://github\.com/[\w.-]+/[\w.-]+/blob/(?![0-9a-f]{7,40}[/#])[\w.-]+/" +) +# ── item 13: dump-as-resolver ────────────────────────────────────────────── +DUMP_LINK = re.compile(r"(?i)https?://\S+\.(?:log|json|har|txt)\b") +IMAGE_EMBED = re.compile( + r"(?i)(?:!\[|.) is + excused unconditionally — that template is itself the correct convention + for a comment (exogram-core: ghostwrite-review-reply-register), so + following it is the signal of doing this right, not a loophole. + """ + if REPLY_TEMPLATE_OPENER.search(body.strip()): + return [] + paras = [p.strip() for p in re.split(r"\n\s*\n", body) if p.strip()] + if len(paras) < 3: + return [] # short reply, even with a link, isn't a standalone report + if not re.search(r"https?://\S+", body): + return [] # no cited evidence — not the report shape either + snip = re.sub(r"\s+", " ", paras[0])[:120] + return [{"token": "standalone finding", "snippet": snip, "kind": "enrichment"}] + + +# The text following --body/--body-file, up to the next argument. If it carries a variable, +# a command substitution, or a backtick, this hook cannot know what will actually be sent. +BODY_ARG = re.compile(r"(?:--body-file|--body|-F\s+body|--field\s+body|--raw-field\s+body)[=\s]+(\S+)") + + +def _body_arg_is_unresolvable(cmd): + m = BODY_ARG.search(cmd) + if not m: + return False + arg = m.group(1) + return bool(re.search(r"\$|`", arg)) + + +def _scan(body): + # Strip bot-generated summary block — not our claim. + body = re.sub(r".*?", + "", body, flags=re.DOTALL) + violations = [] + section = "" + for block in re.split(r"(?m)^(?=\s*#{1,6}\s)", body): + hm = re.match(r"\s*#{1,6}\s*(.+)", block) + if hm: + section = hm.group(1) + section_in_scope = bool(SCOPE_HEADING.search(section)) + for para in re.split(r"\n\s*\n", block): + scan_lines = [] + for ln in para.splitlines(): + s = ln.strip() + if re.match(r"-\s*\[[ xX]\]", s): # checklist item + continue + if s.startswith(">"): # blockquote (bot NOTE) + continue + if s.startswith("_Status key"): # legend + continue + if s.startswith("#"): # heading line + continue + scan_lines.append(ln) + chunk = "\n".join(scan_lines) + if not chunk.strip(): + continue + if not (section_in_scope or SCOPE_PARA.search(chunk)): + continue + # A markdown table row is its own claim unit — scan each row so an + # artifact two rows down cannot excuse a bare row. + units = chunk.splitlines() if chunk.lstrip().startswith("|") else [chunk] + for unit in units: + _scan_unit(unit, violations) + return violations + + +def _add(violations, kind, token, unit): + violations.append({ + "kind": kind, + "token": token, + "snippet": re.sub(r"\s+", " ", unit.strip())[:120], + }) + + +def _positive_verdict(unit): + """A non-negated verdict token in this unit, or None.""" + for m in VERDICT.finditer(unit): + if not _negated(unit, m.start()): + return m.group(0) + return None + + +def _scan_unit(unit, violations): + # ── VERDICT: excused by a co-located inspectable artifact. + if not ARTIFACT.search(unit): + 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): + for m in OBSERVATION.finditer(unit): + if _negated(unit, m.start()): + continue + _add(violations, "observation", m.group(0), unit) + break + + # ── DEFERRAL: excused by a co-located tracker, NOT by an artifact. + if not TRACKER.search(unit): + dm = DEFERRAL.search(unit) + if dm: + _add(violations, "deferral", dm.group(0), unit) + + # ── CI RESTATEMENT (item 11): unconditional in validation scope. No + # verdict co-location required, no "beyond-CI"/"as context" excuse — + # a carve-out here is an instruction to phrase every violation as the + # exception. + cm = CI_RESTATEMENT.search(unit) + if cm: + _add(violations, "ci-restatement", cm.group(0), unit) + + # ── INFLATED VERDICT (item 11): proof language co-located with an + # admission the surface was not exercised. + nm = NOT_EXERCISED.search(unit) + if nm and _positive_verdict(unit): + _add(violations, "inflated-verdict", nm.group(0), unit) + + # ── STEP WAIVER (item 14): an impossibility argument never discharges a + # lane derived from an executable Manual testing step. + sm = STEP_WAIVER.search(unit) + if sm: + _add(violations, "step-waiver", sm.group(0), unit) + + # ── TRUNCATED IDENTIFIER (item 16): a co-located resolver does NOT + # excuse — the resolver resolves the full id, not the fragment the + # reader holds. Hash-equality prose is exempt. + if not HASH_EQUALITY.search(unit): + tm = TRUNCATED_ID.search(unit) + if tm: + _add(violations, "truncated-identifier", tm.group(0), unit) + + # ── 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): + bm = BARE_ID.search(unit) + if bm: + _add(violations, "bare-identifier", bm.group(0), unit) + + # ── MUTABLE REF (item 16): pin evidence links to a SHA. + mm = MUTABLE_REF.search(unit) + if mm: + _add(violations, "mutable-ref", mm.group(0)[:60], unit) + + # ── DUMP RESOLVER (item 13): a positive verdict whose only resolver is a + # raw dump behind a link. The digging moved a hop away, it did not + # disappear. + if _positive_verdict(unit) and DUMP_LINK.search(unit) and not IMAGE_EMBED.search(unit) \ + and not LIVE_LINK.search(unit): + _add(violations, "dump-resolver", DUMP_LINK.search(unit).group(0)[:60], unit) + + # ── LINK-ONLY EXHIBIT (item 15): a live permalink defers validation + # behind click + auth + query rendering. Needs the visual too. + if _positive_verdict(unit) and LIVE_LINK.search(unit) and not IMAGE_EMBED.search(unit): + _add(violations, "link-only-exhibit", LIVE_LINK.search(unit).group(0)[:60], unit) + + # ── DATA-ONLY EXHIBIT (item 17): telemetry claim with neither a visual + # nor a live link carries no liveness provenance — extracted data is + # indistinguishable from data typed by hand. + if _positive_verdict(unit) and TELEMETRY_VOCAB.search(unit) \ + and not IMAGE_EMBED.search(unit) and not LIVE_LINK.search(unit): + _add(violations, "data-only-exhibit", TELEMETRY_VOCAB.search(unit).group(0), unit) + + +def _negated(text, pos): + """A verdict token preceded by a negator is a hedge, not a claim.""" + pre = text[max(0, pos - 16):pos].lower() + if re.search(r"\b(not|never|no|isn't|aren't|cannot|can't|without|un|yet)\s*$", pre): + return True + # 'unverified' / 'unproven' — negator fused onto the token + if pre.endswith("un"): + return True + return False + + +if __name__ == "__main__": + main() diff --git a/domains/pr-workflow/skills/evidence/probes/metametrics-context.test.tsx b/domains/pr-workflow/skills/evidence/probes/metametrics-context.test.tsx new file mode 100644 index 00000000..ae61df61 --- /dev/null +++ b/domains/pr-workflow/skills/evidence/probes/metametrics-context.test.tsx @@ -0,0 +1,68 @@ +// Probe — MetaMetrics context value identity. +// +// PLACEMENT: copy to `ui/contexts/__render_probe__.test.tsx` in a metamask-extension tree. +// The imports below are relative to `ui/contexts/`, so a different destination resolves +// nothing and the suite fails to run with "Cannot find module" — which is a failed probe, +// not a measurement. `probe_dest` in the evidence workflow must match this path. +// +// The claim under test is about breadth: "all N consumers avoid unnecessary re-renders". +// `useContext` re-renders a consumer when the value's IDENTITY changes, and that is not a +// per-consumer property — so one distinct value across N parent renders means every +// consumer is spared, and N distinct values means none is. Counting distinct values is +// therefore the measurement the claim actually rests on; counting one consumer's renders +// would only ever describe that consumer. +// +// Resolves against both `metametrics.js` and `metametrics.tsx`, so the same file measures a +// base commit and a head commit that renamed it — the comparison is the point. +import React, { useContext, useRef, useState } from 'react'; +import { act } from '@testing-library/react'; +import configureStore from '../store/store'; +import { renderWithProvider } from '../../test/lib/render-helpers-navigate'; +import mockState from '../../test/data/mock-state.json'; +import { MetaMetricsContext, MetaMetricsProvider } from './metametrics'; + +let consumerRenders = 0; +let distinctValues = 0; +let bump: (() => void) | undefined; + +function Consumer() { + const value = useContext(MetaMetricsContext); + const last = useRef(null); + if (last.current !== value) { + last.current = value; + distinctValues += 1; + } + consumerRenders += 1; + return null; +} + +function Parent() { + const [, setN] = useState(0); + bump = () => setN((n) => n + 1); + return ( + + + + ); +} + +describe('MetaMetrics context value identity', () => { + it('counts distinct context values across parent re-renders', () => { + const PARENT_RENDERS = 5; + consumerRenders = 0; + distinctValues = 0; + + renderWithProvider(, configureStore(mockState)); + for (let i = 0; i < PARENT_RENDERS; i++) { + act(() => { + bump?.(); + }); + } + + // eslint-disable-next-line no-console + console.log( + `RENDER_COUNT consumer=${distinctValues} parentRenders=${PARENT_RENDERS + 1} consumerRenders=${consumerRenders}`, + ); + expect(consumerRenders).toBeGreaterThan(0); + }); +}); diff --git a/domains/pr-workflow/skills/evidence/references/aep-local-run.md b/domains/pr-workflow/skills/evidence/references/aep-local-run.md new file mode 100644 index 00000000..cccb6b6e --- /dev/null +++ b/domains/pr-workflow/skills/evidence/references/aep-local-run.md @@ -0,0 +1,81 @@ +# Running AEP locally + +Everything the local Autonomous Engineering Platform run needs: bring-up, submit, poll, +fetch artifacts, tear down. The hosted instance (`aep.dev.web3factory.consensys.net`) has +not resolved since 2026-06, so local is the only path. + +The skill body links here rather than carrying this inline. An AEP run is the heaviest +lane in the catalog and most validations do not need it — a falsifying test, a single +screenshot, or an artifact CI already produced usually closes the same falsifier. Read +this when you have decided an AEP run is warranted. + +Every bullet below cost a failed run at least once. + +## Preflight — bring up what is down + +The hosted AEP doesn't resolve (`aep.dev.web3factory.consensys.net` is dead as of 2026-06). Everything runs locally. Health-check, then bring up only what's down. + +Fast checks: + +```bash +AEP=~/Code/metamask/metamask-autonomous-engineering-platform +curl -fsS localhost:3000/health >/dev/null && echo "control-plane up" || echo "control-plane DOWN" +curl -fsS localhost:8233 >/dev/null && echo "temporal UI up" || echo "temporal DOWN" +docker ps --format '{{.Names}}' | grep -E 'mm-aep-postgres-dev|mm-aep-temporal-dev' +``` + +Bring-up order (each in its own shell; details + env in the reference): +1. `yarn dev:postgres` (docker `postgres:16-alpine`, `mm-aep-postgres-dev`, port 5432) +2. `yarn dev:temporal` (temporal dev server; UI on 8233) +3. `yarn db:migrate` +4. **worker** — `yarn dev:worker` on **Node ≥ 24.13**, env `ANTHROPIC_API_KEY=host-subscription`, `CLAUDE_CODE_EXECUTABLE=~/.local/bin/claude`, `GITHUB_TOKEN="$(gh auth token)"`, `SANDBOX_PROVIDER=local` (needs JFrog `npm login` first; relies on uncommitted local patches) +5. `yarn dev:control-plane` (`localhost:3000`) + +If any of the local patches (`local-sandbox-adapter.ts` timeout, `claude-agent-runner.ts` auth, the `perf-validation/` graph) are missing from the working tree, the reference says how to restore them — `git status` in the AEP repo should show them modified/untracked. +## Run mechanics — submit, poll, fetch + +The control-plane is a thin REST shell. Submit a PR-validation task, poll the run, pull artifacts from the evidence bundle. + +```bash +CP=localhost:3000 +PR="https://github.com/MetaMask/metamask-extension/pull/" + +# Submit (publishEvidence:false ALWAYS for local runs — the platform otherwise +# writes to the public PR body even on failure, leaking local paths/usernames) +RUN_ID=$(curl -fsS -X POST "$CP/v1/tasks" -H 'content-type: application/json' -d '{ + "repo": "MetaMask/metamask-extension", + "title": "Visual validation — PR #", + "taskClass": "visual_validation", + "externalRef": "'"$PR"'", + "payload": { "prUrl": "'"$PR"'", "description": "", "publishEvidence": false } +}' | node -e 'process.stdin.on("data",d=>console.log(JSON.parse(d).runId||JSON.parse(d).id))') + +# Poll +curl -fsS "$CP/v1/runs/$RUN_ID" | node -e 'const r=JSON.parse(require("fs").readFileSync(0));console.log(r.status); (r.evidenceBundle?.artifactRefs||[]).forEach(a=>console.log(a.name,a.mediaType))' + +# Fetch an artifact +curl -fsS "$CP/v1/runs/$RUN_ID/artifacts/" -o /tmp/ +``` + +- `taskClass`: `visual_validation` or `perf_validation`. The worker auto-enriches the payload from `prUrl` (pulls headSha, base, diff, files, linked issues via the GitHub app) — you only supply `prUrl` + a `description` targeting hint. +- The **targeting hint** (`payload.description`) is how you steer the agent to the surface under test. Be specific: which screen, which control, what to toggle. For hard-to-reach surfaces, name the reachable fallback (e.g. the Shield entry modal stands in for the Perps tutorial modal, which is gated in the default fixture). +- Artifact regex allows **png/jpg/log/txt only** — no video. Screen recordings need the side-channel recipe (catalog + publishing reference). + +### Concurrent runs (multiple agents / parallel lanes) + +Five shared resources need per-run isolation on one machine — collisions cross-contaminate evidence *silently* (wrong session's logs attributed to a run), which is an integrity failure, not flakiness: **(1)** CDP debug ports — derive per run, never hardcode; **(2)** e2e harness service ports (anvil/proxy/fixture/mocha) — one e2e run at a time per worktree, one worktree per agent (`wt new`), and never rebuild `dist/` in a worktree with an active run; **(3)** artifact dirs — per-run namespaces; `test-artifacts/` is per-worktree shared state, harvest failure artifacts before the next run overwrites the same test-title dir; **(4)** evidence-repo uploads — run-scoped paths (`pr-//`), retry-with-fresh-sha on 409, never overwrite another run's published files; **(5)** commit-pinning — pin only after your own final upload lands, verifying your files exist at that sha. Safe to share: JFrog login, a read-only `dist/`, the AEP stack itself. + +### Trust the evidence (anti-reward-hacking) + +A green result is not proof. The vacuous-pass trap is the floor: if `promptCrafter` errors, the chain "passes" via skip with **zero artifacts** — a pass is only real if `evidenceBundle.artifactRefs` is non-empty with the expected media. Beyond that, every lane must clear a trustworthiness gate before you believe or publish it: **does the artifact show the *claimed* surface** (not a spinner/wrong screen), **does the test exercise the *changed* code** (fails on `main`), **does the signal exceed noise**, **could the assertion have failed**? The Claim Card's Falsifier is the anchor. Full gate + per-lane traps: **[references/evidence-trustworthiness.md](references/evidence-trustworthiness.md).** + +### perf_validation caveat + +The `perf-validation/` graph is **uncommitted local AEP work** (added 2026-06-11). It writes falsifiable network/static/smoke assertions and gives the tester deterministic `.aep/` helpers (CDP netlog, phase segmentation, source-map chunk membership). It requires a `yarn webpack --test` build first (the browserify `build:test` has no code splitting, so `import()` never hits the network there). Temporal caps activity results at ~2MB — artifact refs must be content-free; only `evidenceBundle` carries base64. If the graph isn't in the working tree, perf runs won't register — fall back to manual DevTools/CDP capture (catalog). +## Teardown — always, on every exit path + +The stack is the heaviest thing this skill starts — postgres + temporal + a Node worker + control-plane — and the worker holds a live Claude session while the autonomous run itself spends tokens. It is **on-demand, not resident**: bring it up for the validation window, **tear it down when the run(s) finish**. Left up, it's the single largest reclaimable footprint on a shared host and quietly keeps a Claude seat warm. + +- **On a host managed by `aep-stack` (systemd):** `aep-stack up` to preflight, **`aep-stack down` when done** — stops the services; the `--rm` postgres/temporal containers are removed, so state resets on the next `up` (fine — each run is fresh anyway). +- **Otherwise:** stop the `yarn dev:*` processes and `docker rm -f mm-aep-postgres-dev mm-aep-temporal-dev`. +- **Tear down on every exit path** — pass, refutation, *or* abort. A failed or abandoned run leaves the stack up exactly as much as a passing one; the usual leak is walking away after a refutation without stopping it. diff --git a/domains/pr-workflow/skills/evidence/references/claim-extraction.md b/domains/pr-workflow/skills/evidence/references/claim-extraction.md new file mode 100644 index 00000000..e5654608 --- /dev/null +++ b/domains/pr-workflow/skills/evidence/references/claim-extraction.md @@ -0,0 +1,63 @@ +# Claim extraction + +The linchpin of evidence: before choosing any lane, turn the PR into a **falsifiable, surface-specific claim**. Every lane is only as good as the claim it tests. A vague claim ("improves perf", "fixes the bug") can't be proven or refuted; a sharp claim names the precondition, action, observable outcome, and what would disprove it. + +## Read these, in order + +1. **PR body** — Description (what/why), `Fixes #N`, Manual testing steps, the Before/After intent. +2. **Linked issue(s)** — the bug report / acceptance criteria; "Steps to reproduce" and "Expected vs actual" are the claim in the reporter's words. +3. **The diff** (`gh pr diff`) — what actually changed: which surfaces, controllers, modules. The claim must be anchored to what the code can do, not only what the body promises. +4. **Labels / type** — bug vs feat vs perf vs refactor changes the claim shape (see [special cases](#special-cases)). + +## Extraction steps + +1. **Asserted change** — what does the PR say it does? (body + issue) +2. **Anchor to the diff** — which surface/module changed? Reconcile intent with the diff. If the body promises X but the diff can't deliver X, **flag the drift** — that's a finding, not a claim. +3. **Phrase as falsifiable** — `Given , when , then .` The outcome must be observable and checkable. Replace vague verbs (improve / fix / handle / support) with the concrete observable. +4. **Pin the surface + reachability** — exact screen / API / metric. Reachable in the default fixture, or does it need state seeding, a feature flag, or a fallback surface? + - **A surface need not be a screen.** A pipeline's job graph, a build artifact, a policy file, a telemetry shape, or a harness's determinism are all legitimate surfaces with their own falsifiers. Do not force a user-visible observable onto a claim that does not have one — routing a CI or build claim through a product effect is the *wrong* bar, not a stricter one. + - **When the changed code is the automation, the PR's own run may not exercise it.** A CI-config diff commonly skips the very path it edits (build reuse, `needs-*` resolution, event-type conditions). Execute the changed workflow where its trigger conditions hold — a test fork, a branch whose name satisfies the condition — with the failure state forced. Name that substitution explicitly; a claim about *this* repo's pipeline is not proven by a run on another. +5. **Classify the type** → routes to lanes via the matching guide: visible UI · non-visible perf · telemetry · persisted-state · build-output · behavior-no-UI. +6. **Decompose mixed claims** — a PR that changes UI *and* shifts a metric is two claims; validate each. + +## Claim Card (output) + +``` +Claim: Given , when , then . +Surface: (reachable? seed / flag / fallback: …) +Type: → lanes +Falsifier: +Baseline: +``` + +One card per claim. For a refactor, the claim is a **negation** (see below). + +## Claim quality bar + +A good claim is **falsifiable** (observable outcome + clear falsifier), **surface-specific** (names the exact screen/API/metric, not "the app"), **diff-anchored** (the changed code can plausibly produce it), **bounded** (one behavior, one precondition), and **measurable** where quantitative (a number + threshold, not "faster"). + +## Anti-patterns → refinements + +| Vague claim | Refined | +|---|---| +| "Improves performance" | "Opening the Activity tab: TBT drops below 200ms (was >600ms)" — name the interaction, metric, threshold | +| "Fixes the bug" | "With privacy mode on, the Perps tab balance is masked" — observable behavior + precondition + surface | +| "Refactor, no behavior change" | Negation claim: "behavior of `` is unchanged" → prove via a red-on-base test that stays green / snapshot / identical output, **not** a screenshot | +| "Adds a null check" (restates the diff) | "No crash when `` is null on ``" — the behavior, not the code | +| Body promises X, diff does Y | Not a claim — **flag the drift** to the author | + +## Special cases + +- **Refactor / no-op:** the claim is "nothing observable changed." Falsifier = any behavior/output diff. Lanes: regression test stays green, snapshot diff empty, bundle/output identical (D1/D2), benchmark within noise. A passing screenshot proves nothing here. +- **Bug fix:** the strongest claim form ships its own falsifier — a test that fails on `main` and passes on the branch (catalog **B3**). Extract the claim straight from the issue's "Expected vs actual." +- **Perf:** always quantify — metric + interaction + threshold + baseline. Without a number it isn't falsifiable. +- **Persisted-state / migration:** claim = "upgrading from `` preserves `` and applies ``." Falsifier = corrupted/lost state. Baseline = a profile from the prior version (catalog **F1**). +- **Flag-gated:** two claims, one per flag state (catalog **F5**). + +## Worked examples + +- **Visible (#42683):** body "privacy mode doesn't hide the Perps balance"; issue: expected masked, actual visible; diff touches the Perps balance component. → **Claim:** *Given privacy mode on, when I open the Perps tab, the balance is masked.* **Surface:** Perps tab (gated → fallback: Shield entry modal). **Type:** visible → A1/B1. **Falsifier:** balance digits visible under privacy mode. **Baseline:** same flow on base reproduces the bug. +- **Perf:** body "defer Rive wasm at startup"; diff: dynamic `import()` of the Rive runtime. → **Claim:** *On cold start of the home view, the Rive wasm chunk is not requested until the animation surface mounts.* **Surface:** startup network + chunk graph. **Type:** perf → A2/C6/D2. **Falsifier:** the chunk appears in the cold-start waterfall. **Baseline:** base requests it at startup. +- **Migration:** diff adds migration NNN. → **Claim:** *Loading a profile from `` applies migration NNN; `changedKeys = {}`; all other state intact.* **Type:** state → F1. **Falsifier:** an untouched controller mutated, or migrated state malformed. **Baseline:** a prior-version profile. + +A sharp claim is also a good recipe **proof target** (ADR-0058): precondition → action → observable maps to pre-conditions → assertions → screenshot points. Extraction pays off in both lanes. diff --git a/domains/pr-workflow/skills/evidence/references/evidence-catalog.md b/domains/pr-workflow/skills/evidence/references/evidence-catalog.md new file mode 100644 index 00000000..adca4077 --- /dev/null +++ b/domains/pr-workflow/skills/evidence/references/evidence-catalog.md @@ -0,0 +1,300 @@ +# Evidence catalog + +The menu of evidence kinds for validating a MetaMask **extension** PR, with **what each proves**, **how to capture it (verified against the live repo)**, and **when to reach for it**. AEP is the primary autonomous engine; the rest are complementary. The skill's job is to **match evidence to the claim** and to **proactively suggest kinds the author didn't think of**. + +Pick the evidence that would **falsify the claim if it were false**. Prefer a lane that yields an artifact a reviewer can independently re-check (a link, an image, a number, a replayable trace) over prose. Don't run the whole menu — match, then capture. Capture commands cite `~/Code/metamask/metamask-extension`; verify script names against its `package.json` (they drift). + +Legend: **first-class lanes** are `##`-headed; closely-related variants are sub-bullets. Capture marked *(manual)* has no repo helper — it's a DevTools/CDP action. + +--- + +## Lanes at a glance + +43 lanes in 7 families. Each lane below has a full spec in its own section — what it proves, how to capture it, and its trust gate. Family G is written as one-liners rather than full sections, because those lanes are links and counts rather than captures. + +| Family | Lanes | | +|---|---|---| +| **A. AEP harness (primary, autonomous)** | 3 | `A1` visual_validation · `A2` perf_validation · `A3` AEP bundle byproducts | +| **B. Behavior & flow proof** | 7 | `B1` Visual before/after via the mm CLI · `B2` E2E trace + video · `B3` Falsifying regression test · `B4` Component / Storybook visual · `B5` Accessibility · `B6` Flaky-stability rerun · `B7` Deterministic interleaving test | +| **C. Performance & render** | 9 | `C1` Startup / custom traces + phase segmentation · `C2` Web vitals · `C3` Long-task / TBT · `C4` React render & selector proof · `C5` Benchmark A/B · `C6` DevTools / CDP profiling · `C7` Memory stability over a flow · `C8` Same-window app + DevTools capture · `C9` Retention-path analysis | +| **D. Build** | 7 | `D1` Bundle-size diff · `D2` Chunk membership / source-map · `D3` LavaMoat policy / supply-chain capability diff · `D4` Manifest permissions diff · `D5` Build-variant matrix · `D6` Authored-vs-authoritative substitution A/B · `D7` Build & rebuild duration A/B | +| **E. Production telemetry** | 3 | `E1` Sentry query links · `E2` Tempo distributed traces · `E3` Sentry error-event / breadcrumb shape | +| **F. Extension integrity (high-stakes, extension-specific)** | 8 | `F1` State migration / upgrade · `F2` Vault / keyring round-trip · `F3` Transaction simulation / gas · `F4` Provider / dapp connectivity · `F5` Feature-flag matrix · `F6` Snaps / multichain execution · `F7` i18n usage · `F8` SES lockdown / runtime containment | +| **G. CI, review & process** | 6 | `G1` CI check links · `G2` Coverage delta · `G3` Automated-reviewer output · `G4` Manual reproduction steps · `G5` CI-workflow change, run on a test fork · `G6` CI job-duration delta | +--- + +# A. AEP harness (primary, autonomous) + +## A1. visual_validation — before/after screenshots +- **Proves:** a visible UI change on the real surface. Deterministic state seed + agent navigation; PNG artifacts in `evidenceBundle.artifactRefs`. +- **Capture:** `taskClass: visual_validation`, `payload.prUrl` + `description` hint. See [aep-local-run.md](aep-local-run.md). +- **Reach for it:** anything a human would screenshot for the PR's `### After`. + +## A2. perf_validation — falsifiable network/static/smoke assertions +- **Proves:** non-visible behavior (hover-preload, no double-fetch, chunk membership, smoke boot). CDP netlog / phase segmentation / source-map membership. +- **Capture:** `taskClass: perf_validation` (local/uncommitted graph; needs `yarn webpack --test`). Falls back to C6/D2 manually if the graph isn't present. + +## A3. AEP bundle byproducts (free with any run) +- Test results (`executionResult`/`checkResults`), diff stats, automated `reviewResult` findings, and the **LangSmith trace** of the run. Include the relevant subset; link the trace for auditability. + +--- + +# B. Behavior & flow proof + +## B1. Visual before/after via the `mm` CLI (`visual-testing`) +- **Proves:** UI behavior on a real headed build, with controlled state/network. Defers to the public `visual-testing` skill. +- **Capture:** `yarn build:test:webpack` → `dist/chrome`; `yarn mm launch` → `mm describe-screen` / `mm screenshot` / `mm click` / `mm type` / `mm navigate`. README: `test/e2e/playwright/llm-workflow/`. + - **Degraded-path:** `mm mock-network` to force error/slow responses (session-scoped; add after launch, before the action; can't intercept pre-launch startup). + - **a11y / DOM:** `mm accessibility-snapshot` and `mm cdp` (per the `visual-testing` skill; `a11yRef`s are ephemeral — re-describe after navigation). + +## B2. E2E trace + video (Playwright / Selenium) +- **Proves:** a full flow works, replayably. The strongest "it works end-to-end" artifact. +- **Capture (Playwright):** `yarn playwright test `; trace is `'on'` by default (`playwright.config.ts`), video is `'off'` (enable in config if needed). View: `yarn test:e2e:pw:report`. Artifacts under `public/playwright/`. +- **Capture (Selenium):** `yarn test:e2e:single --browser chrome|firefox|all [--retries n]`; screenshots auto-captured on failure to `test/test-results/e2e/`. + +## B3. Falsifying regression test ⭐ +- **Proves — strongest single proof a fix targets the bug:** a new test that **fails on `main` and passes on the branch**. Show both runs. + - **Engine: the `red-on-base` skill.** +- **Capture:** add the test, run it on the PR branch (pass) and on the PR's **merge-base** (fail) — pin the base, don't use whatever `main` points at today. Pair with the PR's `Fixes #N`. **Read the base failure's message, not its exit code:** it must fail on the assertion that encodes the bug. A `ModuleNotFoundError`, a missing fixture, or an unrelated pre-existing red produces an identical non-zero exit and falsifies nothing. +- **Reach for it:** every bug-fix PR. If you can't write a test that fails on main, question whether the fix addresses the reported bug. + +## B4. Component / Storybook visual +- **Proves:** a component renders across states/props in isolation. +- **Capture:** `.storybook/` present; `yarn storybook` (port 6006), `yarn storybook:build`, `yarn test-storybook` (visual + a11y via `@storybook/addon-a11y`). Jest snapshot diffs for serialized output. + +## B5. Accessibility (a11y) +- **Proves:** no a11y regression / an a11y improvement. +- **Capture:** `yarn test-storybook` (Storybook a11y addon) for components; `mm accessibility-snapshot` for live flows. (No axe-core in the e2e suite — don't claim it.) + +## B6. Flaky-stability rerun +- **Proves:** a flow/test is not flaky (or that a fix removed flakiness). +- **Capture:** Playwright retries `1` on CI / `0` local (`playwright.config.ts`); Selenium `--retries n`; benchmarks default `--retries 2`. Run N× and report the pass rate. See `e2e-flakiness-patterns`. +- Sub: jest snapshot diffs; a unit run for just the changed module (`yarn test:unit `); fuzz/property tests for parsers/encoders. + +--- + +## B7. Deterministic interleaving test (concurrency / temporal-ordering) ⭐ +- **Engine:** `race-condition-repro` — run it rather than hand-rolling the harness. +- **Proves:** an ordering guarantee under interleaving — retry, cancellation, supersession, debounce, locks, queues, async state machines — where the correctness *is* the ordering under races, not a value. +- **Capture:** force each race deterministically — `jest.useFakeTimers()` + `advanceTimersByTimeAsync(DELAY)` to fire the delayed action at a known point; `Promise.all([opA, opB])` to overlap operations; `advanceTimersByTimeAsync(0)` to step to a precise interleaving point; then assert the ordering/cancellation outcome for **each** guarantee, including asymmetric ones (one path canceled → its recovery event `.not.toHaveBeenCalled()`; another must complete → `.toHaveBeenCalledWith(...)`). Corroborate with transition telemetry; for the integration path, a live forced-race capture (C8/CDP, the #44610 technique). +- **Trust-gate:** the test must **actually interleave** — time advanced into the pending window, the superseding op injected *during* it. A sequential run exercises no race and is a vacuous green. Verify the interleaving, not just the assertion. + +# C. Performance & render + +## C1. Startup / custom traces + phase segmentation +- **Proves:** which startup phase moved (init → FirstRender → interactive), per named span. +- **Capture:** `shared/lib/trace.ts` `TraceName` enum (UIStartup, LoadScripts, FirstRender, …); read in test/debug via `window.stateHooks.getCustomTraces()`. LCP fallback mark: `performance.mark('mm-hero-painted')`. `driver.collectMetrics()` aggregates paint/navigation/long-task/custom traces in e2e. + +## C2. Web vitals — INP / FCP / LCP / CLS +- **Proves:** a user-centric metric moved. `ui/helpers/utils/web-vitals.ts` via `web-vitals/attribution` (attribution names the causing element). +- **Capture:** `window.stateHooks.getWebVitalsMetrics()` (test/debug) → `{inp, fcp, lcp, cls, *Rating}`. Thresholds: INP good<200/poor>500, FCP<1800/3000, LCP<2500/4000, CLS<0.1/0.25. +- **Caveat:** **INP fires on all pages; FCP/LCP/CLS do not fire on popup pages** (sidepanel/E2E only). For extensions, INP is the high-value runtime metric. + +## C3. Long-task / TBT +- **Proves:** main-thread blocking during an interaction dropped. This is where **TBT** lives (the web-vitals lib lane does *not* collect TBT). +- **Capture:** `ui/helpers/utils/performance-observers.ts`; `window.stateHooks.getLongTaskMetricsWithTBT()` → `{count, totalDuration, maxDuration, tbt, tbtRating}`. TBT good<200 / needs-improvement<600 / poor>600. Sampled 10% prod / 100% test. + +## C4. React render & selector proof + - **Engine: the `react-render-delta` skill.** Delegate the measurement to it; it runs the source/delivery/metric gates, derives the needle from real build output, repeats the capture, and returns a band (or "not resolvable at this n" with an MDE). evidence packages the result. +- **Proves:** a component/selector stopped over-rendering (cascade-amplification before/after). +- **Capture:** WDYR via `ENABLE_WHY_DID_YOU_RENDER` (`.metamaskrc` or env) — wired in `app/scripts/development/wdyr.ts` (`trackAllPureComponents`); console logs each unnecessary re-render. `yarn devtools:react` for the Profiler flame graph. Selectors use `reselect`'s `createSelector`, which **does expose a real `.recomputations()` counter** — read it (sample on an interval if the count should visibly climb) rather than injecting a log into the selector body; an injected log is an authored claim, a library API is an observation. *(This entry previously said there was no built-in counter. There is.)* +- **Bar:** the delivery check comes before the number. An arm whose manipulation cannot be observed in the built bundle produces a null indistinguishable from "small effect" — and reports as the second. + +## C5. Benchmark A/B +- **Proves:** a startup/journey/interaction timing moved, with a distribution not one sample. +- **Capture:** `yarn test:e2e:benchmark` (`test/e2e/benchmarks/run-benchmark.ts`); presets in `shared/constants/benchmarks.ts` (`startupStandardHome`, `sendTransactions`, `swap`, `dappPageLoad`, …). +- **Caveat:** the rolling baseline (`MetaMask/extension_benchmark_stats`) can **silently freeze** behind a green check (the `store-benchmark-stats` step is `continue-on-error`; happened 2026-04-02, PR #42947). Prefer a **paired A/B** (build both refs now, compare directly) over the stored baseline. +- **Treatment check first** — before trusting any delta, confirm the mechanism under test is actually active in each arm (split chunk present in head and absent in base; the span emitted; the flag evaluated). An arm without the treatment delivered is a no-op, not a control (2026-07-22, #42795). +- **A null needs its power stated** — "no change" and "underpowered" print the same result. When the run-to-run spread exceeds the effect under test, report **not resolvable at this n** and name the smallest detectable effect; never let it read as "no effect". Correcting a known bias (discarding a warm-up, alternating the starting arm) removes *that* bias and nothing more — it is not a trust gate, and the confounds you did not enumerate (thermal drift, background load, ordering within a round) stay live. + +### Capturing an authenticated view (the in-situ requirement) + +Headless Chrome's `--screenshot` cannot set cookies, so an authenticated dashboard +(Grafana/Tempo, Sentry Discover, an internal panel) screenshots as a login page. Drive +Chrome over CDP instead — inject the session cookie, navigate, capture: + +```bash +COOKIE_NAME=grafana_session COOKIE_VALUE="$sess" COOKIE_DOMAIN= \ + cdp-shot "" out.png 25000 1500 2400 +``` + +- **Deep-link to the exact view** so the capture and the reader's verification path are the + same URL (Grafana: `/explore?schemaVersion=1&panes=`). +- **Wait generously** — a trace waterfall or Discover table renders well after `load`. +- **Capture tall + `captureBeyondViewport`**, then crop; the interesting span is usually + below the fold, and cropping after the fact beats guessing a viewport. +- **Crop out the chrome that identifies the operator** (profile avatar, org switcher) + before the image leaves the machine. +- **Keep the trace/query id, timestamp, and result count in frame** — that is what makes + the exhibit reproducible rather than decorative. +- Never echo the cookie value, never commit it, never pass it to a subagent. + +## C6. DevTools / CDP profiling *(manual)* +- **Proves:** a flame-chart hot path shrank, a request was removed/deferred, frame rate held, or it holds on slow hardware. +- **Capture (manual via DevTools or `mm cdp`):** performance profile / flame chart; network waterfall (HAR) + request-count delta; **CPU throttling** (CDP `Emulation.setCPUThrottlingRate` — *no repo helper*, set it in DevTools); **animation/Rive FPS / dropped frames** (DevTools rendering FPS meter — *no repo helper*); JS coverage for dead-code. + +## C7. Memory stability over a flow *(manual)* +- **Proves:** a leak is fixed across repeated interactions (not one snapshot): retained heap stays flat, detached DOM nodes / listeners don't accumulate. +- **Capture:** DevTools heap snapshots before/after N cycles of the flow; compare retained size + detached nodes. +- Sub: redux dispatch/action count per interaction; network payload bytes; forced-reflow / layout-thrash count (DevTools Performance). + +## C8. Same-window app + DevTools capture *(manual)* +- **Proves:** the UI behavior **and** its internal evidence (console log, network row, storage state) in **one frame** — cause and effect temporally correlated in a single artifact. Two separate captures can't prove they came from the same run; one frame can. Canonical use: "the toast does NOT appear *while* the console shows the silent-handling path executed". +- **Capture (macOS, OS-level — Playwright `recordVideo` sees only the page viewport, never DevTools):** + 1. Tab-target DevTools: launch Chrome with `--auto-open-devtools-for-tabs` so DevTools opens **docked in the same window** (dock side persists per profile; set once via the DevTools ⋮ menu if a fresh profile defaults to undocked). + 2. MV3 **service-worker console has no dockable host** — open its dedicated inspector (`chrome://extensions` → *Inspect views: service worker*) and tile it flush beside the app window: `osascript -e 'tell application "Google Chrome" to set bounds of front window to {x, y, w, h}'` (the SW inspector is a Chrome window too and tiles the same way; CDP `Browser.setWindowBounds` also works per `windowId`).\ + 3. Record the union region, not a single window: stills `screencapture -x -R out.png`; video `screencapture -v -V -R out.mov`, then ffmpeg two-pass palette → GIF (recipe in [evidence-publishing](evidence-publishing.md)). First use prompts for macOS Screen Recording permission for the terminal. +- **Legibility rule:** console text dies in GIF downscale. Keep the GIF ≥720px wide, and pair it with (a) a full-res PNG of the same frame and (b) a text dump of the console via CDP (`Runtime.consoleAPICalled` on the SW target, `npx mm cdp` or a 20-line ws script) so the log lines are quotable/searchable. +- **Trust note:** arrange windows *before* triggering the behavior so the recording shows trigger → console line → UI (non-)reaction as one continuous take; a post-hoc composite of separate captures is exactly what this lane exists to avoid. + +--- + +## C9. Retention-path analysis — memory leak from code ⭐ *(static; lead for leak claims)* +- **Engine: the `memory-leak` skill.** For a memory-leak claim, delegate the analysis to `memory-leak` — it runs Phase-1 static pairing (and Phase-2 heap investigation if a primitive can't be paired) and returns the paired/unpaired sites + verdict. evidence keeps **memory leak** as the evidence category: it invokes the skill on the diff and packages the result (in-situ scan capture, plus the lifecycle test / retainer graph if Phase 2 ran) as the category's evidence. The lane spec below is the method that skill implements. +- **Proves:** "X is retained past its lifecycle boundary" / "collection Y grows unboundedly" — argued from code, no runtime needed. This is the lane that works at **review time** (does this PR *introduce* retention?) and leads fix-side validation (does the fix *break* the retention path?). C7 is the runtime corroborator, not the lead — leaks need many cycles to exceed noise. +- **Capture — the holder → held → boundary triple, per suspect:** (1) the **holder** (listener, closure, module singleton, accumulating collection, timer); (2) the **held set** — the *specific* objects pinned (list the closure's captures; note when a closure links two objects' GC); (3) the **outlived boundary** (`destroy()`, stream close, instance replacement, request completion). Method: **pair every acquire with its release site** (`on`↔`removeListener`, push↔drain, assign↔null) — the absence of the pair, cited at the acquire site, IS the finding. Four canonical shapes: unbounded accumulator (defeated guard, no drain) · stale-instance listeners on replacement · unremoved listener + capture set · retention past `destroy()`. +- **Scope to the diff, or you invent findings.** Classify every flagged primitive as *introduced by this PR* (in the added lines) vs *pre-existing* (already in the file). Charge only the introduced ones to the PR; report pre-existing un-paired primitives separately and uncharged. On extension#40684 the two new stream listeners each had a `removeListener` on `onStreamClosed` (the exact fix a reviewer suggested) and the new pending-request Map had its `.delete` — no leak introduced — while three pre-existing un-torn-down listeners were surfaced but left uncharged, matching how the human/bot reviewers treated them in-thread. This lane *is* the retention review automated; a heap snapshot (C7) is warranted only for an introduced primitive it cannot pair. +- **Corroborate:** a falsifying lifecycle test (force the boundary, assert release — listener count zero, singleton nulled, collection drained); C7 heap-over-flow with the **retainer graph naming the same path** the static argument named. +- **Trust-gate:** the triple must be specific ("this listener holds `patchStore` after `patchStore.destroy()`", not "might leak"); distinguish **bounded staleness vs unbounded growth** (severity differs); attribute **introduced vs pre-existing** honestly. + +# D. Build + +## D1. Bundle-size diff +- **Proves:** the build grew/shrank by a measured amount. Use the bundle-size CI output or a local build size comparison. + +## D2. Chunk membership / source-map +- **Proves:** a module moved to the intended (lazy) chunk and no longer ships on the critical path. Requires the webpack build. Mirrors AEP `perf-chunks`. + +## D3. LavaMoat policy / supply-chain capability diff + - **Engine: the `supply-chain-audit` skill** (umbrella — lockfile/manifest diff, advisories, Socket Security, install scripts), which delegates capability grants to **`lavamoat-policy`** (per-grant call-site justification). Delegate the dependency change to the umbrella; it returns a disposition per lane. evidence keeps **supply-chain capability diff** as the evidence category and packages the output. Note the lanes are independent: a clean policy diff does not mean a safe dependency, and a known CVE never appears as a new grant. +- **Proves:** a dependency change (bump/add/lockfile) grants **no *unjustified* new capability** — the supply-chain-risk lane. Note the bar: for a bump the policy *will* change, so "empty diff" is the WRONG test; the right test is **every new grant is justified by the dep's function**.. +- **Capture:** **Prefer the CI-generated policy whenever one is available.** `@metamaskbot update-policies` regenerates the policy files from a real run of the code and `validate-lavamoat-policies` fails the build on drift, so the committed policy on a bot-run PR *is* the authoritative artifact — diff that. Regenerating locally when a current CI policy exists only re-does a machine that is already trusted, and a local run's provenance is weaker (your node/OS/lockfile resolution, not CI's). **Local regen is the fallback**, for when the bot hasn't run yet, the branch is unpushed, or you need a variant CI didn't cover: `yarn webpack:lavamoat:policy:build` (`:mv2` / `:mv3` for variants) over `lavamoat/webpack/build/policy.json` (+ `policy-override.json`). Either way, `git diff` the policy across **all 8 variants** (mv{2,3}/{main,beta,flask,experimental}) — a grant can appear in one and not others. Then audit **grant-by-grant**: new **globals** (`fetch`, `importScripts`, `WebAssembly`) / **builtins** (`fs`, `child_process`) on a dep that shouldn't need them, new **packages** edges to powerful APIs, or an identifier substitution (`pkgC>name` replacing `pkgB>pkgA>name` = possible dep swap). Falsifier = a surprising grant ("I wonder what it's using this for"). Guide: lavamoat.github.io/guides/policy-diff/. `allowScripts` in `package.json` gates install scripts. + +## D4. Manifest permissions diff +- **Proves:** no permission/host-permission scope creep. +- **Capture:** `git diff app/manifest/v3/_base.json app/manifest/v2/_base.json` (+ `chrome.json`/`firefox.json`). Flag new sensitive perms (webRequest, broad host patterns). + +## D5. Build-variant matrix +- **Proves:** the change works across build types, not just main. +- **Capture:** `yarn build:test:flask` / `:beta` / `:mv2` (`ENABLE_MV3=false`, Firefox). Run the relevant lane per variant when behavior is build-type-gated. + +## D6. Authored-vs-authoritative substitution A/B ⭐ *(fixed head; lead for "the artifact restates a source" claims)* +- **Proves:** whether an artifact the PR *hand-wrote* agrees with the source it restates — a type vs the value's real type, a hand-maintained schema vs the generated one, a vendored constant vs the upstream export, a checked-in policy vs `update-policies` output. The finding is the **delta in a checker's output**, not a reading of the diff. +- **Shape:** both arms sit at the **same commit**; they differ by a *substitution*, not by a ref — so there is no build, no rebase, and no merge boundary to confound. + - **Arm A** — the PR as written, run through the checker. Must be **silent**. A non-empty Arm A means the instrument is broken and Arm B is unreadable (see trustworthiness gate item 19). + - **Arm B** — same tree, with the authored artifact replaced by the **derived** equivalent, exercised exactly as the real code exercises it. Every new diagnostic is a disagreement the authored version concealed. +- **Capture (TypeScript worked example — extension#44397, 2026-07-30):** + ```bash + # Arm A — baseline. Expect zero errors. + NODE_OPTIONS='--max-old-space-size=9216' npx tsc -p tsconfig.json --noEmit + # Arm B — probe files that substitute the derived type and call it as the caller does. + mkdir -p app/scripts/derive-probe && cp probe-*.ts app/scripts/derive-probe/ + NODE_OPTIONS='--max-old-space-size=9216' npx tsc -p tsconfig.json --noEmit # diagnostics = the findings + rm -rf app/scripts/derive-probe + ``` + One probe per claim, each naming the authoritative source in a header comment and calling the derived type the way the real call site does. Keep the probes as the artifact — they are the re-runnable falsifier. +- **Why it finds what review and CI miss:** the authored artifact compiles, so CI is green *by construction*. In a partially-migrated repo the asymmetry is structural — with `checkJs` off, a type written for a function whose callers are still `.js` is checked against nothing, and drifts silently forever. Those boundaries are where the lane pays. +- **Traps:** (a) **a substitution can fail for the wrong reason** — a diagnostic on an earlier property short-circuits the one under test, and counting exit codes reads that as confirmation; assert on the *specific* diagnostic, and re-probe with the earlier cause neutralised (`NonNullable<…>`, a targeted assertion) to isolate each claim. Same hazard as B3's "fails on base for the wrong reason." (b) **no authoritative source may exist** — an unshipped package's types, a lib not in tsconfig `lib`, a genuinely new boundary the repo owns. Hand-writing is then *correct*; report it as a cleared falsifier, not a finding. +- **Pairs with:** [lane-assertions.md](lane-assertions.md) for the recipe form; D3 when the substituted artifact is a LavaMoat policy. + +--- + +## D7. Build & rebuild duration A/B *(paired; lead for toolchain-change claims)* +- **Proves:** what a toolchain change costs or saves in the **dev loop** — a loader, transform, linter, or bundler swap. Distinct from `C5`, which times the shipped app at runtime; this times the build that produces it. The two move independently and in opposite directions often enough that measuring one and inferring the other is the failure this lane exists to prevent (`React Compiler` builds slower and runs faster; `thread-loader` builds faster and runs identically). +- **Shape:** paired A/B, both arms built now, on one machine, alternating order. **Cold and warm are separate questions and get separate numbers** — never one figure labelled "build time". +- **Capture:** N ≥ 5 per arm per mode, alternating. Cold: clear the cache explicitly between arms (`node_modules/.cache`, webpack `cache.cacheDirectory`) and state what was cleared. Warm: touch one source file, rebuild, discard the first result as pool warmup. Report median **and spread**; a median without spread hides a bimodal cache effect. +- **Falsifiers — each returns a favourable number when uncontrolled:** warm cache leaking into the "cold" arm (the largest confound, and the easiest to introduce by running arms in sequence); worker-pool startup counted once and amortised across rebuilds; core count, since parallel loaders scale with the runner and a laptop result does not transfer; watch-rebuild numbers presented as cold-build numbers. +- **Trust-gate:** state machine, core count, N, and cache handling per arm, or the number is unreproducible. A null result states the smallest effect the sample could have detected — "no difference" from N=3 is not a finding. Renders **no ship verdict**: a change that costs build time and buys runtime is a trade, and pricing it is not the same as taking it. +- **Corroborate:** `G6` for the CI half (different machine, different confounds), `C5` for the runtime half. A toolchain claim is not closed by one surface. + +# E. Production telemetry + +## E1. Sentry query links (before/after) +- **Proves:** error-rate / transaction count / latency moved in prod. A link a reviewer opens beats a chart screenshot. +- **Capture:** Sentry MCP (`search_events`/`search_issues`) → hand the discover/dashboard link with the before/after window, scoped to the release. Projects: `metamask` = prod, `metamask-performance` = CI. +- **Boundary:** PRs that *add/change span instrumentation* (volume/quota) → `/sentry-quota`, not this lane. +- **Perf-PR promotion (standard, not just complementary):** for a **performance-focused PR**, the main-branch Sentry **trend** for the affected metric across the PR's merge (before/after the merge commit's release) is **standalone lead evidence** — CI already sends every main/release `startupPowerUserHome` / journey benchmark to Sentry, so the trend is a real before/after on the actual metric, continuously tracked, with no local run. Prefer it over a local paired A/B when a clean merge-boundary window exists: it sidesteps the stale committed-baseline trap (`benchmark-baseline-staleness-paired-ab`). Still bound to the trust gate — a **windowed, release-scoped, one-click-resolvable** trend link with the merge boundary visible, never a prose "looks fine." A local interleaved paired A/B (C5) remains the precision complement when the merge window is noisy or the metric CI doesn't track (selector-eval count, re-render count, INP-on-typing — none of which CI captures). + +## E2. Tempo distributed traces +- **Proves:** a span/transaction now appears / is shaped correctly (e.g. background-RPC tracing). Link the trace + note the release. + +## E3. Sentry error-event / breadcrumb shape +- **Proves:** an instrumentation PR captures the intended error-event state / breadcrumbs (relevant after the Sentry-v10 error-event capture changes). Show the captured event payload. + +--- + +# F. Extension integrity (high-stakes, extension-specific) + +## F1. State migration / upgrade ⭐ +- **Proves:** a persisted-state change doesn't corrupt existing users. +- **Capture:** migrations in `app/scripts/migrations/NNN.ts`, runner `app/scripts/lib/migrator/`; scaffold with `./development/generate-migration.sh NNN`. The `NNN.test.js` asserts `meta.version` and that the `changedKeys` Set covers only mutated controllers — i.e. untouched state is preserved. Run it; show old-state-in / new-state-out. + +## F2. Vault / keyring round-trip +- **Proves:** no key/vault corruption; encrypt→decrypt is lossless. +- **Capture:** `app/scripts/lib/encryptor-factory.ts` (`@metamask/browser-passworder`, PBKDF2). E2E: `test/e2e/dist/vault-decryption-chrome.spec.ts`; `test/e2e/tests/vault-corruption/`. Storage-size via `getFileSize` on the encrypted blob. + +## F3. Transaction simulation / gas +- **Proves:** tx behavior/balance-changes/gas are correct before submit. +- **Capture:** `app/scripts/lib/transaction/containers/enforced-simulations.ts`; e2e `test/e2e/tests/simulation-details/`; mock `test/e2e/tests/confirmations/mocks/simulation.ts` (returns `gasUsed`, `callTrace`, `stateDiff`, token balance changes). TX_SENTINEL_URL in `shared/constants/transaction.ts`. + +## F4. Provider / dapp connectivity +- **Proves:** dapp integration works (injection, connect, requests). +- **Capture:** `yarn dapp` (serves `@metamask/test-dapp` on :8080); EIP-6963 `test/e2e/provider/eip-6963.spec.js`; multi-provider `test/e2e/multi-injected-provider/`; EIP-1193 reconnect tests under `test/e2e/tests/mm-connect/`. + +## F5. Feature-flag matrix (on/off) +- **Proves:** correct behavior in both remote-flag states (the Perps-gating class of bug). +- **Capture:** remote-feature-flag-controller (`app/scripts/lib/update-remote-feature-flags.ts`); flags come from `client-config.api.cx.metamask.io/v1/flags` — **not** `.metamaskrc`. In e2e, mock the response (see `test/e2e/tests/remote-feature-flag/`) to force each state; read via `uiState.metamask.remoteFeatureFlags`. + +## F6. Snaps / multichain execution +- **Proves:** snap behavior across multichain (e.g. `snap_startTrace`/`snap_endTrace`). +- **Capture:** `test/e2e/flask/snaps/preinstalled-example.spec.ts` (the snap-trace test), broader `test/e2e/snaps/`. Build flask (`yarn build:test:flask`). + +## F7. i18n usage +- **Proves:** no hardcoded strings; locales resolve. +- **Capture:** `yarn verify-locales` (`development/verify-locale-strings.js`); locales in `app/_locales/`. `yarn verify-locales:fix` to auto-fix. + +## F8. SES lockdown / runtime containment ⭐ +- **Proves:** the runtime defenses are **actually in force in the shipped artifact** — SES `lockdown()` and its taming levels, LavaMoat global scuttling, Snow's anti-escape hooks, Snaps compartments. Distinct from D3: D3 is the build-time *policy* (what a package may reach), this is whether containment *holds at runtime*. A correct policy ships alongside a lockdown that silently failed, and no policy diff would show it. +- **Capture:** `Runtime.evaluate` over CDP against the **built variant under discussion** — `Object.isFrozen(Object.prototype)`; a scuttled global throws while an exception-list global still resolves; `typeof SNOW === 'function'`; the `lockdown({…})` options as they appear *in the bundle*. Pair a positive with a negative — a check that only confirms the permitted case passes in a completely unlocked environment. +- **Bar — three divergences make this a lane, not a checkbox:** (1) the `lockdown()` call is wrapped in `try/catch` that logs to Sentry and **continues unlocked** (added for Firefox v56 contentscript injection), so it is a runtime assertion, never a guaranteed precondition; (2) **scuttling is off entirely in DEV builds** (`shouldScuttle = entryTask !== BUILD_TARGETS.DEV`); (3) **TEST builds widen the scuttling exception list** for chromedriver (`Proxy`, `ret_nodes`, `browser`, `chrome`, `indexedDB`). So **a green e2e run is evidence about a wider-open global than users get** — always state which build variant produced the evidence. +- **Reach for it:** any change touching the lockdown call site or its ordering (lockdown must precede untrusted code), the scuttling exception list, a taming level, compartment boundaries, or a `@lavamoat/snow` bump (Snow is patched in-repo — re-read the patch; see `supply-chain-audit`'s patch lane). + +--- + +# G. CI, review & process + +- **G1. CI check links** — `gh pr checks `; link the full suite (AEP's bundle is often `partial`). Always worth a one-line "all green" + link. +- **G2. Coverage delta** — `yarn test:unit:coverage` → `coverage/unit/` (and `yarn test:unit:webpack:coverage`); `codecov.yml`. Proves the new code is exercised. +- **G3. Automated-reviewer output** — independent bot (e.g. cursor[bot]) found nothing blocking. Complements, never replaces, behavior evidence. +- **G4. Manual reproduction steps** — human-followable steps that reproduce the fixed behavior; populates the PR template's Manual testing steps. +- **G5. CI-workflow change, run on a test fork** — a CI-YAML-only PR usually **cannot exercise the workflow it edits**: identical build output ⇒ builds reused from base ⇒ `needs-=false` ⇒ the workflow is *skipped* (`get-requirements.yml:654`). Escape: push to a branch literally named **`main`** (or `stable`) on your own test fork of the repo — `IS_RUN_EVERYTHING_BRANCH` (line 48) disables `find-reusable-builds` (line 310), so the workflow runs; `IS_CROSS_REPO_PR` is false inside the fork. Requires the workflow's secrets on the fork (`INFURA_PROJECT_ID`, `TEST_SRP_*` for benchmarks; `vars.`-gated Sentry/AWS steps skip cleanly) and a fork sync first. **State fork-scope in the published evidence** — it proves the workflow logic, not a run on the canonical repo.. +- **G6. CI job-duration delta** — compare job wall-clock across arms in the Actions UI or `gh run view`. **Falsifier: build reuse.** `get-requirements.yml` skips jobs when build output matches base, so a measured "speedup" is often a skipped job — confirm each arm actually ran the work before comparing. Runner class and queue time vary independently of the change; report job time, not wall-clock from push. Pairs with `D7`, which measures the same change on a machine you control. + +--- + +# Matching guide (claim → lanes) + +| The PR claims… | Lead with | Corroborate | +|---|---|---| +| a visible UI behavior | A1 / B1 visual | B2 recording for motion; B5 a11y | +| a fixed bug (any) | **B3 falsifying test** | A1/B1 if visible; E1 if it errored | +| preload / no-double-fetch / lazy-load | A2 perf | C6 netlog, D2 chunk | +| a render/over-render fix | C4 WDYR/profiler | C1 traces | +| interaction responsiveness | C2 INP, C3 TBT | C6 profile | +| startup/load timing | C5 benchmark (paired) | C1 phase traces, C2 FCP/LCP | +| smaller/cleaner bundle | D1 size | D2 chunk | +| a memory leak fixed / introduced | **C9 retention-path from code** (holder → held → boundary) | C7 heap-over-flow + retainer graph; falsifying lifecycle test | +| an error/crash fixed | E1 Sentry rate→0 | B3 test, A1 if visible | +| a dep change is safe | D3 LavaMoat + D4 manifest | D1 size; supply-chain-audit's patch/resolutions/ignore lanes | +| a mechanical migration / "rename-only" refactor | **D6 substitution A/B** (authored artifact vs its authoritative source) | B3 if behavior-visible; D1 for accidental output change | +| a hand-written type/schema/policy restates a source | **D6 substitution A/B** | G1 checks (as the *premise*: it compiles, which is why nobody noticed) | +| runtime containment / SES / scuttling | **F8 runtime containment** (on the shipped variant) | D3 policy; E1 for `Lockdown failed` events | +| persisted-state change | **F1 migration** | F2 vault | +| tx/confirmation behavior | F3 simulation | B2 e2e | +| dapp/provider behavior | F4 connectivity | B2 e2e | +| flag-gated behavior | F5 flag matrix | A1/B1 per state | +| snap behavior | F6 snaps | E2 trace | +| copy/localization | F7 i18n | A1 visual | +| CI workflow behavior | **G5 fork run** (branch named `main`) | G1 checks, G4 repro steps | + +Run the cheapest lane that yields an independently re-checkable artifact, confirm the claim holds, then escalate. Don't over-instrument a one-line copy fix; don't under-prove a startup-latency or migration claim with a single screenshot. diff --git a/domains/pr-workflow/skills/evidence/references/evidence-gate-setup.md b/domains/pr-workflow/skills/evidence/references/evidence-gate-setup.md new file mode 100644 index 00000000..21a2c998 --- /dev/null +++ b/domains/pr-workflow/skills/evidence/references/evidence-gate-setup.md @@ -0,0 +1,58 @@ +# Evidence gate — setup (optional, Claude Code only) + +`hooks/pr-evidence-gate.py` is an **optional** mechanical enforcement of the disciplines documented in [`evidence-trustworthiness.md`](./evidence-trustworthiness.md). It is a Claude Code `PreToolUse:Bash` hook: before an outward-facing write runs, it scans the body for a validation-scoped claim the trustworthiness gate would reject, and blocks the write if it finds one. + +**Surfaces policed:** the `gh pr|issue edit|create|comment` porcelain (`--body`, `--body-file`) *and* `gh api` body writes (`-f body=…`, `-F body=@file`, `--input file.json`) — a PATCH to a comment is the same publish with a different spelling, so a porcelain-only matcher is a hole rather than a gate. Read-only `gh api` calls pass through untouched. + +**Classes enforced:** `verdict`, `observation`, `deferral`, `ci-restatement`, `inflated-verdict`, `bare-identifier`, `truncated-identifier`, `mutable-ref`, `dump-resolver`, `link-only-exhibit`, `data-only-exhibit`, `step-waiver` — each implementing a numbered item of [`evidence-trustworthiness.md`](./evidence-trustworthiness.md). What the hook cannot see (whether a screenshot shows the resolving UI, whether a deferral's blocker matches its step, quotation fidelity) stays reader-applied. + +The hook is **Claude-Code-specific**. Other operators (Cursor, Codex, plain review) don't get the mechanical gate — for them the same disciplines apply as *documentation*, self-enforced by reading `evidence-trustworthiness.md`. The hook is not required to use the skill; it just moves the checklist from "remember to run it" to "runs automatically at emit time." + +It **fails open**: anything it cannot parse (non-`gh` command, unreadable body, malformed JSON) is allowed through, so it never bricks unrelated Bash commands. It uses the Python 3 standard library only (`json`, `re`, `sys`) — no dependencies to install. + +## Wire it up (Claude Code `settings.json`) + +Add a `PreToolUse` hook with matcher `Bash` that runs the script with `python3`. Put this in your user `~/.claude/settings.json` or a project `.claude/settings.json`: + +```json +{ + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": "python3 /absolute/path/to/evidence/hooks/pr-evidence-gate.py" + } + ] + } + ] + } +} +``` + +Resolve the path to wherever `evidence` lives on disk. Note that `tools/install` copies only the `references`/`scripts`/`assets`/`adapters` bundles into `~/.claude/skills/mms-evidence/` — the `hooks/` directory is **not** part of the installed bundle. Point the `command` at your checked-out skills repo instead: + +``` +/domains/pr-workflow/skills/evidence/hooks/pr-evidence-gate.py +``` + +**When it blocks:** the hook exits `2` and prints the reason (which claim, what artifact/tracker it needs) to stderr. Claude Code surfaces that to the model, which self-corrects — attaches the missing artifact/tracker or downgrades the verdict — and re-posts. No manual intervention needed. + +## Two other setup requirements the skill needs + +These are independent of the hook; the skill needs them whether or not you install the gate. + +1. **`gh pr comment` must be permitted — pick a grant model.** evidence posts its evidence bundle as a PR review comment (`gh pr edit` if publishing into your own PR body). Four options, in descending order of standing safety: + + | Model | How | Tradeoff | + |---|---|---| + | **`ask` (recommended)** | `"Bash(gh pr comment:*)"` in `permissions.ask` | Per-post confirmation prompt. Combined with this hook (content gate) and a draft-confirm habit, that's three independent layers. | + | **`allow` + hook** | same pattern in `permissions.allow`, hook wired | Frictionless posting; safety rests entirely on the hook and your draft discipline. Only sensible where the hook is actually installed — not for operators without hook support. | + | **Allowlisted wrapper** | keep raw `gh pr comment` denied; allowlist a small script that takes `--repo`/`--pr`/`--body-file`, checks preconditions (canonical header present), and is the only sanctioned path | Tightest scoping — the raw verb stays blocked; costs a script to maintain. | + | **No grant — manual post** | the model prepares the body file; you run `gh pr comment --repo --body-file ` yourself | Zero standing grant; you are the bottleneck. The universal fallback, and the only option on operators with no permission system. | + + Avoid a bare **deny** on the comment verbs if you use this skill: it hard-blocks the publish step with no prompt, which reads as a mysterious failure mid-run. + +2. **Image re-hosting needs your own public evidence repo.** Screenshots and recordings captured locally must be re-hosted to a public URL before a reviewer can see them (see items 8–9 in `evidence-trustworthiness.md`). This repo is **yours to provide** — set it to a public repo you control, referenced here as ``. There is no shared/default host: parameterize it in your own configuration and push captures there, then reference the resulting raw URLs in the PR comment. Do not hardcode someone else's host. diff --git a/domains/pr-workflow/skills/evidence/references/evidence-publishing.md b/domains/pr-workflow/skills/evidence/references/evidence-publishing.md new file mode 100644 index 00000000..d86337b3 --- /dev/null +++ b/domains/pr-workflow/skills/evidence/references/evidence-publishing.md @@ -0,0 +1,335 @@ +# Publishing the evidence bundle to a PR body + +How to take run artifacts + complementary evidence and write a clean, idempotent, reviewer-familiar section into the PR body — **matching AEP's own format** so a re-run replaces in place instead of stacking duplicates. + +Canonical source for the format: `~/Code/metamask/metamask-autonomous-engineering-platform/packages/github/src/pr-body-builder.ts` (`upsertVisualValidationSection`). Mirror it. + +> **Publishing is public and outward-facing. Always render the section and get explicit confirmation before writing the PR body. Use `publishEvidence: false` on the run; this manual flow is the only publish path.** + +## Step 1 — Re-host images (artifacts are localhost) + +Control-plane artifact URLs (`localhost:3000/v1/runs/:id/artifacts/:name`) won't render on GitHub. Re-host each artifact and link the hosted URL. + +**Host: an S3 bucket you configure, prefix `public/`.** Set `EVIDENCE_BUCKET` and +`EVIDENCE_REGION` in your environment; this file does not name a bucket, because a bucket name +published here is an anonymously-readable endpoint advertised to everyone who reads it. + +``` +s3://$EVIDENCE_BUCKET/public/metamask/pr-// +https://$EVIDENCE_BUCKET.s3.$EVIDENCE_REGION.amazonaws.com/public/metamask/pr-// +``` + +The bucket must allow anonymous `GetObject` under `public/*` and must **not** allow listing, so +the prefix is not browsable — link individual files, and don't promise readers an index. If you +do not have one, that is the whole policy: + +```json +{ + "Version": "2012-10-17", + "Statement": [{ + "Effect": "Allow", + "Principal": "*", + "Action": "s3:GetObject", + "Resource": "arn:aws:s3:::YOUR-BUCKET/public/*" + }] +} +``` + +with `BlockPublicPolicy` and `RestrictPublicBuckets` disabled on that bucket and +`s3:ListBucket` granted to nobody. An org-owned bucket is preferable to a personal one: artifact +links outlive the person who published them. + +**Do not re-host to a personal repo.** A personal private repo returns 404 for every reader but +its owner, so every artifact link published from one is dead on arrival. That was the previous +target here, and this file simultaneously said such links were unreachable — guidance that +instructed you to publish dead links. Verified live in a published artifact. + +The test is **audience-reachability, not public-vs-private.** An org repo may be private and still +readable by colleagues, so an internal-audience link to one is fine. A personal repo is unreachable +by colleagues *and* by the public, so it fails for every audience. Re-host to an org-owned +destination, or to the configured bucket. + +- Path convention: `pr-//` keeps runs from colliding. +- **Verify unauthenticated before shipping**: `curl -s -o /dev/null -w "%{http_code}"` on each + published URL. A 200 from your own browser proves nothing — you are logged in. + +```bash +RUN_ID=; PR=; CP=localhost:3000 +BUCKET="$EVIDENCE_BUCKET" +BASE="https://$BUCKET.s3.$EVIDENCE_REGION.amazonaws.com" +for name in ; do + curl -fsS "$CP/v1/runs/$RUN_ID/artifacts/$name" -o "/tmp/$name" + key="public/metamask/pr-$PR/$RUN_ID/$name" + aws s3 cp "/tmp/$name" "s3://$BUCKET/$key" --only-show-errors + url="$BASE/$key" + # the link is not shippable until it resolves WITHOUT credentials + code=$(curl -s -o /dev/null -w '%{http_code}' --max-time 25 "$url") + [ "$code" = "200" ] || { echo "UNREACHABLE ($code): $url" >&2; exit 1; } + echo "$url" +done +``` + +No base64 round-trip and no 1 MB contents-API ceiling — the ceiling silently truncated a +1.7 MB gif to **0 bytes** on one run, and the loop reported success. Size-check anything you +transfer by another route. + +Files >1MB exceed `ARG_MAX` for an inline `-f content=` — use `gh api -F content=@` (write the base64 to a file first). For GIFs, re-host the same way. + +## Step 2 — Build the section (canonical header + mirror AEP) + +**Canonical header (2026-07-21):** every validation-run output — a PR comment *or* the PR-body section — leads with the exact literal `## 🧪 Validation Run`. Never reworded, never demoted to `###`: the constant string is the identifiability anchor, exactly like Copilot's fixed `## Pull request overview`. `hooks/pr-evidence-gate.py` blocks any `gh` write whose body has a validation/verification/evidence heading or AEP marker without this literal. + +Marker pairs, used so re-runs replace idempotently: + +- Whole section: `` … `` +- AEP status block (nested): `` … `` +- Screenshots block: `` … `` + +AEP prefers to inject screenshots into the PR template's `### **After**` section (replacing the `` placeholder), falling back to a `### Screenshots` block inside the status block when there's no After scaffold. Do the same. + +Section shape: + +```markdown + +## 🧪 Validation Run + +**Verdict:** ✅ proven — **Claim:** +head `` · · lanes: + + + + +### AEP Visual Validation + +**✅ Passed** + + + +
Validation details + +** — . " lines> + +
+ +Run `` · [LangSmith trace]() + + +``` + +Verdict icon: `✅` Passed, `❌` Failed, `ℹ️` otherwise. For perf, retitle the nested block `### AEP Perf Validation` and put `M/M assertions proven` in the headline. When AEP's *service* publishes its own `## AEP Visual Validation` block (publishEvidence:true, not the local flow), leave that block's heading alone — the demotion to `###` applies to hand-assembled bundles under the canonical header. + +Screenshots block (injected into `### After`, or appended under `### Screenshots`): + +```markdown + +
+ +<artifact-name> + +[Open full-size image]() + +
+ +``` + +`
` so reviewers see evidence without a click. One block per image; before/after read top-to-bottom. + +## Step 3 — Choose the surface by ownership, then publish + +**Publish surface depends on your relationship to the PR.** Determine it FIRST: + +```bash +PR=; REPO=MetaMask/metamask-extension +ME=$(gh api user --jq .login) +# Piped to jq rather than `gh --jq`: gh's built-in filter takes no --arg, and passing one +# fails with "accepts at most 1 arg(s)". +SURFACE=$(gh pr view "$PR" --repo "$REPO" --json author,commits | jq -r --arg me "$ME" ' + if .author.login==$me then "body" + elif ([.commits[] | select(.authors[].login==$me) + | select([.authors[].login] | map(select(.!=$me and .!="Copilot" and (test("claude|anthropic")|not))) | length == 0)] | length) > 0 + then "comment" else "skip" end') +``` + +- `body` — you authored the PR → upsert into the PR body (below). Validation is + part of your own claim. +- `comment` — not author but I have a solo commit (no HUMAN co-author) → post a + `gh pr comment` under the canonical `## 🧪 Validation Run` header. Never edit + someone else's PR body. +- `skip` — my only commits are co-authored with a human (review/pairing) OR I + have no commits → **do not publish**. Not my PR to validate outward. + +### Publish the script that produced a computed artifact, next to the artifact + +Any number you derived rather than read off a tool — a hash comparison, a count, a delta, a +statistic — is only as trustworthy as the reader's ability to re-run it. **A prose `method:` field +is not provenance.** Reviewers discount computed figures from an agent by default, and correctly: +on extension#45024 a reviewer dismissed a policy-identity check as *"we know LLMs are really bad at +this"*. It had in fact been a deterministic `sha256`, not the model counting — but the script lived +in a throwaway `python3 - <<'PY'` heredoc, so nothing could show that. The objection was +unanswerable because of how the evidence was packaged, not because of what it said. + +So: + +- **Write the script to a file, never an inline heredoc**, when its output will be published. The + heredoc survives only in the transcript, which the reader does not have. +- **Publish the script alongside its output**, and cross-reference: the artifact carries + `provenance: { script, script_sha256, command }`; the comment links the artifact. +- **Include the exact command** with its inputs (PR ref, head SHA), so the run is reproducible + rather than merely described. +- **Verify the round trip** — fetch the published script anonymously, hash it, and confirm it + matches `script_sha256`. A link that 200s is not proof the bytes are the ones you ran. +- Prefer a script that takes arguments and is re-runnable against a different PR. A one-off that + only works on your paths is weak provenance even when published. + +State plainly what the script does and does not do (`no model judgement; not a count of '+' +characters`) — that sentence is what actually retires the reviewer's prior. + +### Before any of the commands below: show the body in the response + +Every publish path here uses `--body-file`, so the **permission prompt displays a file path, not +the content**. The user is then asked to authorize publishing something under their name that they +cannot read, and the correct answer to that is no. + +**Paste the complete body inline in the response first, then run the command.** For an edit, also +say what changed relative to what is currently live. "I've drafted it, shall I post?" with a path +instead of the text is incomplete — pointing at `/tmp/validation-run.md` is the same failure as the +prompt itself. If the body is too long to show comfortably, that is a signal to trim it. +(Three consecutive denials on extension#45024, 2026-07-30, all from this.) + +### body surface (I own the PR) +```bash +gh pr view "$PR" --json body -q .body > /tmp/pr-body.md +# Replace the region between VALIDATION_RUN markers if present, else append. +# (Legacy bodies: replace the AEP_VISUAL_VALIDATION region and re-wrap it under +# the canonical "## 🧪 Validation Run" header + VALIDATION_RUN markers.) +# Replace the region between AEP_SCREENSHOTS markers if present, else inject after +# the "### **After**" heading (replacing the [screenshots/recordings] placeholder). +# ...edit /tmp/pr-body.md... +gh pr edit "$PR" --body-file /tmp/pr-body.md +``` + +### comment surface (I contributed but don't own) +```bash +# Same canonical "## 🧪 Validation Run" header + bundle; post as a comment. +gh pr comment "$PR" --repo "$REPO" --body-file /tmp/validation-run.md +``` + +Idempotency: because both regions are marker-delimited, re-running replaces them — never append a second copy. If the markers are absent (human-authored body), append the status block at the end and inject screenshots into `### After` when that heading exists. + +## Step 4 — Privacy scrub (before writing) + +Failure summaries and agent narratives leak the dev environment. Before publishing, strip: + +- Absolute local paths (`/Users//…`, `~/Code/…`) → describe the surface, not the path. +- The username anywhere it appears. +- `localhost` / `127.0.0.1` URLs → must be re-hosted public URLs only. +- Internal hostnames, JFrog/registry URLs, tokens. + +A failed run still must not publish raw — either omit the section or publish a scrubbed `❌ Failed` summary, with confirmation. + +## Recordings → GIF (for flows/motion a still can't prove) + +The platform can't collect video (artifact regex = png/jpg/log/txt). Capture out-of-band: + +1. In a **built** PR checkout (mm's fixture infra is required — a bare `dist/chrome` won't boot), write a preload `/tmp/patch-record.mjs` that monkey-patches `playwright-core`'s `chromium.launchPersistentContext` to inject `recordVideo: { dir }`. Resolve the module via `createRequire(/package.json)` so the patch hits the same module instance the `mm` daemon uses. +2. `NODE_OPTIONS="--import /tmp/patch-record.mjs" npx mm launch --state onboarding` → drive the flow (or let it sit) → `npx mm stop` flushes the `.webm`. States: `default | onboarding | custom`. +3. Convert with `ffmpeg` two-pass palette (better color than single-pass): + ```bash + ffmpeg -i in.webm -vf "fps=12,scale=480:-1:flags=lanczos,palettegen" -y /tmp/pal.png + ffmpeg -i in.webm -i /tmp/pal.png -lavfi "fps=12,scale=480:-1:flags=lanczos[x];[x][1:v]paletteuse" -y out.gif + ``` + webm/mp4 don't render inline in GitHub PR bodies; GIF does. +4. Re-host the GIF (Step 1) and embed like a screenshot. + +**Same-window app + DevTools (lane C8):** when the claim needs UI + console/network in one frame (e.g. "no toast *while* the log shows the silent path ran"), skip `recordVideo` entirely — it can't see DevTools. Use the OS-level region capture in [evidence-catalog C8](evidence-catalog.md): dock tab DevTools with `--auto-open-devtools-for-tabs`, tile the SW inspector window via `osascript`/CDP `Browser.setWindowBounds`, then `screencapture -v -V -R` → same ffmpeg GIF recipe. Publish the GIF + one full-res PNG + the CDP console text dump (GIF downscale makes log lines illegible on their own). + +## Re-validation runs: delta-first presentation, every verdict re-earned (2026-07-21) + +The common loop — a run refutes a claim, the author pushes a fix, `/evidence` re-runs at the new head — gets a **delta report**, not a second full bundle: + +- **Presentation is delta-only.** Full exhibits only for lanes whose outcome changed (flipped verdict / new lane / new residual). Unchanged lanes collapse to a `Prior run | This run` ledger, each row with a fresh run-log link from the new head plus one link to the prior run's comment for the full exhibits — and say so ("unchanged rows re-run at ``; full exhibits in the prior run"). +- **Evidence is never delta.** Evidence is head-pinned: re-run every automated lane at the new head and re-earn every verdict with a fresh artifact. "Unchanged" is a conclusion from the re-run, never a carried-over assumption (the stale-baseline trap at report level). Re-running is cheap — the falsifier harness already exists from the first run. +- Same canonical header + markers; the meta line names the fix commit and links the prior run. Comments: one per run, chronological, each linking its predecessor. PR-body section: replaced in place via markers. +- New head → **new hosted artifact directory keyed to the fix commit** (`pr-/fix-/`), commit-pinned raw URLs; never overwrite a prior run's published files. +- Residuals the fix intentionally leaves get their own row/section — don't round a fixed-with-residual claim up to fully proven. + + +## Lead with a lane-status ledger (no silent absence) + +The published section must **enumerate every lane the claim type calls for and give each an explicit status** — never render only the lanes you happen to have and let the rest be silently absent. An unmarked gap is indistinguishable from a lane that ran and came back empty; the reader (and you, on the next pass) can't tell "no evidence because none needed" from "no evidence because not done." This is the vacuous-pass trap at the publish layer — carry the run's `✅/❌/⚠️` verdict into the PR body, don't leave it in the internal report-back. + +Open the evidence section with a ledger: + +```markdown +| Lane | Status | Evidence | +|---|---|---| +| B3 falsifying test | ✅ proven | 32/32 head, 3/32 reverted | +| E1 Sentry before/after | ✅ proven | [discover](…) — distinct trace ids | +| A1 visual | ➖ N/A | background change, no UI surface | +| C6 CDP netlog | ⏳ not-captured | — | +``` + +Status vocabulary: `✅ proven` (link) · `⚠️ inconclusive` (name what's missing) · `➖ N/A` (reason) · `⏳ not-captured`. Mirror the `N/A — ` convention the `### Screenshots` block already uses for no-UI PRs. Never upgrade a `⏳`/`⚠️` to a pass by omission. + +**Sibling-PR parity:** when a set of PRs shares a claim shape (same program, same author, "root the X traces"), their ledgers must match lane-for-lane. A lane present on one and absent on another is either added or explicitly marked `➖ N/A — ` — a bar that silently drifts between siblings is a finding (postmortem 2026-07-17, #43929/#43930). + +## Non-visual & multi-lane evidence + +Screenshots are only one lane. Most claims (perf, telemetry, state, build) publish as **text/links/tables**, not images. Put them in the same verdict-first section so a reviewer sees one coherent bundle, not scattered comments. + +Per-lane rendering: + +- **Sentry / Tempo (E1/E2):** a markdown link to the discover/trace query with the before/after window baked in, plus the headline numbers inline (`errors: 1.2% → 0.0% over 24h post-release`). Link, not screenshot — reviewers re-run it. +- **Benchmark / web-vitals / TBT (C2/C3/C5):** a small before/after table (metric · base · head · Δ · threshold). State it's a **paired A/B** if the stored baseline was bypassed. +- **Migration (F1):** the `changedKeys` set + a before/after state-shape snippet, and a link to the migration-test run. +- **Bundle / chunk / LavaMoat / manifest (D1–D4):** the diff or size delta in a fenced block; for policy/manifest, the actual `git diff` (or "diff empty — no new capability"). +- **Trace artifacts (B2):** link the Playwright trace-viewer report / attach the `trace.zip`; don't paste raw. + +Multi-claim PRs get one sub-block per claim under the status section, each with its own ✅/❌/⚠️ verdict — mirror the Claim Cards. Keep the visual block (markers + `### After` injection) for the image lanes; render the rest as text beneath it. + +### One comment per evidence *kind*, not one comment per PR (2026-07-30) + +Sub-blocks are for several claims **of the same kind**. When a PR draws two different +kinds — say an executed Validation Run *and* a read-level capability triage — they get +**separate comments**, each with its own header, its own marker pair, and its own format. + +| | Validation Run | LavaMoat policy diligence | +|---|---|---| +| header | `## 🧪 Validation Run` | `## 🔒 LavaMoat Grants — → ` | +| markers | `VALIDATION_RUN_*` | `LAVAMOAT_DILIGENCE_*` | +| opens on | `**Verdict:** ✅/⚠️/❌` | the finding; **no verdict at all** | +| body | lane ledger, artifacts per lane | deny candidates, enumeration folded | +| audience | whoever owns the PR's claim | whoever owns the policy | + +Merging them forces one frame onto both. A read-level triage has no run to verdict, so it +would land as `⚠️ inconclusive` on a header promising a run; and a `⏳ not-captured` lane +needs a tracker it does not have. The marker pairs also collide — a re-run replacing the +`VALIDATION_RUN` region would silently eat the diligence output sharing it. + +**So: choose the format from the evidence kind, not from this document's default.** The +canonical `## 🧪 Validation Run` header applies when a run produced artifacts. An engine +skill that defines its own output contract (`lavamoat-policy`) publishes in that +contract. `hooks/pr-evidence-gate.py` enforces the canonical literal only on bodies that +*claim* validation/evidence framing — a diligence comment that renders no verdict does not +trip it, which is the tell that the two are different artifacts rather than one with a +different skin. + +**Per-scenario presentation (2026-07-21):** the same applies one level down — when the evidence spans multiple test scenarios (flag-on vs flag-off, control vs treatment in an A/B falsifier, numbered manual-testing steps), give each scenario its **own sub-section**: a heading naming the scenario in observation terms, one line on what it tests plus its verdict, and that scenario's artifacts co-located under it. Never bunch all scenarios' artifacts into one large evidence dump — the reviewer verifies "under condition X, artifact shows Y" one condition at a time, and a merged block destroys that mapping even when every artifact is real. For long artifact sets use a `
` block *per scenario*, not a merge. (Instance: #44610.) + +## Artifact contract (ADR-0058 alignment) + +To stay interoperable with the recipe-based verification system (MetaMask/decisions#173), shape the bundle like its reviewer-visible contract where practical: a `summary.json` (claim → verdict → evidence refs), a `trace.json` (the run/assertion log), and an artifact manifest (names + media types), with screenshots/video as the confidence layer. Publishing then becomes "render `summary.json` into the PR section." This keeps evidence's output and a recipe's output the same shape — see [lane-assertions.md](lane-assertions.md). Don't hand-roll a divergent format. + +## Checklist before you publish + +- [ ] Section opens with a **lane-status ledger** — every claim-required lane marked `✅`/`⚠️`/`➖ N/A`/`⏳`; no lane silently absent (and sibling PRs' ledgers match lane-for-lane) +- [ ] `evidenceBundle.artifactRefs` non-empty with expected media (not a vacuous pass) +- [ ] Each lane passed the [trustworthiness gate](evidence-trustworthiness.md) (shows the claimed surface, signal > noise, could-have-failed) +- [ ] Multi-scenario evidence rendered **per scenario** (own heading + verdict + co-located artifacts), not bunched into one block +- [ ] **Automated-process voice, no first person** — published validation output never says "I ran/captured/verified"; attribute to the process ("Automated validation ran…", "the harness captured…") so readers know the evidence is machine-generated, not a manual account under the author's name +- [ ] Every image/GIF re-hosted to the configured bucket under `public/…`; no localhost/local-path URLs in the body +- [ ] **Every published link curl'd unauthenticated and returning 200** — never a personal private repo +- [ ] Work cited by **PR link** rather than tracking-ticket id, unless the ticket's own content (an RCA, a spec) is the referent +- [ ] Narrative scrubbed of username/paths/internal hosts +- [ ] Marker pairs present so the upsert is idempotent +- [ ] Section rendered and **confirmed by the user** diff --git a/domains/pr-workflow/skills/evidence/references/evidence-trustworthiness.md b/domains/pr-workflow/skills/evidence/references/evidence-trustworthiness.md new file mode 100644 index 00000000..f398b2ef --- /dev/null +++ b/domains/pr-workflow/skills/evidence/references/evidence-trustworthiness.md @@ -0,0 +1,42 @@ +# Evidence trustworthiness (anti-reward-hacking) + +A green result is not proof. An agent — or an eager run — can produce evidence that *looks* like it validates the claim but doesn't. Before believing or publishing any lane, run it through this gate. It extends the vacuous-pass trap to all lanes; the Claim Card's **Falsifier** is the anchor: trustworthy evidence is evidence that *could* have shown the falsifier and didn't. + +## The gate (per lane, before publish) + +1. **Non-empty & expected media** — the bundle has artifacts of the expected kind. Zero artifacts = not a pass (the vacuous-pass guard). +2. **Shows the claimed surface** — the screenshot/recording is the Claim Card surface in the asserted state — not a loading spinner, an error toast, the wrong screen, or a pre-action frame. Eyeball it. +3. **Exercises the changed code** — the test/flow actually hits the diff. For a test: it **fails on `main`** (catalog B3). For a flow: the changed component/route is on the path. A green test that never imports the changed module proves nothing. +4. **Signal exceeds noise — and a null states its power** — a perf delta must be beyond run-to-run variance (paired A/B, multiple iterations); a 3% move on a noisy metric is not evidence. The same bar applies in reverse: when the spread is wider than the effect being looked for, the finding is **"not resolvable at this sample size"**, never "no change" — an underpowered run and a true null print the same word, and reporting the word alone lets the reader infer the stronger claim. State the smallest effect the design could have detected. + - **Removing a bias is not establishing validity.** Correcting a flaw you found (discarding a warm-up, alternating the starting arm, pinning CPU governor) removes *that* bias and licenses no more than that. It is not a trust gate, because a trust gate names how the evidence could **still** be vacuous — residual risk, not completed work. List what remains uncontrolled (thermal drift, background load, ordering within a round); an unenumerated confound reads as a nonexistent one. + - **When correcting an overclaim, cut the certainty, not the evidence.** A falsifier that actually caught something is the strongest thing on the page — downgrade the conclusion around it, don't delete it with the overclaim. +5. **Could have failed** — the assertion has a reachable failure mode. Always-true assertions (`expect(true)`, a screenshot with no assertion, a Sentry query with no time bound) can't falsify anything. +6. **Right baseline** — "before" is the actual base ref / prior version / pre-window, not a stale or mismatched comparison. +7. **Artifacts are independent & honestly labeled** — checksum every capture set (`md5 *`). Byte-identical files across supposedly independent runs/cases cannot stand as separate observations: either explain the identity in the artifact bundle (deterministic fixture rendering) with per-run provenance that *does* differ (the harness state dump, timestamps, a manifest), or re-capture at distinct moments. Labels must describe the observation, not the interpretation — a file named for the state it *should* show under the claim (`steady-state`, `no-toast`) misleads when the capture shows the refutation. +8. **The finding ships with its artifacts** — a findings comment (including a refutation shared privately) carries functional links to the re-hosted observation artifacts at *draft* time, not descriptions of artifacts that exist only on the capturing machine. "Would need re-hosting" is not a reason to omit: re-hosting is the procedure ([evidence-publishing](evidence-publishing.md) Step 1). Code permalinks + a runnable repro are corroboration, not a substitute for the observation itself. +9. **Signal is surfaced — least-effort validation** — evidence is judged at the reader's eyes, not the author's disk: signal the reader must excavate from a mountain of attached data is, for evidence purposes, no evidence. Every published exhibit leads with a one-line pointer — *what to open, where to look, what it should show*. Deltas are presented **as** deltas (annotated side-by-side, diff, before→after crop of the differing region), never two full captures for the reader to compare by eye; if the claim is "no visual change," publish one image plus the hash-equality line, never N identical-looking copies as separate exhibits. Bulk artifacts (MB-scale JSON, full logs) are excerpted inline to the discriminating lines, with the full file linked as appendix. Emit-time test, per exhibit: can a reader who did not run the session confirm the claim in ~30 seconds from what is directly visible? If not, restructure the presentation — attaching more data cannot fix it. Coverage is the converse constraint (2026-07-21): this item governs *form*, never column-set minimalism — a valid, relevant dimension is never omitted because it duplicates another's signal (redundant corroboration costs a skippable glance; an omitted column is unfalsifiable and reads as cherry-picking). Exclusion requires invalidity (metric void on this surface, e.g. TTFB on `chrome-extension://` pages) or irrelevance (different claim/different data → sibling exhibit, not a column), each stated in a one-line disposition. +10. **Parallel exhibits are format-uniform** — sibling exhibits (table rows, per-scenario blocks, the legs of an A/B pair) carry the same evidence format and quality. If one row links its artifact inline, every row does; if one scenario gets an annotated timeline, action-log provenance, and co-located full-res/raw links, every scenario does. The bar is the **best sibling**: when the presentation standard improves mid-session, re-normalize the whole document up to it before publish — never apply the improvement only to the exhibit being produced (append-only drafting). Any asymmetry carries an explicit stated reason co-located with the weaker exhibit ("close-event variant unit-uncoverable", "manual-only trigger"); an unexplained format gap reads as an evidence gap — the reader cannot tell an unlinked artifact from a missing one, and inconsistency spends credibility on *every* exhibit, including the strong ones. Emit-time test: enumerate the sibling sets, diff each against the best-formatted member, and for every deviation either normalize it or state the reason. +11. **Lanes derive from the Manual testing steps — a CI-green row is not a lane** — the Validation Run's rows are generated top-down from the claim and the PR's own **Manual testing steps**, never bottom-up from whatever links already exist. For each step the claim depends on, the lane's payload is the **captured output of executing that step** (step "in Discover, group by `trace`" → a Discover permalink / **linked** trace-id table showing N rounds → N distinct `trace_id`s, per item 12), or an honest ⏳ naming the missing capture with a tracker. A row restating CI ("tests green at head `` in [CI run]") duplicates the Checks tab and is deleted — and a validation surface carries **zero** CI references, full stop: no `actions/runs` links, no "green at head" clauses, no "as context (only)" retention. The earlier carve-out here ("a CI link is admissible as context on a beyond-CI row") was itself the next costume: within a day all four sibling bodies (extension#43928–#43931) shipped restatements phrased as the exception — rows *leading* with "green at head … in [Unit tests CI]", the same link repeated 3× per body, the remediated row keeping it re-labeled "as context only" — while the gate's excuse regex matched the mere word "revert", so vocabulary, not evidence, discharged the class. The revert lane cites the revert **outcome** (which blocks failed, at which commit); its green-at-head half is the Checks tab's information and is omitted. A carve-out in an emit-time gate is an instruction to generation to phrase every violation as the exception — deliberate exceptions route through the human, never through an excuse predicate. Borrowed evidence — a sibling PR's capture, a unit falsifier standing in for the named live surface — never upgrades an uncaptured lane to ✅: "mechanism live-proven" co-located with "was not exercised" is an inflated verdict; downgrade it. Emit-time trigger: `pr-evidence-gate.py` classes `ci-restatement` (unconditional since 2026-07-21: any CI link / CI-green phrase in validation scope fires — no verdict co-location required, no beyond-CI excuse) and `inflated-verdict`, with the shipped extension#43928 rows and the carve-out-blessed "as context" shape as regression cases (2026-07-21). +12. **Identifiers resolve in one click — a bare id is a digging assignment** — trace ids, event ids, run ids, SHAs are *pointers into a system*, not evidence. Publishing a bunch of raw trace ids hands the reviewer the job of reconstructing project/environment/time window and querying Sentry themselves — it fails item 9's ~30-second test by construction (item 9 makes the signal *findable*; this item makes it *checkable*). Every identifier published as evidence is either hyperlinked to its resolving surface (the Sentry trace/event permalink, or an absolute-windowed Discover query pre-filtered to exactly those ids) or accompanied by the re-hosted captured output (query-result rows / envelope excerpt showing the discriminating fields) — ideally both. Special case that produced the rule: ids captured **locally** (mockttp forwarder, envelope intercept) never reached Sentry, so no permalink can exist — the re-hosted capture is the *only* admissible form, and pasting the id fragments plus a re-run recipe is the "spec necessary / output sufficient" violation wearing ids as decoration (extension#43931 Validation row, 2026-07-21). Rule of construction: when any item in this gate blesses an evidence class by name ("trace-id table", "envelope log"), it means the class's *resolvable instance*, never its bare tokens — a blessed class name is otherwise the next costume. Emit-time trigger: `pr-evidence-gate.py` class `bare-identifier`; converse-of-gate note: the prior gate *whitelisted* `trace_ids?` as beyond-CI payload and its own fix-message recommended "trace-id table" unqualified — second occurrence of "audit the gate for whitelists of the violating shape." +13. **Terminal exhibits are reader-native — a live link or a visual; a dump behind a link is still an opaque reference** — item 12 makes every pointer resolve in one click; this item constrains what it may resolve *to*. A positive verdict's terminal artifact is one of the two media a reviewer natively consumes: a **live link into the resolving system** (Sentry trace/event permalink, absolute-windowed Discover query pre-filtered to the claim) or a **visual capture** (screenshot/recording, annotated or cropped to the discriminating region). Raw files (`.log`/`.json`/`.har`, MB-scale dumps) are **appendix-only** — linked once for auditability, never the exhibit a claim rests on: a link whose target is a raw dump passes item 12 and fails item 9 one click later; the digging moved a hop away, it did not disappear (extension#43931 *second* remediation, 2026-07-21: the `bare-identifier` fix shipped a ✅ row whose sole resolver was a re-hosted ~70KB run log). Two corollaries: (a) **the gate items are conjunctive** — a fix for the newest item must re-pass all prior items; satisfying resolvability with an artifact that fails legibility is the generator's next costume; (b) **ascertain the terminal medium at step zero and pick the capture lane that can produce it** — a local intercept (mockttp envelope forwarder) can never yield a live Sentry permalink, so for Sentry-observable claims it is the supplementary falsifier lane and live ingest (dev build → `SENTRY_DSN_DEV`/test-metamask) is primary, precisely because it terminates in permalinks + screenshots; choosing a lane that cannot produce the terminal medium silently displaces it. Emit-time trigger: `pr-evidence-gate.py` class `dump-resolver`, with the remediated extension#43931 row as the regression case (2026-07-21). +14. **Manual testing steps are the validation contract — steps present ⇒ live evidence definitionally required; an impossibility waiver contradicting an executable step is invalid** — the PR's own **Manual testing steps** are the author's assertion that the claimed behavior *is* live-observable, and how: each numbered step is an executability proof (a step a human reviewer can run, `/evidence` can run) and its text is the capture spec. Item 11 derives the lanes from the steps top-down; this item closes the other side of the hatch — a lane derived from a step may not then be *waived by argument*. Emit-time procedure: build the per-step coverage map (step → executed-output artifact); for any step without one, the only admissible state is a **per-step** ⏳ + tracker whose blocker is that step's *own* unmet precondition, checked against the step text. Three waiver-inflation patterns from the producing instance (extension#43228/#42869/#44538, 2026-07-21 — all three shipped articulate impossibility rationales in the same body whose Manual testing steps asserted the opposite): (a) **borrowed impossibility** — the excuse imported from a different mechanism or sibling PR (#43228 waived its live lane citing the async remote-flag read race, which is #44538's mechanism; #43228's overrides are build-time env vars, and its steps 1–4 are directly executable in a dev build); (b) **lane-limitation universalized** — one harness's gap stated as global impossibility (#42869: "the e2e harness emits no error `event` envelope, so … not capturable pre-merge" — the step says *dev build with Sentry enabled, trigger an error*, which ingests error events into test-metamask without the e2e harness); (c) **blocked-scope inflation** — a genuinely blocked precondition of one half of the claim expanded to waive the whole lane (#44538: LaunchDarkly provisioning [#7482](https://github.com/MetaMask/MetaMask-planning/issues/7482) blocks only the *prod-flag* half; the step's own text names the dev-injectable alternative — "or inject it into persisted `RemoteFeatureFlagController` state"). If a step is *truly* non-executable, the waiver is still inadmissible alone: the Manual testing steps are then wrong and are corrected in the same edit — a document may not simultaneously instruct a reviewer to observe X and declare X unobservable. The honest-⏳ lane blessed throughout this gate is for *not yet done*, never for *argued away*: an eloquent impossibility rationale is the cheapest token sequence that satisfies every prior item (no fake capture, no CI link, no bare id, no dump) — the generator's costume for the coverage axis. Emit-time trigger: `pr-evidence-gate.py` class `step-waiver` ("not demonstrable / capturable / observable", "not separately captured", "not attached", "rests entirely/solely on the falsifiers/unit/revert" — unconditional in validation scope; no tracker or artifact excuses it), with the three shipped waiver paragraphs as regression cases and the gate verified-blocking on all three live bodies (2026-07-21). Detection gap: the per-step consistency check (does a deferral's blocker match the step's own precondition?) stays procedural — the gate sees vocabulary, not step semantics. +15. **The exhibit lives in the body — link AND visual; a live link alone is the verification path, not the exhibit** — item 13 blessed the terminal media as a *disjunction* (live link OR visual), and generation took the cheaper disjunct: a Discover permalink is producible from the API token alone, a screenshot needs a browser session — so extension#44540's live-ingestion exhibit shipped as a permalink + prose counts, with nothing in the PR body a reader could look at (2026-07-21: "only sentry link and not screenshot that makes it immediately obvious how evidence validates pr"). A live link defers validation behind **click + auth + query rendering + column interpretation** — the dump-resolver displacement one hop further, with the mountain now behind a login: it fails item 9's ~30-second test at the moment of the click, and for any reader *without* Sentry org access (most PR reviewers) a link-only exhibit degrades to a bare identifier (item 12) behind an auth wall. The repaired rule is a **conjunction**: a positive verdict's headline exhibit is an **embedded visual** — screenshot/recording of the linked resolving view (Discover result rows, trace waterfall), cropped/annotated to the discriminating region, captioned with what it should show — **and** the co-located live permalink (absolute-windowed) as the independent-verification path. Neither substitutes for the other: link-only hides the exhibit; visual-only is independently unverifiable. The 2026-07-16 clause "screenshots ride along when a browser session is available; the API token alone yields links + JSON, which is the automatable minimum" was the self-authored escape hatch of this axis (family: the "as context" carve-out, the honest-⏳ waiver): the *automatable minimum* got promoted to the shipped standard because it was the cheapest compliant artifact. A capture lane that cannot screenshot its resolving view is a lane gap to fix before publish (drive a browser session to the Discover URL), never a licensed downgrade — deliberate exceptions route through the human. Emit-time trigger: `pr-evidence-gate.py` class `link-only-exhibit` (non-negated verdict + `sentry.io` link + no image/recording embed in the unit), with the shipped #44540 paragraph as the regression case and the prior suite's permalink-only ALLOW cases flipped/augmented — third occurrence of "an ALLOW case containing the violating tokens is a specification of the next costume." Detection gaps: verdict co-location is required, so a no-verdict link-only paragraph evades mechanically; the visual-without-link converse stays procedural under item 12. +16. **The audit chain is mechanical — quote, don't transcribe; pin, don't point** — an exhibit's inline data must be a **verbatim, greppable excerpt** of the artifact (full-length identifiers, raw capture lines quoted exactly), and every repo-hosted artifact link must be a **commit-pinned, line-anchored permalink** (`/blob//…#Lx-Ly`) to the discriminating lines. Producing real artifacts and then hand-transcribing digests severs the claim→artifact bridge at every link: an ellipsized id (`24b1e2da…`) cannot be grepped against any artifact even when the log is linked in the same block; a reformatted data block cannot be distinguished from confabulation without redoing the dig, so it reads as *claims in the form of data*; a branch-ref `/blob//` link is a mutable pointer whose target can be rewritten after review (not tamper-evident); a file-level link without line anchors lands the reader at the top of a 1,000-line dump. The producing instance (extension#43929 validation comment, 2026-07-21) had every number substantiated by four re-hosted run logs — real, included, resolvable — and still drew "still no immediately auditable evidence just claims in the form of data": each prior item individually near-passed while the exhibit↔artifact binding stayed **editorial** (transcription + file link) instead of **mechanical** (quotation + pinned line anchor). Emit-time procedure: for every inline datum, quote the raw capture line it comes from (fenced, verbatim, full ids) and anchor it (`#L`); pin every evidence link to the SHA (press `y` on the GitHub file view). Emit-time trigger: `pr-evidence-gate.py` classes `truncated-identifier` (a co-located resolver does NOT excuse it — the resolver resolves the full id, not the fragment the reader holds; hash-equality prose exempt) and `mutable-ref`, plus the **surface hole** — the shipped comment was published via `gh api` PATCH, which the porcelain-only matcher (`gh pr|issue edit|create|comment`) never saw; fifth occurrence of the converse-of-gate rule, one level down: audit the gate for *spellings of the write it cannot see*, not just tokens it excuses. **CORRECTION 2026-07-30 — this hole was recorded as closed and is not.** Verified against the deployed `hooks/pr-evidence-gate.py` (259 lines): line 47 is the only command matcher, `\bgh\s+(?:pr|issue)\s+(?:edit|create|comment)\b`, so `gh api` body writes are still invisible; and the file implements essentially one check (verdict-needs-artifact), **not** the ~9 classes named across items 11–18 (`ci-restatement`, `bare-identifier`, `dump-resolver`, `link-only-exhibit`, `data-only-exhibit`, `step-waiver`, `truncated-identifier`, `mutable-ref`, `inflated-verdict`). Treat every "Emit-time trigger: `pr-evidence-gate.py` class …" line in this document as **specified, not implemented**, until re-verified in the code — a doc asserting a class the code lacks retires the vigilance it claims to replace, which is the failure this very item warns about. Consequence observed the same day: 14 unlinked `path:line` references shipped across 12 review comments via `gh api`, with the gate both classless for that shape and unwired in `settings.json`. Detection gaps: the line-anchor half of pinning and the paraphrase-vs-quotation judgment stay procedural — the gate sees ellipses and branch refs, not editorial fidelity. +17. **Evidence is captured in its environment — data alone is insufficient even when correct** — item 16 makes the data trustworthy as *transcription* (verbatim, greppable, pinned); this item polices what transcription can never carry: **liveness provenance**. A quoted `EVIDENCE trace_id=…` line, a re-hosted gist, a hand-assembled id table can all be correct and still show nothing about *where they came from* — extracted data is indistinguishable from data typed by hand, so it cannot make it immediately apparent that the evidence was captured **live** from a **functioning** system. The exhibit for a system-of-record-observable claim therefore includes an **in-environment capture**: a screenshot/recording of the resolving system's own UI (the Sentry Discover/trace view with the query, project/environment selectors, absolute time window, and result rows all in-frame) — the environmental chrome is not decoration, it *is* the provenance: it shows the query really ran, in the real dashboard, over the real window, and returned these rows. Correctness was never the failing dimension (2026-07-21: "just the data is insufficient even if correct — it needs to be immediately apparent that evidence was captured live and is functional"). Relation to prior items: item 15's link+visual conjunction fired only when a `sentry.io` link was present, and item 13's `NATIVE_MEDIUM` blessed an inline fenced excerpt as a terminal medium — so a no-link, quoted-data exhibit (the fidelity-remediated shape: full ids, verbatim excerpts, pinned line anchors, zero environment captures) passed the whole regime while carrying zero liveness provenance. The joint rule after this item: a telemetry-observable positive verdict always carries the in-environment visual (plus the live permalink per item 15); quoted excerpts, gists, and data files are appendix beside it, never the exhibit. Emit-time trigger: `pr-evidence-gate.py` class `data-only-exhibit` (non-negated verdict + telemetry-observation vocabulary + no image/recording embed + no sentry link — with a sentry link, `link-only-exhibit` already fires), with the re-hosted-gist ALLOW case flipped (fifth occurrence of "the ALLOW case was the next costume's spec") and the #43929 quoted-excerpt shape as a regression case. Detection gaps: vocabulary-scoped (telemetry-observation terms, not bare code tokens like `trace.test.ts`), so a claim phrased entirely without them evades mechanically; and the gate cannot see whether an embedded image actually shows the environment's chrome — screenshot content stays procedural (item 2's "eyeball it" applies: the capture must show the *resolving UI*, not a cropped data region indistinguishable from a spreadsheet). +18. **"Successful" is an evidence predicate, not a run status — and the default Sentry exhibit is fixed in advance** — a validation run may be scored/reported "successful"/"validated" only when its published surface already carries, for every Sentry-observable lane, the default exhibit pair: an **in-environment Sentry-UI screenshot** (item 17) **plus the co-located live permalink** (item 15). Completed runs, green falsifiers, staged drafts, and honest ⏳ lanes do not confer success — a run without the pair is at most "run-complete, evidence-owed." The default recipe needs no per-PR ascertainment: for Sentry, **generally capture actual screenshots from the Sentry UI and attach the link** — that pair is step zero's pre-computed answer for any Sentry-observable claim, never the terminus of axis-by-axis escalation. Capture-first ordering: the capture executes before any rule/gate/postmortem authoring may close a validation session — writing a new rule or gate class discharges nothing (2026-07-22: ten postmortems and 17 gate items shipped while zero Sentry-UI screenshots did; every "successful" run was claims-only, because success was assigned by run-completion and meta-work substituted for capture work). Emit-time trigger: `pr-evidence-gate.py` `VERDICT` vocabulary now includes the status spellings `successful`/`validated`/`live-proven`, so a claims-only unit scoring itself successful blocks like any bare "confirmed." Detection gap: the gate fires only on re-emit — already-shipped "successful" surfaces are audited by backward re-score, enumerated from live state, never from the ledger (the discharge-granularity rule applies to success statuses verbatim). + +19. **A substitution A/B is readable only if the unmodified arm is silent — and only if each diagnostic fires for the reason claimed** — the substitution lane (catalog D6) derives its finding from the *delta* in a checker's output between the PR as written (Arm A) and the PR with one authored artifact replaced by its authoritative equivalent (Arm B). Two ways that delta lies, both of which look like a confirmed finding. (a) **A noisy Arm A destroys attribution.** If the unmodified tree already emits diagnostics, nothing in Arm B is attributable to the substitution — the reader cannot tell a concealed disagreement from ambient breakage, and "N errors in Arm B" is then a count, not a finding. Publish Arm A's result explicitly (`0 errors`, verbatim) as the delivery check; if it is non-empty, the instrument is broken and the lane is **inconclusive**, not a pass — fix the baseline (pin the toolchain, raise the heap, exclude the unrelated project) or drop the lane. This is the substitution analogue of item 1's vacuous-pass guard: item 1 asks whether the artifact exists, this asks whether the *comparison* means anything. (b) **A diagnostic can fire for the wrong reason.** A checker reports the first failure it reaches, so an earlier cause short-circuits the claim under test and an exit-code read scores it as confirmation — the same hazard item 3 polices for tests that fail on base for an import error rather than the bug. Producing instance (extension#44397, 2026-07-30): a probe asserting a hand-written provider return type was unsound errored on *nullability* one property earlier, and the return-type claim — re-probed with the nullability neutralised via `NonNullable<…>` — turned out to be **sound**, i.e. a finding that would have shipped as real. Emit-time procedure: for every substitution claim, assert on the *specific* expected diagnostic (code + message + line), not on non-zero exit; where an earlier cause intervenes, isolate it and re-run; and report the claims the re-probe **cleared** alongside the ones it confirmed — a substitution sweep that only ever confirms is indistinguishable from one that never isolated anything. Corollary on the negative case: when no authoritative source exists (a package that ships no types, a lib absent from tsconfig `lib`, a boundary the repo genuinely owns), hand-writing is *correct* — record it as a cleared falsifier with the reason, never as an unreported non-finding. Detection gap: procedural — a gate can see whether Arm A's result is published, not whether the diagnostic it cites is the one the claim needs. + +## Lane-specific traps + +- **Visual:** spinner/skeleton mistaken for the loaded state; the toggle (privacy/redaction) not actually flipped; a cached screenshot from a prior run; the fallback surface shown without saying so. +- **Perf / benchmark:** stale frozen baseline (catalog C5 caveat); single sample; warm-vs-cold mismatch; measuring a different interaction than the claim. +- **Test:** snapshot regenerated to match the bug (`--updateSnapshot` masking a regression); the test mocks out the changed path; it passes on `main` too (so it's not a regression test). +- **Substitution A/B (D6):** a non-silent Arm A (attribution destroyed); a diagnostic that fires one property earlier than the claim (isolate and re-probe); a substitution the checker never reaches because the caller is untyped JS or `any`; treating "no authoritative source exists" as a null result rather than a cleared falsifier. +- **Telemetry:** query window excludes the release; the error regrouped under a different fingerprint; sample-rate makes "0 events" meaningless. +- **Migration:** only the happy path asserted; `changedKeys` not checked against actual mutations; no real prior-version fixture. +- **Coverage:** a line covered ≠ a behavior asserted (executed but never checked). + +## When evidence fails the gate + +Don't publish it. Either re-capture correctly, **downgrade the verdict to ⚠️ inconclusive** and name what's missing, or — if the evidence shows the claim is false — switch to the [refutation path](SKILL.md). Never round a weak pass up to "proven." diff --git a/domains/pr-workflow/skills/evidence/references/lane-assertions.md b/domains/pr-workflow/skills/evidence/references/lane-assertions.md new file mode 100644 index 00000000..7ba6dca7 --- /dev/null +++ b/domains/pr-workflow/skills/evidence/references/lane-assertions.md @@ -0,0 +1,26 @@ +# Lane → declarative assertion mapping (ADR-0058 bridge) + +Maps each evidence-catalog lane to a declarative assertion form, so a Claim Card can be expressed as an ADR-0058 recipe (pre-conditions → proof targets → assertions → screenshot points) where possible — and so we know which lanes are **CDP-expressible** vs **out-of-band**. State/log assertions give determinism; screenshots/video give reviewer confidence. See [[../ITERATION]] items 9–11 and MetaMask/decisions#173. + +| Lane | Assertion form | Expressible as a CDP recipe action? | +|---|---|---| +| A1 / B1 visual | screenshot at a proof point + (optional) DOM/a11y assertion | **yes** — Chrome CDP | +| B2 e2e | the spec's own assertions; trace.zip as artifact | yes — it *is* a driver | +| B3 falsifying test | test exit code: fail@`main`, pass@branch | out-of-band (test runner) | +| C1 startup traces | `stateHooks.getCustomTraces()[name] < threshold` | **yes** — `Runtime.evaluate` | +| C2 web-vitals | `stateHooks.getWebVitalsMetrics().inp < 200` | **yes** | +| C3 long-task / TBT | `stateHooks.getLongTaskMetricsWithTBT().tbt < 200` | **yes** | +| C4 render (WDYR) | console-log assertion: 0 unnecessary re-renders | partial — needs console capture | +| C5 benchmark | metric delta vs paired baseline > threshold | out-of-band (benchmark runner) | +| C6 DevTools/CDP | netlog: request absent/present; profile metric | **yes** | +| D1 / D2 bundle/chunk | static: chunk-manifest membership / size delta | out-of-band (build artifact) | +| D3 LavaMoat | static: `policy.json` diff empty / justified | out-of-band (git diff) | +| D4 manifest | static: permissions diff empty | out-of-band | +| E1 / E2 Sentry/Tempo | external query link (before/after window) | out-of-band (dashboard) | +| F1 migration | `changedKeys == expected` + state shape valid | out-of-band (migration test) | +| F3 simulation | `simulationData.{gasUsed,stateDiff}` matches | **yes** — `Runtime.evaluate` on state | +| F5 flag matrix | the same assertion repeated per `remoteFeatureFlags` state | **yes** | +| F7 i18n | static: `verify-locales` exit 0 | out-of-band | +| F8 runtime containment | `Object.isFrozen(Object.prototype)`; scuttled global throws + exception resolves; `typeof SNOW` | **yes** — `Runtime.evaluate`, but only against the SHIPPED build variant (dev is unscuttled, test's exception list is wider) | + +**Takeaway.** UI-state and runtime-metric lanes (A/B-visual, C1–C3/C6, F3/F5) map cleanly to CDP recipe assertions — ADR-0058's sweet spot. Static (D, F7), test-runner (B3, C5, F1), and dashboard (E) lanes are **out-of-band**: the recipe should *reference* them as proof targets without executing them. That out-of-band reference is precisely the **non-UI scaling gap** flagged in review of decisions#173 — a recipe schema that admits out-of-band assertion references (not only CDP actions) closes it. This table is the proposed taxonomy to contribute back (ITERATION item 10). diff --git a/domains/pr-workflow/skills/evidence/references/output-templates.md b/domains/pr-workflow/skills/evidence/references/output-templates.md new file mode 100644 index 00000000..752ac449 --- /dev/null +++ b/domains/pr-workflow/skills/evidence/references/output-templates.md @@ -0,0 +1,81 @@ +# Output templates + +The shape a validation run ships in. **This file is the generator.** A correction to how a +run reads is a defect here, not in the comment it was noticed on — fix it here and regenerate, +or the same correction arrives again on the next run. + +Drafting a template in a scratch directory is how that goes wrong: the comment gets better and +nothing else does. + +## The template + +```markdown + +## 🧪 Validation Run + +**Verdict:** — **Claim:** head `` · · + +> [!NOTE] +> Trial run of the [MetaMask evidence skills]() — feedback welcome, on the finding or +> on whether this format is useful to a reviewer. Not a review verdict; nothing here blocks +> the PR. + + + + + + + + + +**Follows from the above** + +- +- + +**Open for review:** + + +``` + +## What each slot is for + +**Verdict line.** The conclusion, not the topic. *"one of the two conjuncts is tested"* and +*"six renders where one would do"* are conclusions; *"tested the hash predicate"* is a topic. +Icons: `✅` proven · `⚠️` partial or scoped · `📋` measured, no verdict asserted · `❌` failed. +Never `❌` for a gap in *evidence* — that reads as a verdict on the author's work. + +**Check name, in words.** *red-on-base check*, *render-count check*, +*dependency-containment check*. Never the lane id: `B3` is an address into +[evidence-catalog.md](evidence-catalog.md), which the reviewer cannot open. + +**The exhibits.** Whatever the runner wrote, pasted whole and unfolded. They should outweigh +everything else in the comment; 70% is a reasonable floor. Do not summarise them above +themselves — a table of your own restating theirs turns a measurement into your word for it. + +**Follows from the above.** Bullets, each traceable to a number in an exhibit. If a bullet +needs three sentences, the exhibit is not carrying its weight. + +**The disclaimer sits directly under the verdict, and stays a callout.** It is the frame a +reviewer needs *before* they read a verdict on their own PR from a source they have not seen +before — where feedback goes, and that nothing here blocks them. Edited by the same rules as +prose it drifts to the foot of the page in ``, where it arrives after the reaction it +exists to shape. Check 11 tests its position, not just its presence. + +**Open for review.** One question, about this change. The runners' generic limits go to stderr +and the `.json` precisely so they do not end up here three times over; read them, and write +the thing a human should actually look at. + +## Assembly + +Templates carry `@@TOKEN@@` placeholders, one per exhibit, substituted with the runner's `.md` +verbatim. Substitution — never retyping — is what keeps the provenance line attached to the +numbers it vouches for. + +Before posting, `scripts/attest-gate.sh ` must exit 0. + +## Worked instantiations + +Three runs assembled from this template, with the reasoning behind each choice, are in +[worked-examples.md](worked-examples.md). diff --git a/domains/pr-workflow/skills/evidence/references/worked-examples.md b/domains/pr-workflow/skills/evidence/references/worked-examples.md new file mode 100644 index 00000000..e98a5f1f --- /dev/null +++ b/domains/pr-workflow/skills/evidence/references/worked-examples.md @@ -0,0 +1,30 @@ +# Worked examples (end-to-end) + +Full runs: claim → lanes → capture → trust-gate → publish. The visual case is in SKILL.md; these cover the non-visual claim shapes. + +## Perf — "defer Rive wasm at startup" +- **Claim:** on cold start of the home view, the Rive wasm chunk isn't requested until the animation surface mounts. **Surface:** startup network + chunk graph. **Falsifier:** the chunk appears in the cold-start waterfall. **Baseline:** base requests it at startup. +- **Lanes:** A2 `perf_validation` (primary) → D2 chunk membership + C6 CDP netlog (corroborate). +- **Capture:** paired build of base vs head (`yarn webpack --test`); CDP netlog over cold start for each; source-map chunk membership of the Rive runtime. +- **Trust gate:** cold-vs-cold (not warm); the chunk truly absent (not deferred by a few ms); the netlog covers the whole startup window. +- **Publish:** before/after request list + a chunk-membership table in the PR body. No screenshot needed. + +## Migration — "add migration NNN" +- **Claim:** loading a profile from `` applies NNN; `changedKeys = {X, Y}`; all other state intact. **Falsifier:** an untouched controller mutated / malformed state. **Baseline:** a prior-version profile. +- **Lanes:** F1 migration test (primary) → F2 vault round-trip (if the vault is touched). +- **Capture:** run `NNN.test.js` (old-state-in → new-state-out); assert `changedKeys`; load a real prior-version profile and confirm boot. +- **Trust gate:** the test asserts more than the happy path; `changedKeys` matches the actual mutations; the fixture is a real prior profile, not synthetic. +- **Publish:** the `changedKeys` assertion + before/after state shape; link the test run. + +## Flag-gated — "Perps banner behind a remote feature flag" +- **Claim (×2):** flag on → banner shows; flag off → banner absent. **Surface:** home/Perps. **Falsifier:** banner state ≠ flag state. **Baseline:** each flag state is its own baseline. +- **Lanes:** F5 flag matrix → A1/B1 visual per state. +- **Capture:** mock the client-config response for each flag state; screenshot each. +- **Trust gate:** the flag is actually toggled (read `remoteFeatureFlags`); two distinct states are shown, not the same frame twice. +- **Publish:** a two-up before/after (flag off / flag on) in the PR body. + +## Refactor / no-op — "extract a hook, no behavior change" +- **Claim (negation):** behavior of `` is unchanged. **Falsifier:** any output/behavior diff. **Baseline:** base behavior. +- **Lanes:** B3 regression suite stays green + B4 snapshot diff empty + D1 bundle within noise. +- **Trust gate:** snapshots were *not* regenerated to hide a diff; the tests actually cover the surface; bundle delta is within noise, not "small but real". +- **Publish:** "no behavior change — regression suite green, snapshots unchanged, bundle ±0"; link CI. A passing screenshot is not evidence here. diff --git a/domains/pr-workflow/skills/evidence/scripts/attest-gate.sh b/domains/pr-workflow/skills/evidence/scripts/attest-gate.sh new file mode 100755 index 00000000..4168e740 --- /dev/null +++ b/domains/pr-workflow/skills/evidence/scripts/attest-gate.sh @@ -0,0 +1,300 @@ +#!/usr/bin/env bash +# +# attest-gate — phase 0 of /attest. Mechanical, no model, hard fails only. +# +# Everything checkable is checked before anything is asked of a model, because a +# model asked "is this good evidence?" answers from inside the frame that produced +# the text. These are greppable, so they are not a matter of judgement. +# +# Usage: attest-gate.sh [--reference ] +# +# 0 all checks pass → proceed to the dispatched passes +# 1 one or more failed → BLOCKED, do not publish +# +# --target owner/repo#N is how check 12 learns where this is going. Without it the gate +# cannot tell a live review from a merged one, and the difference is the whole point. +# 2 usage error +set -uo pipefail + +FILE="${1:-}"; REF=""; TARGET=""; MODE="run" +shift || true +while [ $# -gt 0 ]; do + case "$1" in + --reference) REF="${2:-}"; shift 2 ;; + --target) TARGET="${2:-}"; shift 2 ;; + --diligence) MODE="diligence"; shift ;; + *) shift ;; + esac +done +[ -n "$FILE" ] || { echo "usage: attest-gate.sh [--reference ] [--target ] [--diligence]" >&2; exit 2; } +[ -f "$FILE" ] || { echo "attest-gate: not found: $FILE" >&2; exit 2; } + +FAILED=0 +pass() { printf ' PASS %s\n' "$1"; } +fail() { printf ' FAIL %s\n %s\n' "$1" "$2"; FAILED=$((FAILED+1)); } +has() { grep -qF "$1" "$FILE"; } +hasre(){ grep -qE "$1" "$FILE"; } +hasi() { grep -qiE "$1" "$FILE"; } # case-insensitive; a separate function because + # `hasre -i ''` silently greps for "-i". + +echo "attest-gate: $FILE" +echo + +# A diligence comment (lavamoat-policy and its siblings) renders no verdict and deliberately +# does not use the Validation Run envelope — see "One comment per evidence kind" in +# references/evidence-publishing.md. That exemption used to mean it was checked by nothing at +# all: attest-gate only knew the Validation Run shape, and pr-evidence-gate.py by design does +# not trip on a body claiming no verdict. So every rule the diligence skills state about their +# own output had no execution path, and a comment shipped with an unwitnessed local `npm pack` +# result and untraceable integers. --diligence swaps the envelope checks for that contract's +# own; everything downstream of the envelope is shared, because those defects are shared. +if [ "$MODE" = diligence ]; then + has 'LAVAMOAT_DILIGENCE_START' && has 'LAVAMOAT_DILIGENCE_END' \ + && pass "1 marker pair" \ + || fail "1 marker pair" "no LAVAMOAT_DILIGENCE_START/_END — a re-run appends a duplicate instead of replacing, and the pair must not be VALIDATION_RUN_* or an evidence re-run would eat this region" + + hasre '^\*\*LavaMoat grants|^LavaMoat grants' \ + && pass "2 canonical header" \ + || fail "2 canonical header" "missing the 'LavaMoat grants — -> ' opener" + + printf ' SKIP %s\n' "3 verdict line — a diligence comment renders none, by contract" +else +has 'VALIDATION_RUN_START' && has 'VALIDATION_RUN_END' \ + && pass "1 marker pair" \ + || fail "1 marker pair" "no VALIDATION_RUN_START/_END — a re-run appends a duplicate instead of replacing" + +has '## 🧪 Validation Run' \ + && pass "2 canonical header" \ + || fail "2 canonical header" "missing '## 🧪 Validation Run'" + +hasre '^\*\*Verdict:\*\*.*\*\*Claim:\*\*' \ + && pass "3 verdict line" \ + || fail "3 verdict line" "no '**Verdict:** … — **Claim:** …' — valence is not legible at a glance" +fi + +# A run outside the repo's toolchain pins a different thing. A browser-memory lane +# names "Firefox 153.0"; a repo lane names a head SHA and a lockfile hash. Both are +# pins, and a check that only knows the second one fails every run of the first — +# telling an author their pinned environment is unpinned. +# A diligence comment pins a read, not a run: its citations are permalinks, and the thing +# that can rot is a branch-head link drifting out from under the line it names. +if [ "$MODE" = diligence ]; then + if grep -qE 'https://github\.com/[^ )]+/blob/(main|master|develop|HEAD)/' "$FILE"; then + fail "4 citations pinned" "a permalink points at a branch head; it will drift off the line it cites. Pin a tag or a 40-char SHA" + elif grep -qE 'https://github\.com/[^ )]+/blob/[^/]+/' "$FILE"; then + pass "4 citations pinned" + else + fail "4 citations pinned" "no source permalink at all — the permalink IS the evidence here; a retyped 'it needs X' proves nothing about provenance" + fi +else +hasre 'head `[0-9a-f]{7,}|sha256|node `v|yarn\.lock `|[Ff]irefox [0-9]+\.[0-9]|[Cc]hrom(e|ium) [0-9]+\.|[Ss]afari [0-9]+\.|[Nn]ode v?[0-9]+\.[0-9]' \ + && pass "4 environment pinned" \ + || fail "4 environment pinned" "no head SHA, lockfile hash, or pinned toolchain/browser version" +fi + +# 5 — the one that matters, and it asks for a MEDIUM, not for better text. +# +# Every earlier version of this check tested a property of the plaintext: does it carry a +# provenance marker, does the command contain a placeholder, is the path local. Each caught +# one defect and missed the next, because every property of plaintext is forgeable by +# whatever emits the plaintext. Four runs shipped that way. +# +# So the block below is necessary but is no longer the evidence. The evidence is an image +# of the tool's own surface, a link that re-executes, or a hosted artifact the reader +# fetches without going through the author. If the artifact is small, nothing was attached. +# +# `Produced by` attests who WROTE the block, not that the block is the tool's own output. +# A script that composes a summary table and stamps itself passes on the marker alone — +# which is how a run shipped with a table the script had written, one grepped line, and a +# command reading `yarn jest `. A `$` line carrying a placeholder is the +# tell: it looks reproducible and cannot be run. +# An image, a re-executing link, or a hosted artifact — verification that does not route +# through the author. `Produced by` and `evidence-artifacts/` are provenance, not this. +# In diligence mode the medium is the permalink, already required by check 4 — a reader +# clicks it and lands on the line. What a permalink cannot witness is what the AUTHOR RAN, +# and that is the defect this variant catches: an `npm pack` unpacked locally, a grep over a +# tarball, a byte-comparison across policy files. Those read as properties of the package +# and are actually properties of an unwitnessed local run. State them as the search +# ("searched N files, no match") or publish the output; do not assert them as fact. +if [ "$MODE" = diligence ]; then + # A permalink is the medium for a CITATION — a reader clicks it and lands on the line. It is + # not the medium for anything you RAN. The first version of this branch tested for phrases + # ("npm pack", "complete specifier set") and passed an artifact whose entire results section + # was hand-typed to look like terminal output, because none of those words appeared in it. + # That is the regression the block below already documents as having shipped four times: + # every property of plaintext is forgeable by whatever emits the plaintext. So the test is + # the same one, on the same terms — if the artifact shows a command or a run result, it owes + # the reader something fetchable. + # `/blob/` is a CITATION, never a capture — it witnesses a line in a file, not a run. + # Excluding it matters: a permalink to `policy-override.json` ends in `.json` and satisfied + # a naive extension test, so an artifact whose entire results section was hand-typed passed + # on the strength of a source link. + if hasre '^\$ |^ *\$ |\bexit [0-9]|\bexit=[0-9]' \ + && ! grep -qE 'actions/runs/[0-9]|/gist\.|!\[[^]]*\]\(https?://' "$FILE" \ + && ! grep -E 'https?://[^ )]+\.(txt|log|json)\b' "$FILE" | grep -qv '/blob/'; then + fail "5 captured artifact" "shows a command or a run result with nothing a reader can fetch — a fenced block is your transcription, whatever produced it. Publish the log/gist/run and link it" + else + pass "5 captured artifact" + fi +elif ! hasre '!\[[^]]*\]\(https?://|]*src="https?://|actions/runs/[0-9]|/gist\.|https?://[^ )]+\.(png|jpg|jpeg|gif|svg|txt|log|json)\b'; then + fail "5 captured artifact" "no reader-verifiable capture — an image of the tool surface, a run link, or a hosted artifact. A fenced block is the author\'s transcription, whatever produced it" + # No separate attribution test: a hosted artifact the reader fetches is its own + # attribution, and requiring `Produced by` on top of it only fails runs whose + # evidence is stronger than a stamped fenced block. +elif grep -qE '^\$ .*<[a-z][a-z ._-]*>' "$FILE"; then + fail "5 captured artifact" "a console command contains a placeholder — $(grep -m1 -oE '^\$ .*' "$FILE") is not a command a reader can run" +elif grep -qE '^\$ .*(/tmp/|/home/|/Users/)' "$FILE"; then + # A helper script in /tmp, or any absolute local path, is unreproducible by + # construction. `capture.sh` records the command honestly — but honestly + # recording `bash /tmp/dup.sh` still publishes a recipe nobody else can follow. + # Inline the commands, or ship the helper where the reader can reach it. + fail "5 captured artifact" "a console command references a local-only path — $(grep -m1 -oE '^\$ .*(/tmp/|/home/|/Users/)[^ ]*' "$FILE") cannot be run by a reader" +elif [ "$(grep -cE '^\$ ' "$FILE")" -gt 1 ] && \ + [ "$(grep -E '^\$ ' "$FILE" | sed 's/ *#.*$//' | sort -u | wc -l)" -lt "$(grep -cE '^\$ ' "$FILE")" ]; then + # Two identical commands shown as producing different outputs. The difference + # came from an edit made between runs, so the block misstates its own cause: + # running it twice reproduces the first number twice. + fail "5 captured artifact" "two console commands are identical but shown with different output — the block does not say what actually differed between them" +else + pass "5 captured artifact" +fi + +if hasi 'what would close it|what would prove it|closing it requires'; then + fail "6 no prescriptions" "contains a 'what would close it' section — that is an unfinished run, formatted to look finished" +elif hasre '^\s*(Run|Switch|Assert|Scroll|Compare) '; then + fail "6 no prescriptions" "imperative-mood instructions to the reader — the artifact does not exist" +else + pass "6 no prescriptions" +fi + +# Drafting history is the author's, not the reader's: a reader who never saw the earlier +# version learns nothing from being told it existed, and the byline may not be yours. +# The list grew after a comment shipped a '### Correction:' section retracting its own +# previous revision in place — right instinct, wrong surface. Retract by restating the +# finding correctly; the account of how it changed belongs in a postmortem. +if hasi "I originally|correction to my earlier|filed by me|hard to calibrate|I withdraw|my earlier comment|earlier revision|previous revision|an earlier version of this|is withdrawn|that claim was wrong|^#{1,4} *Correction[: ]|corrected below|see the correction"; then + fail "7 no process narration" "contains first-person process commentary — the reader did not see the earlier draft, and the byline may not be yours" +else + pass "7 no process narration" +fi + +if [ "$MODE" = diligence ]; then + printf ' SKIP %s\n' "8 verdict is earned — no verdict rendered" + printf ' SKIP %s\n' "9 verdict matches artifact — no verdict rendered" +else +if hasi '\*\*Verdict:\*\*.*proven' && ! hasre 'Produced by |actions/runs|evidence-artifacts/'; then + fail "8 verdict is earned" "claims 'proven' with no execution artifact — reading yields 'unverified'" +else + pass "8 verdict is earned" +fi + +# 9 — the wrapper's verdict must not contradict the artifact it embeds. A comment is +# assembled by hand around machine output, and the hand-written header is exactly where +# a "vacuous" result acquires a "proven" label. +HDR="$(grep -m1 '^\*\*Verdict:\*\*' "$FILE" | tr 'A-Z' 'a-z')" +BODY="$(grep -ioE 'vacuous|value unstable|no delta|nothing falsified|broke the module|substitution silent|probe-failed' "$FILE" | head -1 | tr 'A-Z' 'a-z')" +if printf '%s' "$HDR" | grep -q 'proven' && [ -n "$BODY" ]; then + fail "9 verdict matches artifact" "header claims 'proven' while the embedded artifact reports '$BODY'" +else + pass "9 verdict matches artifact" +fi +fi + +# 10 — the positive counterpart to check 6. A run succeeds by putting concerns in front +# of a reviewer, so an artifact that floats nothing has reported only what it happened to +# measure and called that the whole picture. This is NOT satisfied by a "what would close +# it" section, which check 6 rejects: that hands the reader the run's own unfinished work, +# whereas this names a limit or a question the run is right to leave open. +# +# The vocabulary is a fixed list because this phase asks no model anything. That makes +# it blind to a limit phrased outside the list — a real run stated its limit as "what it +# does not establish" and the check called it absent. Add phrases when that happens; +# judging whether the stated limit is substantive is the dispatched passes' job. +if hasi 'open for review|raise with a human|falsifier|worth a look|left unmeasured|not covered by this run|no verdict offered|does not establish|what it does not|cannot attribute'; then + pass "10 floats something for review" +else + fail "10 floats something for review" "no limit, open question, or falsifier named — an artifact that floats nothing implies its measurement was the whole surface" +fi + +# 11 — the trial-run disclaimer, and its POSITION. This is not content, it is the frame +# the reader needs before they read a verdict on their own PR from an unfamiliar source. +# Compressed and moved to the foot of the page — which is what happens when it is edited +# by the same rules as prose — it arrives after the reaction it exists to shape. +DISC="$(grep -n -i 'trial run' "$FILE" | head -1 | cut -d: -f1)" +FIRST_EXHIBIT="$(grep -n '^```' "$FILE" | head -1 | cut -d: -f1)" +if [ -z "$DISC" ]; then + fail "11 disclaimer present and early" "no trial-run disclaimer — a reviewer cannot tell what this is or where to send feedback" +elif ! grep -qi 'trial run' "$FILE" || ! grep -q 'skills/pull/\|MetaMask/skills' "$FILE"; then + fail "11 disclaimer present and early" "disclaimer does not link the skills PR, so feedback has nowhere to go" +elif [ -n "$FIRST_EXHIBIT" ] && [ "$DISC" -gt "$FIRST_EXHIBIT" ]; then + fail "11 disclaimer present and early" "disclaimer is at line $DISC, after the first exhibit at line $FIRST_EXHIBIT — it frames nothing from there" +else + pass "11 disclaimer present and early" +fi + +if [ -n "$REF" ] && [ -f "$REF" ]; then + r=$(grep -coE '!\[|/dev/null 2>&1; then + # Blocks rather than warns. An earlier version printed UNVERIFIED and exited 0, so on a + # machine without `gh` — which is to say, running locally — this check announced that it + # had not run and passed anyway. That is the shape it exists to catch, one level up. + fail "12 destination is open" "gh not on PATH, so the destination was not checked. Unverified is not passing: install gh, or confirm the target is open and re-run." +else + t_repo="${TARGET%%#*}"; t_num="${TARGET##*#}" + t_state="$(gh api "repos/$t_repo/pulls/$t_num" --jq 'if .merged_at then "merged" else .state end' 2>/dev/null || echo unknown)" + case "$t_state" in + open) pass "12 destination is open" ;; + unknown) fail "12 destination is open" "could not read $TARGET — do not publish to a destination you could not check" ;; + *) fail "12 destination is open" "$TARGET is $t_state. A run published to a closed pull request reaches no reviewer and changes no decision." ;; + esac +fi + +# 13 — a number in the prose that appears in no exhibit. Check 9 compares verdict WORDS; +# nothing compared the figures. Measured on a demonstration artifact built to test this +# gate: the prose said "0 errors over 48 skills" directly above an exhibit reading +# "47 skill(s) checked", and named a warning class with zero instances in the output it +# was describing. Both survived every other check. Prose drifts from the exhibit it sits +# beside, and it is the most common way one of these goes wrong. +# +# Deliberately narrow, because a noisy check is an ignored check: integers of two or more +# digits only, and only those absent from every fenced block. Excluded as references +# rather than measurements — whole URLs, issue refs, version strings, dates, SHAs, +# file:line citations, hyphenated identifiers like P-256, and regex quantifiers. Every +# one of those was added after a control run flagged something that was not a figure. +echo +NUM_ORPHANS="$( + awk '/^```/{f=!f; next} f{print}' "$FILE" > "$FILE.exh" 2>/dev/null + awk '/^```/{f=!f; next} !f{print}' "$FILE" \ + | sed -E 's#https?://[^ )]*##g' \ + | sed -E 's/#[0-9]+//g; s/\bv?[0-9]+\.[0-9]+(\.[0-9]+)?\b//g; s/\b[0-9]{4}-[0-9]{2}-[0-9]{2}\b//g; s/\b[0-9a-f]{7,}\b//g; s/:[0-9]+\b//g; s/[A-Za-z]+-[0-9]+//g; s/\{[0-9,]+\}//g' \ + | grep -oE '\b[0-9]{2,}\b' | sort -u \ + | while read -r n; do grep -qF "$n" "$FILE.exh" || printf '%s ' "$n"; done + rm -f "$FILE.exh" +)" +if [ -n "$NUM_ORPHANS" ]; then + fail "13 figures trace to an exhibit" "these appear in the prose and in no exhibit: $NUM_ORPHANS — either they came from somewhere the reader cannot see, or they disagree with what is shown" +else + pass "13 figures trace to an exhibit" +fi + +echo +if [ "$FAILED" -eq 0 ]; then + echo "attest-gate: phase 0 clean — proceed to /outframe ‖ /missing ‖ /press" + exit 0 +fi +echo "attest-gate: BLOCKED — $FAILED check(s) failed. Do not publish." +exit 1 diff --git a/domains/pr-workflow/skills/evidence/scripts/capture.sh b/domains/pr-workflow/skills/evidence/scripts/capture.sh new file mode 100755 index 00000000..298cd619 --- /dev/null +++ b/domains/pr-workflow/skills/evidence/scripts/capture.sh @@ -0,0 +1,154 @@ +#!/usr/bin/env bash +# +# capture — turn any analysis command into a contract-compliant evidence artifact. +# +# The analysis scripts in this repo (retention-scan.py, policy-audit.py, a jest +# probe, a selector recomputation counter) all print to stdout. Printing to stdout +# means the operator is the capture device: they read it, retype some of it into a +# comment, and the result carries their provenance rather than the measurement's. +# +# This wraps any command so the ARTIFACT is written by the tool. Nothing is retyped. +# +# capture.sh --label --lane --claim "" [--verdict ] +# [--open ""] +# [--max-log-lines N | --head-lines N --tail-lines N] -- +# +# --verdict is stated by the caller, never inferred from the exit code: a wrapped +# tool's exit convention is its own, and guessing prints "pass" over real findings. +# +# --open is the same discipline pointed the other way. A run succeeds by putting +# concerns in front of a reviewer, not by closing them, so what the wrapped tool +# could not reach is publishable content. Omitting it is recorded, not hidden. +# +# Emits, under --out (default evidence-artifacts/): +# " +} > "$STAMP.md" + +printf 'capture: %s (exit %s)\n %s\n %s\n %s\n' "$VERDICT" "$CODE" "$STAMP.log" "$STAMP.json" "$STAMP.md" >&2 +# Stated limits reach the orchestrator, not the pasted exhibit: one open question per +# comment, about this diff, beats the same sentence repeated under every block. +if [ -n "$OPEN" ]; then + printf 'limits: %s\n' "$OPEN" >&2 +else + printf 'limits: none stated. This tool answered one question; what it does not cover was +not recorded, which is not the same as it covering everything.\n' >&2 +fi +exit "$CODE" diff --git a/domains/pr-workflow/skills/evidence/scripts/falsify-probe.sh b/domains/pr-workflow/skills/evidence/scripts/falsify-probe.sh new file mode 100755 index 00000000..46fed7f0 --- /dev/null +++ b/domains/pr-workflow/skills/evidence/scripts/falsify-probe.sh @@ -0,0 +1,214 @@ +#!/usr/bin/env bash +# +# falsify-probe — prove a test is falsifying, by mutation rather than by reading. +# +# A test is evidence only if it FAILS when the mechanism it guards is removed. +# Reading the test establishes its shape; only this establishes its power. +# +# Runs two arms against the same tree: +# Arm A baseline — the suite as committed +# Arm B mutant — one line replaced, suite re-run, source restored +# +# Emits a captured artifact (JSON + markdown) written by this script, not +# transcribed by an operator. Exit code IS the verdict, so CI can gate on it. +# +# 0 falsifying arm A passed, arm B failed ON ASSERTIONS → the test has power +# 1 vacuous arm A passed, arm B ALSO passed → the test proves nothing +# 2 broken arm A failed, or arm B did not run → nothing to conclude +# 3 usage/env error +# +# Arm B failing is NOT sufficient. A mutation that breaks syntax fails every test in +# the file, which looks identical to a falsification and is worth nothing: the suite +# never executed. So arm B must run the SAME number of tests as arm A and fail some of +# them. A dropped test count means the mutation broke the module, not the mechanism. +# +# Usage: +# falsify-probe.sh --test --source --line --replace +# [--expect-fail ]... +# [--label ] [--out ] [--runner ""] +# +# Example: +# falsify-probe.sh \ +# --test ui/hooks/perps/coalesceBackgroundRequest.test.ts \ +# --source ui/hooks/perps/coalesceBackgroundRequest.ts \ +# --line 54 --replace ' const existing = undefined as Promise | undefined;' \ +# --label coalesce-inflight +set -uo pipefail + +# A run's artifact has to say whether a reader can verify it. In CI the run URL is that +# verification; locally there is none, and the artifact says so rather than leaving the +# omission for a gate to catch later. +capture_provenance() { + if [ -n "${GITHUB_RUN_ID:-}" ]; then + printf 'Run: %s/%s/actions/runs/%s — logs and artifacts attached there.' \ + "${GITHUB_SERVER_URL:-https://github.com}" "${GITHUB_REPOSITORY:-}" "$GITHUB_RUN_ID" + else + printf 'Produced on a local machine: no reader-verifiable capture. Re-run through the evidence workflow before publishing.' + fi +} + +RUNNER="yarn jest" +OUT_DIR="evidence-artifacts" +LABEL="" +TEST="" SOURCE="" LINE="" REPLACE="" +# Which test names the caller predicts will fail. Caller-stated, like every other judgement +# word here, and checked rather than trusted: the guards ask whether arm B failed and whether +# it ran the same tests, never whether the RIGHT ones failed. A mutation silently corrupted +# before it reached the file failed a different case than it aimed at, ran the full suite, and +# was reported `falsifying` — a green verdict for a mechanism the run never touched. +EXPECT="" + +die() { printf 'falsify-probe: %s\n' "$1" >&2; exit 3; } + +while [ $# -gt 0 ]; do + case "$1" in + --test) TEST="${2:-}"; shift 2 ;; + --source) SOURCE="${2:-}"; shift 2 ;; + --line) LINE="${2:-}"; shift 2 ;; + --replace) REPLACE="${2:-}"; shift 2 ;; + --label) LABEL="${2:-}"; shift 2 ;; + --out) OUT_DIR="${2:-}"; shift 2 ;; + --runner) RUNNER="${2:-}"; shift 2 ;; + --expect-fail) EXPECT="$EXPECT${EXPECT:+\n}${2:-}"; shift 2 ;; + -h|--help) sed -n '2,32p' "$0"; exit 0 ;; + *) die "unknown argument: $1" ;; + esac +done + +[ -n "$TEST" ] || die "--test is required" +[ -n "$SOURCE" ] || die "--source is required" +[ -n "$LINE" ] || die "--line is required" +[ -n "$REPLACE" ] || die "--replace is required (use '' only if deleting the line)" +[ -f "$TEST" ] || die "test not found: $TEST" +[ -f "$SOURCE" ] || die "source not found: $SOURCE" +case "$LINE" in ''|*[!0-9]*) die "--line must be numeric: $LINE" ;; esac +[ "$LINE" -le "$(wc -l < "$SOURCE")" ] || die "--line $LINE is past the end of $SOURCE" + +LABEL="${LABEL:-$(basename "$SOURCE" | sed 's/\.[^.]*$//')-L$LINE}" +mkdir -p "$OUT_DIR" || die "cannot create $OUT_DIR" +STAMP="$OUT_DIR/falsify-$LABEL" + +# --- environment pin: two operators on different machines must be comparable --- +HEAD_SHA="$(git rev-parse HEAD 2>/dev/null || echo unknown)" +DIRTY="$(git status --porcelain 2>/dev/null | grep -v '^??' | wc -l | tr -d ' ')" +NODE_V="$(node -v 2>/dev/null || echo unknown)" +LOCK_SHA="$( { sha256sum yarn.lock 2>/dev/null || shasum -a 256 yarn.lock 2>/dev/null; } | cut -c1-16)" +ORIGINAL_LINE="$(sed -n "${LINE}p" "$SOURCE")" + +BACKUP="$(mktemp)" || die "mktemp failed" +cp "$SOURCE" "$BACKUP" +restore() { cp "$BACKUP" "$SOURCE"; rm -f "$BACKUP"; } +trap restore EXIT INT TERM + +run_arm() { # $1=logfile ; prints "passed|failed" + if $RUNNER "$TEST" > "$1" 2>&1; then echo passed; else echo failed; fi +} + +total_tests() { sed -n 's/.*Tests:.*[^0-9]\([0-9][0-9]*\) total.*/\1/p' "$1" | head -1; } +load_failed() { grep -qiE "SyntaxError|Cannot find module|Unexpected token|Transform failed" "$1"; } + +ARM_A="$(run_arm "$STAMP-armA.log")" + +if [ "$ARM_A" != "passed" ]; then + VERDICT="baseline-already-failing"; CODE=2; ARM_B="not-run" + : > "$STAMP-armB.log" +else + # Mutate exactly one line. The replacement travels through the environment, not + # through `awk -v`: awk runs escape processing on a `-v` assignment, so `[\s\S]` + # arrived as `[sS]` and the mutation written to the file was not the mutation asked + # for — it narrowed the regex it was meant to widen, failed a different test, and the + # runner reported `falsifying` for a mechanism it never touched. `ENVIRON` does no + # such processing. + MUTANT_LINE="$REPLACE" awk -v n="$LINE" 'NR==n{print ENVIRON["MUTANT_LINE"]; next}{print}' \ + "$SOURCE" > "$SOURCE.tmp" && mv "$SOURCE.tmp" "$SOURCE" || die "mutation failed" + # What the artifact reports as the mutation is read back off disk, never taken from the + # argument. The two differed once and nothing in the output said so. + APPLIED_LINE="$(sed -n "${LINE}p" "$SOURCE")" + if [ "$APPLIED_LINE" != "$REPLACE" ]; then + printf 'falsify-probe: the line written differs from --replace\n asked: %s\n written: %s\n' \ + "$REPLACE" "$APPLIED_LINE" >&2 + fi + ARM_B="$(run_arm "$STAMP-armB.log")" + restore; trap - EXIT INT TERM + A_TOTAL="$(total_tests "$STAMP-armA.log")"; A_TOTAL="${A_TOTAL:-0}" + B_TOTAL="$(total_tests "$STAMP-armB.log")"; B_TOTAL="${B_TOTAL:-0}" + if [ "$ARM_B" != "failed" ]; then + VERDICT="vacuous"; CODE=1 + elif load_failed "$STAMP-armB.log" || [ "$B_TOTAL" -lt "$A_TOTAL" ]; then + # The suite did not execute under mutation, so nothing was falsified. Reported as + # broken rather than falsifying: a module that will not load fails every test, which + # is indistinguishable from a real failure by exit code alone. + VERDICT="mutation broke the module — suite ran $B_TOTAL of $A_TOTAL tests, nothing falsified" + CODE=2 + else + VERDICT="falsifying"; CODE=0 + fi +fi + +# Runs last, on the verdict the guards already reached: a mutation can only fail the wrong +# case if it failed something, so this narrows `falsifying` and never widens it. +MISSED="" +if [ "$CODE" -eq 0 ] && [ -n "$EXPECT" ]; then + FAILED_SO_FAR="$(grep -E "^[[:space:]]+.[^\u203a]*\u203a" "$STAMP-armB.log" 2>/dev/null)" + printf '%b\n' "$EXPECT" | while IFS= read -r want; do + [ -n "$want" ] || continue + printf '%s' "$FAILED_SO_FAR" | grep -qF "$want" || printf '%s\n' "$want" + done > "$STAMP.missed" + MISSED="$(tr '\n' '|' < "$STAMP.missed" | sed 's/|$//;s/|/, /g')" + rm -f "$STAMP.missed" + if [ -n "$MISSED" ]; then + VERDICT="falsified a different case — predicted failure absent: $MISSED" + CODE=2 + fi +fi + +summarise() { grep -E '^(Tests|Test Suites):' "$1" 2>/dev/null | tr '\n' ' ' | sed 's/ */ /g'; } +A_SUM="$(summarise "$STAMP-armA.log")" +B_SUM="$(summarise "$STAMP-armB.log")" +FAILED_NAMES="$(grep -E '^\s+●[^›]*›' "$STAMP-armB.log" 2>/dev/null | sed 's/^ *//' | head -10)" + +cat > "$STAMP.json" <Produced by \`falsify-probe.sh\` at \`$HEAD_SHA\` · node \`$NODE_V\` · yarn.lock \`$LOCK_SHA\` · $DIRTY tracked changes. $(capture_provenance)" +} > "$STAMP.md" + +printf 'falsify-probe: %s (exit %s)\n %s\n %s\n' "$VERDICT" "$CODE" "$STAMP.json" "$STAMP.md" >&2 +# The limits below are identical on every run: they describe the instrument, not the +# change under review. Pasted into a PR comment they read as boilerplate to a reviewer +# who has no stake in this tooling, so they go to stderr and to the .json instead. The +# orchestrator reads them and writes ONE open question about THIS diff. +printf 'limits: one line of one file was mutated. Says nothing about other paths into the +same mechanism, whether it is reachable in production, or whether the guarded behaviour is +correct. A falsifying test proves the test has power, not that the fix is right.%s\n' \ + "$([ "$VERDICT" = vacuous ] && printf '\n vacuous: the mechanism is unguarded by this suite — what else depends on it?')" >&2 +exit "$CODE" diff --git a/domains/pr-workflow/skills/evidence/scripts/render-count.sh b/domains/pr-workflow/skills/evidence/scripts/render-count.sh new file mode 100755 index 00000000..ab3dcca2 --- /dev/null +++ b/domains/pr-workflow/skills/evidence/scripts/render-count.sh @@ -0,0 +1,157 @@ +#!/usr/bin/env bash +# +# render-count — lane C4, the component half. +# +# `selector-recompute` answers "how often does this selector recompute". This +# answers the other C4 question: "how many times does a consumer actually +# render". A memoization claim about context or props is a claim about that +# count, and a count of call sites is not it — 149 consumers can mean 149 +# avoided renders or none. +# +# Generates a probe that mounts a provider with a counting consumer, forces the +# parent to re-render N times with the memoised value unchanged, and reports the +# consumer's render count. Arm B re-runs with one line changed, so the delta is +# attributable rather than assumed. +# +# Usage: +# render-count.sh --probe [--defeat --defeat-line --defeat-with ] +# [--arm-b