From a8ff487cb0ab04ba9d9ab3defd78348445cdf3bc Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sat, 24 Oct 2026 13:29:11 -0400 Subject: [PATCH 1/5] feat(pr-review-cli): skip the key job while the key's credit stop is recent A key out of credits still started the key job on every run: runner isolation, checkout and prep, a credit stop, then the handoff. A new key-credit job reads this workflow's completed runs from the last key-credit-window-hours (default 6) with the job token (actions: read): one list call and at most three job-list calls. When the key job ended in the launcher's credit-stop class in that window, the key job is skipped and the subscription job reviews; the status job then reads the subscription job's scope, and the last summary falls back to that leg. The key job records a credit stop as a step named for its run, run only after lane-class credit-stop, so an auth stop never counts; a skipping run carries the origin forward the same way, so the window counts from the credit stop. Any read failure or unclear answer runs the key job as before. Callers grant actions: read. Co-Authored-By: Claude Opus 5.5 --- .github/actions/key-credit-gate/action.yml | 46 ++++ .github/actions/key-credit-gate/gate.cjs | 241 ++++++++++++++++++++ .github/scripts/key-credit-gate.test.cjs | 245 +++++++++++++++++++++ .github/scripts/pr-review-cli.test.cjs | 194 +++++++++++++++- .github/workflows/pr-review-cli.yml | 140 ++++++++++-- README.md | 12 +- 6 files changed, 855 insertions(+), 23 deletions(-) create mode 100644 .github/actions/key-credit-gate/action.yml create mode 100644 .github/actions/key-credit-gate/gate.cjs create mode 100644 .github/scripts/key-credit-gate.test.cjs diff --git a/.github/actions/key-credit-gate/action.yml b/.github/actions/key-credit-gate/action.yml new file mode 100644 index 00000000..8e0683c0 --- /dev/null +++ b/.github/actions/key-credit-gate/action.yml @@ -0,0 +1,46 @@ +name: Key credit gate +description: > + Before a keyed lane's key job: whether that job should skip its key because + the key ended in a credit stop in this workflow inside the window. Reads + the workflow's completed runs created inside the window (one list call) + and at most three of their job lists, with the job token, which needs + actions: read. Any failure or an unclear answer leaves skip-key `false`, + so the key job runs as before. Writes one log line either way; a skip is a + notice naming the run whose credit stop caused it. See gate.cjs. + +inputs: + key-job: + description: >- + The key job's display name, lower-case kebab (for example + claude-review-key). The record steps are looked up in this job and in + `-credit`, the job that runs this action. + required: true + window-hours: + description: How far back a credit stop counts, in hours (0 turns the gate off; at most 168). + required: false + default: "6" + github-token: + description: The job token, with actions: read. + required: false + default: ${{ github.token }} + +outputs: + skip-key: + description: "`true` when the key job should skip its key, else `false` or empty." + value: ${{ steps.gate.outputs.skip-key }} + origin-run: + description: The run id whose credit stop caused the skip, or empty. + value: ${{ steps.gate.outputs.origin-run }} + +runs: + using: composite + steps: + - name: Read the key's credit history + id: gate + shell: bash + env: + KEY_JOB: ${{ inputs.key-job }} + WINDOW_HOURS: ${{ inputs.window-hours }} + WORKFLOW_REF: ${{ github.workflow_ref }} + GITHUB_TOKEN: ${{ inputs.github-token }} + run: node "$GITHUB_ACTION_PATH/gate.cjs" || true diff --git a/.github/actions/key-credit-gate/gate.cjs b/.github/actions/key-credit-gate/gate.cjs new file mode 100644 index 00000000..5f17262d --- /dev/null +++ b/.github/actions/key-credit-gate/gate.cjs @@ -0,0 +1,241 @@ +// key-credit-gate: whether a keyed lane's key job should skip the model +// because the key ended in a credit stop inside the window. +// +// It reads the calling workflow's completed runs created inside the window +// (one list call) and, newest first, at most MAX_JOB_READS of their job lists +// (one call each), and stops at the first run that settles the question: +// - the key job or this gate's job has a successful step whose name is +// RECORD_STEP and an origin run id that is one of the listed runs: skip, +// citing that origin; +// - that record names a run outside the list: the credit stop expired, run; +// - the key job's model step (MODEL_STEP) ran with no record: the key ran +// with no credit stop, run. +// A run that settles nothing (cancelled, no key job, no review needed) is +// passed over. Any failure, an unreadable answer or no settling run within +// the cap runs the key, as before this gate: it never skips the subscription +// path and never fails the step. +// +// The record step runs in the key job only after the launcher's lane-class +// `credit-stop` (run-claude-cli: classify.cjs isCreditStop), so an auth stop +// (401, or 402 without the credit text) never counts, and names its own run. +// When this gate skips the key, its job's record step names the origin run +// instead, so the window runs from the credit stop itself, not from the +// last skip. A same-repository pull request that edits the caller workflow +// could write a record; the worst it does is move reviews to the +// subscription for the window, and such writers already hold the secrets. +// More than 100 completed runs in the window can push the origin off the +// list, which runs the key. +"use strict"; + +const fs = require("node:fs"); +const path = require("node:path"); + +const RECORD_STEP = "Record the key's credit stop in run "; +const RECORD_PATTERN = /^Record the key's credit stop in run (\d{1,20})$/u; +const MODEL_STEP = "Claude review"; +const MAX_JOB_READS = 3; +const MAX_WINDOW_HOURS = 168; +const WORKFLOW_REF = /^[\w.-]+\/[\w.-]+\/\.github\/workflows\/([\w.-]+\.ya?ml)@/u; + +// The rate-limit reading report-api-budget reads, as the scope step writes +// it. A failed write only drops the reading. +function recordBudget(response, runnerTemp) { + if (!runnerTemp) return; + const values = ["limit", "remaining", "used", "reset", "resource"].map( + (key) => { + const value = String(response.headers?.get?.(`x-ratelimit-${key}`) ?? ""); + return (key === "resource" ? /^[a-z_]+$/u : /^\d+$/u).test(value) + ? value + : "na"; + }, + ); + try { + const dir = path.join(runnerTemp, "api-budget"); + fs.mkdirSync(dir, { recursive: true }); + fs.appendFileSync( + path.join(dir, "github-token"), + `${response.status} ${values.join(" ")}\n`, + ); + } catch { + // No reading: the notice is only missing one. + } +} + +// The job named `name` in one run's job list: exactly, or ` / +// name` when a caller's job runs the reusable. +function jobNamed(jobs, name) { + return jobs.find( + (job) => + typeof job?.name === "string" && + job.status === "completed" && + Array.isArray(job.steps) && + (job.name === name || job.name.endsWith(` / ${name}`)), + ); +} + +// What one run's job list settles: { skip, origin } or { skip: false, +// reason }, or null when it settles nothing. The record is the key job's +// own, or the gate job's (`-credit`) carried from an earlier run. +function judgeRun(jobs, keyJob, windowIds) { + const list = Array.isArray(jobs) ? jobs : []; + const key = jobNamed(list, keyJob); + const gate = jobNamed(list, `${keyJob}-credit`); + for (const step of [...(gate?.steps ?? []), ...(key?.steps ?? [])]) { + const match = + step?.conclusion === "success" && typeof step.name === "string" + ? step.name.match(RECORD_PATTERN) + : null; + if (!match) continue; + return windowIds.has(match[1]) + ? { skip: true, origin: match[1] } + : { + skip: false, + reason: `the credit stop in run ${match[1]} is outside the window`, + }; + } + const model = key?.steps.find((step) => step?.name === MODEL_STEP); + if (model?.conclusion === "success" || model?.conclusion === "failure") { + return { skip: false, reason: "the key ran with no credit stop" }; + } + return null; +} + +// The decision, from injected readers; never throws. +async function decide({ + listRuns, + listJobs, + currentRunId, + keyJob, + maxJobReads = MAX_JOB_READS, +}) { + let runs; + try { + runs = await listRuns(); + } catch (error) { + return { skip: false, reason: `the run list was not read (${error.message})` }; + } + if (!Array.isArray(runs)) { + return { skip: false, reason: "the run list was not a list" }; + } + const candidates = runs.filter( + (run) => Number.isSafeInteger(run?.id) && String(run.id) !== currentRunId, + ); + const windowIds = new Set(candidates.map((run) => String(run.id))); + for (const run of candidates.slice(0, maxJobReads)) { + let jobs; + try { + jobs = await listJobs(run.id); + } catch (error) { + return { + skip: false, + reason: `run ${run.id}'s jobs were not read (${error.message})`, + }; + } + const verdict = judgeRun(jobs, keyJob, windowIds); + if (verdict) return { ...verdict, run: String(run.id) }; + } + return { + skip: false, + reason: + candidates.length === 0 + ? "no completed run in the window" + : `no run of the newest ${Math.min(candidates.length, maxJobReads)} settles it`, + }; +} + +function readEnv(env) { + const keyJob = env.KEY_JOB ?? ""; + const hours = Number(env.WINDOW_HOURS); + const workflow = (env.WORKFLOW_REF ?? "").match(WORKFLOW_REF)?.[1] ?? null; + const repository = env.GITHUB_REPOSITORY ?? ""; + return { + keyJob, + hours, + workflow, + repository, + api: env.GITHUB_API_URL || "https://api.github.com", + server: env.GITHUB_SERVER_URL || "https://github.com", + runId: env.GITHUB_RUN_ID ?? "", + token: env.GITHUB_TOKEN ?? "", + runnerTemp: env.RUNNER_TEMP ?? "", + valid: + /^[a-z][a-z0-9-]{0,99}$/u.test(keyJob) && + /^[\w.-]+\/[\w.-]+$/u.test(repository) && + workflow !== null && + /^\d{1,20}$/u.test(env.GITHUB_RUN_ID ?? ""), + }; +} + +async function main(env = process.env, fetchImpl = globalThis.fetch, now = () => new Date()) { + const config = readEnv(env); + const lines = []; + let verdict; + if (!(config.hours > 0 && config.hours <= MAX_WINDOW_HOURS)) { + verdict = { skip: false, reason: "the window is off" }; + } else if (!config.valid || config.token === "") { + verdict = { skip: false, reason: "the run, workflow, job or token is unreadable" }; + } else { + const get = async (url) => { + const response = await fetchImpl(url, { + headers: { + Accept: "application/vnd.github+json", + Authorization: `Bearer ${config.token}`, + "X-GitHub-Api-Version": "2022-11-28", + }, + signal: AbortSignal.timeout(15000), + }); + recordBudget(response, config.runnerTemp); + if (!response.ok) throw new Error(`HTTP ${response.status}`); + return response.json(); + }; + const since = new Date(now().getTime() - config.hours * 3600 * 1000) + .toISOString() + .replace(/\.\d{3}Z$/u, "Z"); + const base = `${config.api}/repos/${config.repository}/actions`; + verdict = await decide({ + currentRunId: config.runId, + keyJob: config.keyJob, + listRuns: async () => + ( + await get( + `${base}/workflows/${config.workflow}/runs?status=completed&exclude_pull_requests=true&per_page=100&created=${encodeURIComponent(`>=${since}`)}`, + ) + )?.workflow_runs, + listJobs: async (id) => + (await get(`${base}/runs/${id}/jobs?filter=latest&per_page=100`))?.jobs, + }); + } + if (verdict.skip) { + lines.push( + `::notice title=key-credit-gate::${config.keyJob} skips the key: the key's credit stop in run ${verdict.origin} (${config.server}/${config.repository}/actions/runs/${verdict.origin}) is inside the ${config.hours}-hour window; the subscription job reviews`, + ); + } else { + lines.push(`key-credit-gate: ${config.keyJob || "the key job"} runs the key: ${verdict.reason}`); + } + if (env.GITHUB_OUTPUT) { + fs.appendFileSync( + env.GITHUB_OUTPUT, + `skip-key=${verdict.skip}\norigin-run=${verdict.skip ? verdict.origin : ""}\n`, + ); + } + return { verdict, lines }; +} + +if (require.main === module) { + main() + .then(({ lines }) => process.stdout.write(`${lines.join("\n")}\n`)) + .catch(() => { + // Fail open: no output leaves skip-key empty, so the key runs. + process.stdout.write("key-credit-gate: the check failed; the key runs\n"); + }); +} + +module.exports = { + MAX_JOB_READS, + MODEL_STEP, + RECORD_PATTERN, + RECORD_STEP, + decide, + judgeRun, + main, +}; diff --git a/.github/scripts/key-credit-gate.test.cjs b/.github/scripts/key-credit-gate.test.cjs new file mode 100644 index 00000000..df8f23c7 --- /dev/null +++ b/.github/scripts/key-credit-gate.test.cjs @@ -0,0 +1,245 @@ +"use strict"; + +// key-credit-gate reads a keyed lane's own recent runs and skips the key job +// only on a credit-stop record inside the window; every other answer, and +// every failure, runs the key. Its calls: one run list, then at most +// MAX_JOB_READS job lists. + +const assert = require("node:assert/strict"); +const fs = require("node:fs"); +const path = require("node:path"); +const test = require("node:test"); + +const { + MAX_JOB_READS, + decide, + judgeRun, + main, +} = require("../actions/key-credit-gate/gate.cjs"); +const { makeTemporaryDirectory } = require("./temporary-directories.cjs"); + +const KEY = "claude-review-key"; +const step = (name, conclusion = "success") => ({ name, conclusion }); +const keyJob = (steps, name = `pr-review / ${KEY}`) => ({ + name, + status: "completed", + steps, +}); +const RAN = step("Claude review", "failure"); +const credit = (id) => step(`Record the key's credit stop in run ${id}`); + +test("a run settles on a record in the window, a record outside it, or a key that ran", () => { + const ids = new Set(["10", "11"]); + assert.deepEqual(judgeRun([keyJob([RAN, credit(10)])], KEY, ids), { + skip: true, + origin: "10", + }); + // The gate job's carried record counts the same. + assert.deepEqual( + judgeRun( + [keyJob([credit(11)], `pr-review / ${KEY}-credit`), keyJob([], "x")], + KEY, + ids, + ), + { skip: true, origin: "11" }, + ); + assert.equal(judgeRun([keyJob([credit(9)])], KEY, ids).skip, false); + assert.equal(judgeRun([keyJob([RAN])], KEY, ids).skip, false); + assert.equal( + judgeRun([keyJob([step("Claude review", "success")])], KEY, ids).skip, + false, + ); +}); + +test("a run settles nothing without a finished key job whose model or record ran", () => { + const ids = new Set(["10"]); + for (const jobs of [ + [], + null, + [keyJob([step("Claude review", "skipped")])], + [{ ...keyJob([RAN]), status: "in_progress" }], + // A record that did not succeed, and another lane's job. + [keyJob([credit(10).name].map((name) => step(name, "skipped")))], + [keyJob([RAN], "pr-review / security-review-key")], + [keyJob([RAN], `pr-review / not-${KEY}`)], + ]) { + assert.equal(judgeRun(jobs, KEY, ids), null, JSON.stringify(jobs)); + } +}); + +test("an auth stop or any other class never reads as a credit stop", () => { + // Only the record step's exact name counts, and the workflow runs it only + // after lane-class credit-stop (pr-review-cli.test.cjs holds that). + const ids = new Set(["10"]); + for (const name of [ + "Record the key's credit stop in run ", + "Record the key's credit stop in run 10 ", + "Record the key's auth stop in run 10", + `Record the key's credit stop in run \${{ github.run_id }}`, + ]) { + assert.equal( + judgeRun([keyJob([RAN, step(name)])], KEY, ids).skip, + false, + name, + ); + } +}); + +test("decide reads newest first, skips the current run and caps its job reads", async () => { + const runs = [ + { id: 7 }, + { id: 6 }, + { id: 5 }, + { id: 4 }, + { id: 3 }, + { id: 2 }, + ]; + const read = []; + const verdict = await decide({ + currentRunId: "7", + keyJob: KEY, + listRuns: async () => runs, + listJobs: async (id) => { + read.push(id); + return id === 2 ? [keyJob([credit(2)])] : []; + }, + }); + assert.deepEqual(read, [6, 5, 4]); + assert.equal(read.length, MAX_JOB_READS); + assert.equal(verdict.skip, false); + + const read2 = []; + const skip = await decide({ + currentRunId: "9", + keyJob: KEY, + listRuns: async () => runs, + listJobs: async (id) => { + read2.push(id); + return id === 6 ? [keyJob([credit(3)], `r / ${KEY}-credit`)] : []; + }, + }); + assert.deepEqual(skip, { skip: true, origin: "3", run: "6" }); + assert.deepEqual(read2, [7, 6]); +}); + +test("decide fails open on every read failure", async () => { + const boom = async () => { + throw new Error("HTTP 403"); + }; + for (const args of [ + { listRuns: boom, listJobs: async () => [] }, + { listRuns: async () => ({}), listJobs: async () => [] }, + { listRuns: async () => [{ id: 1 }], listJobs: boom }, + { listRuns: async () => [], listJobs: async () => [] }, + ]) { + const verdict = await decide({ currentRunId: "9", keyJob: KEY, ...args }); + assert.equal(verdict.skip, false); + } +}); + +function env(dir, extra = {}) { + return { + KEY_JOB: KEY, + WINDOW_HOURS: "6", + WORKFLOW_REF: + "o/r/.github/workflows/pr-review-hosted.yml@refs/pull/1/merge", + GITHUB_REPOSITORY: "o/r", + GITHUB_RUN_ID: "100", + GITHUB_TOKEN: "t", + GITHUB_API_URL: "https://api.example", + GITHUB_SERVER_URL: "https://example", + GITHUB_OUTPUT: path.join(dir, "output"), + RUNNER_TEMP: dir, + ...extra, + }; +} + +function fakeFetch(routes) { + const urls = []; + const fetchImpl = async (url, init) => { + urls.push(url); + assert.equal(init.headers.Authorization, "Bearer t"); + const [, body] = routes.find(([pattern]) => pattern.test(url)) ?? []; + const headers = new Map([ + ["x-ratelimit-limit", "1000"], + ["x-ratelimit-remaining", String(1000 - urls.length)], + ["x-ratelimit-used", String(urls.length)], + ["x-ratelimit-reset", "1"], + ["x-ratelimit-resource", "core"], + ]); + return { + ok: body !== undefined, + status: body === undefined ? 404 : 200, + headers, + json: async () => body, + }; + }; + return { urls, fetchImpl }; +} + +test("main skips on a credit stop in the window with one list call and one jobs call, and says which run", async () => { + const dir = makeTemporaryDirectory("key-credit-gate-"); + const { urls, fetchImpl } = fakeFetch([ + [ + /\/workflows\/pr-review-hosted\.yml\/runs\?/u, + { workflow_runs: [{ id: 99 }, { id: 98 }] }, + ], + [/\/runs\/99\/jobs\?/u, { jobs: [keyJob([RAN, credit(98)])] }], + ]); + const now = () => new Date("2026-10-24T12:00:00Z"); + const { verdict, lines } = await main(env(dir), fetchImpl, now); + assert.deepEqual(verdict, { skip: true, origin: "98", run: "99" }); + assert.equal(urls.length, 2); + assert.match(urls[0], /status=completed/u); + assert.match(urls[0], /created=%3E%3D2026-10-24T06%3A00%3A00Z/u); + assert.equal(lines.length, 1); + assert.match( + lines[0], + /^::notice title=key-credit-gate::claude-review-key skips the key: the key's credit stop in run 98 \(https:\/\/example\/o\/r\/actions\/runs\/98\)/u, + ); + assert.equal( + fs.readFileSync(path.join(dir, "output"), "utf8"), + "skip-key=true\norigin-run=98\n", + ); + // Each response's rate-limit reading, for report-api-budget. + assert.equal( + fs.readFileSync(path.join(dir, "api-budget", "github-token"), "utf8"), + "200 1000 999 1 1 core\n200 1000 998 2 1 core\n", + ); +}); + +test("main runs the key on a refused read, an unreadable ref or a window of 0, spending nothing on the last two", async () => { + for (const [extra, calls] of [ + [{}, 1], + [{ WINDOW_HOURS: "0" }, 0], + [{ WINDOW_HOURS: "abc" }, 0], + [{ WORKFLOW_REF: "nonsense" }, 0], + [{ GITHUB_TOKEN: "" }, 0], + ]) { + const dir = makeTemporaryDirectory("key-credit-gate-"); + const { urls, fetchImpl } = fakeFetch([]); + const { verdict, lines } = await main(env(dir, extra), fetchImpl); + assert.equal(verdict.skip, false, JSON.stringify(extra)); + assert.equal(urls.length, calls, JSON.stringify(extra)); + assert.match(lines[0], /^key-credit-gate: .* runs the key: /u); + assert.equal( + fs.readFileSync(path.join(dir, "output"), "utf8"), + "skip-key=false\norigin-run=\n", + ); + } +}); + +test("the action passes the caller's workflow ref and the window to the gate", () => { + const { parseWorkflow } = require("./workflow-yaml.cjs"); + const action = parseWorkflow( + fs.readFileSync( + path.join(__dirname, "..", "actions", "key-credit-gate", "action.yml"), + "utf8", + ), + ); + const run = action.runs.steps[0]; + assert.equal(run.env.WORKFLOW_REF, `\${{ github.workflow_ref }}`); + assert.equal(run.env.WINDOW_HOURS, `\${{ inputs.window-hours }}`); + assert.equal(action.inputs["window-hours"].default, "6"); + assert.match(run.run, /\|\| true$/u); +}); diff --git a/.github/scripts/pr-review-cli.test.cjs b/.github/scripts/pr-review-cli.test.cjs index 862ef032..afd0a4b6 100644 --- a/.github/scripts/pr-review-cli.test.cjs +++ b/.github/scripts/pr-review-cli.test.cjs @@ -119,7 +119,9 @@ test("each secret-bearing job turns away forks, drafts, Dependabot, unlisted bot github, inputs: { "allowed-bot-sender-ids": "", ...inputs }, needs: { + "key-credit": { result: "success", outputs: {} }, "review-key": { + result: "success", outputs: { handoff: "true", "credential-present": "true", @@ -129,7 +131,7 @@ test("each secret-bearing job turns away forks, drafts, Dependabot, unlisted bot }, }), ); - for (const job of [key, subscription]) { + for (const job of [workflow.jobs["key-credit"], key, subscription]) { assert.equal(passes(job, PR), true, job.name); const refused = [ withPr((g) => { @@ -176,15 +178,32 @@ test("each secret-bearing job turns away forks, drafts, Dependabot, unlisted bot } }); -test("the subscription job starts only on an exact handoff or a key job with no key", () => { - const starts = (outputs) => +test("the subscription job starts only on an exact handoff, a key job with no key or a credit skip", () => { + const starts = (outputs, result = "success", skipKey = "") => Boolean( evaluate(subscription.if, { github: PR, inputs: { "allowed-bot-sender-ids": "" }, - needs: { "review-key": { outputs } }, + needs: { + "key-credit": { outputs: { "skip-key": skipKey } }, + "review-key": { result, outputs }, + }, }), ); + // A key job skipped by key-credit's exact `true`, and no other skip. + assert.equal(starts({}, "skipped", "true"), true); + for (const skipKey of ["", "false", "True", "true ", "1"]) { + assert.equal( + starts({}, "skipped", skipKey), + false, + JSON.stringify(skipKey), + ); + } + // A key job that failed or was cancelled starts nothing, as before. + for (const result of ["failure", "cancelled"]) { + assert.equal(starts({ handoff: "true" }, result, "true"), false, result); + } + assert.deepEqual(subscription.needs, ["key-credit", "review-key"]); assert.equal(starts({ handoff: "true", "credential-present": "true" }), true); assert.equal(starts({ handoff: "", "credential-present": "false" }), true); for (const handoff of ["", "false", "True", "true ", "1"]) { @@ -196,10 +215,175 @@ test("the subscription job starts only on an exact handoff or a key job with no } // A review the scope step skipped sets no credential-present at all. assert.equal(starts({ handoff: "", "credential-present": "" }), false); - assert.equal(subscription.needs, "review-key"); assert.doesNotMatch(subscription.if, /always\(\)|failure\(\)/u); }); +test("the key job runs unless key-credit says exactly true, a failed key-credit included", () => { + const runs = (skipKey, result = "success") => + Boolean( + evaluate(key.if, { + github: PR, + inputs: { "allowed-bot-sender-ids": "" }, + needs: { "key-credit": { result, outputs: { "skip-key": skipKey } } }, + }), + ); + assert.equal(runs("true"), false); + for (const skipKey of ["", "false", "True"]) { + assert.equal(runs(skipKey), true, JSON.stringify(skipKey)); + } + assert.equal(runs("", "failure"), true); + assert.equal(runs("", "skipped"), true); + assert.equal(key.needs, "key-credit"); + const gate = workflow.jobs["key-credit"]; + assert.deepEqual(gate.permissions, { actions: "read" }); + assert.equal(gate.steps[0].uses, "$/.github/actions/key-credit-gate"); + assert.equal(gate.outputs["skip-key"], `\${{ steps.gate.outputs.skip-key }}`); + assert.doesNotMatch(JSON.stringify(gate), /secrets\./u); +}); + +test("the credit record steps carry the names and conditions key-credit-gate reads", () => { + const { + MODEL_STEP, + RECORD_PATTERN, + } = require("../actions/key-credit-gate/gate.cjs"); + const render = (name, context) => + name.replace(/\$\{\{\s*([^}]+?)\s*\}\}/gu, (_, expr) => + String(evaluate(expr, context)), + ); + for (const job of [key, subscription]) { + const record = job.steps.find((s) => s.name.startsWith("Record the key")); + assert.match( + render(record.name, { github: { run_id: "123" } }), + RECORD_PATTERN, + ); + assert.equal(step(job, "claude").name, MODEL_STEP); + const runsAfter = (laneClass) => + Boolean( + evaluate(record.if, { + steps: { claude: { outputs: { "lane-class": laneClass } } }, + }), + ); + assert.equal(runsAfter("credit-stop"), true); + for (const laneClass of ["auth", "rate-limit", "", "other"]) { + assert.equal(runsAfter(laneClass), false, laneClass); + } + } + const carry = workflow.jobs["key-credit"].steps[1]; + const context = (skip) => ({ + steps: { gate: { outputs: { "skip-key": skip, "origin-run": "99" } } }, + }); + assert.match(render(carry.name, context("true")), RECORD_PATTERN); + assert.equal( + render(carry.name, context("true")).match(RECORD_PATTERN)[1], + "99", + ); + assert.equal(Boolean(evaluate(carry.if, context("true"))), true); + assert.equal(Boolean(evaluate(carry.if, context(""))), false); +}); + +test("after a credit skip the status job runs and reads the subscription job's scope", () => { + const needs = (keyResult, subResult) => ({ + "review-key": { + result: keyResult, + outputs: + keyResult === "skipped" + ? {} + : { "scope-review": "key", "pr-files": "k" }, + }, + "review-subscription": { + result: subResult, + outputs: + subResult === "skipped" + ? {} + : { "scope-review": "sub", "pr-files": "s" }, + }, + }); + const runs = (k, s) => Boolean(evaluate(status.if, { needs: needs(k, s) })); + assert.equal(runs("skipped", "success"), true); + assert.equal(runs("success", "skipped"), true); + assert.equal(runs("skipped", "skipped"), false); + for (const name of ["Publish the review", "Apply the hold"]) { + const env = status.steps.find((s) => s.name === name).env; + const read = (k, s, v) => evaluate(env[v], { needs: needs(k, s) }); + assert.equal(read("skipped", "success", "PR_FILES"), "s", name); + assert.equal(read("success", "success", "PR_FILES"), "k", name); + assert.equal(read("success", "skipped", "PR_FILES"), "k", name); + } + const publish = status.steps.find((s) => s.name === "Publish the review").env; + for (const v of [ + "SCOPE_REVIEW", + "SKIP", + "SKIP_REASON", + "SINCE", + "SCOPE", + "MARKER_ID", + "MERGE_BASE", + ]) { + assert.match( + publish[v], + /needs\.review-key\.result == 'skipped' && needs\.review-subscription\.outputs\./u, + v, + ); + } + assert.equal( + evaluate(publish.SCOPE_REVIEW, { needs: needs("skipped", "success") }), + "sub", + ); + // The subscription leg records the skip as the credit stop it follows. + assert.equal( + evaluate(step(subscription, "review-outcome").with["fallback-reason"], { + needs: { "review-key": { result: "skipped", outputs: {} } }, + }), + "credit-stop", + ); +}); + +test("the status job takes the last summary from the subscription leg when the key leg left none", async () => { + const dir = makeTemporaryDirectory("pr-review-cli-handoffs-"); + const crypto = require("node:crypto"); + const write = (leg, data) => { + const bytes = Buffer.from(JSON.stringify(data)); + fs.writeFileSync(path.join(dir, `${leg}.json`), bytes); + return crypto.createHash("sha256").update(bytes).digest("hex"); + }; + const check = status.steps.find((s) => s.id === "handoffs"); + const run = async (env) => { + const outputs = {}; + const core = { + setOutput: (k, v) => { + outputs[k] = v; + }, + warning: () => {}, + }; + const saved = { ...process.env }; + Object.assign(process.env, { + HANDOFF_DIR: dir, + KEY_SHA256: "", + SUB_SHA256: "", + ...env, + }); + try { + await new AsyncFunction("require", "core", check.with.script)( + require, + core, + ); + } finally { + for (const k of ["HANDOFF_DIR", "KEY_SHA256", "SUB_SHA256"]) { + if (k in saved) process.env[k] = saved[k]; + else delete process.env[k]; + } + } + return outputs; + }; + const sub = write("review-subscription", { "last-summary": "from sub" }); + assert.equal((await run({ SUB_SHA256: sub }))["last-summary"], "from sub"); + const keyLeg = write("review-key", { "last-summary": "from key" }); + assert.equal( + (await run({ KEY_SHA256: keyLeg, SUB_SHA256: sub }))["last-summary"], + "from key", + ); +}); + test("the subscription job repeats the key job's steps but for the credential and the fallback reason", () => { const strip = (steps, secret, input) => steps.map((s) => { diff --git a/.github/workflows/pr-review-cli.yml b/.github/workflows/pr-review-cli.yml index 743df5f1..bad87ad6 100644 --- a/.github/workflows/pr-review-cli.yml +++ b/.github/workflows/pr-review-cli.yml @@ -22,6 +22,7 @@ name: pr-review-cli # # request against this grant at startup, so summary mode needs it # # too, though it writes nothing. # permissions: +# actions: read # the key-credit job reads this workflow's runs # contents: read # pull-requests: write # uses: melodic-software/ci-workflows/.github/workflows/pr-review-cli.yml@ @@ -48,6 +49,14 @@ name: pr-review-cli # That job runs on a fresh ubuntu-24.04 runner, repeats every hardening # step and takes no artifact or cache from the key job. Either secret # may be empty. +# - The key-credit job runs first, holds no secret and no write, and reads +# only this workflow's own runs with the job token (actions: read, so +# the caller grants it). When the key job ended in a credit stop +# (lane-class `credit-stop`) in a run created inside +# key-credit-window-hours, the key job is skipped and the subscription +# job reviews, its scope read by the status job in the key job's place. +# A failed or unclear read runs the key job as before +# (.github/actions/key-credit-gate). # - The model jobs cannot write: contents: read and pull-requests: read, # no id-token, and `cache-mode: none`. The launcher step's env holds the # one Claude credential and no GitHub token. Before the checkout the job @@ -83,7 +92,8 @@ name: pr-review-cli # output; it downloads from this run only, with no token. It # checks every inline anchor against the pull request's diff at the head # (one bad anchor sends the whole batch to the summary): the hunk ranges -# the key job's scope step read before any model step ran, used only +# the key job's scope step (the subscription job's when key-credit +# skipped the key job) read before any model step ran, used only # when a fresh read shows the head has not moved, else a read of its # own. Each step writes its own outputs file, so the later model step # cannot rewrite the scope step's outputs; the status job already trusts @@ -237,6 +247,14 @@ on: scrutiny. A match never holds the pull request by itself. type: string default: "" + key-credit-window-hours: + description: >- + Hours a key's credit stop in this workflow keeps later runs off the + key: their key job is skipped and the subscription job reviews. + The first run after the window tries the key again, so a new cycle + or a replaced key is picked up. 0 always tries the key. + type: number + default: 6 outputs: hold: description: "`true` when this run held the pull request, else `false`." @@ -258,11 +276,70 @@ permissions: contents: read jobs: + # Whether the key is still out of credits: a key-job credit stop in this + # workflow inside the window skips the key job. Its record step carries the + # origin run forward in its name, which the next run's read finds, so the + # window counts from the credit stop, not the last skip. Every failure runs + # the key job: its `if` starts with !cancelled(). + key-credit: + name: ${{ inputs.lane == 'security' && 'security-review-key-credit' || 'claude-review-key-credit' }} + runs-on: ubuntu-24.04 + timeout-minutes: 10 + if: >- + github.event_name == 'pull_request' + && github.event.pull_request.state == 'open' + && github.event.pull_request.draft == false + && github.event.pull_request.head.repo.full_name == github.repository + && github.actor != 'dependabot[bot]' + && github.event.pull_request.user.login != 'dependabot[bot]' + && (!endsWith(github.actor, '[bot]') + || (inputs.allowed-bot-sender-ids != '' + && !startsWith(inputs.allowed-bot-sender-ids, ',') + && !endsWith(inputs.allowed-bot-sender-ids, ',') + && !contains(inputs.allowed-bot-sender-ids, ',,') + && github.event.sender.id + && contains(format(',{0},', inputs.allowed-bot-sender-ids), + format(',{0},', github.event.sender.id)))) + permissions: + actions: read + cache-mode: none + outputs: + skip-key: ${{ steps.gate.outputs.skip-key }} + steps: + - name: Read the key's credit history + id: gate + continue-on-error: true + timeout-minutes: 2 + uses: $/.github/actions/key-credit-gate + with: + key-job: ${{ inputs.lane == 'security' && 'security-review-key' || 'claude-review-key' }} + window-hours: ${{ inputs.key-credit-window-hours }} + + - name: Record the key's credit stop in run ${{ steps.gate.outputs.origin-run }} + if: steps.gate.outputs.skip-key == 'true' + continue-on-error: true + timeout-minutes: 1 + run: echo "carried to the next run's key-credit read" + + # The job token's api-budget notice, from the readings the gate's + # requests left. It spends no request. + - name: Log the API budget + if: always() + continue-on-error: true + timeout-minutes: 1 + uses: $/.github/actions/report-api-budget + with: + tokens: github-token + review-key: name: ${{ inputs.lane == 'security' && 'security-review-key' || 'claude-review-key' }} + needs: key-credit runs-on: ubuntu-24.04 - timeout-minutes: ${{ inputs.lane == 'security' && 45 || 43 }} + timeout-minutes: ${{ inputs.lane == 'security' && 46 || 44 }} if: >- + needs.key-credit.outputs.skip-key != 'true' + && !cancelled() + && github.event_name == 'pull_request' && github.event.pull_request.state == 'open' && github.event.pull_request.draft == false @@ -1101,6 +1178,16 @@ jobs: cli-version: ${{ steps.compose.outputs.cli-version }} leg-timeout-minutes: ${{ inputs.lane == 'security' && '13' || '11' }} + # Read by the next runs' key-credit job: a step named for this run that + # succeeds only after the launcher's credit stop, which only the key + # leg can end in. Its name is the record; keep it equal to + # key-credit-gate's RECORD_PATTERN. + - name: Record the key's credit stop in run ${{ github.run_id }} + if: always() && steps.claude.outputs.lane-class == 'credit-stop' + continue-on-error: true + timeout-minutes: 1 + run: echo "the key ended in a credit stop" + # The CLI's init message must report the mode asked for; dontAsk may # read default, which run-claude-cli's `--permission-prompts none` # makes deny every call no rule allows, as dontAsk does. A mismatch, or @@ -1329,11 +1416,14 @@ jobs: # writes them into a script. review-subscription: name: ${{ inputs.lane == 'security' && 'security-review-subscription' || 'claude-review-subscription' }} - needs: review-key + needs: [key-credit, review-key] runs-on: ubuntu-24.04 - timeout-minutes: ${{ inputs.lane == 'security' && 45 || 43 }} + timeout-minutes: ${{ inputs.lane == 'security' && 46 || 44 }} if: >- - (needs.review-key.outputs.handoff == 'true' || needs.review-key.outputs.credential-present == 'false') + ((needs.review-key.result == 'success' + && (needs.review-key.outputs.handoff == 'true' || needs.review-key.outputs.credential-present == 'false')) + || (needs.review-key.result == 'skipped' && needs.key-credit.outputs.skip-key == 'true')) + && !cancelled() && github.event_name == 'pull_request' && github.event.pull_request.state == 'open' @@ -2173,6 +2263,16 @@ jobs: cli-version: ${{ steps.compose.outputs.cli-version }} leg-timeout-minutes: ${{ inputs.lane == 'security' && '13' || '11' }} + # Read by the next runs' key-credit job: a step named for this run that + # succeeds only after the launcher's credit stop, which only the key + # leg can end in. Its name is the record; keep it equal to + # key-credit-gate's RECORD_PATTERN. + - name: Record the key's credit stop in run ${{ github.run_id }} + if: always() && steps.claude.outputs.lane-class == 'credit-stop' + continue-on-error: true + timeout-minutes: 1 + run: echo "the key ended in a credit stop" + # The CLI's init message must report the mode asked for; dontAsk may # read default, which run-claude-cli's `--permission-prompts none` # makes deny every call no rule allows, as dontAsk does. A mismatch, or @@ -2381,7 +2481,7 @@ jobs: known-failure-class: ${{ steps.claude.outputs.lane-class }} handoff: ${{ steps.claude.outputs.handoff }} # Why this job ran, for this leg's lane record. - fallback-reason: ${{ needs.review-key.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.review-key.outputs.lane-class == 'auth' && 'auth' || needs.review-key.outputs.credential-present == 'false' && 'no-key' || '' }} + fallback-reason: ${{ (needs.review-key.result == 'skipped' || needs.review-key.outputs.lane-class == 'credit-stop') && 'credit-stop' || needs.review-key.outputs.lane-class == 'auth' && 'auth' || needs.review-key.outputs.credential-present == 'false' && 'no-key' || '' }} # The job token's api-budget notice, from the readings the scope # step's requests and the shell steps' `gh api -i` files left. It @@ -2405,7 +2505,7 @@ jobs: status: name: ${{ inputs.lane == 'security' && 'security-review' || 'claude-review-status' }} needs: [review-key, review-subscription] - if: always() && needs.review-key.result != 'skipped' + if: always() && (needs.review-key.result != 'skipped' || needs.review-subscription.result != 'skipped') runs-on: ubuntu-24.04 timeout-minutes: 20 permissions: @@ -2481,6 +2581,9 @@ jobs: script: | const fs = require("node:fs"); const env = process.env; + // The key leg's last summary, else the subscription leg's (both + // read the same marker; the key job is skipped after a credit stop). + let lastSummary = false; for (const [leg, prefix, expected] of [ ["review-key", "key", env.KEY_SHA256], ["review-subscription", "sub", env.SUB_SHA256], @@ -2499,7 +2602,10 @@ jobs: } const data = JSON.parse(bytes.toString("utf8")); if (typeof data?.review === "string") core.setOutput(`${prefix}-review`, data.review); - if (prefix === "key" && typeof data?.["last-summary"] === "string") core.setOutput("last-summary", data["last-summary"]); + if (!lastSummary && typeof data?.["last-summary"] === "string") { + core.setOutput("last-summary", data["last-summary"]); + lastSummary = true; + } } - name: Publish the review @@ -2514,15 +2620,15 @@ jobs: BASE_REF: ${{ github.event.pull_request.base.ref }} PUBLISH_GATE: ${{ steps.gate.outputs.path }} ARTIFACT_DIR: ${{ runner.temp }}/shadow-report - SCOPE_REVIEW: ${{ needs.review-key.outputs.scope-review }} - SKIP: ${{ needs.review-key.outputs.skip }} - SKIP_REASON: ${{ needs.review-key.outputs.skip-reason }} - SINCE: ${{ needs.review-key.outputs.since }} - SCOPE: ${{ needs.review-key.outputs.scope }} - MARKER_ID: ${{ needs.review-key.outputs.marker-id }} - MERGE_BASE: ${{ needs.review-key.outputs.merge-base }} + SCOPE_REVIEW: ${{ needs.review-key.result == 'skipped' && needs.review-subscription.outputs.scope-review || needs.review-key.outputs.scope-review }} + SKIP: ${{ needs.review-key.result == 'skipped' && needs.review-subscription.outputs.skip || needs.review-key.outputs.skip }} + SKIP_REASON: ${{ needs.review-key.result == 'skipped' && needs.review-subscription.outputs.skip-reason || needs.review-key.outputs.skip-reason }} + SINCE: ${{ needs.review-key.result == 'skipped' && needs.review-subscription.outputs.since || needs.review-key.outputs.since }} + SCOPE: ${{ needs.review-key.result == 'skipped' && needs.review-subscription.outputs.scope || needs.review-key.outputs.scope }} + MARKER_ID: ${{ needs.review-key.result == 'skipped' && needs.review-subscription.outputs.marker-id || needs.review-key.outputs.marker-id }} + MERGE_BASE: ${{ needs.review-key.result == 'skipped' && needs.review-subscription.outputs.merge-base || needs.review-key.outputs.merge-base }} LAST_SUMMARY: ${{ steps.handoffs.outputs.last-summary }} - PR_FILES: ${{ needs.review-key.outputs.pr-files }} + PR_FILES: ${{ needs.review-key.result == 'skipped' && needs.review-subscription.outputs.pr-files || needs.review-key.outputs.pr-files }} KEY_LANE_CLASS: ${{ needs.review-key.outputs.lane-class }} SUB_RESULT: ${{ needs.review-subscription.result }} KEY_PRESENT: ${{ needs.review-key.outputs.credential-present }} @@ -2565,7 +2671,7 @@ jobs: HEAD_SHA: ${{ github.event.pull_request.head.sha }} BASE_REF: ${{ github.event.pull_request.base.ref }} PUBLISH_GATE: ${{ steps.gate.outputs.path }} - PR_FILES: ${{ needs.review-key.outputs.pr-files }} + PR_FILES: ${{ needs.review-key.result == 'skipped' && needs.review-subscription.outputs.pr-files || needs.review-key.outputs.pr-files }} KEY_LANE_CLASS: ${{ needs.review-key.outputs.lane-class }} SUB_RESULT: ${{ needs.review-subscription.result }} KEY_PRESENT: ${{ needs.review-key.outputs.credential-present }} diff --git a/README.md b/README.md index aa82d55c..46269c3b 100644 --- a/README.md +++ b/README.md @@ -717,6 +717,14 @@ consumer to audit it. step: the `api-budget` notice (`token=github-token ... cost=N`) the hub collector reads, built from the `x-ratelimit-*` headers of the job's own requests, so it spends none (its `action.yml` describes what it does). +- `.github/actions/key-credit-gate` — `pr-review-cli.yml`'s `key-credit` + job: skips the key job while the key's credit stop in the same workflow is + inside `key-credit-window-hours` (default 6), reading the workflow's runs + with the job token (one list call, at most three job-list calls), and runs + the key on any failure. Another keyed lane adopts it with the same three + pieces: a gate job before its key job, a record step named for its run + after the launcher's `credit-stop`, and a subscription job that also starts + on the gate's skip (its `action.yml` describes the inputs). Each input's meaning and default is documented inline in the action's `inputs:` block. @@ -1131,7 +1139,9 @@ GitHub continues the normal weekly patching of each hosted image generation. credential-scanned JSON artifact instead, lane records under a `-shadow` lane id, and no job ever red. The caller grants `contents: read` and `pull-requests: write` in both modes and, in summary mode, does not cancel - in-progress runs. The workflow header carries the canonical caller and + in-progress runs, and grants `actions: read` for the `key-credit` job, + which skips the key job for `key-credit-window-hours` after the key's + credit stop. The workflow header carries the canonical caller and the security model. With `hold-label` set, comments mode also holds the pull request for a person: when the review returns a `hold` with criterion `cost` (only a change that can cause a real charge, as the compose step's From 85f8f0c922750b5fd5f9206ecfb98b6417cc5717 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sat, 24 Oct 2026 13:32:00 -0400 Subject: [PATCH 2/5] test(key-credit-gate): hold that empty job steps are no evidence and run the key Co-Authored-By: Claude Opus 5.5 --- .github/scripts/key-credit-gate.test.cjs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/.github/scripts/key-credit-gate.test.cjs b/.github/scripts/key-credit-gate.test.cjs index df8f23c7..6a87fea5 100644 --- a/.github/scripts/key-credit-gate.test.cjs +++ b/.github/scripts/key-credit-gate.test.cjs @@ -62,6 +62,9 @@ test("a run settles nothing without a finished key job whose model or record ran [keyJob([credit(10).name].map((name) => step(name, "skipped")))], [keyJob([RAN], "pr-review / security-review-key")], [keyJob([RAN], `pr-review / not-${KEY}`)], + // The jobs API sometimes returns every job of a run with no steps. + [keyJob([]), keyJob([], `pr-review / ${KEY}-credit`)], + [{ name: `pr-review / ${KEY}`, status: "completed" }], ]) { assert.equal(judgeRun(jobs, KEY, ids), null, JSON.stringify(jobs)); } @@ -122,6 +125,21 @@ test("decide reads newest first, skips the current run and caps its job reads", assert.deepEqual(read2, [7, 6]); }); +test("runs whose jobs all come back with empty steps run the key after the cap", async () => { + const read = []; + const verdict = await decide({ + currentRunId: "9", + keyJob: KEY, + listRuns: async () => [{ id: 8 }, { id: 7 }, { id: 6 }, { id: 5 }], + listJobs: async (id) => { + read.push(id); + return [keyJob([]), keyJob([], `r / ${KEY}-credit`)]; + }, + }); + assert.equal(verdict.skip, false); + assert.deepEqual(read, [8, 7, 6]); +}); + test("decide fails open on every read failure", async () => { const boom = async () => { throw new Error("HTTP 403"); From 38e9250d70942e26a9b36b6e4980e0b4c5082c99 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sat, 24 Oct 2026 13:53:14 -0400 Subject: [PATCH 3/5] fix(pr-review-cli): name the credit gate check-key-credit and quote its token description The naming lint requires a verb-first action directory, so the action and its job become check-key-credit. An unquoted "actions: read" in the token input's description broke the action's YAML for actionlint and zizmor. Co-Authored-By: Claude Opus 5.5 --- .../action.yml | 2 +- .../gate.cjs | 8 ++--- ...ate.test.cjs => check-key-credit.test.cjs} | 14 ++++----- .github/scripts/pr-review-cli.test.cjs | 26 ++++++++-------- .github/workflows/pr-review-cli.yml | 30 +++++++++---------- README.md | 4 +-- 6 files changed, 42 insertions(+), 42 deletions(-) rename .github/actions/{key-credit-gate => check-key-credit}/action.yml (96%) rename .github/actions/{key-credit-gate => check-key-credit}/gate.cjs (94%) rename .github/scripts/{key-credit-gate.test.cjs => check-key-credit.test.cjs} (93%) diff --git a/.github/actions/key-credit-gate/action.yml b/.github/actions/check-key-credit/action.yml similarity index 96% rename from .github/actions/key-credit-gate/action.yml rename to .github/actions/check-key-credit/action.yml index 8e0683c0..6617325d 100644 --- a/.github/actions/key-credit-gate/action.yml +++ b/.github/actions/check-key-credit/action.yml @@ -20,7 +20,7 @@ inputs: required: false default: "6" github-token: - description: The job token, with actions: read. + description: "The job token, with actions: read." required: false default: ${{ github.token }} diff --git a/.github/actions/key-credit-gate/gate.cjs b/.github/actions/check-key-credit/gate.cjs similarity index 94% rename from .github/actions/key-credit-gate/gate.cjs rename to .github/actions/check-key-credit/gate.cjs index 5f17262d..d6c42e9d 100644 --- a/.github/actions/key-credit-gate/gate.cjs +++ b/.github/actions/check-key-credit/gate.cjs @@ -1,4 +1,4 @@ -// key-credit-gate: whether a keyed lane's key job should skip the model +// check-key-credit: whether a keyed lane's key job should skip the model // because the key ended in a credit stop inside the window. // // It reads the calling workflow's completed runs created inside the window @@ -207,10 +207,10 @@ async function main(env = process.env, fetchImpl = globalThis.fetch, now = () => } if (verdict.skip) { lines.push( - `::notice title=key-credit-gate::${config.keyJob} skips the key: the key's credit stop in run ${verdict.origin} (${config.server}/${config.repository}/actions/runs/${verdict.origin}) is inside the ${config.hours}-hour window; the subscription job reviews`, + `::notice title=check-key-credit::${config.keyJob} skips the key: the key's credit stop in run ${verdict.origin} (${config.server}/${config.repository}/actions/runs/${verdict.origin}) is inside the ${config.hours}-hour window; the subscription job reviews`, ); } else { - lines.push(`key-credit-gate: ${config.keyJob || "the key job"} runs the key: ${verdict.reason}`); + lines.push(`check-key-credit: ${config.keyJob || "the key job"} runs the key: ${verdict.reason}`); } if (env.GITHUB_OUTPUT) { fs.appendFileSync( @@ -226,7 +226,7 @@ if (require.main === module) { .then(({ lines }) => process.stdout.write(`${lines.join("\n")}\n`)) .catch(() => { // Fail open: no output leaves skip-key empty, so the key runs. - process.stdout.write("key-credit-gate: the check failed; the key runs\n"); + process.stdout.write("check-key-credit: the check failed; the key runs\n"); }); } diff --git a/.github/scripts/key-credit-gate.test.cjs b/.github/scripts/check-key-credit.test.cjs similarity index 93% rename from .github/scripts/key-credit-gate.test.cjs rename to .github/scripts/check-key-credit.test.cjs index 6a87fea5..b06a2220 100644 --- a/.github/scripts/key-credit-gate.test.cjs +++ b/.github/scripts/check-key-credit.test.cjs @@ -1,6 +1,6 @@ "use strict"; -// key-credit-gate reads a keyed lane's own recent runs and skips the key job +// check-key-credit reads a keyed lane's own recent runs and skips the key job // only on a credit-stop record inside the window; every other answer, and // every failure, runs the key. Its calls: one run list, then at most // MAX_JOB_READS job lists. @@ -15,7 +15,7 @@ const { decide, judgeRun, main, -} = require("../actions/key-credit-gate/gate.cjs"); +} = require("../actions/check-key-credit/gate.cjs"); const { makeTemporaryDirectory } = require("./temporary-directories.cjs"); const KEY = "claude-review-key"; @@ -196,7 +196,7 @@ function fakeFetch(routes) { } test("main skips on a credit stop in the window with one list call and one jobs call, and says which run", async () => { - const dir = makeTemporaryDirectory("key-credit-gate-"); + const dir = makeTemporaryDirectory("check-key-credit-"); const { urls, fetchImpl } = fakeFetch([ [ /\/workflows\/pr-review-hosted\.yml\/runs\?/u, @@ -213,7 +213,7 @@ test("main skips on a credit stop in the window with one list call and one jobs assert.equal(lines.length, 1); assert.match( lines[0], - /^::notice title=key-credit-gate::claude-review-key skips the key: the key's credit stop in run 98 \(https:\/\/example\/o\/r\/actions\/runs\/98\)/u, + /^::notice title=check-key-credit::claude-review-key skips the key: the key's credit stop in run 98 \(https:\/\/example\/o\/r\/actions\/runs\/98\)/u, ); assert.equal( fs.readFileSync(path.join(dir, "output"), "utf8"), @@ -234,12 +234,12 @@ test("main runs the key on a refused read, an unreadable ref or a window of 0, s [{ WORKFLOW_REF: "nonsense" }, 0], [{ GITHUB_TOKEN: "" }, 0], ]) { - const dir = makeTemporaryDirectory("key-credit-gate-"); + const dir = makeTemporaryDirectory("check-key-credit-"); const { urls, fetchImpl } = fakeFetch([]); const { verdict, lines } = await main(env(dir, extra), fetchImpl); assert.equal(verdict.skip, false, JSON.stringify(extra)); assert.equal(urls.length, calls, JSON.stringify(extra)); - assert.match(lines[0], /^key-credit-gate: .* runs the key: /u); + assert.match(lines[0], /^check-key-credit: .* runs the key: /u); assert.equal( fs.readFileSync(path.join(dir, "output"), "utf8"), "skip-key=false\norigin-run=\n", @@ -251,7 +251,7 @@ test("the action passes the caller's workflow ref and the window to the gate", ( const { parseWorkflow } = require("./workflow-yaml.cjs"); const action = parseWorkflow( fs.readFileSync( - path.join(__dirname, "..", "actions", "key-credit-gate", "action.yml"), + path.join(__dirname, "..", "actions", "check-key-credit", "action.yml"), "utf8", ), ); diff --git a/.github/scripts/pr-review-cli.test.cjs b/.github/scripts/pr-review-cli.test.cjs index afd0a4b6..d1357ea9 100644 --- a/.github/scripts/pr-review-cli.test.cjs +++ b/.github/scripts/pr-review-cli.test.cjs @@ -119,7 +119,7 @@ test("each secret-bearing job turns away forks, drafts, Dependabot, unlisted bot github, inputs: { "allowed-bot-sender-ids": "", ...inputs }, needs: { - "key-credit": { result: "success", outputs: {} }, + "check-key-credit": { result: "success", outputs: {} }, "review-key": { result: "success", outputs: { @@ -131,7 +131,7 @@ test("each secret-bearing job turns away forks, drafts, Dependabot, unlisted bot }, }), ); - for (const job of [workflow.jobs["key-credit"], key, subscription]) { + for (const job of [workflow.jobs["check-key-credit"], key, subscription]) { assert.equal(passes(job, PR), true, job.name); const refused = [ withPr((g) => { @@ -185,12 +185,12 @@ test("the subscription job starts only on an exact handoff, a key job with no ke github: PR, inputs: { "allowed-bot-sender-ids": "" }, needs: { - "key-credit": { outputs: { "skip-key": skipKey } }, + "check-key-credit": { outputs: { "skip-key": skipKey } }, "review-key": { result, outputs }, }, }), ); - // A key job skipped by key-credit's exact `true`, and no other skip. + // A key job skipped by check-key-credit's exact `true`, and no other skip. assert.equal(starts({}, "skipped", "true"), true); for (const skipKey of ["", "false", "True", "true ", "1"]) { assert.equal( @@ -203,7 +203,7 @@ test("the subscription job starts only on an exact handoff, a key job with no ke for (const result of ["failure", "cancelled"]) { assert.equal(starts({ handoff: "true" }, result, "true"), false, result); } - assert.deepEqual(subscription.needs, ["key-credit", "review-key"]); + assert.deepEqual(subscription.needs, ["check-key-credit", "review-key"]); assert.equal(starts({ handoff: "true", "credential-present": "true" }), true); assert.equal(starts({ handoff: "", "credential-present": "false" }), true); for (const handoff of ["", "false", "True", "true ", "1"]) { @@ -218,13 +218,13 @@ test("the subscription job starts only on an exact handoff, a key job with no ke assert.doesNotMatch(subscription.if, /always\(\)|failure\(\)/u); }); -test("the key job runs unless key-credit says exactly true, a failed key-credit included", () => { +test("the key job runs unless check-key-credit says exactly true, a failed check-key-credit included", () => { const runs = (skipKey, result = "success") => Boolean( evaluate(key.if, { github: PR, inputs: { "allowed-bot-sender-ids": "" }, - needs: { "key-credit": { result, outputs: { "skip-key": skipKey } } }, + needs: { "check-key-credit": { result, outputs: { "skip-key": skipKey } } }, }), ); assert.equal(runs("true"), false); @@ -233,19 +233,19 @@ test("the key job runs unless key-credit says exactly true, a failed key-credit } assert.equal(runs("", "failure"), true); assert.equal(runs("", "skipped"), true); - assert.equal(key.needs, "key-credit"); - const gate = workflow.jobs["key-credit"]; + assert.equal(key.needs, "check-key-credit"); + const gate = workflow.jobs["check-key-credit"]; assert.deepEqual(gate.permissions, { actions: "read" }); - assert.equal(gate.steps[0].uses, "$/.github/actions/key-credit-gate"); + assert.equal(gate.steps[0].uses, "$/.github/actions/check-key-credit"); assert.equal(gate.outputs["skip-key"], `\${{ steps.gate.outputs.skip-key }}`); assert.doesNotMatch(JSON.stringify(gate), /secrets\./u); }); -test("the credit record steps carry the names and conditions key-credit-gate reads", () => { +test("the credit record steps carry the names and conditions check-key-credit reads", () => { const { MODEL_STEP, RECORD_PATTERN, - } = require("../actions/key-credit-gate/gate.cjs"); + } = require("../actions/check-key-credit/gate.cjs"); const render = (name, context) => name.replace(/\$\{\{\s*([^}]+?)\s*\}\}/gu, (_, expr) => String(evaluate(expr, context)), @@ -268,7 +268,7 @@ test("the credit record steps carry the names and conditions key-credit-gate rea assert.equal(runsAfter(laneClass), false, laneClass); } } - const carry = workflow.jobs["key-credit"].steps[1]; + const carry = workflow.jobs["check-key-credit"].steps[1]; const context = (skip) => ({ steps: { gate: { outputs: { "skip-key": skip, "origin-run": "99" } } }, }); diff --git a/.github/workflows/pr-review-cli.yml b/.github/workflows/pr-review-cli.yml index bad87ad6..43adacb9 100644 --- a/.github/workflows/pr-review-cli.yml +++ b/.github/workflows/pr-review-cli.yml @@ -22,7 +22,7 @@ name: pr-review-cli # # request against this grant at startup, so summary mode needs it # # too, though it writes nothing. # permissions: -# actions: read # the key-credit job reads this workflow's runs +# actions: read # the check-key-credit job reads this workflow's runs # contents: read # pull-requests: write # uses: melodic-software/ci-workflows/.github/workflows/pr-review-cli.yml@ @@ -49,14 +49,14 @@ name: pr-review-cli # That job runs on a fresh ubuntu-24.04 runner, repeats every hardening # step and takes no artifact or cache from the key job. Either secret # may be empty. -# - The key-credit job runs first, holds no secret and no write, and reads +# - The check-key-credit job runs first, holds no secret and no write, and reads # only this workflow's own runs with the job token (actions: read, so # the caller grants it). When the key job ended in a credit stop # (lane-class `credit-stop`) in a run created inside # key-credit-window-hours, the key job is skipped and the subscription # job reviews, its scope read by the status job in the key job's place. # A failed or unclear read runs the key job as before -# (.github/actions/key-credit-gate). +# (.github/actions/check-key-credit). # - The model jobs cannot write: contents: read and pull-requests: read, # no id-token, and `cache-mode: none`. The launcher step's env holds the # one Claude credential and no GitHub token. Before the checkout the job @@ -92,7 +92,7 @@ name: pr-review-cli # output; it downloads from this run only, with no token. It # checks every inline anchor against the pull request's diff at the head # (one bad anchor sends the whole batch to the summary): the hunk ranges -# the key job's scope step (the subscription job's when key-credit +# the key job's scope step (the subscription job's when check-key-credit # skipped the key job) read before any model step ran, used only # when a fresh read shows the head has not moved, else a read of its # own. Each step writes its own outputs file, so the later model step @@ -281,7 +281,7 @@ jobs: # origin run forward in its name, which the next run's read finds, so the # window counts from the credit stop, not the last skip. Every failure runs # the key job: its `if` starts with !cancelled(). - key-credit: + check-key-credit: name: ${{ inputs.lane == 'security' && 'security-review-key-credit' || 'claude-review-key-credit' }} runs-on: ubuntu-24.04 timeout-minutes: 10 @@ -310,7 +310,7 @@ jobs: id: gate continue-on-error: true timeout-minutes: 2 - uses: $/.github/actions/key-credit-gate + uses: $/.github/actions/check-key-credit with: key-job: ${{ inputs.lane == 'security' && 'security-review-key' || 'claude-review-key' }} window-hours: ${{ inputs.key-credit-window-hours }} @@ -319,7 +319,7 @@ jobs: if: steps.gate.outputs.skip-key == 'true' continue-on-error: true timeout-minutes: 1 - run: echo "carried to the next run's key-credit read" + run: echo "carried to the next run's check-key-credit read" # The job token's api-budget notice, from the readings the gate's # requests left. It spends no request. @@ -333,11 +333,11 @@ jobs: review-key: name: ${{ inputs.lane == 'security' && 'security-review-key' || 'claude-review-key' }} - needs: key-credit + needs: check-key-credit runs-on: ubuntu-24.04 timeout-minutes: ${{ inputs.lane == 'security' && 46 || 44 }} if: >- - needs.key-credit.outputs.skip-key != 'true' + needs.check-key-credit.outputs.skip-key != 'true' && !cancelled() && github.event_name == 'pull_request' @@ -1178,10 +1178,10 @@ jobs: cli-version: ${{ steps.compose.outputs.cli-version }} leg-timeout-minutes: ${{ inputs.lane == 'security' && '13' || '11' }} - # Read by the next runs' key-credit job: a step named for this run that + # Read by the next runs' check-key-credit job: a step named for this run that # succeeds only after the launcher's credit stop, which only the key # leg can end in. Its name is the record; keep it equal to - # key-credit-gate's RECORD_PATTERN. + # check-key-credit's RECORD_PATTERN. - name: Record the key's credit stop in run ${{ github.run_id }} if: always() && steps.claude.outputs.lane-class == 'credit-stop' continue-on-error: true @@ -1416,13 +1416,13 @@ jobs: # writes them into a script. review-subscription: name: ${{ inputs.lane == 'security' && 'security-review-subscription' || 'claude-review-subscription' }} - needs: [key-credit, review-key] + needs: [check-key-credit, review-key] runs-on: ubuntu-24.04 timeout-minutes: ${{ inputs.lane == 'security' && 46 || 44 }} if: >- ((needs.review-key.result == 'success' && (needs.review-key.outputs.handoff == 'true' || needs.review-key.outputs.credential-present == 'false')) - || (needs.review-key.result == 'skipped' && needs.key-credit.outputs.skip-key == 'true')) + || (needs.review-key.result == 'skipped' && needs.check-key-credit.outputs.skip-key == 'true')) && !cancelled() && github.event_name == 'pull_request' @@ -2263,10 +2263,10 @@ jobs: cli-version: ${{ steps.compose.outputs.cli-version }} leg-timeout-minutes: ${{ inputs.lane == 'security' && '13' || '11' }} - # Read by the next runs' key-credit job: a step named for this run that + # Read by the next runs' check-key-credit job: a step named for this run that # succeeds only after the launcher's credit stop, which only the key # leg can end in. Its name is the record; keep it equal to - # key-credit-gate's RECORD_PATTERN. + # check-key-credit's RECORD_PATTERN. - name: Record the key's credit stop in run ${{ github.run_id }} if: always() && steps.claude.outputs.lane-class == 'credit-stop' continue-on-error: true diff --git a/README.md b/README.md index 46269c3b..01c1470c 100644 --- a/README.md +++ b/README.md @@ -717,7 +717,7 @@ consumer to audit it. step: the `api-budget` notice (`token=github-token ... cost=N`) the hub collector reads, built from the `x-ratelimit-*` headers of the job's own requests, so it spends none (its `action.yml` describes what it does). -- `.github/actions/key-credit-gate` — `pr-review-cli.yml`'s `key-credit` +- `.github/actions/check-key-credit` — `pr-review-cli.yml`'s `key-credit` job: skips the key job while the key's credit stop in the same workflow is inside `key-credit-window-hours` (default 6), reading the workflow's runs with the job token (one list call, at most three job-list calls), and runs @@ -1139,7 +1139,7 @@ GitHub continues the normal weekly patching of each hosted image generation. credential-scanned JSON artifact instead, lane records under a `-shadow` lane id, and no job ever red. The caller grants `contents: read` and `pull-requests: write` in both modes and, in summary mode, does not cancel - in-progress runs, and grants `actions: read` for the `key-credit` job, + in-progress runs, and grants `actions: read` for the `check-key-credit` job, which skips the key job for `key-credit-window-hours` after the key's credit stop. The workflow header carries the canonical caller and the security model. With `hold-label` set, comments mode also holds the From 8cacbe6b3267336f5e5f78e95f8976c46af5032c Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sat, 24 Oct 2026 13:56:55 -0400 Subject: [PATCH 4/5] style(pr-review-cli): format the renamed gate test lines Co-Authored-By: Claude Opus 5.5 --- .github/scripts/pr-review-cli.test.cjs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/scripts/pr-review-cli.test.cjs b/.github/scripts/pr-review-cli.test.cjs index d1357ea9..cd8c56f8 100644 --- a/.github/scripts/pr-review-cli.test.cjs +++ b/.github/scripts/pr-review-cli.test.cjs @@ -224,7 +224,9 @@ test("the key job runs unless check-key-credit says exactly true, a failed check evaluate(key.if, { github: PR, inputs: { "allowed-bot-sender-ids": "" }, - needs: { "check-key-credit": { result, outputs: { "skip-key": skipKey } } }, + needs: { + "check-key-credit": { result, outputs: { "skip-key": skipKey } }, + }, }), ); assert.equal(runs("true"), false); From 1d0d86a53d15b8d21ea7ac7faea0973e7695b13b Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sat, 24 Oct 2026 13:59:47 -0400 Subject: [PATCH 5/5] docs(readme): name the check-key-credit job by its renamed id Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 01c1470c..5760320c 100644 --- a/README.md +++ b/README.md @@ -717,7 +717,7 @@ consumer to audit it. step: the `api-budget` notice (`token=github-token ... cost=N`) the hub collector reads, built from the `x-ratelimit-*` headers of the job's own requests, so it spends none (its `action.yml` describes what it does). -- `.github/actions/check-key-credit` — `pr-review-cli.yml`'s `key-credit` +- `.github/actions/check-key-credit` — `pr-review-cli.yml`'s `check-key-credit` job: skips the key job while the key's credit stop in the same workflow is inside `key-credit-window-hours` (default 6), reading the workflow's runs with the job token (one list call, at most three job-list calls), and runs