feat(pr-workflow): add pr-validate skill — evidence-based PR validation - #56
Closed
MajorLift wants to merge 23 commits into
Closed
feat(pr-workflow): add pr-validate skill — evidence-based PR validation#56MajorLift wants to merge 23 commits into
MajorLift wants to merge 23 commits into
Conversation
… sibling pointers)
…rrent-run and publishing rules
MajorLift
force-pushed
the
jongsun/add/260626-pr-validate-skill
branch
from
July 22, 2026 16:44
6c0e22c to
c66e45b
Compare
…nership guidance `evidence-catalog.md`: a paired A/B is only trustworthy once the mechanism under test is confirmed active in each arm (a null arm reports noise as signal); and for a perf-targeting PR the lead evidence is the measured impact CI already publishes (metamaskbot's base-vs-merge-base matrix and the Sentry trend), read from the primary table rather than the moving-baseline section. `worked-examples.md`: publish into the PR body when you authored the PR, as a comment when validating someone else's.
…liance to the catalog The LavaMoat lane's bar was "no new capability" — wrong for a dependency bump, whose policy legitimately changes. Reframed to a per-grant audit: every new global/builtin/package grant justified by the dep's function, read across all build variants, with the identifier-substitution check. Also: rely on CI for routine coverage (lint/build/typecheck/suite/changelog) and cite the check result rather than re-collecting it — spend independent evidence only on the load-bearing falsifier or noteworthy areas.
…talog Don't restate CI results in the published comment — the reviewer already sees the Checks tab; cite one only to highlight something specific. And publish falsifier-forward: foreground what would falsify the claim and how each falsifier is closed, not a lane inventory.
For a retry/cancellation/supersession/debounce claim the correctness is the ordering under races: force each race with fake timers + concurrent launch + microtask stepping and assert the outcome, including asymmetric guarantees. Trust-gate: the test must actually interleave — a sequential run is a vacuous green.
The lead lane for a leak claim is static: name the holder, the specific held set, and the outlived lifecycle boundary, pairing every acquire with its release site — the absent pair is the finding. Works at review time, before any runtime exists; heap-over-a-flow with the retainer graph corroborates.
… setup doc `hooks/pr-evidence-gate.py` mechanically enforces the evidence-trustworthiness disciplines at emit time: it blocks an outward `gh pr|issue edit|create|comment` whose body carries an unbacked verdict, a bare identifier, an untracked deferral, or a validation section missing the canonical header, exiting 2 with the reason so the model self-corrects and re-posts. Optional and Claude-Code-only (stdlib, fails open); other operators get the same disciplines as documentation. `references/evidence-gate-setup.md` covers the settings.json PreToolUse wiring, the `gh pr comment` permission requirement, and the user-owned evidence-host repo for image re-hosting. Note: `tools/install` copies only `references scripts assets adapters`, so the hook does not land via the installer yet — the setup doc points settings.json at the skills-repo checkout path as the workaround.
ask (recommended; three independent layers with the hook and draft-confirm), allow+hook, allowlisted wrapper (raw verb stays denied), and no-grant manual post. Warns against a bare deny, which hard-blocks the publish step with no prompt mid-run.
For a flow-spanning claim, derive falsifiers by enumeration, not intuition: map the flow (each node/edge anchored to file:line), probe every node with standard axes (failure-at-step, ordering permutations, lifecycle boundaries, population boundaries, state-freshness splits, entry-condition variants), and close each row with a lane or an honest open item. The map + matrix is itself publishable evidence; an unanchored diagram is narrative.
…I diff the PR skips
The transform is CI-covered by each `NNN.test.js`; independent evidence goes to what those per-path fixtures never do — run the whole `migrate()` twice on combined state and assert the second application is a no-op. Exemplar: #42297. Claude-Session: https://claude.ai/code/session_01MfxHHLzB3qETfMgyvAEbgY
The bar is not that the reader can re-run it (a working link is the floor) but that they needn't — the exhibit is complete enough that reading it makes the result near-certain. The re-issuable link is a backstop, offered second. Claude-Session: https://claude.ai/code/session_01MfxHHLzB3qETfMgyvAEbgY
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.
What
Adds
pr-validatetopr-workflow. For a PR's specific falsifiable claim, it names the observation that would prove the claim false, gathers it, and publishes it into the PR body.Why it's different from capture and readiness skills
A reviewer takes the author's tested locally, works on trust. Capture skills (
visual-testing,performance-testing,pr-manual-testing) replay a flow and photograph the screen, but a clean screenshot is also what a silently-swallowed failure produces.pr-readiness-checkproves tests are present, not that behaviour is correct.pr-validateis the missing layer:pr-evidence-gate.pyhook blocks a bundle that is empty, truncated, or link-only.Impact: the reviewer reads evidence built to fail instead of re-running the author's testing in their head. The same check runs in the dev loop, so a developer finds their own claim false before anyone else does.
Fit
Complements
pr-readiness-check(presence) by proving behaviour. References, does not duplicate,visual-testing/performance-testing/pr-manual-testing/pr-description. Describes the AEP (visual_validation/perf_validation) as an autonomous evidence engine at the capability level.Files
domains/pr-workflow/skills/pr-validate/:skill.md, sixreferences/files, ahooks/pr-evidence-gate.pycompleteness gate, andrepos/metamask-extension.md. Draft: feedback wanted on domain placement (pr-workflow vs testing) and AEP scope.