From 0c2d08d5d98a63f1638b37844593655ed1420401 Mon Sep 17 00:00:00 2001 From: Ossie Irondi Date: Sat, 7 Nov 2026 22:47:06 -0600 Subject: [PATCH 1/2] chore(pr-kit): install PR kit v84 Updates the kit from v67 to v84. Adds the merge-danger scorer workflow and cancels a PR's superseded body checks. Co-Authored-By: Claude Opus 5.5 --- .claude/skills/land-pr/.pr-kit | 2 +- .claude/skills/land-pr/SKILL.md | 9 +- .claude/skills/land-pr/danger.ts | 221 ++++++++++++ .claude/skills/land-pr/merge.ts | 10 +- .claude/skills/open-pr/.pr-kit | 2 +- .claude/skills/open-pr/SKILL.md | 1 + .claude/skills/open-pr/VERSION | 2 +- .claude/skills/open-pr/command.ts | 103 +++++- .claude/skills/open-pr/contract.ts | 21 +- .claude/skills/open-pr/gate.ts | 25 +- .claude/skills/open-pr/kit.ts | 32 +- .claude/skills/open-pr/merge-danger.yml | 46 +++ .claude/skills/open-pr/pr-kit.yml | 6 + .claude/skills/open-pr/score.ts | 438 ++++++++++++++++++++++++ .github/workflows/merge-danger.yml | 46 +++ .github/workflows/pr-kit.yml | 6 + 16 files changed, 939 insertions(+), 31 deletions(-) create mode 100644 .claude/skills/land-pr/danger.ts create mode 100644 .claude/skills/open-pr/merge-danger.yml create mode 100644 .claude/skills/open-pr/score.ts create mode 100644 .github/workflows/merge-danger.yml diff --git a/.claude/skills/land-pr/.pr-kit b/.claude/skills/land-pr/.pr-kit index 0d9f3d3c..17eec338 100644 --- a/.claude/skills/land-pr/.pr-kit +++ b/.claude/skills/land-pr/.pr-kit @@ -1,2 +1,2 @@ Installed by the PR kit (open-pr/kit.ts). Edit the kit at its source, not here. -version 67 sha256:d0:c2:68:8a:e0:a8:c2:fc:32:62:f5:ed:8c:5e:c5:74:4a:6e:c0:d7:a8:ca:42:6e:5b:40:6d:5f:26:50:07:f8 +version 84 sha256:80:38:ce:a0:56:8d:6d:83:38:06:31:f5:2f:5f:24:80:e8:e8:26:24:b4:2c:95:1c:dc:6b:3b:31:9c:99:99:67 diff --git a/.claude/skills/land-pr/SKILL.md b/.claude/skills/land-pr/SKILL.md index 46f297ac..7e4181dc 100644 --- a/.claude/skills/land-pr/SKILL.md +++ b/.claude/skills/land-pr/SKILL.md @@ -21,8 +21,9 @@ The PR gate refuses a merge unless the PR body passes the contract and the revie | Conflict fix | A conflict-resolution commit changes the head. Wait for a successful `pullfrog-approval` check-run on that exact head before merging. An exempt repo has no check-run to wait for. It needs green CI and a local review of the new head instead. | | CI | Required checks pass on the same head. | | Threads | Fix actionable findings. Resolve bot threads with evidence. Resolve or disposition every other thread. | -| Agent merge | The agent merges without asking when all of these hold on the exact head: the PR body's Merge danger shows a blast radius of `none`, `low`, or `medium`, required CI passes, the project's verify skills pass (step 6), and the pass signal passes. A repo with no review bot qualifies only when this kit's exemption list (`merge.ts`) names it, because that list is the owner's record that the repo has no bot on purpose. Never add a repo to that list to make a merge pass. The rating counts only as the local review left it. If the blast radius was lowered after that review, the agent does not merge on its own. The gate checks the body and the bot verdict, not the rating, so this rule is the agent's to keep. The owner set this rule on 2026-10-30. | -| Owner's word | Every other merge takes the owner's word: a `high` blast radius, a rating lowered after review, or a repo with no review bot and no exemption. The word is an explicit "merge", or a request whose wording implies it ("ship it", "get it merged", "merge when green"). For "merge when green", wait for required checks and exact-head approval, then use a merge pinned to that approved head. Without the word, stop at merge-ready and report the head SHA, CI, bot verdict, threads, spec fidelity, verify skills, and merge danger. A `high` PR also gets an explainer video first (see Explainer video for a high PR). | +| Agent merge | The agent merges without asking when all of these hold on the exact head: the merge danger that counts shows a blast radius of `none`, `low`, or `medium`, required CI passes, the project's verify skills pass (step 6), and the pass signal passes. A repo with no review bot qualifies only when this kit's exemption list (`merge.ts`) names it, because that list is the owner's record that the repo has no bot on purpose. Never add a repo to that list to make a merge pass. The rating counts only as the local review left it. If the blast radius was lowered after that review, the agent does not merge on its own. The gate checks the body and the bot verdict, not the rating, so this rule is the agent's to keep. The owner set this rule on 2026-10-30. | +| Merge danger that counts | The higher of two ratings: the PR body's Merge danger, and the `merge-danger` commit status that the kit's scorer posts on the exact head (open-pr step 5). `bun /danger.ts ` reads both and decides. With no status and no scorer for the PR (no workflow on the default branch, or a fork or Dependabot PR it skips), the body's rating alone counts, as before. When the scorer runs for the PR, wait for its status like a CI check. A PR opened before the scorer was installed gets its status on its next push, or on a close and reopen. When no status comes 60 minutes after the last push, as when the scorer job died before posting, stop and report it under the convergence rule. That merge takes the owner's word. The newest status decides, so a rerun can replace an `error`. The PR body contract still requires a Door and a Blast radius line, so every PR, a cloud agent's included, carries a body rating. A body that breaks the contract takes the owner's word. When the scorer rated the head, its rating stands in for the local review of the body's rating, so a cloud PR with no local review merges on its own when the higher of the two is `medium` or lower. A newest status of `error` or `failure` takes the owner's word. Any workflow can post as `github-actions[bot]`, so a rating counts only when its status links a finished run of `merge-danger.yml` on the exact head. Any other `merge-danger` status takes the owner's word. The owner set this rule on 2026-11-05. | +| Owner's word | Every other merge takes the owner's word: a `high` blast radius, a rating lowered after review, a `merge-danger` status of `error` or `failure`, or a repo with no review bot and no exemption. The word is an explicit "merge", or a request whose wording implies it ("ship it", "get it merged", "merge when green"). For "merge when green", wait for required checks and exact-head approval, then use a merge pinned to that approved head. Without the word, stop at merge-ready and report the head SHA, CI, bot verdict, threads, spec fidelity, verify skills, and merge danger. A `high` PR also gets an explainer video first (see Explainer video for a high PR). | | Always | Never pass `--admin` or `--auto`. The gate refuses both. Never merge when the repo's AGENTS.md forbids agent merges. | | Other bots | Open Code Review, CodeRabbit, Greptile, and Codex are advisory unless branch rules require them. | @@ -51,12 +52,14 @@ A stale PR costs more than one more round. Keep fixing while each round makes pr ``` 4. **Wait on the real state change.** Watch the check-run, not a timer. Count the round and apply the convergence rule. + - A scorer status must match the PR's current head, base commit, and scored title and body. An edit requests a new score. If the base branch advances without a PR event, close and reopen the PR to request a score for the new comparison. Wait for that score before merging. Old scores without an input fingerprint need a fresh score too. + - The matching fingerprint and actual radius must also come from the linked scorer run's own job log, read through GitHub. Status URL text alone proves nothing. If that log proof is missing, invalid, or unavailable, the owner decides. Request a fresh score when the old run's logs have expired. When no scorer applies and only stale statuses remain, the body-only rule applies. - When a T3 watch is the wake source and Bash supports `run_in_background`, also arm one bounded `until` loop on the head's `pullfrog-approval` check-run as a backstop. 5. **Check spec fidelity before merge.** Diff the result against the issue, the plan, or the mock. Post every deviation, or "no deviations", as a PR comment. 6. **Run the project's verify skills on the head.** Run `git fetch origin ` and check out the exact head you will merge. Run `bun /../open-pr/verify.ts --base origin/` in the repo, then read each listed `SKILL.md` in full and run it, in the listed order. The listed paths are relative to the repo root. - When one fails, stop. Do not merge. Report the skill, the failing step, and its output on the PR. A skill that cannot start counts as a failure, and so does a non-zero exit from `verify.ts`. Never read a `verify.ts` error as nothing to run. A fix is a new head, so it goes through this skill again. - When it prints `verify: none.`, note that line in the merge-ready report and go on. -7. **Merge under the merge policy.** Read the Merge danger section of the PR body as it is now, since a fix round can change it. If the policy leaves this merge to the owner and the owner has not given the word, stop here and report merge-ready. For a `high` blast radius, make the explainer video below before you report. Otherwise, first check allowed methods with `gh api repos/{owner}/{repo} --jq '{allow_squash_merge,allow_merge_commit,allow_rebase_merge}'`. Follow the repo's required method if allowed. Otherwise fetch main and run `git log origin/main --first-parent -10 --format='%h %p %s'`. Two parents show a merge commit. Squash and rebase history can look the same. Use an allowed method supported by policy or history, and prefer squash when neither names one. Stop if the evidence conflicts with the settings. Run `gh pr merge --match-head-commit ` alone in its call, with `-R /` when the current directory is not that repo's checkout or the repo is exempt. The gate checks a merge only when nothing runs before it but a literal `cd` or a `set` of `-e`, `-u`, `-x`, or `-o pipefail`, and nothing runs beside or after it, not even a pipe into a filter, since any other command can move the head. Nonexempt REST merges must also name that same head in the `sha` field. The asynchronous REST `merge-async` endpoint is refused. The merge is the only command in its call, so `git pull` or `git checkout main` runs in a later call. The gate checks the policy again. +7. **Merge under the merge policy.** Run `bun /danger.ts `. It reads the PR body's Merge danger as it is now, since a fix round can change it, and the scorer's status on the head. Exit 0 lets the agent merge, 1 takes the owner's word, and 3 means the scorer has not rated the head yet. Wait for it as step 4 says. If the policy leaves this merge to the owner and the owner has not given the word, stop here and report merge-ready. For a `high` blast radius, make the explainer video below before you report. Otherwise, first check allowed methods with `gh api repos/{owner}/{repo} --jq '{allow_squash_merge,allow_merge_commit,allow_rebase_merge}'`. Follow the repo's required method if allowed. Otherwise fetch main and run `git log origin/main --first-parent -10 --format='%h %p %s'`. Two parents show a merge commit. Squash and rebase history can look the same. Use an allowed method supported by policy or history, and prefer squash when neither names one. Stop if the evidence conflicts with the settings. Run `gh pr merge --match-head-commit ` alone in its call, with `-R /` when the current directory is not that repo's checkout or the repo is exempt. The gate checks a merge only when nothing runs before it but a literal `cd` or a `set` of `-e`, `-u`, `-x`, or `-o pipefail`, and nothing runs beside or after it, not even a pipe into a filter, since any other command can move the head. REST merges, exempt ones included, must also name that same head in the `sha` field. The asynchronous REST `merge-async` endpoint is refused. The merge is the only command in its call, so `git pull` or `git checkout main` runs in a later call. The gate checks the policy again. 8. **Prove the merge.** Only `gh api repos/{owner}/{repo}/pulls/ --jq .merged` returning `true` proves it. A tool's success message does not. ## Explainer video for a high PR diff --git a/.claude/skills/land-pr/danger.ts b/.claude/skills/land-pr/danger.ts new file mode 100644 index 00000000..12f3b449 --- /dev/null +++ b/.claude/skills/land-pr/danger.ts @@ -0,0 +1,221 @@ +#!/usr/bin/env bun +/** + * The merge danger that counts for the agent-merge rule: the higher of two ratings, the PR + * body's Merge danger and the `merge-danger` commit status that open-pr/score.ts posts on the + * exact head. A blast radius of medium or lower lets the agent merge on its own. + * + * bun danger.ts + * + * Exit 0: the agent may merge. Exit 1: the owner's word. Exit 3: wait for the scorer. + * + * - No status, and no scorer for this PR (none on the default branch, or a fork or Dependabot PR + * it skips): the body's rating alone counts, as before. + * - The scorer runs for this PR but has not rated the head, or its newest status is pending: wait. + * A score must also match the current base commit and scored title/body. An edit requests a + * new score. After a base-branch push, close and reopen the PR to score the new comparison. + * - The newest status is `error` or `failure`: the owner's word. A later success replaces an + * earlier error, so a rerun can rate a head after an outage. + * - Any workflow's token posts as github-actions[bot], so a rating counts only when its status + * links a finished, successful pull_request_target attempt of merge-danger.yml on this exact head. + * A PR cannot start such a run with its own code. A merge-danger status from any other account, + * or one that links anything else, refuses. + * That attempt's own job log must record the current input fingerprint and the status's radius. + * A caller-supplied URL hash alone is not evidence. Missing log proof takes the owner's word. + * The gate never reads this rating, so the rule stays the agent's to keep, as land-pr says. + */ + +import { checkPullRequest, type PullRequest } from "../open-pr/contract"; +import { inputFingerprint, parseEvent, PROOF_PREFIX, RADII, RATING, STATUS_CONTEXT, type Radius, type ScoreEvidence } from "../open-pr/score"; + +export type RunProof = ({ kind: "recorded" } & ScoreEvidence) | { kind: "unavailable"; why: string }; + +/** The commit status fields this reads, as the REST API returns them, newest first. */ +export interface Status { + state: string; + description: string | null; + context: string; + creator: { login: string } | null; + targetUrl: string | null; +} + +/** The workflow run fields this reads, as the REST API returns them. */ +export interface ScorerRun { + path: string; + event: string; + headSha: string; + status: string; + conclusion: string | null; + proof: RunProof; +} + +/** What readStatus needs besides the statuses: whether the scorer runs for this PR, and its linked attempts, by ID/attempts/number. */ +export interface Scorer { + installed: boolean; + repo: string; + head: string; + inputs: string; + runs: ReadonlyMap; +} + +export type StatusReading = + | { kind: "absent" } + | { kind: "waiting"; why: string } + | { kind: "refused"; why: string } + | { kind: "rated"; radius: Radius }; + +export type Danger = { verdict: "agent" | "owner"; radius: Radius; why: string } | { verdict: "owner" | "wait"; why: string }; + +const SCORER = "github-actions[bot]"; +const SCORER_PATH = ".github/workflows/merge-danger.yml"; + +/** The run and attempt a status links, when it links a scorer attempt in `repo`. */ +export function runAttemptOf(repo: string, url: string | null): string | undefined { + const prefix = `https://github.com/${repo}/actions/runs/`.toLowerCase(); + return url?.toLowerCase().startsWith(prefix) ? url.slice(prefix.length).match(/^([1-9]\d*\/attempts\/[1-9]\d*)#inputs=[a-f0-9]{64}$/)?.[1] : undefined; +} + +/** False when the merge-danger workflow is not on the default branch, or it skips this PR (forks and Dependabot, as its `if:` says). */ +export const scorerWillRun = (workflowOnDefault: boolean, pr: { headRepo: string | null; baseRepo: string; author: string | null }): boolean => + workflowOnDefault && pr.headRepo === pr.baseRepo && pr.author !== "dependabot[bot]"; +const rank = (radius: Radius) => RADII.indexOf(radius); +const isRadius = (word: string | undefined): word is Radius => RADII.some((r) => r === word); + +/** + * The scorer's statuses on the head (newest first), as one reading. Only current inputs count. + * The newest matching status decides the state, with the highest matching successful rating. + */ +export function readStatus(statuses: readonly Status[], scorer: Scorer): StatusReading { + const ours = statuses.filter((s) => s.context === STATUS_CONTEXT); + const stranger = ours.find((s) => s.creator?.login !== SCORER); + if (stranger) return { kind: "refused", why: `a ${STATUS_CONTEXT} status comes from ${stranger.creator?.login ?? "a deleted account"}, not ${SCORER}` }; + const unlinked = ours.find((s) => s.state === "success" && runAttemptOf(scorer.repo, s.targetUrl) === undefined); + if (unlinked) return { kind: "refused", why: `a ${STATUS_CONTEXT} status links ${JSON.stringify(unlinked.targetUrl)}, not a run of ${SCORER_PATH} in ${scorer.repo}` }; + if (ours.length === 0) return scorer.installed ? { kind: "waiting", why: "the scorer has not rated this head yet" } : { kind: "absent" }; + const current = ours.filter((s) => s.targetUrl?.endsWith(`#inputs=${scorer.inputs}`)); + const [newest] = current; + if (newest === undefined) return scorer.installed ? { kind: "waiting", why: "the scorer has not rated the current base, title, and body. Wait for the run a push or edit started. If none is running, close and reopen the PR to request a fresh score" } : { kind: "absent" }; + if (newest.state === "error" || newest.state === "failure") return { kind: "refused", why: `the ${STATUS_CONTEXT} status is ${newest.state}: ${newest.description ?? "no description"}` }; + if (newest.state !== "success") return { kind: "waiting", why: `the ${STATUS_CONTEXT} status is ${newest.state}` }; + let radius: Radius | undefined; + for (const s of current.filter((s) => s.state === "success")) { + const id = runAttemptOf(scorer.repo, s.targetUrl); + const run = id === undefined ? undefined : scorer.runs.get(id); + if (run === undefined) return { kind: "refused", why: `a ${STATUS_CONTEXT} status links ${JSON.stringify(s.targetUrl)}, not a run of ${SCORER_PATH} in ${scorer.repo}` }; + if (run.path !== SCORER_PATH || run.event !== "pull_request_target" || run.headSha !== scorer.head) { + return { kind: "refused", why: `a ${STATUS_CONTEXT} status links run ${id}, which is ${run.path} on ${run.event} for ${run.headSha.slice(0, 7)}, not the scorer on this head` }; + } + if (run.status !== "completed") return { kind: "waiting", why: `scorer run ${id} is ${run.status}` }; + if (run.conclusion !== "success") return { kind: "refused", why: `scorer run ${id} concluded ${run.conclusion}, yet a success status links it` }; + const word = s.description?.match(RATING)?.[1]; + if (!isRadius(word)) return { kind: "refused", why: `the ${STATUS_CONTEXT} status says ${JSON.stringify(s.description)}, which names no blast radius` }; + if (run.proof.kind === "unavailable") return { kind: "refused", why: `scorer run ${id} has no verified output: ${run.proof.why}` }; + if (run.proof.inputs !== scorer.inputs || run.proof.radius !== word) return { kind: "refused", why: `the ${STATUS_CONTEXT} status does not match the inputs and rating recorded by scorer run ${id}` }; + if (radius === undefined || rank(word) > rank(radius)) radius = word; + } + return radius === undefined ? { kind: "waiting", why: "the scorer has not rated this head yet" } : { kind: "rated", radius }; +} + +/** The higher of the body's blast radius word and the scorer's reading, and who may merge. */ +export function dangerThatCounts(bodyWord: string | undefined, status: StatusReading): Danger { + if (status.kind === "waiting") return { verdict: "wait", why: `${status.why}. Wait for its status like a CI check.` }; + if (status.kind === "refused") return { verdict: "owner", why: `${status.why}. The owner decides.` }; + const word = bodyWord?.toLowerCase(); + if (word !== undefined && !isRadius(word)) return { verdict: "owner", why: `the body's blast radius "${bodyWord}" is not one of ${RADII.join(", ")}. The owner decides.` }; + const ratings = [word, status.kind === "rated" ? status.radius : undefined].filter((r) => r !== undefined); + if (ratings.length === 0) return { verdict: "owner", why: "the PR has no blast radius, from its body or the scorer. The owner decides." }; + const radius = ratings.reduce((a, b) => (rank(b) > rank(a) ? b : a)); + const source = status.kind === "rated" ? `the higher of body ${word ?? "none given"} and scorer ${status.radius}` : "the body, since no scorer runs here"; + const verdict = rank(radius) <= rank("medium") ? "agent" : "owner"; + return { verdict, radius, why: `blast radius ${radius}, from ${source}. ${verdict === "agent" ? "The agent may merge on its own." : "The owner decides."}` }; +} + +/** + * dangerThatCounts with the body's rating read through the PR contract. A body that breaks the + * contract takes the owner's word, since the gate refuses its merge and its rating cannot be read. + */ +export function dangerOf(pr: PullRequest, status: StatusReading): Danger { + const body = checkPullRequest(pr); + if (!body.ok) return { verdict: "owner", why: `the body breaks the PR contract (${body.violations.map((v) => v.rule).join(", ")}), so its rating cannot be read. Fix the body first.` }; + return dangerThatCounts(body.blastRadius, status); +} + +function gh(args: string[]): { ok: true; out: string } | { ok: false; why: string } { + const r = Bun.spawnSync(["gh", "api", ...args], { stdout: "pipe", stderr: "pipe" }); + return r.exitCode === 0 ? { ok: true, out: r.stdout.toString() } : { ok: false, why: r.stderr.toString().trim() }; +} + +function ghJson(args: string[]): unknown { + const r = gh(args); + if (!r.ok) throw new Error(`gh api ${args.join(" ")} failed: ${r.why}`); + return JSON.parse(r.out); +} + +const isObj = (x: unknown): x is Record => typeof x === "object" && x !== null && !Array.isArray(x); +const str = (x: unknown) => (typeof x === "string" ? x : null); + +function parseStatus(x: unknown): Status { + if (!isObj(x) || typeof x.state !== "string" || typeof x.context !== "string") throw new Error(`GitHub returned a status without state or context: ${JSON.stringify(x)}`); + const login = isObj(x.creator) ? str(x.creator.login) : null; + return { state: x.state, context: x.context, description: str(x.description), creator: login === null ? null : { login }, targetUrl: str(x.target_url) }; +} + +function parseRun(x: unknown): Omit { + const [path, event, headSha, status] = isObj(x) ? [str(x.path), str(x.event), str(x.head_sha), str(x.status)] : []; + if (!isObj(x) || !path || !event || !headSha || !status) throw new Error(`GitHub returned a workflow run without path, event, head_sha, or status: ${JSON.stringify(x)}`); + return { path, event, headSha, status, conclusion: str(x.conclusion) }; +} + +/** Accept exactly one well-formed record from a job log, with GitHub's timestamp prefix. */ +export function readProof(log: string): RunProof { + const records = log.split(/\r?\n/).map((line) => line.replace(/^\d{4}-\d{2}-\d{2}T[\d:.]+Z /, "")).filter((line) => line.startsWith(PROOF_PREFIX)); + if (records.length !== 1) return { kind: "unavailable", why: `expected one proof record, found ${records.length}` }; + let value: unknown; + try { + value = JSON.parse(records[0].slice(PROOF_PREFIX.length)); + } catch { + return { kind: "unavailable", why: "the proof record is not JSON" }; + } + const radius = isObj(value) ? str(value.radius) : null; + if (!isObj(value) || typeof value.inputs !== "string" || !/^[a-f0-9]{64}$/.test(value.inputs) || radius === null || !isRadius(radius)) return { kind: "unavailable", why: "the proof record has invalid inputs or radius" }; + return { kind: "recorded", inputs: value.inputs, radius }; +} + +/** Job logs are scoped to the linked Actions attempt, never the latest rerun. */ +function readRunProof(repo: string, id: string): RunProof { + const jobs = gh([`repos/${repo}/actions/runs/${id}/jobs?per_page=100`, "--jq", '.jobs[] | select(.name == "Merge danger") | .id']); + if (!jobs.ok) return { kind: "unavailable", why: jobs.why }; + const job = jobs.out.trim(); + if (!/^\d+$/.test(job)) return { kind: "unavailable", why: `run ${id} has no single Merge danger job` }; + // Capture raw log bytes for the parser, never render terminal control sequences. + const log = gh([`repos/${repo}/actions/jobs/${job}/logs`, "--allow-escape-sequences"]); + return log.ok ? readProof(log.out) : { kind: "unavailable", why: log.why }; +} + +if (import.meta.main) { + const [repo, number] = process.argv.slice(2); + if (!repo?.includes("/") || !/^\d+$/.test(number ?? "")) { + console.error("usage: bun danger.ts "); + process.exit(2); + } + const pr = ghJson([`repos/${repo}/pulls/${number}`]); + const head = isObj(pr) && isObj(pr.head) ? str(pr.head.sha) : null; + const headRepo = isObj(pr) && isObj(pr.head) && isObj(pr.head.repo) ? str(pr.head.repo.full_name) : null; + const baseRepo = isObj(pr) && isObj(pr.base) && isObj(pr.base.repo) ? str(pr.base.repo.full_name) : null; + const base = isObj(pr) && isObj(pr.base) && isObj(pr.base.repo) ? str(pr.base.repo.default_branch) : null; + const author = isObj(pr) && isObj(pr.user) ? str(pr.user.login) : null; + if (!isObj(pr) || head === null || base === null || baseRepo === null) throw new Error(`GitHub returned no head SHA, base repo, or default branch for ${repo}#${number}`); + const pages = ghJson(["--paginate", "--slurp", `repos/${repo}/commits/${head}/statuses?per_page=100`]); + const statuses = (Array.isArray(pages) ? pages.flat() : []).map(parseStatus); + const workflow = gh([`repos/${repo}/contents/${SCORER_PATH}?ref=${encodeURIComponent(base)}`, "--jq", ".path"]); + if (!workflow.ok && !workflow.why.includes("HTTP 404")) throw new Error(`could not tell whether ${repo} runs the scorer: ${workflow.why}`); + const ids = new Set(statuses.filter((s) => s.context === STATUS_CONTEXT && s.state === "success").map((s) => runAttemptOf(repo, s.targetUrl)).filter((id) => id !== undefined)); + const runs = new Map([...ids].map((id) => { + const run = parseRun(ghJson([`repos/${repo}/actions/runs/${id}`])); + const proof: RunProof = run.status === "completed" && run.conclusion === "success" ? readRunProof(repo, id) : { kind: "unavailable", why: "the run has not completed successfully" }; + return [id, { ...run, proof }] as const; + })); + const scorer: Scorer = { installed: scorerWillRun(workflow.ok, { headRepo, baseRepo, author }), repo, head, inputs: inputFingerprint(parseEvent({ pull_request: pr })), runs }; + const danger = dangerOf({ title: str(pr.title) ?? "", body: str(pr.body) ?? "" }, readStatus(statuses, scorer)); + console.log(`merge danger on ${head.slice(0, 7)}: ${danger.verdict}. ${danger.why}`); + process.exit(danger.verdict === "agent" ? 0 : danger.verdict === "wait" ? 3 : 1); +} diff --git a/.claude/skills/land-pr/merge.ts b/.claude/skills/land-pr/merge.ts index e91e4dd9..92479f02 100644 --- a/.claude/skills/land-pr/merge.ts +++ b/.claude/skills/land-pr/merge.ts @@ -11,7 +11,10 @@ * * The kit's source repo is the owner's curated settings repo, and no review bot gates it. * The owner also exempted two repos with no review bot installed, which take local reviews only - * (2026-10-27), and six product meta (planning) repos with no review bot installed (2026-10-29). The list lives here in the kit, never in a file the target repo could edit. It holds SHA-256 + * (2026-10-27), six product meta (planning) repos with no review bot installed (2026-10-29), motion-studio + * (2026-10-30), for now, acp-church-media while Pullfrog is paused for its org (2026-11-03), and desktop-commander, which has no + * Pullfrog (2026-11-05). An exempt merge + * still pins its head, since the local review is the only check of that head. The list lives here in the kit, never in a file the target repo could edit. It holds SHA-256 * digests of lowercase `owner/name`, so the kit can ship to public repos without naming a * private one. The exemption holds only when the merge command names the repo itself, * because a repo guessed from the cwd or the environment can differ from the one gh merges. @@ -50,6 +53,11 @@ const BOT_EXEMPT = new Set([ "sha256:8a:60:35:cf:9f:fe:e9:f6:e6:73:7e:2b:f7:84:b6:13:af:92:3f:a7:e4:f4:28:ba:d5:3e:d5:de:f6:7a:9b:07", "sha256:51:1f:ed:46:6f:52:26:97:e0:71:43:ad:a6:19:ec:44:c7:32:52:01:90:5d:9c:ac:b5:5f:b5:76:35:fa:7c:a8", "sha256:af:68:ff:91:e8:1e:9f:10:fe:b5:34:02:ca:5f:43:53:b5:be:14:60:2f:2d:36:0c:a7:c6:a6:4b:8d:f4:67:e0", + // Temporary. The owner exempted ACP-Media/acp-church-media on 2026-11-03 while Pullfrog is paused for its org. + // Remove this entry once the Pullfrog replacement publishes the pass signal there (ACP-Media/acp-church-media#420). + "sha256:fa:b3:57:7b:6d:2f:98:ad:06:df:f2:08:3f:db:2a:f3:69:cd:c9:86:df:92:b8:c1:1a:89:46:e7:61:5b:19:53", + // AojdevStudio/desktop-commander has no Pullfrog. The owner exempted it on 2026-11-05. + "sha256:4a:c0:47:14:8a:92:08:ac:39:97:92:99:cf:6c:d0:c8:7c:99:98:3c:9f:c9:9f:c9:ff:5e:c5:b9:af:e1:3b:a2", ]); const CODERABBIT_GATED = new Set([ diff --git a/.claude/skills/open-pr/.pr-kit b/.claude/skills/open-pr/.pr-kit index 0d9f3d3c..17eec338 100644 --- a/.claude/skills/open-pr/.pr-kit +++ b/.claude/skills/open-pr/.pr-kit @@ -1,2 +1,2 @@ Installed by the PR kit (open-pr/kit.ts). Edit the kit at its source, not here. -version 67 sha256:d0:c2:68:8a:e0:a8:c2:fc:32:62:f5:ed:8c:5e:c5:74:4a:6e:c0:d7:a8:ca:42:6e:5b:40:6d:5f:26:50:07:f8 +version 84 sha256:80:38:ce:a0:56:8d:6d:83:38:06:31:f5:2f:5f:24:80:e8:e8:26:24:b4:2c:95:1c:dc:6b:3b:31:9c:99:99:67 diff --git a/.claude/skills/open-pr/SKILL.md b/.claude/skills/open-pr/SKILL.md index 467e5321..b7814245 100644 --- a/.claude/skills/open-pr/SKILL.md +++ b/.claude/skills/open-pr/SKILL.md @@ -39,6 +39,7 @@ The PR body is a briefing for a reviewer who has one minute. A gate checks the b 5. **Measure the merge danger.** - **Door.** Two-way when a revert undoes it. One-way when it deletes something you didn't make (dead entries that point at nothing stay two-way), deletes data or work that cannot be put back, migrates a schema, publishes, sends, deploys to users, spends money, changes prod user data, or changes a contract that others consume. The door rates the change, not the merge. - **Blast radius.** Find what the change can break outside the diff: callers, consumers, config, jobs, other repos. Prove the one fact that makes it safe by running code, not by reading it. Name the radius in one word: `none`, `low`, `medium`, `high`. Rate it honestly, and include the rating in the local review. A radius of `medium` or lower lets the agent merge without asking, and `high` waits for the owner's word and gets an explainer video before he decides (see `land-pr`). + - **The scorer.** In a repo with the kit's `merge-danger.yml` workflow, a check on the homelab Linux runners also rates every PR, whoever wrote it. It asks Jev (TypeSafe AI) four questions about what the diff does (door, reach, likelihood, and whether it weakens a control), from the diff, the title, and the body without its Merge danger section. Code combines the answers into a blast radius and posts it as the `merge-danger` commit status on the head. land-pr counts the higher of your rating and the scorer's, so rate honestly and never lower yours to match it. 6. **Write the body** to a file with the template below. Write in ASD-STE100: - Start with the problem. No preamble, no "This PR". - One topic per sentence, at most 25 words. Active voice, present tense. diff --git a/.claude/skills/open-pr/VERSION b/.claude/skills/open-pr/VERSION index bd5ec792..12dd42c2 100644 --- a/.claude/skills/open-pr/VERSION +++ b/.claude/skills/open-pr/VERSION @@ -1 +1 @@ -67 sha256:d0:c2:68:8a:e0:a8:c2:fc:32:62:f5:ed:8c:5e:c5:74:4a:6e:c0:d7:a8:ca:42:6e:5b:40:6d:5f:26:50:07:f8 +84 sha256:80:38:ce:a0:56:8d:6d:83:38:06:31:f5:2f:5f:24:80:e8:e8:26:24:b4:2c:95:1c:dc:6b:3b:31:9c:99:99:67 diff --git a/.claude/skills/open-pr/command.ts b/.claude/skills/open-pr/command.ts index ee4221d8..58898c79 100644 --- a/.claude/skills/open-pr/command.ts +++ b/.claude/skills/open-pr/command.ts @@ -32,7 +32,7 @@ * before gh or a cd the gate cannot follow can change the repo gh picks, or the command can change gh's * host or config. * - * Limit: this is a static read of the command text. `eval "gh ..."`, `gh api "$P"`, aliases, shell + * Limit: this is a static read of the command text. `eval "gh ..."`, `trap "gh ..." EXIT`, `gh api "$P"`, aliases, shell * functions, globs, and profile settings are not followed. A script a shell runs as an operand * (`bash script.sh`), a program a shell reads through a pipe from any other command (`curl ... | bash`), or * one the outer shell splices an expansion into as an argument, is not read, and the gate allows it, as it @@ -54,6 +54,8 @@ * The literal /dev/stdin, /dev/fd/0, and /proc/self/fd/0 are the exception: stdin is then the program, and the gate reads it. * `< /dev/null` gives no text, so it leaves the operand a file. * A command name the shell computes, which can be a shell, is opaque when a later word or its stdin holds a PR command. + * A command name written as one quoted variable, `"$P"`, that this shell set to a literal word before it is read as that word, + * but only when nothing in the text can change P unseen (see `knownNames`). * A `-c` script or ssh command the outer shell computes is opaque when a command name in it is computed, since that can be gh. * Output the outer shell computes, piped into a shell, is opaque too, whatever command or group prints it. */ @@ -240,6 +242,8 @@ interface Segment { pipe?: Segment; /** The commands of the `{ }`, `( )`, or compound command this segment closes. They print to its pipe. */ group?: Segment[]; + /** By word index: the source text of a plain word, before quote removal. */ + raw?: Record; } /** @@ -305,11 +309,106 @@ function envOf(scope: Scope, outer: Env): Env { return env; } +/** Builtins that run code the gate does not read, or set variables it does not name: by arithmetic, a nameref, an alias, or a zsh option. */ +const HIDDEN_SETS = [...RUNS_CODE, "trap", "alias", "enable", "let", "declare", "typeset", "local", "integer", "float", "noglob", "nocorrect", "-", "vared", "zparseopts", "zstyle", "zformat", "zregexparse", "strftime", "zstat", "sysread", "getln", "zle", "zmodload", "autoload"]; +/** Builtins that set a variable named in an option or argument, where the name can be glued to an option letter, or that run a callback. */ +const VAR_SETTERS = ["printf", "print", "wait", "set", "read", "mapfile", "readarray", "getopts", "unset", "compgen", "select", "coproc"]; +/** Variables bash or zsh set or guard on their own, so a plain assignment may fail or not hold. */ +const SHELL_SET = + /^(?:_|HOME|(?:CD|F|MAN|MAIL|MODULE_)?PATH|BASH\w*|ZSH\w*|COMP\w*|PWD|OLDPWD|DIRSTACK|REPLY|MAPFILE|COPROC|OPT(?:ARG|IND)|(?:PIPE)?STATUS|ARGV|ARGC|FUNCNAME|FUNCSTACK|GROUPS|S?RANDOM|SECONDS|EPOCH\w+|LINENO|HISTCMD|COLUMNS|LINES|PPID|E?UID|E?GID|USERNAME|SHELLOPTS|SHLVL|TTY\w*|ERRNO|PSVAR|WATCH|FIGNORE|MATCH|MBEGIN|MEND)$/i; + +/** + * Gives a command name written as one quoted variable, `"$P"` or `"${P}"`, the literal word this command set P to before it, + * so `PY=/usr/bin/python3; "$PY" run.py` runs python, not a computed name that could be a shell. The literal is then read as + * if it were typed there, so gh, a shell, ssh, or a wrapper is checked as before. A name resolves only when all of these hold: + * - P is set only by plain `P=word` commands at the top of this shell: assignments alone, in no subshell, group, compound body, + * pipeline, or background job, and not after `&&` or `||`. Each sets P in order. A value with an expansion, or none, leaves P unknown. + * - The text, both as written and after quote removal, names P nowhere else but in `$P` and `${P}` reads, so no prefix assignment, + * `unset`, `read`, `export`, loop, or `${P:=x}` changes it, even with the name split by quotes, as in `read P'Y'`. + * - P is not a variable the shell sets or guards on its own, such as PWD, REPLY, PATH, or BASH_*. + * - No setter builtin (printf, print, read, mapfile, readarray, getopts, wait, set, unset, compgen, select, coproc) takes an + * option that names a variable or a callback (-a, -A, -v, -p, -C, -V), or a computed word, since `printf -vP` hides the name P. + * - Nothing in the text can set a variable without naming it: no eval, source, trap, alias, declare, let, arithmetic, array, + * function (zsh's anonymous `() { }` too), subscript assignment (zsh's `functions[f]=body` defines one), setter with a computed + * word, or command name that stays computed. A function body would also run where it is called, and in zsh an EXIT trap set + * in a function runs as that function returns. + * A trap set only on EXIT (or 0) is the exception, since it runs as the shell exits, after every command a name resolves for. + * A trap on any other signal, such as DEBUG or ERR, runs while the script still runs. + */ +function knownNames(scope: Scope, src: string): void { + const texts = [src, src.replaceAll("\\\n", "")]; + if (scope.assigns.length > 0 || texts.some((t) => /\(\s*\)|\bfunction\b|=\(|\$\{\(|\w\[.*?\]\+?=/.test(t))) return; + const plain = (seg: Segment): boolean => + seg.always && + !seg.subshell && + !seg.pipe && + seg.after !== "|" && + seg.after !== "&" && + seg.stdin.length + seg.other.length + seg.out.length + seg.copies === 0 && + seg.words.every((_, i) => /^[A-Za-z_]\w*=/.test(seg.raw?.[i] ?? "")); + const sets = new Map(); + for (const word of scope.segments.filter(plain).flatMap((s) => s.words)) { + const name = word.split("=")[0]; + sets.set(name, (sets.get(name) ?? 0) + 1); + } + // Quote removal can join a name the source splits, as `read P'Y'` names PY, so the parsed words are counted too. + const words = [scope.segments, ...scope.subs.map((s) => s.segments)].flat().flatMap((s) => s.words.map((w) => w.replace(MARKER, " "))).join(" "); + const named = (name: string): boolean => { + const mention = new RegExp(`(? t.replace(read, "")), words].every((t) => t.match(mention)?.length === sets.get(name)); + }; + const known = new Map(); + const resolved = new Map(); + for (const seg of scope.segments) { + const words = [...seg.words]; + for (let found = head(words, (w) => computed(w, scope)); !found.sure && found.at >= 0; found = head(words, (w) => computed(w, scope))) { + const [, braced, bare] = /^"\$(?:\{([A-Za-z_]\w*)\}|([A-Za-z_]\w*))"$/.exec(seg.raw?.[found.at] ?? "") ?? []; + const value = known.get(braced ?? bare ?? ""); + if (value === undefined) break; + words[found.at] = value; + } + resolved.set(seg, words); + if (!plain(seg)) continue; + for (const word of seg.words) { + const name = word.split("=")[0]; + const value = word.slice(name.length + 1); + if (value !== "" && !value.includes("\0") && named(name)) known.set(name, value); + else known.delete(name); + } + } + // A setter builtin can set P where the name never stands apart, glued to an option letter as in `printf -vP gh` or `read -aP`, + // or through a callback as in `mapfile -C 'eval ...'`. So a setter word anywhere, under any wrapper, stops every resolution + // when an option after it holds a letter that takes a name or a callback, or when any word after it is computed. + // The words are read after resolution, since `P=printf; "$P" -vQ gh` reaches the setter through a name. + const all = [scope.segments, ...scope.subs.map((s) => s.segments)].flat(); + const hides = (seg: Segment): boolean => { + const words = resolved.get(seg) ?? seg.words; + return words.some((w, i) => VAR_SETTERS.includes(w) && words.slice(i + 1).some((a) => computed(a, scope) || /^-[^-]*[aAvpCV]/.test(a))); + }; + if (all.some(hides)) return; + for (const seg of all) { + const words = resolved.get(seg) ?? seg.words; + const found = head(words, (w) => computed(w, scope)); + if (!found.sure) return; + if (found.at < 0) continue; + const [cmd, first = "", ...rest] = words.slice(found.at); + // An EXIT trap runs only as its shell exits, after every command here. Its text still counts as mentions above. + const trap = first === "--" ? rest : [first, ...rest]; + const atExit = cmd === "trap" && trap.length > 1 && !trap[0].startsWith("-") && trap.slice(1).every((w) => w === "EXIT" || w === "0"); + if (HIDDEN_SETS.includes(cmd) && !atExit) return; + if (cmd === "[[" && [first, ...rest].some((w) => /^-(?:eq|ne|lt|le|gt|ge)$/.test(w))) return; + if (["export", "readonly"].includes(cmd) && [first, ...rest].some((w) => w.split("=")[0].includes("\0"))) return; + } + for (const [seg, words] of resolved) seg.words = words; +} + /** Reads `src`. When its words can set HOME, it is read again with `~` and `$HOME` as computed text. */ function lex(src: string, outer: Env = { names: new Set() }): Scope { const parse = (home: boolean): Scope => { const scope: Scope = { segments: [], docs: [], subs: [], assigns: [], tries: 0, home, env: outer }; scope.segments = new Lexer(src, scope).commands(false); + knownNames(scope, src); return scope; }; const first = parse(true); @@ -495,6 +594,7 @@ class Lexer { this.caseHead = false; this.pattern = true; } + (seg.raw ??= {})[seg.words.length] = this.src.slice(start, this.pos); seg.words.push(word); } } @@ -2004,6 +2104,7 @@ function command(seg: Segment, found: Head, ctx: Ctx): PrCommand[] { if (name !== undefined && found.sure && isGh(name)) { return ghCommand({ ...seg, words: words.slice(found.at) }, { ...ctx, lost, known: !ctx.wrapped && found.at === 0 }, false); } + if (found.sure && dataCommand(words, found)) return []; if (!found.sure && found.at >= 0 && hidesShell(seg, found.at, ctx)) return [{ ...at(seg, ctx), kind: "opaque", part: "shell" }]; const start = gh >= 0 ? gh : found.sure ? -1 : found.at; if (start < 0) return []; diff --git a/.claude/skills/open-pr/contract.ts b/.claude/skills/open-pr/contract.ts index 81af6ed8..0ed9da90 100644 --- a/.claude/skills/open-pr/contract.ts +++ b/.claude/skills/open-pr/contract.ts @@ -83,7 +83,8 @@ function plain(line: string): string { interface Parsed { preamble: string; - sections: Array<{ heading: string; text: string }>; + /** `line` is the heading line as written, so a rebuilt body keeps it whole. */ + sections: Array<{ heading: string; line: string; text: string }>; } function parse(body: string): Parsed { @@ -93,7 +94,7 @@ function parse(body: string): Parsed { for (const [i, line] of lines.entries()) { const heading = !mask[i] && line.match(/^##\s+(.+?)\s*#*\s*$/); if (heading) { - out.sections.push({ heading: heading[1], text: "" }); + out.sections.push({ heading: heading[1], line, text: "" }); continue; } const last = out.sections.at(-1); @@ -103,6 +104,15 @@ function parse(body: string): Parsed { return out; } +/** + * The author's body without every section named `name`, read the way the contract reads sections: + * an unfenced `##` heading in any case, through the next one, its subheadings included. + */ +export function withoutSection(body: string, name: Section): string { + const { preamble, sections } = parse(authorBody(body)); + return preamble + sections.filter((s) => s.heading.toLowerCase() !== name.toLowerCase()).map((s) => `${s.line}\n${s.text}`).join(""); +} + function sentences(text: string): string[] { return stripFences(text) .split("\n") @@ -171,13 +181,16 @@ function stripPrLens(body: string): string { return lines.join("\n"); } -/** Checks one pull request against the contract. Every violation carries the fix in its message. */ // CodeRabbit appends its release notes inside these markers after the author's body. const CODERABBIT_SUMMARY = /\n*[\s\S]*?\n*/g; +/** The author's own body: the CodeRabbit and PR Lens blocks removed. The contract reads only this. */ +const authorBody = (body: string) => stripPrLens(body.replace(CODERABBIT_SUMMARY, "\n")); + +/** Checks one pull request against the contract. Every violation carries the fix in its message. */ export function checkPullRequest(input: PullRequest): Verdict { - const pr = { ...input, body: stripPrLens(input.body.replace(CODERABBIT_SUMMARY, "\n")) }; + const pr = { ...input, body: authorBody(input.body) }; const v: Violation[] = []; if (pr.draft) v.push({ rule: "draft", message: "drafts get no review-bot coverage. Open the PR ready for review." }); const title = pr.title === undefined ? null : titleViolation(pr.title); diff --git a/.claude/skills/open-pr/gate.ts b/.claude/skills/open-pr/gate.ts index 11aeff0b..79a9c69a 100644 --- a/.claude/skills/open-pr/gate.ts +++ b/.claude/skills/open-pr/gate.ts @@ -47,12 +47,22 @@ function parseInput(raw: string): HookInput { class Refusal extends Error {} -function run(argv: string[], cwd: string): string { - const r = Bun.spawnSync(argv, { cwd, stdout: "pipe", stderr: "pipe", timeout: 8_000 }); +/** The hook gives the gate 120 s. Every call shares this deadline, so a slow GitHub refuses here before the hook times out. */ +const DEADLINE = Date.now() + 100_000; + +/** Runs a command for at most `limit` ms: 8 s for one read, longer for a read of every page. */ +function run(argv: string[], cwd: string, limit = 8_000): string { + const timeout = Math.min(limit, DEADLINE - Date.now()); + if (timeout <= 0) throw new Refusal(`the gate ran out of time before \`${argv.join(" ")}\`, so it cannot check this command. Retry it.`); + const r = Bun.spawnSync(argv, { cwd, stdout: "pipe", stderr: "pipe", timeout }); + if (r.exitedDueToTimeout) throw new Refusal(`\`${argv.join(" ")}\` did not finish in ${Math.round(timeout / 1000)} s, so the gate cannot check this command. Retry it.`); if (r.exitCode !== 0) throw new Refusal(`\`${argv.join(" ")}\` failed: ${(r.stderr.toString() || r.stdout.toString()).trim()}`); return r.stdout.toString().trim(); } +/** A `--paginate` read makes one request per page inside its one gh process, so it gets more time than a single read. */ +const PAGINATED = 45_000; + const ghJson = (path: string, cwd: string): unknown => JSON.parse(run(["gh", "api", path], cwd)); type Json = Record; @@ -301,7 +311,7 @@ const THREADS = `query($owner: String!, $name: String!, $number: Int!, $endCurso /** Every review thread on the PR, across all pages. */ function allThreads(repo: string, number: number, cwd: string): Thread[] { const [owner, name] = repo.split("/"); - const pages: unknown = JSON.parse(run(["gh", "api", "graphql", "--paginate", "--slurp", "-f", `query=${THREADS}`, "-F", `owner=${owner}`, "-F", `name=${name}`, "-F", `number=${number}`], cwd)); + const pages: unknown = JSON.parse(run(["gh", "api", "graphql", "--paginate", "--slurp", "-f", `query=${THREADS}`, "-F", `owner=${owner}`, "-F", `name=${name}`, "-F", `number=${number}`], cwd, PAGINATED)); if (!isArr(pages)) throw new Refusal("the review threads GitHub returned are not a list of pages, so this gate cannot read CodeRabbit's open findings."); return pages.flatMap((page) => { const pull = field(field(field(page, "data", isObj, "a thread page"), "repository", isObj, "a thread page"), "pullRequest", isObj, "a thread page"); @@ -318,7 +328,7 @@ function parsePaths(x: unknown): string[] { /** Every item of a paginated REST list, read with `--paginate --slurp`. */ function allPages(path: string, what: string, cwd: string): unknown[] { - const pages: unknown = JSON.parse(run(["gh", "api", `${path}?per_page=100`, "--paginate", "--slurp"], cwd)); + const pages: unknown = JSON.parse(run(["gh", "api", `${path}?per_page=100`, "--paginate", "--slurp"], cwd, PAGINATED)); if (!isArr(pages) || !pages.every(isArr)) throw new Refusal(`the ${what} GitHub returned is not a list of pages, so this gate cannot read CodeRabbit's verdict.`); return pages.flat(); } @@ -406,12 +416,13 @@ function checkMerge(cmd: Extract, cwd: string): vo if (named !== undefined && named.toLowerCase() !== sha.toLowerCase()) { throw new Refusal(`the merge names head ${named}, but the gate checked ${sha}, so it cannot vouch for the head gh would merge. Name the head the gate checked.`); } - if (botExempt(pull.baseRepo) && cmd.pinned) return; - const verdict = decideMerge({ repo: pull.baseRepo, pinned: cmd.pinned, sha, evidence: evidenceFor(repo, pull, cwd) }); - if (!verdict.ok) throw new Refusal(`${repo}#${pull.number} cannot merge: ${verdict.reason} Follow the land-pr skill.`); + // Every merge pins its head, an exempt one too: without a review bot, the local review is the only check of that head. if (named === undefined) { throw new Refusal(`pin the approved head ${sha} with --match-head-commit for gh pr merge, or the sha field for the REST merge endpoint. Otherwise a concurrent push can change the head that merges.`); } + if (botExempt(pull.baseRepo) && cmd.pinned) return; + const verdict = decideMerge({ repo: pull.baseRepo, pinned: cmd.pinned, sha, evidence: evidenceFor(repo, pull, cwd) }); + if (!verdict.ok) throw new Refusal(`${repo}#${pull.number} cannot merge: ${verdict.reason} Follow the land-pr skill.`); } function classifyIssue(cmd: Extract, cwd: string): "issue" | "pull-request" { diff --git a/.claude/skills/open-pr/kit.ts b/.claude/skills/open-pr/kit.ts index bfb0f19d..39955b93 100644 --- a/.claude/skills/open-pr/kit.ts +++ b/.claude/skills/open-pr/kit.ts @@ -23,6 +23,7 @@ * .agents/skills/{open-pr,land-pr} symlinks for Codex and Cursor * .claude/settings.json the PreToolUse gate hook * .github/workflows/pr-kit.yml the PR body check in CI + * .github/workflows/merge-danger.yml the merge-danger scorer, on the homelab Linux runners * AGENTS.md (and CLAUDE.md when it does not load AGENTS.md) the rule block * only the real one of AGENTS.md and CLAUDE.md when the other is a symlink whose text is exactly its name */ @@ -253,8 +254,8 @@ function linkSkill(name: string): void { const isObj = (x: unknown): x is Record => typeof x === "object" && x !== null && !Array.isArray(x); /** - * The gate hook's settings. 120 seconds covers a merge check's chain of `gh` calls (8 seconds - * each). `onFailure: "block"` makes a gate that times out, fails to start, or crashes deny + * The gate hook's settings. 120 seconds covers a merge check's chain of `gh` calls: 8 seconds + * for a single read, 45 for a paginated one, and 100 for all of them together. `onFailure: "block"` makes a gate that times out, fails to start, or crashes deny * the call, since Claude Code otherwise lets it run. */ const HOOK_SETTINGS = { timeout: 120, onFailure: "block" } as const; @@ -279,8 +280,8 @@ function wireHook(): void { const RUNS_ON = /^\s*runs-on:\s*(\S.*?)\s*$/gm; /** - * The runner label: the flag, else the label the installed pr-kit.yml already uses, else the - * one label most of the repo's other workflows use. Inference never picks a paid hosted + * The runner label for pr-kit.yml: the flag, else the label the installed pr-kit.yml already + * uses, else the one label most of the repo's other workflows use. Inference never picks a paid hosted * label, and a tie for most used is refused, so the result never depends on file order. */ function runnerLabel(): string { @@ -293,7 +294,8 @@ function runnerLabel(): string { return label; } const counts = new Map(); - for (const f of existsSync(dir) ? readdirSync(dir) : []) { + // merge-danger.yml is the kit's own and names its runners itself, so it never votes. + for (const f of existsSync(dir) ? readdirSync(dir).filter((f) => f !== "merge-danger.yml") : []) { for (const m of readFileSync(join(dir, f), "utf8").matchAll(RUNS_ON)) counts.set(m[1], (counts.get(m[1]) ?? 0) + 1); } const top = Math.max(0, ...counts.values()); @@ -316,11 +318,17 @@ function newer(a: string, b: string): boolean { return false; } -/** Writes pr-kit.yml. A pin the installed copy has at a newer version stays, so an install never downgrades an action the repo's Dependabot moved ahead. */ -function writeWorkflow(label: string): void { - const path = join(repo, ".github", "workflows", "pr-kit.yml"); +/** + * The workflows the kit installs. Only pr-kit.yml takes the runner label. merge-danger.yml names + * the homelab Linux runners itself, since the label can name a Mac. + */ +const WORKFLOWS = ["pr-kit.yml", "merge-danger.yml"] as const; + +/** Writes one workflow. A pin the installed copy has at a newer version stays, so an install never downgrades an action the repo's Dependabot moved ahead. */ +function writeWorkflow(file: (typeof WORKFLOWS)[number], label: string): void { + const path = join(repo, ".github", "workflows", file); const installed = new Map(existsSync(path) ? [...readFileSync(path, "utf8").matchAll(PIN)].map((m) => [m[4], { pin: m[2], version: m[5] }] as const) : []); - const yml = readFileSync(join(SOURCE["open-pr"], "pr-kit.yml"), "utf8") + const yml = readFileSync(join(SOURCE["open-pr"], file), "utf8") .replace("__RUNS_ON__", label) .replaceAll(".claude/skills/", `${layout.directory}/`) // Only the pin moves over, so the template keeps its own indent and step layout. @@ -376,7 +384,7 @@ function preflightDestinations(): InstructionWrite[] { } const alias = instructionAlias(); const instructionFiles = alias ? [alias] : ["AGENTS.md", "CLAUDE.md"]; - for (const file of [".claude/settings.json", ".github/workflows/pr-kit.yml", ...instructionFiles]) assertDestination(join(repo, file)); + for (const file of [".claude/settings.json", ...WORKFLOWS.map((w) => `.github/workflows/${w}`), ...instructionFiles]) assertDestination(join(repo, file)); if (layout.name === "skills") assertDestination(join(repo, ".agents", "skills"), true); return instructionFiles.flatMap((file) => { const path = join(repo, file); @@ -406,8 +414,8 @@ guardInstalled(); for (const name of SKILLS) copySkill(name); if (layout.name === "skills") for (const name of SKILLS) linkSkill(name); wireHook(); -writeWorkflow(label); +for (const file of WORKFLOWS) writeWorkflow(file, label); for (const { path, text } of instructions) writeIfChanged(path, text); console.log(changes.length ? changes.map((c) => `- ${c}`).join("\n") : "PR kit already current."); -console.log("Next: commit these files in their own PR, then make the pr-kit check required in the repo ruleset."); +console.log("Next: commit these files in their own PR, then make the pr-kit check required in the repo ruleset. merge-danger needs a homelab-ci Linux runner with Jev's key file (ROLLOUT.md, step 4a)."); diff --git a/.claude/skills/open-pr/merge-danger.yml b/.claude/skills/open-pr/merge-danger.yml new file mode 100644 index 00000000..7093f5ba --- /dev/null +++ b/.claude/skills/open-pr/merge-danger.yml @@ -0,0 +1,46 @@ +name: merge-danger + +# Scores every same-repo PR's merge danger the same way, whoever wrote it, and posts the +# `merge-danger` commit status on the exact head. land-pr reads that status at merge time. +# pull_request_target runs this workflow and score.ts from the default branch, so a PR cannot +# change its own score. The job reads Jev's key from /etc/merge-danger/typesafe_api_key on the +# runner (ROLLOUT.md), not from a repo secret, and holds a token that writes statuses. So it +# checks out the default branch only and reads the PR through the event payload and the API. +# Never check out or run the PR's code here. +on: + pull_request_target: + types: [opened, edited, reopened, synchronize, ready_for_review] + +permissions: + contents: read + statuses: write + +concurrency: + group: merge-danger-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + score: + name: Merge danger + # Fork and Dependabot PRs skip this check, so they never run on our runners. + if: >- + github.actor != 'dependabot[bot]' && + github.event.pull_request.head.repo.full_name == github.repository && + github.event.pull_request.user.login != 'dependabot[bot]' + # The homelab Linux runners, never the Mac minis. This job does not take the pr-kit label, + # which can name any self-hosted runner. + runs-on: [self-hosted, Linux, homelab-ci] + timeout-minutes: 5 + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + with: + ref: ${{ github.event.repository.default_branch }} + persist-credentials: false + - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 + with: + bun-version: 1.4.4 + env: + HOME: ${{ runner.temp }}/setup-bun + - run: bun .claude/skills/open-pr/score.ts --event "$GITHUB_EVENT_PATH" + env: + GITHUB_TOKEN: ${{ github.token }} diff --git a/.claude/skills/open-pr/pr-kit.yml b/.claude/skills/open-pr/pr-kit.yml index 51e11918..d7149ba1 100644 --- a/.claude/skills/open-pr/pr-kit.yml +++ b/.claude/skills/open-pr/pr-kit.yml @@ -12,6 +12,12 @@ on: permissions: contents: read +# Each edit or push starts a new check of the PR's current body. The newest run is the one that +# counts, so an older run for the same PR is cancelled and does not hold a runner slot. +concurrency: + group: pr-kit-${{ github.event.pull_request.number }} + cancel-in-progress: true + jobs: body: name: PR body contract diff --git a/.claude/skills/open-pr/score.ts b/.claude/skills/open-pr/score.ts new file mode 100644 index 00000000..c9590ed9 --- /dev/null +++ b/.claude/skills/open-pr/score.ts @@ -0,0 +1,438 @@ +#!/usr/bin/env bun +/** + * Scores a pull request's merge danger with Jev (TypeSafe AI) and posts it as the + * `merge-danger` commit status on the exact head. The merge-danger workflow runs it from the + * default branch on every same-repo PR, so every PR is scored the same way, whoever wrote it, + * and never by its own author. land-pr reads the status (land-pr/danger.ts). + * + * bun score.ts --event "$GITHUB_EVENT_PATH" + * + * Env: GITHUB_TOKEN (statuses: write), GITHUB_REPOSITORY, GITHUB_API_URL, GITHUB_SERVER_URL, + * GITHUB_RUN_ID, GITHUB_RUN_ATTEMPT. Jev's key comes from TYPESAFE_API_KEY when set, else KEY_FILE on the + * runner. The workflow passes no repo secret. + * + * It reads the PR only through the event payload and the API, never by checking it out. It asks + * Jev four questions in one call (door, reach, likelihood, control) and combines the answers in + * code (combine). A diff too large for one call is rated in parts (statesFor), and the highest + * radius stands. When it cannot rate the PR (no key, Jev or GitHub failed, or the diff is empty + * or cannot be split without losing a line) it posts `error` with the reason, never a rating. + * The run then exits 1. + * The status's run link identifies the comparison and metadata with a SHA-256 fingerprint. + * The job log records that fingerprint and the actual radius. land-pr verifies both through + * GitHub's attempt-scoped log API instead of trusting the caller-supplied status text. + */ + +import { createHash } from "node:crypto"; +import { readFileSync } from "node:fs"; +import { parseArgs } from "node:util"; +import { withoutSection } from "./contract"; + +export const STATUS_CONTEXT = "merge-danger"; +export const RADII = ["none", "low", "medium", "high"] as const; +export type Radius = (typeof RADII)[number]; + +/** The four axes Jev answers, each as its own question. Their option keys are the values code combines. */ +export const DOORS = ["two_way", "one_way", "unclear"] as const; +export const REACHES = ["none", "local", "repo", "external"] as const; +export const LIKELIHOODS = ["low", "elevated", "high"] as const; +export const CONTROLS = ["weakens", "neutral", "no"] as const; +export interface Axes { + door: (typeof DOORS)[number]; + reach: (typeof REACHES)[number]; + likelihood: (typeof LIKELIHOODS)[number]; + control: (typeof CONTROLS)[number]; +} + +const JEV_ENDPOINT = "https://api.typesafe.ai/v1/systemone"; +/** Each GitHub or Jev request gets this long, so a hung request ends in an error status. */ +const REQUEST_TIMEOUT_MS = 30_000; +/** + * Jev reads at most 32k tokens per question: the state plus that one question + * (docs.typesafe.ai/models). Bound the complete request by UTF-8 bytes instead of assuming four + * characters per token. This includes every question and leaves room for provider framing. + */ +const REQUEST_BYTE_BUDGET = 30_000; +/** Bound model work and keep scoring inside the workflow's five-minute job limit. */ +const MAX_PARTS = 24; +const SCORING_REQUEST_WINDOWS = 6; +/** GitHub caps a commit status description at 140 characters. */ +const DESCRIPTION_MAX = 140; +/** Where the homelab-ci runner keeps Jev's key, written from BWS (ROLLOUT.md). */ +export const KEY_FILE = "/etc/merge-danger/typesafe_api_key"; + +/** + * Evidence first. Every option describes something the changed lines do, never a kind of file or + * path. An option above the lowest needs a changed line that shows it. The questions never say + * what a level does to the merge: code combines the axes (combine), so the policy cannot move the + * judgment. Jev reads `title`, `body` (the author's own Merge danger section removed), and `diff`. + */ +const EVIDENCE = "Judge by what the changed lines in `diff` do, not by which files or paths they are in. Pick an answer above the lowest only when a changed line shows it. When no changed line shows it, pick the lower answer."; + +export const QUESTIONS = { + door: { + type: "choice", + instructions: `After a plain revert of the pull request in \`diff\` (described by \`title\` and \`body\`), does anything it did stay changed? ${EVIDENCE}`, + criteria: { + two_way: "No. Every effect of the changed lines ends when they are reverted: code, tests, docs, config, or CI that a revert puts back, or removing entries that nothing reads", + one_way: "Yes. A changed line does something a revert cannot undo: it deletes or rewrites stored data, migrates a schema, publishes a package or release, sends a message, deploys to users when merged, spends money, puts a secret where others can read it, or changes an interface that another repo or person already uses", + unclear: "A changed line starts a deploy, publish, migration, or job, and `diff` does not show whether that happens when the pull request merges", + }, + }, + reach: { + type: "score", + instructions: `If the changed lines in \`diff\` are wrong, what is the widest thing that reads or runs them? Count only what a changed line shows reads or runs it: a call, an import, a config key, a job step, or an instruction that an agent follows. ${EVIDENCE}`, + criteria: [ + "Nothing reads or runs the changed lines: comments, prose for human readers, or tests that only add cases", + "Only the changed code and its direct callers in this repo, in code that does not ship to users: tests, scripts, examples, or internal tooling", + "Code, config, CI steps, scheduled jobs, or agent instructions (AGENTS.md, CLAUDE.md, skills, prompts) that other parts of this repo read or run", + "Something outside this repo reads or runs the changed lines: other repos, deployed services, other machines, credentials, stored data, or users, including runtime code that the project ships in a release, package, app, or service", + ], + }, + likelihood: { + type: "score", + instructions: `How likely is it that the changed lines in \`diff\` do not do what \`title\` and \`body\` say? Size alone is not evidence, and lockfiles and generated files do not count. ${EVIDENCE}`, + criteria: [ + "The changed lines keep behavior the same (a rename, move, format, dead code removal, docs, a test that only adds cases, or a patch version bump), or add small new logic in one place with a test in `diff` that covers it", + "The changed lines add or change logic across several files or areas, change what the code does at run time (not only its shape) with no test in `diff` that checks the new behavior, fix tricky logic, bump a minor or major dependency version, or include parts that cannot be read (generated, binary, or cut off)", + "A changed line shows a concrete defect: a wrong condition, a removed check, a type escape, a weakened or deleted test, or code that does not do what `title` or `body` says", + ], + }, + control: { + type: "choice", + instructions: `Do the changed lines in \`diff\` change how a control works? Controls are auth, permissions, secrets handling, a merge or review gate, CI trust settings (\`pull_request_target\`, tokens, \`permissions:\` blocks), a deploy or release pipeline, branch protection, and safety hooks. ${EVIDENCE}`, + criteria: { + weakens: "Yes, and a changed line lets through something that was blocked before, widens a permission, adds an entry to an allowlist, approved list, or exemption list, skips or bypasses a check, or exposes a secret", + neutral: "Yes, and the control keeps the same behavior or gets stricter: a refactor, a move, or an added check", + no: "No changed line alters how any control works", + }, + }, +} as const; + +/** Reach by likelihood, from the research's matrix (eval/RESEARCH.md, Combining). */ +export const MATRIX: Record> = { + none: { low: "none", elevated: "none", high: "low" }, + local: { low: "low", elevated: "low", high: "medium" }, + repo: { low: "low", elevated: "medium", high: "high" }, + external: { low: "medium", elevated: "high", high: "high" }, +}; + +/** The blast radius, combined in code exactly as eval/RESEARCH.md's Combining section says. */ +export function combine(axes: Axes): Radius { + if (axes.door === "one_way" || axes.control === "weakens") return "high"; + let level = RADII.indexOf(MATRIX[axes.reach][axes.likelihood]); + if (axes.control === "neutral") level = Math.max(level, Math.min(level + 1, RADII.indexOf("medium"))); + if (axes.door === "unclear") level = Math.min(level + 1, RADII.length - 1); + return RADII[level]; +} + +export interface PullEvent { + number: number; + title: string; + body: string; + headSha: string; + baseSha: string; +} + +const SHA = /^[0-9a-f]{40}$/; +const isObj = (x: unknown): x is Record => typeof x === "object" && x !== null && !Array.isArray(x); +const shaOf = (x: unknown, what: string): string => { + const sha = isObj(x) ? x.sha : undefined; + if (typeof sha !== "string" || !SHA.test(sha)) throw new Error(`the event's ${what}.sha is not a commit SHA`); + return sha; +}; + +export function parseEvent(event: unknown): PullEvent { + const pr = isObj(event) ? event.pull_request : undefined; + if (!isObj(pr) || typeof pr.number !== "number") throw new Error("the event holds no pull_request payload"); + return { + number: pr.number, + title: typeof pr.title === "string" ? pr.title : "", + body: typeof pr.body === "string" ? pr.body : "", + headSha: shaOf(pr.head, "head"), + baseSha: shaOf(pr.base, "base"), + }; +} + +/** Each axis's answer with the probability Jev gave it. */ +export type Rating = { [K in keyof Axes]: { value: Axes[K]; p: number } }; + +export type Outcome = { state: "success"; radius: Radius; axes: Rating } | { state: "error"; why: string }; + +const shown = (value: string) => value.replace("_", "-"); +/** The `success` description. danger.ts reads the radius back from its first words. */ +export const describeRating = (r: Extract) => + `Blast radius ${r.radius}. ${(Object.keys(r.axes) as (keyof Axes)[]).map((k) => `${k} ${shown(r.axes[k].value)} ${r.axes[k].p.toFixed(2)}`).join(", ")}.`; +export const RATING = /^Blast radius (none|low|medium|high)\b/; +/** Credential shapes, replaced before anything leaves the runner (from the jev-ask skill's client). */ +const SECRET_SHAPES: readonly (readonly [RegExp, string])[] = [ + [/-----BEGIN [A-Z ]*PRIVATE KEY-----[\s\S]*?-----END [A-Z ]*PRIVATE KEY-----/g, "[REDACTED]"], + [/\b([A-Z][A-Z0-9_]*(?:KEY|TOKEN|SECRET|PASSWORD|PASS))=("[^"]*"|'[^']*'|\S+)/g, "$1=[REDACTED]"], + [/(Authorization:\s*Bearer\s+)\S+/gi, "$1[REDACTED]"], + [/\b(?:sk|pk|rk)-[A-Za-z0-9_-]{16,}/g, "[REDACTED]"], + [/\bgh[pousr]_[A-Za-z0-9]{20,}/g, "[REDACTED]"], + [/\bgithub_pat_[A-Za-z0-9_]{20,}/g, "[REDACTED]"], + [/\bxox[abprs]-[A-Za-z0-9-]{10,}/g, "[REDACTED]"], + [/\bAKIA[0-9A-Z]{16}\b/g, "[REDACTED]"], + [/\beyJ[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}/g, "[REDACTED]"], +]; +const redact = (text: string) => SECRET_SHAPES.reduce((t, [re, to]) => t.replace(re, to), text); +/** Refuse possible private keys before redaction can hide source between unrelated markers. */ +const PRIVATE_KEY_MARKER = /-----(?:BEGIN|END) [A-Z ]*PRIVATE KEY-----/; + +const PNG_URI = /data:image\/png;base64,([A-Za-z0-9+/]+={0,2})/g; +const PNG_SIGNATURE = Buffer.from([137, 80, 78, 71, 13, 10, 26, 10]); + +/** Whether the bytes have complete PNG framing: the signature, IHDR first, chunks that fit, and IEND last. */ +function wholePng(bytes: Buffer): boolean { + if (!bytes.subarray(0, 8).equals(PNG_SIGNATURE)) return false; + for (let at = 8; at + 12 <= bytes.length; ) { + const type = bytes.toString("latin1", at + 4, at + 8); + const end = at + 12 + bytes.readUInt32BE(at); + if (end > bytes.length || (at === 8 && type !== "IHDR")) return false; + if (type === "IEND") return end === bytes.length; + at = end; + } + return false; +} + +/** + * Each PNG data URI's pixels as their size and SHA-256, so screenshots embedded in SVG do not + * fill Jev's budget. Only canonical base64 that decodes to complete PNG framing is replaced. Anything else + * stays as written. This runs before redact, which could otherwise rewrite part of a payload. + */ +const opaqueRasters = (diff: string) => + diff.replace(PNG_URI, (uri, data: string) => { + const bytes = Buffer.from(data, "base64"); + if (bytes.toString("base64") !== data || !wholePng(bytes)) return uri; + return `data:image/png;base64,[opaque PNG: ${bytes.length} bytes, sha256 ${createHash("sha256").update(bytes).digest("hex")}]`; + }); + +/** What Jev reads: the title, the body without the author's Merge danger section, and the diff, each with credentials redacted. */ +export const jevState = (pr: { title: string; body: string }, diff: string) => ({ + title: redact(pr.title), + body: redact(withoutSection(pr.body, "Merge danger")), + diff: redact(opaqueRasters(diff)), +}); +type JevState = ReturnType; +const requestFor = (state: JevState) => ({ model: "jev-latest", state, questions: QUESTIONS }); +const requestBytes = (state: JevState) => Buffer.byteLength(JSON.stringify(requestFor(state)), "utf8"); + +function statesFor(pr: { title: string; body: string }, diff: string): JevState[] | string { + const whole = jevState(pr, diff); + if (requestBytes(whole) <= REQUEST_BYTE_BUDGET) return [whole]; + const lines = whole.diff.split("\n"); + const inventory = lines.filter((line) => line.startsWith("diff --git ")).map((line) => line.replace(/^diff --git a\/(.+) b\/\1$/, "$1")).join("\n"); + const label = `[Scorer note: this is one part of a complete diff, not a patch to apply. No text lines are discarded. File and hunk headers repeat at boundaries. Other parts contain the remaining source. The highest radius across all parts counts. Full file inventory follows.]\n${inventory}\n[Part source follows.]\n`; + const part = (source: string[]): JevState => ({ ...whole, diff: label + source.join("\n") }); + const tooLarge = `cannot split the diff within the ${REQUEST_BYTE_BUDGET}-byte request bound`; + if (requestBytes(part([])) > REQUEST_BYTE_BUDGET) return `${tooLarge}: its title, body, questions, and file inventory do not fit. The owner decides.`; + const parts: JevState[] = []; + let source: string[] = []; + let fileHeaders: string[] = []; + let hunkHeader: string | undefined; + for (const line of lines) { + const startsFile = line.startsWith("diff --git "); + const startsHunk = line.startsWith("@@"); + if (requestBytes(part([...source, line])) > REQUEST_BYTE_BUDGET) { + if (!source.length) return `${tooLarge}: a source line does not fit. The owner decides.`; + parts.push(part(source)); + if (parts.length >= MAX_PARTS) return `${tooLarge}: it needs more than ${MAX_PARTS} parts. The owner decides.`; + source = startsFile ? [] : [...fileHeaders, ...(startsHunk || hunkHeader === undefined ? [] : [hunkHeader])]; + if (requestBytes(part([...source, line])) > REQUEST_BYTE_BUDGET) return `${tooLarge}: a source line and its file context do not fit. The owner decides.`; + } + source.push(line); + if (startsFile) { + fileHeaders = [line]; + hunkHeader = undefined; + } else if (startsHunk) hunkHeader = line; + else if (hunkHeader === undefined) fileHeaders.push(line); + } + if (source.length) parts.push(part(source)); + return parts; +} + +/** Immutable comparison commits and the metadata Jev reads, bound to this PR. */ +export const inputFingerprint = (pr: PullEvent): string => + createHash("sha256").update(JSON.stringify([pr.number, pr.headSha, pr.baseSha, jevState(pr, "")])).digest("hex"); + +export interface ScoreEvidence { + inputs: string; + radius: Radius; +} +export const PROOF_PREFIX = "merge-danger-proof: "; + +/** One structured record in the trusted scorer job's log, read back through GitHub. */ +export const evidenceLine = (pr: PullEvent, radius: Radius): string => + `${PROOF_PREFIX}${JSON.stringify({ inputs: inputFingerprint(pr), radius } satisfies ScoreEvidence)}`; + +const probability = (x: unknown): x is number => typeof x === "number" && x >= 0 && x <= 1; + +type Answer = { value: T; p: number } | string; + +/** The probabilities for every option, which must all be present and sum to 1. */ +function distribution(byOption: Record, keys: readonly string[]): number[] | string { + const ps = keys.map((k) => byOption[k]); + if (!ps.every(probability)) return "unexpected shape"; + const total = ps.reduce((sum, p) => sum + p, 0); + return Math.abs(total - 1) > 0.05 ? `probabilities that do not sum to 1 (they sum to ${total.toFixed(2)})` : ps; +} + +/** A Choice answer: Jev's pick, with its probability. */ +function choiceOf(answer: unknown, options: readonly T[]): Answer { + if (!isObj(answer) || answer.type !== "choice" || !isObj(answer.probabilities)) return "unexpected shape"; + const value = options.find((o) => o === answer.choice); + if (value === undefined) return `not one of ${options.join(", ")}`; + const ps = distribution(answer.probabilities, options); + return typeof ps === "string" ? ps : { value, p: ps[options.indexOf(value)] }; +} + +/** A Score answer: the most likely level. A tie takes the lower level, since a level above the lowest needs evidence. */ +function levelOf(answer: unknown, levels: readonly T[]): Answer { + if (!isObj(answer) || answer.type !== "score" || !isObj(answer.probabilities)) return "unexpected shape"; + const ps = distribution(answer.probabilities, levels.map((_, i) => String(i))); + if (typeof ps === "string") return ps; + const top = ps.reduce((best, p, i) => (p > ps[best] ? i : best), 0); + return { value: levels[top], p: ps[top] }; +} + +/** Jev's four answers as a rating, combined in code. */ +export function rate(response: unknown): Outcome { + const answers = isObj(response) && isObj(response.answers) ? response.answers : {}; + const fail = (axis: string, why: string): Outcome => ({ state: "error", why: `Jev answered ${axis} with ${why}.` }); + const door = choiceOf(answers.door, DOORS); + if (typeof door === "string") return fail("door", door); + const reach = levelOf(answers.reach, REACHES); + if (typeof reach === "string") return fail("reach", reach); + const likelihood = levelOf(answers.likelihood, LIKELIHOODS); + if (typeof likelihood === "string") return fail("likelihood", likelihood); + const control = choiceOf(answers.control, CONTROLS); + if (typeof control === "string") return fail("control", control); + const axes = { door, reach, likelihood, control }; + const radius = combine({ door: door.value, reach: reach.value, likelihood: likelihood.value, control: control.value }); + return { state: "success", radius, axes }; +} + +export interface ScoreRun { + event: PullEvent; + /** `owner/name`. */ + repo: string; + token: string; + /** TYPESAFE_API_KEY from the environment, when set. It wins over keyFile. */ + key: string | undefined; + /** The key file on the runner, read when `key` is unset. KEY_FILE outside tests. */ + keyFile: string; + api: string; + /** The workflow run URL, before the attempt path and input fingerprint. */ + runUrl: string; + /** The producing attempt, from GITHUB_RUN_ATTEMPT. */ + runAttempt: string; + fetch: typeof fetch; + /** Per request. REQUEST_TIMEOUT_MS when absent. */ + timeoutMs?: number; +} + +const firstLine = (text: string) => text.trim().split("\n")[0].slice(0, 80); + +/** The key file's contents, or undefined when the file does not exist. Any other read error throws. */ +function readKeyFile(path: string): string | undefined { + try { + return readFileSync(path, "utf8").trim() || undefined; + } catch (err) { + if (isObj(err) && err.code === "ENOENT") return undefined; + throw err; + } +} + +async function judge(run: ScoreRun): Promise { + const key = run.key || readKeyFile(run.keyFile); + if (!key) return { state: "error", why: "no Jev key on this runner" }; + const { event } = run; + const diffRes = await run.fetch(`${run.api}/repos/${run.repo}/compare/${event.baseSha}...${event.headSha}`, { + headers: { Accept: "application/vnd.github.diff", Authorization: `Bearer ${run.token}`, "X-GitHub-Api-Version": "2022-11-28" }, + signal: AbortSignal.timeout(run.timeoutMs ?? REQUEST_TIMEOUT_MS), + }); + const diff = await diffRes.text(); + if (!diffRes.ok) return { state: "error", why: `GitHub answered ${diffRes.status} for the diff: ${firstLine(diff)}` }; + if (diff.trim() === "") return { state: "error", why: "the diff is empty, so there is nothing to rate." }; + if ([event.title, event.body, diff].some((text) => PRIVATE_KEY_MARKER.test(text))) { + return { state: "error", why: "the inputs contain private-key markers. Remove the key or markers before scoring." }; + } + const states = statesFor(event, diff); + if (typeof states === "string") return { state: "error", why: states }; + const requestTimeout = run.timeoutMs ?? REQUEST_TIMEOUT_MS; + const expiresAt = performance.now() + SCORING_REQUEST_WINDOWS * requestTimeout; + let highest: Extract | undefined; + for (const state of states) { + const remaining = expiresAt - performance.now(); + if (remaining <= 0) return { state: "error", why: "the combined scoring time budget expired. The owner decides." }; + const outcome = await ask({ ...run, timeoutMs: Math.min(requestTimeout, remaining) }, key, state); + if (outcome.state === "error") return outcome; + if (highest === undefined || RADII.indexOf(outcome.radius) > RADII.indexOf(highest.radius)) highest = outcome; + } + return highest ?? { state: "error", why: "the diff gave no part to rate." }; +} + +/** One Jev call on one state, as a rating. */ +async function ask(run: ScoreRun, key: string, state: JevState): Promise { + const jevRes = await run.fetch(JEV_ENDPOINT, { + method: "POST", + headers: { "content-type": "application/json", Authorization: `Bearer ${key}` }, + body: JSON.stringify(requestFor(state)), + signal: AbortSignal.timeout(run.timeoutMs ?? REQUEST_TIMEOUT_MS), + redirect: "error", + }); + const text = await jevRes.text(); + if (!jevRes.ok) return { state: "error", why: `Jev answered ${jevRes.status}: ${firstLine(text.split(key).join("[REDACTED]"))}` }; + let answer: unknown; + try { + answer = JSON.parse(text); + } catch { + return { state: "error", why: "Jev answered in an unexpected shape (not JSON)." }; + } + return rate(answer); +} + +/** Scores the PR and posts the status. Throws only when GitHub refuses the status itself. */ +export async function score(run: ScoreRun): Promise { + const outcome = await judge(run).catch((err: unknown): Outcome => ({ state: "error", why: `the scorer failed: ${err instanceof Error ? err.message : String(err)}` })); + const description = outcome.state === "success" ? describeRating(outcome) : `Not rated: ${outcome.why}`; + const res = await run.fetch(`${run.api}/repos/${run.repo}/statuses/${run.event.headSha}`, { + method: "POST", + headers: { Accept: "application/vnd.github+json", Authorization: `Bearer ${run.token}`, "X-GitHub-Api-Version": "2022-11-28" }, + body: JSON.stringify({ state: outcome.state, context: STATUS_CONTEXT, description: description.slice(0, DESCRIPTION_MAX), target_url: `${run.runUrl}/attempts/${run.runAttempt}#inputs=${inputFingerprint(run.event)}` }), + signal: AbortSignal.timeout(run.timeoutMs ?? REQUEST_TIMEOUT_MS), + }); + if (!res.ok) throw new Error(`GitHub refused the ${STATUS_CONTEXT} status with ${res.status}: ${firstLine(await res.text())}`); + return outcome; +} + +if (import.meta.main) { + const { values } = parseArgs({ options: { event: { type: "string" } } }); + const env = (name: string) => { + const value = process.env[name]; + if (!value) throw new Error(`${name} is not set. Run score.ts from the merge-danger workflow.`); + return value; + }; + if (!values.event) throw new Error("usage: bun score.ts --event "); + const repo = env("GITHUB_REPOSITORY"); + const event = parseEvent(JSON.parse(readFileSync(values.event, "utf8"))); + const outcome = await score({ + event, + repo, + token: env("GITHUB_TOKEN"), + key: process.env.TYPESAFE_API_KEY || undefined, + keyFile: KEY_FILE, + api: process.env.GITHUB_API_URL ?? "https://api.github.com", + runUrl: `${process.env.GITHUB_SERVER_URL ?? "https://github.com"}/${repo}/actions/runs/${env("GITHUB_RUN_ID")}`, + runAttempt: env("GITHUB_RUN_ATTEMPT"), + fetch, + }); + if (outcome.state === "success") { + console.log(evidenceLine(event, outcome.radius)); + console.log(`${STATUS_CONTEXT}: ${describeRating(outcome)}`); + } + else { + console.error(`${STATUS_CONTEXT}: ${outcome.state}. ${outcome.why}`); + process.exit(1); + } +} diff --git a/.github/workflows/merge-danger.yml b/.github/workflows/merge-danger.yml new file mode 100644 index 00000000..7093f5ba --- /dev/null +++ b/.github/workflows/merge-danger.yml @@ -0,0 +1,46 @@ +name: merge-danger + +# Scores every same-repo PR's merge danger the same way, whoever wrote it, and posts the +# `merge-danger` commit status on the exact head. land-pr reads that status at merge time. +# pull_request_target runs this workflow and score.ts from the default branch, so a PR cannot +# change its own score. The job reads Jev's key from /etc/merge-danger/typesafe_api_key on the +# runner (ROLLOUT.md), not from a repo secret, and holds a token that writes statuses. So it +# checks out the default branch only and reads the PR through the event payload and the API. +# Never check out or run the PR's code here. +on: + pull_request_target: + types: [opened, edited, reopened, synchronize, ready_for_review] + +permissions: + contents: read + statuses: write + +concurrency: + group: merge-danger-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + score: + name: Merge danger + # Fork and Dependabot PRs skip this check, so they never run on our runners. + if: >- + github.actor != 'dependabot[bot]' && + github.event.pull_request.head.repo.full_name == github.repository && + github.event.pull_request.user.login != 'dependabot[bot]' + # The homelab Linux runners, never the Mac minis. This job does not take the pr-kit label, + # which can name any self-hosted runner. + runs-on: [self-hosted, Linux, homelab-ci] + timeout-minutes: 5 + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + with: + ref: ${{ github.event.repository.default_branch }} + persist-credentials: false + - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 + with: + bun-version: 1.4.4 + env: + HOME: ${{ runner.temp }}/setup-bun + - run: bun .claude/skills/open-pr/score.ts --event "$GITHUB_EVENT_PATH" + env: + GITHUB_TOKEN: ${{ github.token }} diff --git a/.github/workflows/pr-kit.yml b/.github/workflows/pr-kit.yml index b13649aa..1496e162 100644 --- a/.github/workflows/pr-kit.yml +++ b/.github/workflows/pr-kit.yml @@ -12,6 +12,12 @@ on: permissions: contents: read +# Each edit or push starts a new check of the PR's current body. The newest run is the one that +# counts, so an older run for the same PR is cancelled and does not hold a runner slot. +concurrency: + group: pr-kit-${{ github.event.pull_request.number }} + cancel-in-progress: true + jobs: body: name: PR body contract From 9318f5cc945d0263e0ede1d547e51a50fbc21d4b Mon Sep 17 00:00:00 2001 From: Ossie Irondi Date: Sun, 8 Nov 2026 06:18:10 -0600 Subject: [PATCH 2/2] chore(pr-kit): install PR kit v85 A fresh verified merge-danger score now recovers from superseded scorer runs, and danger.ts reads only runs of the current inputs (agents-config #118). Co-Authored-By: Claude Opus 5.5 --- .claude/skills/land-pr/.pr-kit | 2 +- .claude/skills/land-pr/SKILL.md | 2 +- .claude/skills/land-pr/danger.ts | 51 ++++++++++++++++++++++---------- .claude/skills/open-pr/.pr-kit | 2 +- .claude/skills/open-pr/VERSION | 2 +- 5 files changed, 39 insertions(+), 20 deletions(-) diff --git a/.claude/skills/land-pr/.pr-kit b/.claude/skills/land-pr/.pr-kit index 17eec338..ae5ab648 100644 --- a/.claude/skills/land-pr/.pr-kit +++ b/.claude/skills/land-pr/.pr-kit @@ -1,2 +1,2 @@ Installed by the PR kit (open-pr/kit.ts). Edit the kit at its source, not here. -version 84 sha256:80:38:ce:a0:56:8d:6d:83:38:06:31:f5:2f:5f:24:80:e8:e8:26:24:b4:2c:95:1c:dc:6b:3b:31:9c:99:99:67 +version 85 sha256:66:ab:65:d4:d7:dd:3a:24:86:65:02:cb:59:80:4e:1b:59:87:ad:10:6b:53:92:b6:38:2b:cf:4e:92:a1:81:2d diff --git a/.claude/skills/land-pr/SKILL.md b/.claude/skills/land-pr/SKILL.md index 7e4181dc..0df3724e 100644 --- a/.claude/skills/land-pr/SKILL.md +++ b/.claude/skills/land-pr/SKILL.md @@ -53,7 +53,7 @@ A stale PR costs more than one more round. Keep fixing while each round makes pr 4. **Wait on the real state change.** Watch the check-run, not a timer. Count the round and apply the convergence rule. - A scorer status must match the PR's current head, base commit, and scored title and body. An edit requests a new score. If the base branch advances without a PR event, close and reopen the PR to request a score for the new comparison. Wait for that score before merging. Old scores without an input fingerprint need a fresh score too. - - The matching fingerprint and actual radius must also come from the linked scorer run's own job log, read through GitHub. Status URL text alone proves nothing. If that log proof is missing, invalid, or unavailable, the owner decides. Request a fresh score when the old run's logs have expired. When no scorer applies and only stale statuses remain, the body-only rule applies. + - The matching fingerprint and actual radius must also come from the linked scorer run's own job log, read through GitHub. Status URL text alone proves nothing. A log proof that contradicts its status takes the owner's word. A score whose proof is gone (expired logs, a deleted run, or an attempt that failed or was cancelled after it posted) can only raise the rating. With no verified score for the current inputs, the owner decides, so request a fresh score with a close and reopen. When no scorer applies and only stale statuses remain, the body-only rule applies. - When a T3 watch is the wake source and Bash supports `run_in_background`, also arm one bounded `until` loop on the head's `pullfrog-approval` check-run as a backstop. 5. **Check spec fidelity before merge.** Diff the result against the issue, the plan, or the mock. Post every deviation, or "no deviations", as a PR comment. 6. **Run the project's verify skills on the head.** Run `git fetch origin ` and check out the exact head you will merge. Run `bun /../open-pr/verify.ts --base origin/` in the repo, then read each listed `SKILL.md` in full and run it, in the listed order. The listed paths are relative to the repo root. diff --git a/.claude/skills/land-pr/danger.ts b/.claude/skills/land-pr/danger.ts index 12f3b449..cefacbb3 100644 --- a/.claude/skills/land-pr/danger.ts +++ b/.claude/skills/land-pr/danger.ts @@ -20,7 +20,8 @@ * A PR cannot start such a run with its own code. A merge-danger status from any other account, * or one that links anything else, refuses. * That attempt's own job log must record the current input fingerprint and the status's radius. - * A caller-supplied URL hash alone is not evidence. Missing log proof takes the owner's word. + * A caller-supplied URL hash alone is not evidence. A log proof that contradicts its status refuses. + * A score whose proof is gone may only raise the rating, and at least one verified score must exist. * The gate never reads this rating, so the rule stays the agent's to keep, as land-pr says. */ @@ -96,23 +97,35 @@ export function readStatus(statuses: readonly Status[], scorer: Scorer): StatusR if (newest === undefined) return scorer.installed ? { kind: "waiting", why: "the scorer has not rated the current base, title, and body. Wait for the run a push or edit started. If none is running, close and reopen the PR to request a fresh score" } : { kind: "absent" }; if (newest.state === "error" || newest.state === "failure") return { kind: "refused", why: `the ${STATUS_CONTEXT} status is ${newest.state}: ${newest.description ?? "no description"}` }; if (newest.state !== "success") return { kind: "waiting", why: `the ${STATUS_CONTEXT} status is ${newest.state}` }; - let radius: Radius | undefined; + // A score whose proof is gone (logs expired, a run deleted, an attempt that failed or was + // cancelled after it posted) may still raise the rating, so a rerun never hides a higher score. + // It never lowers the rating, and a fresh verified score is still required. + let verified: Radius | undefined; + let claimed: Radius | undefined; + let unproven: string | undefined; + const higher = (a: Radius | undefined, b: Radius) => (a === undefined || rank(b) > rank(a) ? b : a); for (const s of current.filter((s) => s.state === "success")) { const id = runAttemptOf(scorer.repo, s.targetUrl); const run = id === undefined ? undefined : scorer.runs.get(id); - if (run === undefined) return { kind: "refused", why: `a ${STATUS_CONTEXT} status links ${JSON.stringify(s.targetUrl)}, not a run of ${SCORER_PATH} in ${scorer.repo}` }; - if (run.path !== SCORER_PATH || run.event !== "pull_request_target" || run.headSha !== scorer.head) { - return { kind: "refused", why: `a ${STATUS_CONTEXT} status links run ${id}, which is ${run.path} on ${run.event} for ${run.headSha.slice(0, 7)}, not the scorer on this head` }; - } - if (run.status !== "completed") return { kind: "waiting", why: `scorer run ${id} is ${run.status}` }; - if (run.conclusion !== "success") return { kind: "refused", why: `scorer run ${id} concluded ${run.conclusion}, yet a success status links it` }; const word = s.description?.match(RATING)?.[1]; if (!isRadius(word)) return { kind: "refused", why: `the ${STATUS_CONTEXT} status says ${JSON.stringify(s.description)}, which names no blast radius` }; - if (run.proof.kind === "unavailable") return { kind: "refused", why: `scorer run ${id} has no verified output: ${run.proof.why}` }; + if (run !== undefined && (run.path !== SCORER_PATH || run.event !== "pull_request_target" || run.headSha !== scorer.head)) { + return { kind: "refused", why: `a ${STATUS_CONTEXT} status links run ${id}, which is ${run.path} on ${run.event} for ${run.headSha.slice(0, 7)}, not the scorer on this head` }; + } + if (run !== undefined && run.status !== "completed") return { kind: "waiting", why: `scorer run ${id} is ${run.status}` }; + if (run === undefined || run.conclusion !== "success" || run.proof.kind === "unavailable") { + unproven ??= + run === undefined ? `a ${STATUS_CONTEXT} status links ${JSON.stringify(s.targetUrl)}, which GitHub does not return as a run of ${SCORER_PATH} in ${scorer.repo}` + : run.proof.kind === "unavailable" && run.conclusion === "success" ? `scorer run ${id} has no verified output: ${run.proof.why}` + : `scorer run ${id} concluded ${run.conclusion}, yet a success status links it`; + claimed = higher(claimed, word); + continue; + } if (run.proof.inputs !== scorer.inputs || run.proof.radius !== word) return { kind: "refused", why: `the ${STATUS_CONTEXT} status does not match the inputs and rating recorded by scorer run ${id}` }; - if (radius === undefined || rank(word) > rank(radius)) radius = word; + verified = higher(verified, word); } - return radius === undefined ? { kind: "waiting", why: "the scorer has not rated this head yet" } : { kind: "rated", radius }; + if (verified === undefined) return unproven === undefined ? { kind: "waiting", why: "the scorer has not rated this head yet" } : { kind: "refused", why: `${unproven}. Request a fresh score with a close and reopen` }; + return { kind: "rated", radius: claimed === undefined ? verified : higher(verified, claimed) }; } /** The higher of the body's blast radius word and the scorer's reading, and who may merge. */ @@ -208,13 +221,19 @@ if (import.meta.main) { const statuses = (Array.isArray(pages) ? pages.flat() : []).map(parseStatus); const workflow = gh([`repos/${repo}/contents/${SCORER_PATH}?ref=${encodeURIComponent(base)}`, "--jq", ".path"]); if (!workflow.ok && !workflow.why.includes("HTTP 404")) throw new Error(`could not tell whether ${repo} runs the scorer: ${workflow.why}`); - const ids = new Set(statuses.filter((s) => s.context === STATUS_CONTEXT && s.state === "success").map((s) => runAttemptOf(repo, s.targetUrl)).filter((id) => id !== undefined)); - const runs = new Map([...ids].map((id) => { - const run = parseRun(ghJson([`repos/${repo}/actions/runs/${id}`])); + const inputs = inputFingerprint(parseEvent({ pull_request: pr })); + // Only scores of the current inputs count, so only their runs are read. A run GitHub does not + // return, such as a deleted one, stays out of the map, and readStatus treats its score as unproven. + const current = statuses.filter((s) => s.context === STATUS_CONTEXT && s.state === "success" && s.targetUrl?.endsWith(`#inputs=${inputs}`)); + const ids = new Set(current.map((s) => runAttemptOf(repo, s.targetUrl)).filter((id) => id !== undefined)); + const runs = new Map([...ids].flatMap((id) => { + const found = gh([`repos/${repo}/actions/runs/${id}`]); + if (!found.ok) return []; + const run = parseRun(JSON.parse(found.out)); const proof: RunProof = run.status === "completed" && run.conclusion === "success" ? readRunProof(repo, id) : { kind: "unavailable", why: "the run has not completed successfully" }; - return [id, { ...run, proof }] as const; + return [[id, { ...run, proof }] as const]; })); - const scorer: Scorer = { installed: scorerWillRun(workflow.ok, { headRepo, baseRepo, author }), repo, head, inputs: inputFingerprint(parseEvent({ pull_request: pr })), runs }; + const scorer: Scorer = { installed: scorerWillRun(workflow.ok, { headRepo, baseRepo, author }), repo, head, inputs, runs }; const danger = dangerOf({ title: str(pr.title) ?? "", body: str(pr.body) ?? "" }, readStatus(statuses, scorer)); console.log(`merge danger on ${head.slice(0, 7)}: ${danger.verdict}. ${danger.why}`); process.exit(danger.verdict === "agent" ? 0 : danger.verdict === "wait" ? 3 : 1); diff --git a/.claude/skills/open-pr/.pr-kit b/.claude/skills/open-pr/.pr-kit index 17eec338..ae5ab648 100644 --- a/.claude/skills/open-pr/.pr-kit +++ b/.claude/skills/open-pr/.pr-kit @@ -1,2 +1,2 @@ Installed by the PR kit (open-pr/kit.ts). Edit the kit at its source, not here. -version 84 sha256:80:38:ce:a0:56:8d:6d:83:38:06:31:f5:2f:5f:24:80:e8:e8:26:24:b4:2c:95:1c:dc:6b:3b:31:9c:99:99:67 +version 85 sha256:66:ab:65:d4:d7:dd:3a:24:86:65:02:cb:59:80:4e:1b:59:87:ad:10:6b:53:92:b6:38:2b:cf:4e:92:a1:81:2d diff --git a/.claude/skills/open-pr/VERSION b/.claude/skills/open-pr/VERSION index 12dd42c2..ed2646c0 100644 --- a/.claude/skills/open-pr/VERSION +++ b/.claude/skills/open-pr/VERSION @@ -1 +1 @@ -84 sha256:80:38:ce:a0:56:8d:6d:83:38:06:31:f5:2f:5f:24:80:e8:e8:26:24:b4:2c:95:1c:dc:6b:3b:31:9c:99:99:67 +85 sha256:66:ab:65:d4:d7:dd:3a:24:86:65:02:cb:59:80:4e:1b:59:87:ad:10:6b:53:92:b6:38:2b:cf:4e:92:a1:81:2d