feat(pr-workflow): add PR-audit, reasoning-audit, and diagnostic skills - #106
Draft
MajorLift wants to merge 69 commits into
Draft
feat(pr-workflow): add PR-audit, reasoning-audit, and diagnostic skills#106MajorLift wants to merge 69 commits into
MajorLift wants to merge 69 commits into
Conversation
Replaces #56, whose skill.md shipped frontmatter with no body at all — the installer's bodyAfterFrontmatter() returned empty, so an agent loading it got a description and six references it had no instruction to read. pr-validate: for a PR's specific falsifiable claim, name the observation that would prove the claim false, gather it, and publish it into the PR body. Drives the AEP harness (visual_validation, perf_validation) as the primary engine, backed by a catalog of complementary lanes, a trustworthiness gate that rejects vacuous passes, and a publishing flow with an audience-reachability rule for re-hosted artifacts. falsifying-test: the strongest single proof that a fix targets the reported bug — a test that fails on the base commit and passes on the branch, with both runs shown. Its own falsifier is a base-commit failure for the wrong reason (import error, missing fixture, unrelated red), which looks identical in an exit code and proves nothing. pr-validate calls it as the engine behind lane B3. hooks/pr-evidence-gate.py enforces the trustworthiness gate at emit time, blocking an outward-facing write whose body carries an unbacked verdict, an untracked deferral, a CI restatement, a bare or truncated identifier, a mutable ref, a dump-as-resolver, a link-only or data-only exhibit, or a step waiver. It polices `gh api` body writes as well as the porcelain, since a PATCH to a comment is the same publish with a different spelling.
Over-scrubbed. The audience is the MetaMask org, and the ticket is the evidence for the claim the item makes — that the LaunchDarkly provisioning blocker covers only the prod-flag half of that lane. Without it the example is an assertion. The scrub line is personal references, not org-internal ones.
`pr-validate`'s other engines are placed by subject — `react-render-proof` in `performance`, `memory-leak-hunt` in `stability`, `supply-chain-audit` in `security`. This one was placed by its caller instead. Writing a test that fails on the base commit and passes on the branch is a testing technique, and `testing/` already holds techniques of that kind (`e2e-flakiness-patterns`, `test-layer-placement`, `performance-testing`), while `pr-workflow/` is uniformly PR-lifecycle stages. Both references to it are by name rather than path, so nothing needed updating.
`D6` covers claims where a PR hand-writes an artifact that restates an existing source — a type, schema, vendored constant, or checked-in policy. Both arms sit at the same commit and differ by a substitution rather than a ref, so there is no build or merge boundary to confound the result.
`pr-validate` cost ~9,059 tokens the moment an agent selected it — 5.6× the median of the 26 skills in the open PRs — and a quarter of that was the AEP local-run procedure, which most validations never touch. The skill's own Sufficiency section tells you to prefer a lighter lane; the body charged you for the heavy one regardless. Preflight, run mechanics, and teardown move to `references/aep-local-run.md`, which the body already linked twice and which did not exist. The link was dangling — the same defect class the `knowledge/` guard catches, on a path nothing checks. Publishing keeps the decisions (surface by ownership, post complete once, falsifier-forward, scrub) and points at `references/evidence-publishing.md` for the mechanics it already documents in full. Body 34,431 → 25,736 bytes, so a selected skill is ~6,812 tokens installed rather than ~9,059. Nothing is lost: it sits behind the same progressive disclosure boundary as the other seven references, read when an AEP run is actually warranted. Description trimmed 1,147 → 885 characters. It was over the 1,024 ceiling that #47 enforces, so it would have failed that check on merge.
Trimmed to 885 characters to fit a 1,024 budget that turned out to be unverified — no operator observed rejects or truncates a longer description, and several over 1,024 load today. Back to 1,147, under the 1,536 budget. The description is the discovery surface, so the 262 characters were trigger cues: the subcommand list and the phrasings that route a request here rather than to another skill. Cutting them made the skill harder to select, which is a functional loss and not a cosmetic one. The body restructure is unaffected — that removed duplication behind a reference, which costs nothing at selection time.
2af1db1 carries the reasoning but not the change: its `git add` was chained into a command the commit guard rejected, so the commit was created from an already staged tree and landed empty — same 885-character description as its parent. This applies it. Back to 1,147 characters, under the 1,536 budget. The 262 characters are trigger cues — the subcommand list and the phrasings that route a request here rather than to a sibling skill — so losing them made the skill harder to select, which is a functional loss rather than a cosmetic one.
Two of the three assumptions in the old name were false. The skill runs in the inner loop against uncommitted changes, and on a symptom with no claim and no PR at all, so `pr-` narrowed it to one of three modes. `validate` named an activity where the deliverable is an artifact, and read as a checklist exercise — the posture the skill spends its opening section arguing against. `evidence` names what it produces, and covers a refutation as naturally as a confirmation. The verdicts are proven, refuted, and inconclusive; a name promising proof would make two of those read as failure. The description is rewritten rather than search-replaced. It now states all three modes, since the old one described only the PR case and so under-selected for the other two, and it names the trigger as `mms-evidence` — the form the installer actually emits. Eleven skills across the repo still promise the unprefixed `/<name>` in their descriptions while installing prefixed; this corrects the one being renamed.
The engine table still routed the memory-leak category to `/memory-leak-hunt`, which was renamed to `/memory-leak` — the rename missed the PR that performed it. Two `[[snake_case]]` entries were wiki links to a private authoring vault; they render as literal brackets here and resolve for no reader. One had a real counterpart in `references/` and now links it; the other pointed at a file that does not exist and is dropped rather than left dangling. `falsifying-test` named its evidence categories as `B3` and `B7`. Those are addresses into `evidence-catalog.md`, not names, so both now use the category name and link the catalog.
`race-condition-proof` is now `race-condition-repro` and `react-render-proof` is now `react-render-delta`. Both are named here as engines, in the catalog, the engine table, and the sibling reference — none of which the renaming branches can reach.
The catalog is 41 full lane specs with no summary, so a reader arriving from a link lands mid-document with no way to see the shape of it. Adds a generated "Lanes at a glance" table: family, count, and every lane id with its title. Two placement bugs: - `C9` sat inside the `# D. Build output` section, so scanning family C missed the lane backing `memory-leak`, and scanning D found a stranger. - `B7` sat between `B3` and `B4`. All 41 lanes now read in order. Also removes six pointers into a private authoring vault — four inline `exogram` references and two full `exogram-daemon/...` paths. They resolve for no reader of a public repository. Every substantive claim they were attached to is kept; only the dangling pointer is dropped. `memory-leak-hunt` updated to `memory-leak`.
The catalog had 41 lanes and none for how long a build takes. Family D covered build *output* — size, chunks, policy, permissions, variants — and `C5` covers runtime, so a toolchain change had no category to publish into even though a skill for measuring one is specified in #102. `D7` is the dev-loop half: paired A/B, cold and warm as separate numbers, with the four confounds that each return a favourable result when uncontrolled — warm cache leaking into the cold arm, worker-pool startup amortised away, core count that does not transfer off the measuring machine, and watch rebuilds presented as cold builds. `G6` is the CI half, in family G because its dominant confound is a process one: `get-requirements.yml` skips jobs when build output matches base, so a measured speedup is often a skipped job. Family D is retitled from "Build output" to "Build", since it now covers both. The at-a-glance index is regenerated rather than hand-patched — it is derived from the headings, and hand-editing it is how it drifts.
An 18-comment trial run met none of this skill's output requirements. The cause was structural, not behavioural: `VALIDATION_RUN_START` and the in-situ capture rule occurred zero times in skill.md and only in a reference costing ~5x the body to open, described there as "image re-hosting and the privacy scrub". The publish gate checks none of them either. All three layers failed open. Moves six non-negotiables and the canonical output shape into the body, where they load with the work: 1. Ship an artifact the reader can check without trusting you. Pasted terminal text is indistinguishable from invented terminal text — running the check justifies your belief, not the reader's. 2. `proven` requires execution; reading gives shape, never power. Run arm B against your own probe: one that passes with the mechanism deleted is measuring something else. 3. No "what would close it" section — that is an unfinished run formatted to look finished. Imperative-mood prose means the artifact does not exist. 4. Write to the reviewer who arrives, not whoever commissioned the run. 5. Delete findings whose entire content is test quality, unless critical. 6. Route privacy and security findings to the private tracker. Derived from eight postmortems in exogram-core; the reference keeps the full recipe.
A recipe returns as many answers as it has operators, and any output an operator
retypes carries the operator's provenance rather than the measurement's. This
ships the mechanism instead.
Runs arm A, mutates one line, runs arm B, restores the source, and writes
`falsify-<label>.{json,md}` plus both raw logs itself — nothing is transcribed.
The exit code is the verdict, so CI gates on it directly: 0 falsifying, 1 vacuous,
2 arm A already failing, 3 usage error.
Every artifact pins HEAD, node version, yarn.lock hash, and the tracked-change
count, so two operators either produce comparable results or visibly do not.
Verified against both outcomes on metamask-extension at 796685ce7b7: the perps
coalescing suite reports `falsifying` (10 passed, 2 failed under mutation), and
the token-search suite reports `vacuous` (3 passed both arms, so it does not
test the abort it appears to test).
…o retype them `retention-scan.py` and `policy-audit.py` already do the analysis well; both print to stdout, which makes the operator the capture device and returns provenance to whoever pasted the output. This wraps any command so the tool writes the artifact. Emits <label>.log verbatim, plus .json and an attachable .md that quotes the log rather than summarising it, with HEAD, tracked-change count, node, python, and yarn.lock hash pinned in each. The wrapped exit code passes through for CI. `--verdict` is stated by the caller, never inferred from the exit code. The first run of this script proved why: it labelled a policy audit "pass" while the output listed sixteen newly granted capabilities, because policy-audit.py exits 0 regardless. With no --verdict it now says "ran to completion, no verdict asserted". Verified on both scripts against real PRs, including a deliberately wrong invocation — which produces an artifact containing the traceback rather than a fabricated finding.
A memoization claim is a claim about a count, and reselect publishes the count. This generates a probe, runs it, deletes it, and writes the artifact, so the number never passes through an operator's hands. Three conditions; the middle one discriminates. A selector built on narrowed input selectors is unmoved by a write it does not read, while one taking `state.metamask` wholesale recomputes on every unrelated write in the app. Verified against both shapes on main, so the runner is shown to distinguish them rather than only to report success: getWalletsWithAccounts 1 / 1 / 6 narrowed selectRampsControllerState 1 / 6 / 11 recomputes on unrelated writes That completes runner coverage for the catalog lanes with engine skills: B3 and B7 via falsify-probe, C4 here, C9 and D3 via capture around their existing analysis scripts.
Whether a hand-written type agrees with the source it restates is a question only the compiler can settle. Arm A typechecks the baseline, arm B applies the substitution, and the finding is the error diff. Source is restored on exit, including on interrupt. Carries the warning the lane most needs: a silent arm B is not proof of agreement. Indexing and `.match()` compile against `string` and `string[]` alike, so without `--probe` injecting a deliberately-typed sink, the lane reports false clean on exactly the divergence it exists to find. When arm A already fails it stops and says nothing was established, alongside the module/export error count — but it does not classify from that ratio. The first real run had 124 of 280 errors as install artifacts while tripping no majority rule, because other codes are downstream of the same missing types. A threshold there would be a number I could not justify, so it reports the breakdown and leaves the judgement with the operator.
Gating on arm A being error-free was wrong: the finding was always the diff, so a single unrelated pre-existing error — a local work-in-progress file, in the run that exposed this — vetoed the whole lane. Baseline errors are now subtracted and only errors new under substitution count. Verified end to end on shared/lib/transactions-controller-utils.ts, where the local `LogWithTopicsArray` declares `topics?: string[]` against an upstream `topics?: string`: substitution alone 1 -> 1 errors, 0 new silent substitution + typed sink 1 -> 2 errors, 1 new TS2322 at the sink That is the lane's central caution demonstrated rather than asserted. Indexing and `.match()` compile against both shapes, so the obvious probe reports a false clean; only a deliberately-typed sink surfaces the divergence. A silent arm B means the probe was too weak, not that the types agree. Also fixes a `grep -c ... || echo 0` double-fire that produced "0\n0" and an integer-comparison error — the same shape already fixed once in this script's tracked-change count.
…lished Phase 0 of the /attest command. Everything greppable is checked before a model is asked for judgement, because a model asked "is this good evidence?" answers from inside the frame that produced the text. Marker pair, canonical header, verdict line, environment pin, a captured artifact, no "what would close it", no first-person process narration, and `proven` only where an execution artifact exists. Check 5 carries the weight: if every character of the output is one the operator typed, the run published an assertion. `--reference` compares capture density against a known-good artifact. Verified in both directions. A retracted run-1 comment is BLOCKED on three checks — no captured artifact, a "what would close it" section, and an unearned `proven`. A runner-produced artifact passes all eight. Building it reproduced two bugs it exists to catch: `hasre -i '<pat>'` passed `-i` as the pattern, so three checks silently grepped for the literal string and returned false passes; and an over-escaped backtick made the environment-pin check never match. Both found by running the gate against a file whose expected verdict was already known.
Three runners wrote three different provenance verbs — "Produced by", "Captured by", "Measured by" — so the gate had to know each synonym and silently blocked the one it did not. That is a false BLOCK on valid evidence, the most expensive direction for a gate to fail in. All four now emit `Produced by <script>`, and the gate matches one marker. `tsc-substitution.sh` emitted none at all: an earlier rewrite of its markdown body dropped the footer, so its artifacts failed the captured-artifact check despite being fully machine-produced. Also removes the last escaped backticks from the gate. In ERE a backtick is not special, so `Produced by \`` matched literal-backslash-backtick and never fired — the same defect already fixed once in the environment-pin check and not generalised then. Fifth appearance of this class today; now eliminated rather than patched per-check. Verified by regating five wrapped comments through every fix: three blocked on the synonym mismatch, all six pass once the marker is uniform.
`selector-recompute` answers how often a selector recomputes. This answers the other C4 question: how many times a named consumer actually renders. A memoisation 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. Runs the probe, defeats the memo at a given line, re-runs, reverts. Verified on a synthetic provider: 1 consumer render across 6 parent updates, rising to 6 with the memo defeated. The probe is supplied rather than generated, deliberately. A provider's mount requirements are specific to the component, and a generated probe would either be wrong or need every prop on the command line. That distinction also bounds what this can currently prove about extension#39310: a probe mounting its own memo demonstrates the mechanism but does not measure that PR's provider, which needs the app's store and router. No run was posted there — a synthetic stand-in presented against a real claim is the substitution this lane exists to catch.
falsify-probe reported `falsifying, exit 0` for a mutation that broke syntax. Arm B "failed" because the module would not load and zero tests ran — identical to a real falsification by exit code, and it would have published "the suite fails when the mechanism is removed" about a suite that never executed. Arm B must now run the same test count as arm A and fail on assertions; a load error or a dropped count reports "mutation broke the module, nothing falsified". selector-recompute now gates on correctness before reporting counts. The probe captures the selector's value across all three conditions, and a value that moves under a write the selector does not declare as an input fails the run outright. A memoisation change that alters output is a breaking change the count would never reveal. attest-gate gains check 9: the wrapper's verdict must not contradict the artifact it embeds. Comments are assembled by hand around machine output, and the hand-written header is exactly where a "vacuous" result acquires a "proven" label. Verified by relabelling a real comment — blocked. All three verified in both directions: the poisoned mutation is refused and the genuine one still reports falsifying; the mislabelled comment is blocked and the correct one passes.
Seven runners existed with nothing telling the orchestrator which serves which claim, so routing was left to judgement at the exact point where judgement is what the tooling replaces. The registry pairs every runner with what it CANNOT establish, because that column is where a run stops being evidence: falsify-probe shows a test has power and says nothing about whether the fix is correct; a silent tsc-substitution is not agreement; retention-scan pairs by name and cannot show the release site is reachable; policy-audit and egress-delta cannot rule on acceptability at all. Synthesis rules, in priority order: a lead lane is required and a corroborator never substitutes for one; correctness gates measurement, since a performance number over changed behaviour is a missed regression rather than a result; an exit-2 from any runner caps the whole run at unproven and is reported on its own line rather than averaged away; security and privacy findings route privately whatever the other lanes say; and the uncovered part of the claim is named in the artifact so the covered part cannot imply coverage. Records a known gap rather than papering over it. No runner checks a diff against an architectural decision record. A merged PR added deeplinks accepting unsigned parameters — justified as "read-only screens, so unsigned routing params are safe" — and was reverted. Signed links skip the warning interstitial, so an unsigned parameter inherits the signature's trust without being covered by it, which is exploitable whether or not the destination writes anything. Nothing in the table would have caught that: it is a rule in a document, violated by code that looks unremarkable. ADR-governed surfaces route to a human until a conformance runner exists, and the artifact must say so.
A run succeeds when it puts concerns, falsifiers, and avenues of deeper inquiry in front of a reviewer. Not every concern caught, none required to reach a conclusion. Lower than being right; much higher than staying silent unless certain. Replaces the "known gap" framing, which had the bar wrong. That no runner checks ADR conformance is not a deficiency to apologise for — a run naming the governed surface and its falsifier has done its job while resolving nothing. Held to correctness-to-conclusion the tooling is useless precisely where review matters most, because those cases turn on intent, threat model, or a written decision that is not in the diff, and a tool held to that bar either goes silent or guesses. Guards the obvious failure mode: floating a concern still requires naming what would settle it. "This might be unsafe" is noise; "unsigned parameters on a signed link skip the interstitial, check whether the signature covers them" is actionable. The difference is whether the next step is stated. Keeps the deeplink revert as the worked case, now as an illustration of a sufficient run rather than a missed one.
A runner's job is to put concerns in front of a reviewer, not to close them. The four measurement runners exited 0 or 1 and said nothing about the surface they left unmeasured, which reads as a clean bill of health for the whole mechanism rather than for the one property tested.
`capture.sh` gains `--open`, stated by the caller like `--verdict` and never inferred; when omitted the artifact says so rather than reading as full coverage. `attest-gate.sh` gains check 10, the positive counterpart to check 6 — check 6 rejects handing the reader the run's own unfinished work, check 10 rejects an artifact that names no limit at all.
`capture.sh` elided the tail when output exceeded the line budget, which drops `policy-audit.py`'s RAISE WITH A HUMAN section and leaves a wall of checkboxes in its place — a tool that escalates does it last. It now elides the middle. `render-count.sh` hard-coded arm B as "memo defeated"; when the PR under test is the suspect rather than the fix, that prints the reading backwards, so the label is caller-stated via `--arm-b`.
An absolute `--out` path put the operator's home directory into the `<sub>` line of every artifact, which is the one part of the run that gets pasted into a public comment. The `.json` keeps full paths for machine use.
Check 4 recognised only repo-toolchain pins — head SHA, lockfile hash, node version — so a browser-memory run pinned to `Firefox 153.0 headless` was told its pinned environment was unpinned. Check 10's vocabulary missed a run that stated its limit as "what it does not establish". Both were false negatives against real published runs, not missing content.
Two thirds head, one third tail is a guess. `policy-audit.py` prints 1200 lines of worklist before its escalation section, so the default budget spent itself on checkboxes; `--head-lines 10 --tail-lines 36` cut the embedded block from 9K to 5K with more of the part a reader needs. The elision notice also printed an absolute path, same leak the `<sub>` line had.
Two things in this PR shared a root and a two-arm shape while answering different questions: the skill compares a base commit against a branch to show a test is connected to the reported bug, and `falsify-probe.sh` mutates one line at one commit to show a suite notices the mechanism going away. A test can satisfy either and fail the other, so the names had to stop rhyming. `red-on-base` names the skill's own discipline — the test must be red on the base commit — and shares no root with the runner. The evidence category keeps its name: a falsifying regression test is the artifact, and this is the procedure that produces one. The runner's stderr described its own result as "a falsifying test", which was the same collision inside the output of the thing causing it; it now says what it proves and points at the other experiment by name.
MajorLift
marked this pull request as draft
August 3, 2026 10:11
Every other engine `evidence` names ships in its own pull request — `memory-leak`, `race-condition-repro`, `supply-chain-audit`, `react-render-delta`, `agent-run-cost`. This one was the sole exception, and the reason was authoring order rather than design: it was written alongside the orchestrator before the split between the instrument and the reasoning that points it existed as a concept. It lands in #106 with the other reasoning skills, whose substance is the same kind — what counts as proof, and how a proof can look right while testing the wrong thing. What stays here is the machinery: the runners, the run workflow, the gate, the hooks. The B3 engine cell now names a skill that arrives in #106, which is a dangling name in a table rather than a broken link, and resolves whichever order the two merge.
MajorLift
changed the title
Eleven checks tested properties of the comment text. Text can be perfect and land somewhere nobody will read it, and that is what happened: across one register of published runs, 22 of 27 comments went onto pull requests that had already merged when they were posted — median 22 days after the merge, one 178 days after. The gate passed every one, because no property of a comment reveals the state of its destination. Check 12 takes `--target owner/repo#N` and blocks on anything that is not open. Omitting the target fails rather than passes: an unchecked destination is the condition that produced all 22. Without `gh` on PATH it reports UNVERIFIED and still refuses, since the point is that silence here is indistinguishable from success.
4 tasks
The CI workflow sources runners by `skills_ref`, and every run this week pointed at a fork branch. Four fixes were made there and never reached this one: the mutation now travels through `ENVIRON` instead of an escape-processed `awk -v` assignment, the artifact reports the line read back off disk beside the line requested, `--expect-fail` turns a red arm in the wrong place into its own outcome, and `--metric` lets the caller name what a probe counted. So the defect this PR's own description cites as the reason instruments must report their effect was, until now, still live in the instrument this PR ships. The probe comes across too, with the import fix that made it resolve at the destination the workflow copies it to. Two copies of the same scripts on two branches, edited in both directions — `attest-gate.sh` had a check the fork lacked, so it stays as it is here.
Check 9 is called "verdict matches artifact" and compares verdict words. Nothing compared the numbers, and prose drifting from the exhibit beside it is the most common way one of these goes wrong. Found by building a demonstration artifact to test this gate: the prose read "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. Every other check passed. Two independent readers caught both, which is the argument for moving it into the layer that always runs rather than the one that costs money and sometimes never reports. Narrow on purpose, because a noisy check is an ignored one. Two-plus digits only, and only those absent from every fenced block; whole URLs, issue refs, versions, dates, SHAs, file:line citations, hyphenated identifiers and regex quantifiers are excluded as references rather than measurements. Each exclusion was added after a control run flagged something that was not a figure. Across eight real artifacts it flags one, correctly: a verdict line quoting an author's "730 tests" beside an exhibit measuring 731, where nothing distinguishes the cited figure from the measured one.
With `gh` absent the check printed UNVERIFIED and exited 0, so on a machine without it — running locally — the destination check announced that it had not run and the gate reported clean. A control that cannot run is indistinguishable from one that passed unless the exit code says otherwise.
Renamed on the security-domain branch; installs as `mms-lavamoat-policy`.
A diligence comment renders no verdict and deliberately does not use the Validation Run envelope. That exemption meant it was checked by nothing: this 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 — including "runtime claims need a runtime artifact" — had no execution path. It showed. A lavamoat comment shipped with no marker pair, an `npm pack` specifier set no reader could fetch, and two bare integers traceable to nothing. `--diligence` swaps the four envelope checks for that contract's own — its marker pair, its header, permalinks pinned to a tag or SHA rather than a branch head, and a runtime claim check asking for the thing a `/blob/` link cannot witness. 3, 8 and 9 report SKIP with the reason rather than passing silently, since a check that cannot fail should not read as a check that passed. Everything downstream of the envelope is shared, because those defects are shared. Run against the comment that prompted this, it fails 1, 5 and 13 and passes the rest.
This repository is public. Naming a private repository here discloses its existence,
its owner and roughly its contents to every reader — and a prohibition naming it
("do not re-host to X, it is private") discloses exactly as much as a recommendation
would. Four such references were doing that, and the guidance survives without them:
the rule is audience-reachability, which is stated directly rather than by example.
Two memory-file citations offered as "source of truth" pointed into a private repo, so
a reader was told to follow a rule whose justification they cannot open. The reasoning
is inlined; the pointer is gone.
The publish-surface snippet hardcoded a GitHub username, which decided the destination
for whoever ran it. Now derived from `gh api user --jq .login`, and the surrounding
prose is second-person rather than first — a shared skill has no "my PRs".
`/attest` is no longer linked to a personal repository. That leaves it named but not
resolvable, which is honest and is the smaller problem; the workflow depending on a
command nobody else has is tracked separately.
The re-hosting section named a personal S3 bucket, its region, its prefix layout, and stated that anonymous `GetObject` is enabled under `public/*`. That is a live unauthenticated endpoint advertised, with its structure, to every reader of a public repository — a larger disclosure than the repository names removed alongside it, and one that reads as configuration rather than as a secret, which is why it survived two passes over this file. Now `EVIDENCE_BUCKET` and `EVIDENCE_REGION` from the environment. The requirements the bucket must satisfy — anonymous GetObject under `public/*`, listing disabled — are stated, because those are the load-bearing part; the name never was. The G5 lane likewise named a private test fork, which carried both the org and a personal handle. Now "your own test fork".
…setup Removing the hardcoded username left `gh pr view --jq --arg me "$ME"`, which is not a thing gh supports — its built-in filter takes no --arg and the command dies with "accepts at most 1 arg(s)". Piped to real jq instead, and checked against both branches of the logic: a PR authored by someone else resolves to "skip", one authored by the caller to "body". Replacing the named bucket with `EVIDENCE_BUCKET` removed a working default and put nothing in its place, so the section told you to configure a bucket without saying what "conforming" meant. The policy is now stated: anonymous `s3:GetObject` under `public/*`, public-access blocks off for that bucket, `s3:ListBucket` to nobody. With a note that an org-owned bucket beats a personal one, since artifact links outlive their publisher.
A results section reached a public PR whose entire content was hand-typed to look like
terminal output. Three independent things had to hold for that, and all three did.
The gate is model-invoked, so it can be skipped: the publish and the gate ran as two
statements rather than one chain, and the verdict was read after the write.
The hook that fires on the publish call carried a SECOND, narrower copy of the rules —
keyed on verdict tokens — so a comment rendering no verdict satisfied neither copy.
Two rule sets means the weaker one governs whatever falls between them. The hook now
delegates to `attest-gate.sh`: one rule set, invoked by construction rather than by
choice, in the mode the body's markers imply. It fails CLOSED once it has identified a
body it is about to publish — an enforcement point that waves things through when it
cannot find its rules is not one.
And check 5 in `--diligence` had been rewritten as a phrase denylist ("npm pack",
"complete specifier set"), which is precisely the regression its own comment records as
having shipped four times: every property of plaintext is forgeable by whatever emits
the plaintext. It is a medium test again — if the artifact shows a command or a run
result, it owes the reader something fetchable. `/blob/` links are excluded, because a
permalink to a `.json` file satisfied a naive extension test and was the specific reason
the hand-typed section passed.
Comment-update URLs carry the comment id, not the issue's, so check 12 was asking
whether pull #5177261620 was open. Resolved through the API instead.
Four-arm verified: blocks the exact command and body that shipped; ignores `ls`; ignores
a `gh` read with no body write; refuses when the gate is unreachable.
`mms-evidence` named `/attest` as the gate its output passes and shipped no such thing. Anyone installing the skill got phase 0 as a script and a reference to a command only its author had, which is half a publish path for everyone else. Phase 0 is `attest-gate.sh`, already here. Phase 1 is three briefs sent to fresh instances — frame, coverage, and how it reads to a stranger — written out in `references/dispatched-passes.md` so dispatch does not depend on commands that live elsewhere. The check table in `references/phase-0-checks.md` is generated from the gate rather than retyped, so it cannot drift from what runs. Two things the port makes explicit that the original left to discipline. The gate must be the same shell chain as the publish, because running both and reading the verdict after the write is how a blocked artifact reached a public PR. And softening a check to fit the case in hand is called out as an anti-pattern: if the new version could be satisfied by better prose alone, it is no longer the check.
Three copies of this hook were on one machine and the oldest was the one wired into settings — no `gh api` matcher, no delegation to `attest-gate.sh`. Every publish through that path went ungated, and nothing noticed, because a gate that blocks nothing looks exactly like a gate with nothing to block. `gate-controls.sh` is the thing that would have noticed. Six arms: three publish routes that must block, three inputs that must pass. It copies the hook somewhere with no sibling `scripts/` so `_find_gate()` resolves the way it does in production rather than the way it does in a checkout — the difference matters, and testing the checkout copy is how the deployed one stayed broken. Both halves earned their place immediately. The negative arm caught that delegating to attest-gate ran it over EVERY published body, so an ordinary reply was judged as a failed validation run; wiring that would have blocked every normal comment. The gate now applies only to bodies carrying an artifact marker or a verdict line. And the positive arm caught itself: the enrichment probe was a single sentence, which that rule correctly ignores, so the arm had been passing because attest-gate blocked the body for an unrelated reason. A probe that fires for the wrong reason reports a working rule. The `enrichment` class is ported forward from the older copy, with the constant it depends on — it existed in the deployed version and in neither newer one.
The six arms prove the script blocks what it should. They say nothing about whether anything calls it, and those are different questions — a hook that is unwired, or wired to a path that no longer exists, is indistinguishable from a hook with nothing to block. One session ran start to finish with every PreToolUse hook inert: 306 certification markers written, none enforcing anything, and the ritual read as compliance. The check enumerates config roots rather than trusting `$HOME`. Its first version did trust it, found one settings file, reported it as "the" wiring and never looked at the second — because `$HOME` here points at a per-account directory rather than the login home. That is the same defect one level up, caught only because two configs were known to exist and one was missing from the output. It stops short of claiming liveness, and says so: a settings file naming an existing file is not proof the running session loaded it. Only a command the gate must block, issued in a session and observed to be blocked, shows that.
MajorLift
changed the base branch from
main
to
jongsun/add/pr-validate-falsifying-test
August 5, 2026 12:22
This is the bypass. The hook reads the command as text, so `--body-file $DIR/comment.md` resolves to nothing — and the code called that "can't read it -> nothing to scan" and allowed the write. Every publish in one long session used a shell variable for the path, so every one of them went ungated, including a comment whose entire results section was hand-typed to look like terminal output. That same body, passed by literal path, is blocked. An unreadable body is not an absent risk. By that point the command is already identified as an outward-facing write; not knowing what it carries is the reason to stop. Also rejects a body argument carrying `$` or a backtick — `--body "$(cat f)"` extracts the literal characters, scans clean, and publishes whatever the shell substitutes afterwards. The check reads the command ARGUMENT, not the body. A first version scanned body text for shell metacharacters and blocked every evidence comment ever written, because markdown inline code is backticks and these artifacts are full of them. Caught by the negative arm, which is the half of a control matrix that earns its place on days like this. Three arms added, verified end to end: the command that slipped through minutes earlier is now stopped by the deployed hook.
`falsifiers-first` seals the description until its hypotheses are fixed. A run that takes its falsifiers from the author's sentences probes the mechanisms the author already phrased clearly, mirrors their method so agreement carries no information, and has no bucket for effects nobody claimed — which is where the findings worth having usually sit. `unintended-breakage` reads the description first, because out-of-scope is a relation between code and a stated intent rather than a property of code. It fixes the envelope, enumerates effects against it, and requires a mechanical check per candidate: an export removed still has importers, a locale key still has lookups, a renamed event still has a dashboard. Its most valuable tier is the one nothing fails on — renamed events, flipped defaults, changed state shapes. Each names the other and says why the order differs, so neither gets harmonised into the other later.
Asks whether a mechanism announces its own failure, which is a separate property from whether it can fail and is almost never tested. Since a silent failure cannot be observed, the method induces it and watches for a signal that never arrives — and the verdict reads opposite to a normal mutation probe, where a green suite means a vacuous test. Here green means the failure is undetectable, which is the result. Findings rank by distance to discovery rather than by severity: a small error nobody can see outranks a large one that pages someone in a minute. Half the listed shapes are instrument failures — a name that does not match what is counted, a pattern that cannot match what it searches for — because a measurement that fails quietly gets published.
Each encodes a failure that actually shipped, rather than a principle that sounded right. `unmeasured-join` targets the costliest one — a conclusion assembled from true facts through a step nobody measured, which survives review because the facts check out and the join reads as prose. `instrument-check` requires a positive and a negative control before a measurement counts, after a mutation runner reported a falsification for a line it never wrote. `scope-of-search` makes a negative carry the pattern that produced it, after a grep keyed on a variable name declared absent what its own output printed two blocks later. `coverage-partition` replaces "the test has power" with which cases guard which mechanism. `selection-audit` separates a count from the rule that produced it, after two different selections landed on the same number and the match closed the question. `distinguishing-observation` and `observability-gap` point the same discipline at debugging: design the observation that separates candidates rather than confirms the favourite, and establish what signal exists on a path before reading more of it. The `evidence` links are forward references — that skill ships in #84 and is not on main yet.
Its substance is the same as everything else here: what counts as proof, and how a proof can look right while testing the wrong thing. Its falsifier — a base commit that fails for the wrong reason produces an identical exit code and proves nothing — is the same move `unmeasured-join` and `scope-of-search` make on different material. It sat in #84 because it was written alongside the orchestrator, before the instrument and the reasoning that points it were separate ideas. Every other engine already ships in its own pull request. Renamed from `falsifying-test` before the move, because it and `falsify-probe.sh` shared a root while answering different questions — base-against-branch asks whether a test is connected to the reported bug, one-commit-with-a-mutated-line asks whether a suite notices the mechanism going away.
The description advertised `/red-on-base` while the installer emits `mms-red-on-base`, so a reader who typed what the description told them would find nothing. Caught only with #99 merged alongside this branch — the check lives there, the skill lives here, and neither branch can fail on its own.
MajorLift
force-pushed
the
jongsun/add/audit-skills
branch
from
August 5, 2026 12:29
42270a0 to
5899fd5
Compare
Eleven of them exist across three domains and nothing composes them. Each is individually invocable, so each depends on being recalled at the moment it applies — and that moment is exactly when attention is on the subject rather than on the move being made. Routes on the shape of the move, not the subject: about to report something absent, about to trust a count, about to believe an instrument, about to read a diff through its own description. Eleven rows, every target verified to exist on this branch. Distinct from the two orchestrators already here. `evidence` proves a claim someone has made and costs a build. `lane-graphs` routes a claim to a graph that measures it. This asks which question should be asked, applies before there is a claim, and is cheap enough to run first — which often removes the need for the expensive one. Carries the two properties that keep a router honest: no-match is a result rather than a fallback to the nearest-looking check, and the unrouted read is mandatory, because the table only sees moves someone already anticipated.
MajorLift
force-pushed
the
jongsun/add/pr-validate-falsifying-test
branch
from
August 6, 2026 10:28
d3d153d to
edf65a3
Compare
MajorLift
added a commit
that referenced
this pull request
Aug 6, 2026
Every other engine `evidence` names ships in its own pull request — `memory-leak`, `race-condition-repro`, `supply-chain-audit`, `react-render-delta`, `agent-run-cost`. This one was the sole exception, and the reason was authoring order rather than design: it was written alongside the orchestrator before the split between the instrument and the reasoning that points it existed as a concept. It lands in #106 with the other reasoning skills, whose substance is the same kind — what counts as proof, and how a proof can look right while testing the wrong thing. What stays here is the machinery: the runners, the run workflow, the gate, the hooks. The B3 engine cell now names a skill that arrives in #106, which is a dangling name in a table rather than a broken link, and resolves whichever order the two merge.
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
1 participant
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.
Adds eleven skills. Each encodes a failure mode observed in real validation runs, stated concretely enough to judge the guidance against the mechanism rather than the assertion.
pr-workflow— eight, andtesting— onefalsifiers-firstunintended-breakagesilent-failureunmeasured-joininstrument-checkawk -vescape-processes its assignment, so a mutation runner wrote/^[sS]{1,4096}$/uwhere/^[\s\S]{1,4096}$/uwas requested, narrowing a regex meant to be widened. A different test failed than the one targeted, the suite ran in full, every guard passed, and the run reported a falsification for a line it never wrote. The artifact echoed the requested mutation, so the two could not disagree.scope-of-searchsearchParamsreported(no occurrence)directly above its own output showing fiveparams.get('utm_*')reads. A negative is a fact about the pattern, not about the tree.coverage-partitionselection-audit|
red-on-base| A test that passes on the branch proves the branch is green. Only one that fails on base and passes on branch proves the change is connected to the reported bug — and a base failure for the wrong reason (missing import, absent fixture, unrelated red) produces an identical exit code and proves nothing. Moved from #84; renamed fromfalsifying-test. |coding— twodistinguishing-observationobservability-gapBoth sit beside
flaky-test-detectionand pair with thedebugorchestrator in #98.The decision that needs a call
falsifiers-firstandunintended-breakageread the PR description in opposite orders, which is the substance rather than a style choice:falsifiers-firstunintended-breakageSealing prevents the claims from steering which mechanisms get probed. Reading first is required because out of scope is not a property of code — it is a relation between code and a stated intent, and you cannot detect a departure from an envelope you have not read. Each skill names the other and states why the order differs, so neither gets harmonised into the other later.
Three properties worth knowing before reading the diff
falsifiers-firstsorts results into supported / contradicted / unclaimed. The third is the product, and a description-led run cannot produce it. It catches undersold changes as well as oversold ones.silent-failure, a green suite under an induced failure means the failure is undetectable — the opposite reading from a mutation probe, where green means the test is vacuous. The artifact has to say which reading applies, because the output is identical.Reviewing this
evidence, which ships there and is not onmain. The relative links resolve on merge and are dead if this lands first. Alternative: drop the links, add them in a follow-up.falsifiers-first's phase A dominates cost on a large diff and the skill only says to scope by mechanism, it does not solve it. Sealing is imperfect — CI check names, branch names and commit messages leak the author's framing — so the skill says to record what leaked rather than claim it did not.Test plan
node .github/scripts/lint-skill-entry.mjs— 0 errors across 58 skills## Relatedlink resolves on this branch, except the six forward references toevidenceThis branch alone does not work
The installer resolves one source directory. It does not follow
## Relatedas a dependency edge, does not fetch other refs, and reports a dangling reference as a warning rather than an error — because forward references across open pull requests are expected. So a checkout of this branch installs only the skills on it, with every cross-branch reference dead.To get a tree that actually runs:
Merges clean onto
main. Yields all elevenpr-workflowskills plusevidence, and lints at 0 errors. Three## Relatednames still dangle —react-render-delta,race-condition-repro,lavamoat-policy— because those engines ship in #43, #97 and #83.Worth knowing that the combination is also where defects surface: an error caught only with #99 and this set together — a description advertising a command the installer does not emit — passes on every branch individually.