feat(pr-workflow): add the /mms-evidence orchestrator, runners, and publish gate - #84
Draft
MajorLift wants to merge 188 commits into
Draft
feat(pr-workflow): add the /mms-evidence orchestrator, runners, and publish gate#84MajorLift wants to merge 188 commits into
/mms-evidence orchestrator, runners, and publish gate#84MajorLift wants to merge 188 commits into
Conversation
sentry-mcp-queries: document data-fidelity loss on older releases for longer-range (30D+) queries (sample-rate drift, extrapolation hiding thin samples, retention downsampling) and percentile (p75+) sample-size/quality filtering; add table rows + pitfalls for stored-span-count and superseded-patch releases. performance-attribution: new skill for attributing release-over-release p75/p95 movements to code changes via black-box diff analysis, with an extension repo file (Trace Explorer queries, key transactions, highest-sample-patch version selection, 90d-vs-30d empirics, hot-path files, core-package changelog analysis, worked v13.11->v13.15 catalogue). analytics-instrumentation: cross-link volume-estimation caveats to the new fidelity guidance. CHANGELOG updated.
…n-out, latent trace? param, trace-arg PR-review scan, per-name budget
…cidental instrumentation)
…-types, decompose-large-files
…ary-identification is step one of any migration
…d exception (bivariant callback `any`)
…und unit conversion, swap in the stronger derive-types example, slim CHANGELOG - migration-context-cost: line count is a factor not a non-factor; fan-in is a reading cost (not a change/review surface), fan-out is the change surface; drop the wrong "upstream types land first" and off-topic barrel bullet - decompose-large-files: the point is converting to TS in small self-contained units, not extraction - derive-types: replace the NetworkState restatement with the reinvented-messenger + hand-copied-return example (derive via `ReturnType<Action['handler']>`) - CHANGELOG: list the domain, not each skill
…action is optional
Two-phase retention review for JavaScript/TypeScript. Phase 1 is a static read of a diff: enumerate the retention primitives the change introduces — listeners, timers, pending-request registries, subscriptions, module singletons, growing collections — and pair every acquire with its release site. A primitive with a teardown is safe; one without is the finding. Phase 2 escalates to DevTools/CDP heap snapshots only for a primitive the read cannot pair. Leading with the read rather than the instrument settles most leak claims without ever taking a snapshot.
…oat-policy-diligence Two composing skills for "is this dependency change safe to take". `supply-chain-audit` is the breadth pass: Socket findings, `yarn npm audit` advisories, lockfile and manifest diffs, and the fronts no upstream scanner sees because they are things the repo does to its dependencies afterwards — yarn patches that modify dependency source at install, `resolutions` that force or stub versions, `npmAuditIgnoreAdvisories` suppression lists, CI actions riding mutable tags, and yarn plugins that execute at install. `lavamoat-policy-diligence` is the depth pass it delegates capability containment to. Because a LavaMoat policy is generated from a real run, every grant has a call site by construction — so "each addition is justified" is a tautology, not a finding. It instead reads each grant's use at the installed version to find its gate, and sorts into removable / removable-at-a-cost / load-bearing. Neither renders an accept/reject verdict; disposition belongs to the people who own the dependency. Adds a CODEOWNERS entry for the new domain, defaulted to the platform teams.
CHANGELOG.md tracks consumer-facing changes to the `@metamask/skills` package, per CONTRIBUTING's "CLI / tooling changes" section. No merged skill-only PR adds an entry (#80, #78, #70, #62, #61 all touch zero changelog lines). Carrying one here bought nothing and was the sole source of this branch's conflict with `main`, since every skill PR edits the same `[Unreleased]` block. Restoring the file to its merge-base state makes the branch conflict-free without a merge commit.
CHANGELOG.md tracks consumer-facing changes to the `@metamask/skills` package, per CONTRIBUTING's "CLI / tooling changes" section. No merged skill-only PR adds an entry (#80, #78, #70, #62, #61 all touch zero changelog lines). It was also the sole source of this branch's conflict with `main`, since every skill PR edits the same `[Unreleased]` block.
CHANGELOG.md tracks consumer-facing changes to the `@metamask/skills` package, per CONTRIBUTING's "CLI / tooling changes" section. No merged skill-only PR adds an entry (#80, #78, #70, #62, #61 all touch zero changelog lines). It was also the sole source of this branch's conflict with `main`, since every skill PR edits the same `[Unreleased]` block.
MajorLift
changed the title
pr-validate and falsifying-test skills
MajorLift
marked this pull request as draft
July 30, 2026 14:03
The skill covers runtime retention behaviour, not code authoring, and `coding` reads as language- and style-level guidance. Registers `/domains/stability/` in CODEOWNERS alongside the other platform-owned domains.
…main Was a separate PR against `domains/testing`. It belongs here: its subject is whether a hand-written type agrees with its authoritative source, which is the question `derive-types` answers from the authoring side, and it shares this domain's premise that a green `tsc` is not evidence the types are correct. Directory name and frontmatter `name` already agree; only the domain moved.
`benchmark-design` and `browser-extension-profiling` both cover E2E benchmarking with statistical rigor, and they sat in different domains here while the rest of the measurement work — `data-analysis`, `react-render-proof` — lives in the performance PR. Splitting one subject across two PRs made both harder to review. Moves `benchmark-design`, its `benchmark-statistical-hygiene` knowledge, and `browser-extension-profiling`. Neither skill referenced the other by path, so nothing needed rewriting. What remains is the extension-runtime work this PR is named for.
Generic scripting guidance — paginate an API, handle rate limits, retry transient errors — with no MetaMask specificity and no relationship to this PR's extension-runtime subject. `coding/` otherwise holds MetaMask internals. Nothing referenced it and it referenced nothing, so removal is self-contained. The content stays in this branch's history if it is wanted later.
The keepalive row was the one entry here resting on a live implementation detail rather than on an absent handler or persistent storage, and it was cited as `background.js:750-758` — a line range that drifts. If the interval grows past the idle timeout or the keepalive is removed, the conclusion inverts from "eviction is prevented" to "eviction happens routinely", and a skill still asserting the first is worse than no skill. Replaces the line range with a symbol grep (`saveTimestamp`, `SAVE_TIMESTAMP_INTERVAL_MS`), names the two conditions the conclusion depends on — sub-idle-timeout interval, and an extension API call rather than a bare timer — and records what was verified, against which commit.
MajorLift
marked this pull request as ready for review
July 30, 2026 18:15
…r-check' into HEAD
A block message is the one place the reasoning reaches someone who cannot proceed without reading it. Five of nineteen said only what to do: attest-gate, verdict, deferral, bare-identifier, adhoc-artifact-host. Each now names what breaks and for whom — an untracked deferral decays silently, a bare hash is indistinguishable from a plausible one, an author-controlled host proves reachability rather than independence. gate-missing and gate-error are left terse on purpose: they fire when the gate script is absent or crashed, which is fixed by repairing the script rather than by editing the body.
Two defects, and the second was the worse one. ADHOC_ARTIFACT_HOST matched gist/raw links owned by a single hardcoded handle, so for every other operator the check was silently dead — their personal gist passed as an exhibit. The owner is now EVIDENCE_GATE_SELF, and unset deliberately matches any personal host: over-flagging costs a justification, a hardcoded handle costs the whole check. The adhoc-artifact-host remediation named a bucket the reader cannot write to, which is not a remediation. It now names the configured store and otherwise states the requirement. The destination derives from EVIDENCE_BUCKET, which references/evidence-publishing.md already asks for — that file was de-personalised long ago and the code never followed. principle-coverage.py no longer pins a bucket into the harness env. Controls 13/13 exit 0; suite 73/73.
AEP_HOME replaces a hardcoded clone path in skill.md, aep-local-run.md and evidence-publishing.md, with the old path kept as the fallback so nothing changes for an existing checkout. session-audit now prefers the installed hook over a source clone. Every operator has the installed path; only a contributor has ~/Code/metamask, and resolving to a clone that happens to exist is how one person's layout becomes an unstated requirement. Controls 13/13; suite 73/73; lint 0 errors.
The control matrix was reporting a strictly weaker property for everyone who is not the author. _find_gate's last resort is an installed path the CLI never creates, so on the author's machine the blocking arms exercise attest-gate and elsewhere they take the fail-closed path — printing the same green either way. Blocking arms now assert on the block REASON and refuse [gate-missing] as a reason. Verified falsifiable: under a HOME with nothing installed, two arms fail where they previously passed. Also from the audit: gh auth is a hard precondition that surfaced only 13 checks deep, now stated up front with the rest of the tool assumptions; principle-coverage.py had no shebang at mode 775; its target pinned a live PR that would break the harness for everyone the day it merged, now overridable and documented as the fragility it is; and the mention-block message cited ~/.claude/CLAUDE.md, a file no other operator has. Controls 13/13 here and correctly 11/13 under a bare HOME; suite 73/73; lint 0 errors.
…ght gh auth The preconditions section added one commit ago asserted that docker and jq are not dependencies. Both are wrong. docker gates the AEP harness — yarn dev:postgres on postgres:16-alpine, driven through docker ps — and skill.md opens by calling that harness the primary engine while never naming its runtime. jq is reached as the real binary in the publishing lane's authorship detection, deliberately rather than gh --jq because gh's filter takes no --arg. The bad claim came from a dependency count taken over scripts/ and hooks/ only, then stated as a fact about the skill. Both tools are reached from references/. attest-gate check 12 now distinguishes gh-absent from gh-unauthenticated. Presence is not authentication, and checking only the first sent every unauthenticated operator down a message that reads as a broken target. Verified both ways: fires with 'Run: gh auth login' against an unauthenticated stub, silent against an authenticated one. Also declared: ffmpeg's lane, the macOS-only capture lane, and yarn as supplied by the repo under review. Controls 13/13; suite 73/73; lint 0 errors.
Asserting on the block reason made the harness fail honestly on a fresh box; it did not make the arms work there. The cause is that the harness modelled one layout and production has two. _find_gate resolves ../scripts before its ~/.claude last resort, and `metamask-skills install` does ship hooks/ beside scripts/ — so a real install resolves attest-gate via the sibling. The harness stripped the siblings to reproduce a bare hand-copied hook, which is the incident in its own header, then fell through to a last resort that exists only on the author's machine. Now both: an installed layout with siblings, which the behavioural arms run against, and the bare copy retained for the case it was written for. Measured under a bare HOME, before and after: 2 arms blocked as [gate-missing] and reported nothing; now 0. The remaining 2 failures there are the wiring arms reading this machine's real settings.json through a fake HOME, which is the simulation being wrong rather than the gate. Controls 13/13; suite 73/73; lint 0 errors.
… own The bare layout was kept as a copy and dropped as a case — nothing read $tmp/hook.py after 93bc01d, so coverage inverted rather than widened and the untested layout became the one this file's header incident is about. It now has an arm, and that arm pins HOME to an empty dir: _find_gate's last resort is ~/.claude/skills/..., which exists on the author's machine, so without pinning the arm would assert nothing here and something else elsewhere. Controlling HOME is what makes it mean the same thing twice. EVIDENCE_COVERAGE_TARGET was interpolated raw, so the override added to survive a pinned PR merging reproduced that exact symptom when written as owner/repo#N — the shape every other target interface in this skill takes — and failed identically to the stale-PR case. It now normalises both. Four positive arms block with prose carrying no [class] tag, so the forbid-guard was vacuous on them rather than satisfied; they now require a substring naming why they blocked. Writing those expectations surfaced two wrong guesses of mine before they shipped. Preconditions had two sentences the gh-auth preflight falsified one commit earlier, in the same range; curl was missing from assumed-present. The rewritten mention block restated the paragraph above it. Controls 14/14; suite 73/73; lint 0 errors.
Three of the four got a required substring last commit; the gh-api-body- via-$VAR arm did not, so it still asserted exit=2 and nothing else — the state all four were in before. Its block carries no [class] tag either, so the forbid-guard is vacuous on it rather than satisfied. Mutation-tested: swapping the required phrase for one absent from the message fails the arm, so the guard is load-bearing rather than decorative. Controls 12/12 behavioural; suite 73/73; lint 0 errors.
This was referenced Aug 14, 2026
MajorLift
added a commit
that referenced
this pull request
Aug 20, 2026
Absorbs #103. No file overlap with this branch and no conflict — the two were adjacent in theme rather than entangled, unlike #94, which shared test/cli.test.mjs and the same four citations. Brings three reference checks in the linter: bare lane ids cited without linking the catalog that defines them, [[snake_case]] wiki links from a private authoring vault, and `## Related` entries naming a skill that does not exist on the branch. Plus check-public-refs, which asserts every repository named in the corpus resolves anonymously, and skill-audit. Scope note for reviewers: the lane check has no corpus subject today. Zero matches across 166 skill markdown files here, and zero across 256 non-evidence files on #84's branch, because evidence-catalog.md — the only place [A-G]N vocabulary exists — lives on #84. It is fixture-tested and prospective; it fires the day a non-evidence skill cites a lane. Suite 84/84; lint 52 skills, 0 errors; check-public-refs 6/6 resolve. Falsifiable: stubbing LANE_ID fails 2 lint tests, restoring passes 33/33.
`effect-antipattern-scan`, `selector-antipattern-scan`, `extension-lifecycle-decoupling` and `sentry-quota` carried 89-132 char labels naming their subject but no phrase a developer would type, so the description could not select them. Each now states the trigger conditions explicitly. All four stay under `DESCRIPTION_MAX`.
All three shipped without a `repos/` directory, so they installed into every repo by default rather than by decision — base-for-extension silently meant base for mobile and core too. `extension-lifecycle-decoupling` had its MV3 section, keepalive verification and `background.js` pitfalls in the shared body, shipping to clients with no service worker. `privacy-egress-diligence` named `app/scripts/constants/sentry-state.ts` in its own description. Both move their client-specific half into the overlay. `silent-failure` is genuinely client-agnostic, so its overlay is new content rather than extracted — four extension seams where the process boundary or the platform lifecycle absorbs the failure. Verified by install into all three repos at experimental maturity: extension yes, mobile no, core no.
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Adds
evidencetopr-workflow: an orchestrator, six runners, a CI workflow that runs any runner against any repository at a pinned SHA (evidence-run.yml), a 13-check publish gate, a 41-lane evidence catalog, nine non-negotiables inskill.md, and an optional Claude-Code emit-time hook.The rule is cite what exists, capture what you ran. Its load-bearing check is artifact credibility: a citation counts only if the author could not have authored its bytes, so on
github.comonly/actions/runs/,/user-attachments/,/blob/,/commit/and/pull/qualify. Presence, not resolution — nothing fetches the URL.One hook check runs outside the evidence frame deliberately: addressing a person —
@handle,@org/team— is refused over the whole body and before every other check. The rest ask whether a claim is backed; this asks whether the write notifies somebody, the one violation that cannot be walked back.Bounds and blockers:
hooks/, and check a description names the installed command #99 — the structural linter rejectshooks/besideskill.mduntil ci: shiphooks/, and check a description names the installed command #99 addshookstoBUNDLE_DIRS. Marked experimental.scripts/copies here are temporary — the source is now a standalone runners repository (v0.1.0), and the two have drifted before;check-duplicates.shguards the eight files that must agree, currently agreeing.Motivation
Terminal text pasted into a comment is indistinguishable from terminal text invented for it — and a local run's only witness is its author, which is why measurement runs in CI.
Showcase
Each check ships with the run that would falsify it —
gate-controls.sh, eleven arms, three negative;node --test61/61. The same discipline validated #94 — knowledge cited across domains and #87 — installed references resolve.Trial runs against merged
metamask-extensionPRs nobody flagged:The evidence showcase cites the run page itself for #44724 (workflow executed on a fork) — 2 of 17 gate-carrying legs red and 15 green, the 2 re-executed at new timestamps and the 15 inherited, matrix not re-run.
This branch alone does not work — the installer resolves one source directory, so cross-branch references die:
Two rules here disagree.
evidence-run.ymlmoves measurement into CI; the gate'sci-restatementcheck says a validation surface carries zero CI references — so evidence produced by the first is unpublishable under the second; one already-published body is blocked. I have not weakened either rule: the distinction between "CI ran my measurement" and "CI is green, therefore correct" is real, and where to draw it belongs to whoever owns that check.