From fe8d8728b5599723aef28c912b4f23e1137342a8 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Mon, 2 Nov 2026 21:28:21 -0500 Subject: [PATCH 1/2] fix(pr-pipeline): skip a lane's key leg while its key is out of credit pr-verify and pr-review-main tried the API key first on every run, and with the key out of credit each key leg ended in one turn on "Credit balance is too low" before the subscription leg did the work (#12526). Each workflow now runs ci-workflows' check-key-credit in a probe job before its key leg (check-intent-credit, review-handoffs-credit): actions read only, no secret, no checkout. A key-leg credit stop recorded inside the window (vars.KEY_CREDIT_WINDOW_HOURS, 1 hour when unset) skips the key job, and the subscription leg runs as for a credit-stop handoff. The probe's record step carries the origin run forward, so the key is tried again once the window ends and a topped-up or rotated key is picked up with no edit. Any probe failure leaves skip-key empty and the key runs: each key job's `if` starts with !cancelled(). - pr-verify: check-intent records the stop after a judge step's credit-stop handoff; check-intent-subscription and check-intent-fallback spell out the success their implicit success() gave, and the subscription's fallback-reason names credit-stop for a gated key. - pr-review-main: both lanes share the reviews key, so one gate skips the review job; review-handoffs records a key leg's credit stop, hands every enabled lane on as credit-stop at the trigger job's main commit when the gate skipped the key, and the record job runs when the subscription leg did. - A gated key leg leaves a lane-outcome record (outcome skip, class credit-stop, handoff true) from the probe, so leg counts compare before and after. The tests evaluate the new job gates through a shared expression evaluator and pin the record step names to the gate's RECORD_PATTERN. Refs: #12526 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VcupojVgxFqBnrah9DwFLE --- .github/scripts/lib/gate-expression.mjs | 45 +++++ .../reconcile.test.mjs | 54 +++++- .../scripts/pr-review-main/workflow.test.mjs | 141 ++++++++++++++-- .github/scripts/pr-verify/workflow.test.mjs | 155 +++++++++++++----- .github/workflows/pr-review-main.yml | 136 +++++++++++++-- .github/workflows/pr-verify.yml | 126 +++++++++++--- 6 files changed, 567 insertions(+), 90 deletions(-) create mode 100644 .github/scripts/lib/gate-expression.mjs diff --git a/.github/scripts/lib/gate-expression.mjs b/.github/scripts/lib/gate-expression.mjs new file mode 100644 index 00000000000..e8dc36e077c --- /dev/null +++ b/.github/scripts/lib/gate-expression.mjs @@ -0,0 +1,45 @@ +// A job-level `if:` gate evaluated on one run's context, for the workflow +// tests: `a == b` and `a != b` terms, `!cancelled()`, `&&`, `||` and +// parentheses, a missing value reading as '' as GitHub's null does. Any other +// shape fails the calling test. +import assert from "node:assert/strict"; + +export const admits = (gate, context, { cancelled = false } = {}) => { + const tokens = gate.match(/\|\||&&|==|!=|!|\(|\)|'[^']*'|cancelled\(\)|[A-Za-z_][\w.-]*/g) ?? []; + assert.equal(tokens.join(""), gate.replace(/\s+/g, ""), "the gate holds only known tokens"); + let at = 0; + const peek = () => tokens[at]; + const take = (expected) => { + assert.ok(expected === undefined || tokens[at] === expected, `${expected} at ${at} in ${gate}`); + return tokens[at++]; + }; + const value = (term) => (/^'.*'$/.test(term) ? term.slice(1, -1) : (term.split(".").reduce((node, key) => node?.[key], context) ?? "")); + const primary = () => { + if (peek() === "(") { + take("("); + const inner = or(); + take(")"); + return inner; + } + const left = take(); + if (left === "cancelled()") return cancelled; + const op = take(); + assert.ok(op === "==" || op === "!=", `a comparison after ${left} in ${gate}`); + const right = take(); + return op === "==" ? value(left) === value(right) : value(left) !== value(right); + }; + const unary = () => (peek() === "!" ? (take("!"), !unary()) : primary()); + const and = () => { + let result = unary(); + while (peek() === "&&") result = take("&&") && unary() && result; + return result; + }; + const or = () => { + let result = and(); + while (peek() === "||") result = take("||") && (and() || result); + return result; + }; + const result = or(); + assert.equal(at, tokens.length, `the whole gate parsed: ${gate}`); + return result; +}; diff --git a/.github/scripts/maintenance-reconcile-lost-runs/reconcile.test.mjs b/.github/scripts/maintenance-reconcile-lost-runs/reconcile.test.mjs index 288407be183..13a58bb1ff8 100644 --- a/.github/scripts/maintenance-reconcile-lost-runs/reconcile.test.mjs +++ b/.github/scripts/maintenance-reconcile-lost-runs/reconcile.test.mjs @@ -550,9 +550,21 @@ const legSteps = (body) => body.split(/^(?= {6}- name: )/m).filter((s) => s.star // The reconciler reads every job's steps by name, so a leg whose judge step // another name hid would hide a lost run. +// pr-verify's judging legs; its other job, the credit gate, runs no model +// and writes nothing, so it holds no step the reconciler reads. +const judgingLegs = () => { + const jobs = verifyLegs(); + const legs = jobs.filter(({ body }) => /^ {8}id: judge$/m.test(body)); + for (const { name, body } of jobs.filter((job) => !legs.includes(job))) { + assert.doesNotMatch(body, /run-claude-cli|secrets\.|upload-artifact/, name); + for (const step of [JUDGE_STEP, JUDGE_PREDECESSOR, WRITE_STEP]) assert.ok(!legSteps(body).some((s) => s.startsWith(` - name: ${step}\n`)), `${name}: ${step}`); + } + assert.equal(legs.length, 3); + return legs; +}; + test("pr-verify still names its judge step as the reconciler reads it, in every leg", () => { - const legs = verifyLegs(); - assert.ok(legs.length > 1); + const legs = judgingLegs(); for (const { name, body } of legs) { const steps = legSteps(body); const names = steps.map((s) => s.match(/^ {6}- name: (.+)$/m)[1]); @@ -683,6 +695,41 @@ test("a verify run is read across all three legs: a fallback write held, a fallb assert.deepEqual(records(lost.outputs), [`reconcile-rerun-201-2-${MAIN_TIP}`, "", ""]); }); +// With the key out of credit, the credit gate skips check-intent and the +// subscription leg judges: its own steps tell a write held from a run lost. +test("a verify run whose credit gate skipped the key is read on the subscription leg", async () => { + const gate = { + name: "check-intent-credit", + status: "completed", + conclusion: "success", + steps: [ + { name: "Set up job", status: "completed", conclusion: "success" }, + { name: "Read the key's credit history", status: "completed", conclusion: "success" }, + { name: "Record the key's credit stop in run 200", status: "completed", conclusion: "success" }, + { name: "Complete job", status: "completed", conclusion: "success" }, + ], + }; + const key = { name: "check-intent", status: "completed", conclusion: "skipped", steps: [] }; + const subscription = (write, upload) => ({ + name: "check-intent-subscription", + status: "completed", + conclusion: "failure", + steps: [ + { name: "Set up job", status: "completed", conclusion: "success" }, + { name: JUDGE_PREDECESSOR, status: "completed", conclusion: "success" }, + { name: JUDGE_STEP, status: "completed", conclusion: "success" }, + { name: WRITE_STEP, status: "completed", conclusion: write }, + { name: "Upload the verdict", status: "completed", conclusion: upload }, + { name: "Complete job", status: "completed", conclusion: "success" }, + ], + }); + const held = await reconcile({ pulls: [], runs: [{ ...verifyRun(201, { conclusion: "failure" }), jobs: [gate, key, subscription("success", "failure")] }] }); + assert.deepEqual(held.reruns, []); + const lost = await reconcile({ pulls: [], runs: [{ ...verifyRun(201, { conclusion: "failure" }), jobs: [gate, key, subscription("skipped", "skipped")] }] }); + assert.deepEqual(lost.reruns, [201]); + assert.deepEqual(records(lost.outputs), [`reconcile-rerun-201-2-${MAIN_TIP}`, "", ""]); +}); + test("a verify run that failed on its first attempt is recorded and re-run once", async () => { const { code, reruns, outputs } = await reconcile({ pulls: [], @@ -719,8 +766,7 @@ test("a failed verify run's second attempt is never re-run", async () => { }); test("pr-verify fails a run the kill switch stopped in every leg, so the reconciler sees it", () => { - const legs = verifyLegs(); - assert.ok(legs.length > 1); + const legs = judgingLegs(); for (const { name, body } of legs) { const reports = legSteps(body).filter((s) => s.startsWith(" - name: Report the kill switch\n")); assert.equal(reports.length, 1, name); diff --git a/.github/scripts/pr-review-main/workflow.test.mjs b/.github/scripts/pr-review-main/workflow.test.mjs index c44b28354f1..a8200bfb650 100644 --- a/.github/scripts/pr-review-main/workflow.test.mjs +++ b/.github/scripts/pr-review-main/workflow.test.mjs @@ -10,6 +10,7 @@ import { chmodSync, mkdirSync, symlinkSync, mkdtempSync, readdirSync, readFileSy import { tmpdir } from "node:os"; import path from "node:path"; import { test } from "node:test"; +import { admits } from "../lib/gate-expression.mjs"; const text = readFileSync(new URL("../../workflows/pr-review-main.yml", import.meta.url), "utf8"); const prompt = readFileSync(new URL("./prompt.md", import.meta.url), "utf8"); @@ -17,6 +18,8 @@ const KEY = "review"; const SUB = "review-subscription"; const FB = "review-fallback"; const LEGS = [KEY, SUB, FB]; +// The credit gate before the key leg: no model, no secret. +const PROBE = "review-handoffs-credit"; const jobText = (id) => { const found = new RegExp(`\\n ${id}:\\n([\\s\\S]*?)(?=\\n [a-z-]+:\\n|$)`).exec(text); assert.ok(found, `a job ${id}`); @@ -29,7 +32,7 @@ const stepsOf = (id) => .slice(1) // A step ends before the next step's or job's comment. .map((block) => ({ name: /^ - name: (.*)$/m.exec(block)[1], block: block.replace(/\n\n {2}(?: {4})?#[\s\S]*$/, "") })); -const STEPS = Object.fromEntries(LEGS.map((id) => [id, stepsOf(id)])); +const STEPS = Object.fromEntries([...LEGS, PROBE, "review-handoffs"].map((id) => [id, stepsOf(id)])); const step = (name, job = KEY) => { const found = STEPS[job].find((s) => s.name === name); assert.ok(found, `a step named ${name} in ${job}`); @@ -804,9 +807,17 @@ test("it checks out main only, and runs local actions from that checkout", () => for (const lane of ["claude-review", "claude-security-review"]) { assert.match(jobText(from), new RegExp(`^ {6}${lane}-base-sha: \\$\\{\\{ steps\\.base\\.outputs\\.${lane}-base-sha \\}\\}$`, "m")); } - assert.match(jobText(route), new RegExp(`^ {10}CODE_BASE: \\$\\{\\{ needs\\.${from}\\.outputs\\.claude-review-base-sha \\}\\}$`, "m")); - assert.match(jobText(route), new RegExp(`^ {10}SECURITY_BASE: \\$\\{\\{ needs\\.${from}\\.outputs\\.claude-security-review-base-sha \\}\\}$`, "m")); + // A lane the credit gate handed on had no key leg to record a commit, so + // the key route falls back to the trigger job's. + const gated = route === "review-handoffs" ? " \\|\\| needs\\.trigger\\.outputs\\.main-sha" : ""; + assert.match(jobText(route), new RegExp(`^ {10}CODE_BASE: \\$\\{\\{ needs\\.${from}\\.outputs\\.claude-review-base-sha${gated} \\}\\}$`, "m")); + assert.match(jobText(route), new RegExp(`^ {10}SECURITY_BASE: \\$\\{\\{ needs\\.${from}\\.outputs\\.claude-security-review-base-sha${gated} \\}\\}$`, "m")); } + // The trigger job's commit is the tip it checked out, right after that checkout. + const triggerSteps = jobText("trigger").split(/\n(?= - name: )/).slice(1); + const mainAt = triggerSteps.findIndex((s) => s.startsWith(" - name: Record the main commit\n")); + assert.match(triggerSteps[mainAt - 1], /uses: actions\/checkout@/); + assert.match(triggerSteps[mainAt], /^ {8}id: main\n {8}run: echo "main-sha=\$\(git -C \.base rev-parse --verify HEAD\)" >>"\$GITHUB_OUTPUT"$/m); for (const [, local] of text.matchAll(/^\s+uses: (\.\/\S+)/gm)) { assert.ok(local.startsWith("./.base/.github/actions/"), local); } @@ -892,7 +903,7 @@ test("only a lane-source pull request is reviewed, by the gate's own predicate", // trigger job). The later legs always gate themselves. test("the key leg takes the trigger job's gate only from this attempt, and every value has a fixed shape", () => { const trigger = jobText("trigger"); - assert.equal(needsOf(KEY), "trigger"); + assert.equal(needsOf(KEY), `[trigger, ${PROBE}]`); const outputs = /^ {4}outputs:\n((?: {6}.*\n)+)/m.exec(trigger)[1]; assert.deepEqual( [...outputs.matchAll(/^ {6}([a-z-]+): (.*)$/gm)].map(([, name, value]) => `${name}=${value}`), @@ -904,6 +915,7 @@ test("the key leg takes the trigger job's gate only from this attempt, and every "head-sha=${{ steps.gate.outputs.head-sha }}", "base-sha=${{ steps.gate.outputs.base-sha }}", "judged=${{ steps.judged.outputs.judged }}", + "main-sha=${{ steps.main.outputs.main-sha }}", ], ); assert.doesNotMatch(trigger, /head-ref/); @@ -1008,9 +1020,105 @@ test("the trigger job reads the files first and gates every lane-source pull req // found no lane source, or a head both lanes already reviewed. const KEY_SKIP = " && (needs.trigger.outputs.run-attempt != github.run_attempt\n || needs.trigger.outputs.review != 'false' && needs.trigger.outputs.judged != 'true')\n"; +// The record job runs once a leg could have reviewed: the key leg, or a +// subscription leg the credit gate's skip of the key leg handed every lane to. +const RECORD_IF = /^ {4}if: \$\{\{ !cancelled\(\) && github\.ref == 'refs\/heads\/main' && \(needs\.review\.result != 'skipped' \|\| needs\.review-subscription\.result != 'skipped'\) \}\}$/m; + test("no leg starts, and nothing records, for a head with no lane source or one both lanes reviewed", () => { assert.ok(/^ {4}if: >-\n((?: {6}.*\n)+)/m.exec(jobText(KEY))[1].endsWith(KEY_SKIP)); - assert.match(jobText("record"), /^ {4}if: \$\{\{ !cancelled\(\) && github\.ref == 'refs\/heads\/main' && needs\.review\.result != 'skipped' \}\}$/m); + assert.match(jobText("record"), RECORD_IF); + // The credit gate runs only where this attempt's trigger job starts the key leg. + for (const term of ["needs.trigger.outputs.run-attempt == github.run_attempt", "needs.trigger.outputs.proceed == 'true'", "needs.trigger.outputs.review == 'true'", "needs.trigger.outputs.judged != 'true'"]) { + assert.ok(jobGate(PROBE).includes(`&& ${term}`), term); + } +}); + +const jobGate = (id) => /^ {4}if: >-\n((?: {6}.*\n)+)/m.exec(jobText(id))?.[1].replace(/\s+/g, " ").trim(); +const upstreamRun = { ref: "refs/heads/main", repository: "o/r", run_attempt: "1", event: { workflow_run: { event: "pull_request", conclusion: "success", head_repository: { full_name: "o/r" } } } }; +const triggered = { result: "success", outputs: { "run-attempt": "1", proceed: "true", review: "true", judged: "" } }; + +// ci-workflows check-key-credit (gate.cjs at the pinned SHA): skip-key is +// `true` only while a credit stop recorded in this workflow is inside the +// window; any failure leaves it empty, so the key runs as before the gate. +test("the credit gate skips the key leg only on skip-key true, and a gate failure runs the key", () => { + const keyIf = jobGate(KEY); + assert.ok(keyIf.startsWith(`needs.${PROBE}.outputs.skip-key != 'true' && !cancelled() && needs.trigger.result == 'success' && `), keyIf); + const runs = (probe, trigger = triggered, options) => admits(keyIf, { github: upstreamRun, needs: { trigger, [PROBE]: probe } }, options); + assert.ok(!runs({ result: "success", outputs: { "skip-key": "true" } })); + for (const probe of [{ result: "success", outputs: { "skip-key": "false" } }, { result: "failure", outputs: {} }, { result: "skipped", outputs: {} }]) { + assert.ok(runs(probe), JSON.stringify(probe)); + } + // The trigger job's own result still gates the key leg, as the implicit success() did. + assert.ok(!runs({ result: "failure", outputs: {} }, { ...triggered, result: "failure" })); + assert.ok(!runs({ result: "success", outputs: {} }, triggered, { cancelled: true })); + // The gate itself: the trigger job's checks only, then this attempt's go-ahead. + const probeRuns = (trigger) => admits(jobGate(PROBE), { github: upstreamRun, needs: { trigger } }); + assert.ok(probeRuns(triggered)); + for (const outputs of [{ proceed: "" }, { review: "false" }, { judged: "true" }, { "run-attempt": "2" }]) { + assert.ok(!probeRuns({ ...triggered, outputs: { ...triggered.outputs, ...outputs } }), JSON.stringify(outputs)); + } +}); + +test("the credit gate holds no secret, checks nothing out and reads only this workflow's runs", () => { + const job = jobText(PROBE); + assert.equal(needsOf(PROBE), "trigger"); + assert.match(job, /^ {4}permissions:\n {6}actions: read #.*\n {4}outputs:\n {6}skip-key: \$\{\{ steps\.gate\.outputs\.skip-key \}\}\n/m); + assert.doesNotMatch(job, /secrets\.|actions\/checkout|run-claude-cli|\.\/\.base/); + const gate = step("Read the key's credit history", PROBE); + assert.match(gate, /^ {8}id: gate$/m); + assert.match(gate, /^ {8}continue-on-error: true$/m); + assert.match(gate, /^ {8}uses: melodic-software\/ci-workflows\/\.github\/actions\/check-key-credit@[0-9a-f]{40} # \S/m); + // The gate reads its record in `` and `-credit` by display + // name: review-handoffs and this job, neither renamed. + assert.equal(input(gate, "key-job"), "review-handoffs"); + for (const id of [PROBE, "review-handoffs"]) assert.doesNotMatch(jobText(id), /^ {4}name:/m, id); + assert.equal(input(gate, "window-hours"), "${{ vars.KEY_CREDIT_WINDOW_HOURS || '1' }}"); + // Each skipped key leg leaves its lane record, so leg counts compare. + for (const [lane, sw] of [["claude-review", "CLAUDE_REVIEW_DISABLED"], ["claude-security-review", "CLAUDE_SECURITY_REVIEW_DISABLED"]]) { + const report = step(`Report the skipped ${lane} key leg`, PROBE); + assert.match(report, new RegExp(`^ {8}if: steps\\.gate\\.outputs\\.skip-key == 'true' && vars\\.${sw} != 'true'$`, "m")); + assert.equal(input(report, "outcome"), "skipped"); + assert.equal(input(report, "lane-id"), `pr-review-main-${lane}`); + assert.equal(input(report, "credential"), "REVIEWS_CLAUDE_API_KEY"); + assert.equal(input(report, "known-failure-class"), "credit-stop"); + assert.equal(input(report, "handoff"), '"true"'); + } +}); + +// check-key-credit RECORD_PATTERN: /^Record the key's credit stop in run (\d{1,20})$/. +const RECORD_PATTERN = /^Record the key's credit stop in run (\d{1,20})$/u; +test("the record steps carry the pattern the gate reads, the key's only after a key leg's credit stop", () => { + const carried = "Record the key's credit stop in run ${{ steps.gate.outputs.origin-run }}"; + const own = "Record the key's credit stop in run ${{ github.run_id }}"; + assert.match(carried.replace("${{ steps.gate.outputs.origin-run }}", "12345"), RECORD_PATTERN); + assert.match(own.replace("${{ github.run_id }}", "12345"), RECORD_PATTERN); + assert.match(step(carried, PROBE), /^ {8}if: steps\.gate\.outputs\.skip-key == 'true'$/m); + // The key leg's own handoff reason, never the route's: a lane the gate handed + // on must not restart the window, or the key would never be tried again. + const record = step(own, "review-handoffs"); + assert.match(record, /^ {8}if: \$\{\{ !cancelled\(\) && \(needs\.review\.outputs\.claude-review == 'credit-stop' \|\| needs\.review\.outputs\.claude-security-review == 'credit-stop'\) \}\}$/m); + assert.match(record, /^ {8}continue-on-error: true$/m); + for (const job of LEGS) assert.equal(index(own, job), -1, job); +}); + +test("with the key leg skipped by the credit gate, every enabled lane goes to the subscription leg as a credit stop", () => { + const route = jobText("review-handoffs"); + assert.equal(needsOf("review-handoffs"), `[trigger, ${PROBE}, ${KEY}]`); + const routeIf = jobGate("review-handoffs"); + const handsOn = (review, probe, options) => admits(routeIf, { github: upstreamRun, needs: { review, [PROBE]: probe } }, options); + const gated = { result: "success", outputs: { "skip-key": "true" } }; + assert.ok(handsOn({ result: "skipped", outputs: {} }, gated)); + assert.ok(handsOn({ result: "success", outputs: { "claude-review": "credit-stop" } }, { result: "success", outputs: { "skip-key": "false" } })); + // A key leg skipped for any other reason, or that handed nothing on, routes nothing. + assert.ok(!handsOn({ result: "skipped", outputs: {} }, { result: "skipped", outputs: {} })); + assert.ok(!handsOn({ result: "success", outputs: {} }, { result: "success", outputs: { "skip-key": "false" } })); + for (const [env, lane, sw] of [["CODE", "claude-review", "CLAUDE_REVIEW_DISABLED"], ["SECURITY", "claude-security-review", "CLAUDE_SECURITY_REVIEW_DISABLED"]]) { + assert.match(route, new RegExp(`^ {10}${env}: \\$\\{\\{ needs\\.review\\.outputs\\.${lane} \\|\\| needs\\.review\\.result == 'skipped' && needs\\.${PROBE}\\.outputs\\.skip-key == 'true' && vars\\.${sw} != 'true' && 'credit-stop' \\|\\| '' \\}\\}$`, "m")); + } + // Both lanes handed on as credit-stop at the trigger job's commit. + const { status, matrix } = routeRun("review-handoffs")({ CODE: "credit-stop", SECURITY: "credit-stop", CODE_BASE: SHA_A, SECURITY_BASE: SHA_A }); + assert.equal(status, 0); + assert.deepEqual(matrix.include.map((row) => [row.lane, row.reason, row.base]), [["claude-review", "credit-stop", SHA_A], ["claude-security-review", "credit-stop", SHA_A]]); }); // The trigger job's judged-head read: judged=true only when the merge gate's @@ -1264,10 +1372,11 @@ test("the legs run the review lanes' credentials in the review lanes' order", () const hosted = hostedChain("pr-review-hosted.yml"); assert.deepEqual(hostedChain("pr-review-security-hosted.yml"), hosted); assert.equal(hosted.length, 3); - // Walk this workflow's chain from the key leg through each route. + // Walk this workflow's chain from the key leg through each route, which + // needs the leg before it last (the key route also the trigger and gate). const order = [KEY]; for (let job = KEY; ; ) { - const route = ["review-handoffs", "subscription-handoffs"].find((id) => needsOf(id) === job); + const route = ["review-handoffs", "subscription-handoffs"].find((id) => needsOf(id).replace(/^\[|\]$/g, "").split(", ").at(-1) === job); if (!route) break; job = LEGS.find((id) => needsOf(id) === route); order.push(job); @@ -1372,8 +1481,11 @@ test("a route hands the next leg fixed lane rows, only for a lane handed on with test("a later leg runs only for the lanes the previous leg handed on", () => { for (const [route, from, to] of [["review-handoffs", KEY, SUB], ["subscription-handoffs", SUB, FB]]) { const routeJob = jobText(route); - assert.equal(needsOf(route), from); - assert.match(routeJob, new RegExp(`&& \\(needs\\.${from}\\.outputs\\.claude-review != '' \\|\\| needs\\.${from}\\.outputs\\.claude-security-review != ''\\)`)); + // The key route also reads the trigger job's commit and the credit gate's + // skip, which hands every lane on (its own test above). + assert.equal(needsOf(route), route === "review-handoffs" ? `[trigger, ${PROBE}, ${from}]` : from); + const gated = route === "review-handoffs" ? `\\n {6}\\|\\| needs\\.review\\.result == 'skipped' && needs\\.${PROBE}\\.outputs\\.skip-key == 'true'` : ""; + assert.match(routeJob, new RegExp(`&& \\(needs\\.${from}\\.outputs\\.claude-review != '' \\|\\| needs\\.${from}\\.outputs\\.claude-security-review != ''${gated}\\)`)); // !cancelled(): one lane's failed leg does not drop the other lane's // handoff; the route still passes on only closed-set reasons. assert.match(routeJob, /^ {4}if: >-\n {6}!cancelled\(\)\n {6}&& github\.ref == 'refs\/heads\/main'\n/m); @@ -1404,9 +1516,14 @@ test("a later leg runs only for the lanes the previous leg handed on", () => { assert.doesNotMatch(step(`Review chunk ${k}`, FB), /fallback-ready/); } // The record job waits for every leg and runs when a later one skipped, - // but not when the key leg did (its own test above). + // but not when the key leg and the subscription leg both did (its own test above). assert.match(jobText("record"), /^ {4}needs: \[review, review-subscription, review-fallback\]$/m); - assert.match(jobText("record"), /^ {4}if: \$\{\{ !cancelled\(\) && github\.ref == 'refs\/heads\/main' && needs\.review\.result != 'skipped' \}\}$/m); + assert.match(jobText("record"), RECORD_IF); + const records = (review, subscription) => admits(/^ {4}if: \$\{\{ (.*) \}\}$/m.exec(jobText("record"))[1], { github: { ref: "refs/heads/main" }, needs: { review: { result: review }, "review-subscription": { result: subscription } } }); + assert.ok(records("success", "skipped")); + assert.ok(records("skipped", "success")); + assert.ok(records("skipped", "failure")); + assert.ok(!records("skipped", "skipped")); }); test("the handoff step names its lane and a closed reason, and a handed-off leg stays green", () => { @@ -1483,6 +1600,8 @@ test("every later leg repeats the key leg's steps but for its credential and why /^ {4}if: >-\n((?: {6}.*\n)+)/m .exec(jobText(id))[1] .replace(/^ {6}!cancelled\(\)\n {6}&& needs\.[a-z-]+\.result == 'success'\n {6}&& /, " ") + // The key leg's credit gate and its trigger job's success (their own test above). + .replace(/^ {6}needs\.review-handoffs-credit\.outputs\.skip-key != 'true'\n {6}&& !cancelled\(\)\n {6}&& needs\.trigger\.result == 'success'\n {6}&& /, " ") .replace(KEY_SKIP, ""); assert.equal(gates(job), gates(KEY)); } diff --git a/.github/scripts/pr-verify/workflow.test.mjs b/.github/scripts/pr-verify/workflow.test.mjs index 45224765241..41c8ead5da2 100644 --- a/.github/scripts/pr-verify/workflow.test.mjs +++ b/.github/scripts/pr-verify/workflow.test.mjs @@ -13,12 +13,15 @@ import { cpSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; import path from "node:path"; import { test } from "node:test"; import { tempDir } from "../../../lib/test-tmp.mjs"; +import { admits } from "../lib/gate-expression.mjs"; const text = readFileSync(new URL("../../workflows/pr-verify.yml", import.meta.url), "utf8"); const KEY = "check-intent"; const SUB = "check-intent-subscription"; const FB = "check-intent-fallback"; const JOBS = [KEY, SUB, FB]; +// The credit gate before the key job: no model, no secret. +const PROBE = "check-intent-credit"; const jobText = (id) => { const found = new RegExp(`\\n ${id}:\\n([\\s\\S]*?)(?=\\n [a-z-]+:\\n|$)`).exec(text); assert.ok(found, `a job ${id}`); @@ -30,7 +33,7 @@ const stepsOf = (id) => .split(/\n(?= - name: )/) .slice(1) .map((block) => ({ name: /^ - name: (.*)$/m.exec(block)[1], block })); -const STEPS = Object.fromEntries(JOBS.map((id) => [id, stepsOf(id)])); +const STEPS = Object.fromEntries([...JOBS, PROBE].map((id) => [id, stepsOf(id)])); const index = (name, job = KEY) => STEPS[job].findIndex((step) => step.name === name); const step = (name, job = KEY) => { const found = STEPS[job].find((s) => s.name === name); @@ -49,9 +52,10 @@ const CEILING = Number(/^MAX_CHUNKS=\$\{MAX_CHUNKS:-([1-9][0-9]*)\}$/m.exec(chun const SLOTS = Array.from({ length: CEILING }, (_, i) => i + 1); const PIN = /^melodic-software\/ci-workflows\/\.github\/actions\/([a-z-]+)@[0-9a-f]{40} # \S/; // The subscription job's one lane record names why the key did not judge, -// in report-lane-outcome's fallback-reason (ci-workflows intake-triage.yml). +// in report-lane-outcome's fallback-reason (ci-workflows intake-triage.yml); +// a key the credit gate skipped is a credit stop too. const FALLBACK_REASON = - "${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || '' }}"; + "${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || needs.check-intent-credit.outputs.skip-key == 'true' && 'credit-stop' || '' }}"; // The fallback job's record names why the subscription did not judge, from // its handoff-reason (ci-workflows intake-triage.yml, the fallback job). const FB_REASON = @@ -81,17 +85,6 @@ const CREDENTIAL = { }; const others = (job) => JOBS.filter((id) => id !== job).map((id) => CREDENTIAL[id]); const keyGate = () => /^ {4}if: >-\n((?: {6}.*\n)+)/m.exec(jobText(KEY))?.[1].replace(/\s+/g, " ").trim(); -// Evaluates a gate of `a == b` terms joined by && and parenthesized || groups -// on one run's context; an evaluated term of any other shape fails the test. -const admits = (gate, context) => { - const value = (term) => (/^'.*'$/.test(term) ? term.slice(1, -1) : term.split(".").reduce((at, key) => at?.[key], context)); - const equal = (term) => { - assert.match(term, /^\S+ == \S+$/); - const [left, right] = term.split(" == "); - return value(left) === value(right); - }; - return gate.split(" && ").every((clause) => clause.replace(/^\(|\)$/g, "").split(" || ").some(equal)); -}; // A branch filter pattern per GitHub's filter pattern cheat sheet: `*` stops // at `/`, `**` matches any character including `/`. Every other character is // taken literally, so a pattern using `?`, `+`, `[]` or `!` needs more here. @@ -311,16 +304,70 @@ test("each secret appears only in its own job", () => { // A failed upstream run reaches resolve-pr, which judges it only when it // failed on nothing but holds an approving review of the head lifts, the // `do-not-merge` label, or both (heldOnly). +const TRIGGER_GATE = [ + "github.ref == 'refs/heads/main'", + "&& github.event.workflow_run.event == 'pull_request'", + "&& (github.event.workflow_run.conclusion == 'success' || github.event.workflow_run.conclusion == 'failure')", + "&& github.event.workflow_run.head_repository.full_name == github.repository", +].join(" "); +const gateOf = (id) => /^ {4}if: >-\n((?: {6}.*\n)+)/m.exec(jobText(id))?.[1].replace(/\s+/g, " ").trim(); +const withNeeds = (needs, overrides) => ({ ...upstreamRun(overrides), needs }); + test("the key job runs on main for a same-repository pull_request run that passed or failed", () => { - assert.equal( - keyGate(), - [ - "github.ref == 'refs/heads/main'", - "&& github.event.workflow_run.event == 'pull_request'", - "&& (github.event.workflow_run.conclusion == 'success' || github.event.workflow_run.conclusion == 'failure')", - "&& github.event.workflow_run.head_repository.full_name == github.repository", - ].join(" "), - ); + // !cancelled(): a failed or skipped credit gate still runs the key. + assert.equal(keyGate(), `needs.${PROBE}.outputs.skip-key != 'true' && !cancelled() && ${TRIGGER_GATE}`); + assert.equal(gateOf(PROBE), TRIGGER_GATE); +}); + +// ci-workflows check-key-credit (gate.cjs at the pinned SHA): skip-key is +// `true` only while a credit stop recorded in this workflow is inside the +// window; any failure leaves it empty, so the key runs as before the gate. +test("the credit gate skips the key job only on skip-key true, and a gate failure runs the key", () => { + assert.match(jobText(KEY), /^ {4}needs: check-intent-credit$/m); + assert.ok(!admits(keyGate(), withNeeds({ [PROBE]: { outputs: { "skip-key": "true" } } }))); + for (const skip of ["false", "", undefined]) { + assert.ok(admits(keyGate(), withNeeds({ [PROBE]: { result: skip === undefined ? "failure" : "success", outputs: { "skip-key": skip } } })), String(skip)); + } + assert.ok(!admits(keyGate(), withNeeds({ [PROBE]: { outputs: {} } }), { cancelled: true })); +}); + +test("the credit gate holds no secret, checks nothing out and reads only this workflow's runs", () => { + const job = jobText(PROBE); + assert.match(job, /^ {4}permissions:\n {6}actions: read #.*\n {4}outputs:\n {6}skip-key: \$\{\{ steps\.gate\.outputs\.skip-key \}\}\n/m); + assert.doesNotMatch(job, /secrets\.|actions\/checkout|run-claude-cli|\.\/\.base/); + const gate = step("Read the key's credit history", PROBE); + assert.match(gate, /^ {8}id: gate$/m); + assert.match(gate, /^ {8}continue-on-error: true$/m); + assert.match(gate, /^ {8}uses: melodic-software\/ci-workflows\/\.github\/actions\/check-key-credit@[0-9a-f]{40} # \S/m); + // The key job's display name is its id: the gate reads its record there and + // in this job, `-credit`. + assert.equal(input(gate, "key-job"), KEY); + assert.doesNotMatch(jobText(KEY), /^ {4}name:/m); + assert.equal(input(gate, "window-hours"), "${{ vars.KEY_CREDIT_WINDOW_HOURS || '1' }}"); +}); + +// check-key-credit RECORD_PATTERN: /^Record the key's credit stop in run (\d{1,20})$/. +const RECORD_PATTERN = /^Record the key's credit stop in run (\d{1,20})$/u; +test("the record steps carry the pattern the gate reads, the key's only after a credit-stop handoff", () => { + const carried = "Record the key's credit stop in run ${{ steps.gate.outputs.origin-run }}"; + const own = "Record the key's credit stop in run ${{ github.run_id }}"; + assert.match(carried.replace("${{ steps.gate.outputs.origin-run }}", "12345"), RECORD_PATTERN); + assert.match(own.replace("${{ github.run_id }}", "12345"), RECORD_PATTERN); + assert.match(step(carried, PROBE), /^ {8}if: steps\.gate\.outputs\.skip-key == 'true'$/m); + const record = step(own, KEY); + const condition = /^ {8}if: (.*)$/m.exec(record)[1]; + assert.equal(condition, `always() && (${chainOf((id) => `steps.${id}.outputs.handoff-reason == 'credit-stop'`, "judge")})`); + assert.match(record, /^ {8}continue-on-error: true$/m); + // Only the key job records: a subscription credit stop says nothing of the key. + for (const job of [SUB, FB]) assert.equal(index(own, job), -1, job); + // The skipped key leg still leaves its lane record, so leg counts compare. + const report = step("Report the skipped key leg", PROBE); + assert.match(report, /^ {8}if: steps\.gate\.outputs\.skip-key == 'true'$/m); + assert.equal(input(report, "outcome"), "skipped"); + assert.equal(input(report, "lane-id"), "pr-verify"); + assert.equal(input(report, "credential"), CREDENTIAL[KEY][1]); + assert.equal(input(report, "known-failure-class"), "credit-stop"); + assert.equal(input(report, "handoff"), '"true"'); }); // The trigger's branch filter matches the branch the pr-require-checks run ran @@ -354,24 +401,37 @@ test("a same-repository pull_request run on a feature branch passes the trigger assert.ok(!admits(keyGate(), upstreamRun({ head_repository: { full_name: "someone/fork" } }))); }); -test("the subscription job runs only on a clean handoff or an empty key, on a fixed runner", () => { +test("the subscription job runs only on a clean handoff, an empty key or a key the credit gate skipped, on a fixed runner", () => { const job = jobText(SUB); - assert.match(job, /^ {4}needs: check-intent$/m); - const gate = /^ {4}if: >-\n((?: {6}.*\n)+)/m.exec(job)?.[1].replace(/\s+/g, " ").trim(); + assert.match(job, /^ {4}needs: \[check-intent-credit, check-intent\]$/m); + const gate = gateOf(SUB); assert.equal( gate, [ - "github.ref == 'refs/heads/main'", - "&& github.event.workflow_run.event == 'pull_request'", - "&& (github.event.workflow_run.conclusion == 'success' || github.event.workflow_run.conclusion == 'failure')", - "&& github.event.workflow_run.head_repository.full_name == github.repository", + "!cancelled()", + `&& ${TRIGGER_GATE}`, + "&& ((needs.check-intent.result == 'success'", "&& ((needs.check-intent.outputs.handoff == 'true'", "&& needs.check-intent.outputs.key-leg-clean == 'true')", - "|| needs.check-intent.outputs.credential-present == 'false')", + "|| needs.check-intent.outputs.credential-present == 'false'))", + "|| (needs.check-intent.result == 'skipped'", + "&& needs.check-intent-credit.outputs.skip-key == 'true'))", ].join(" "), ); - // No status function, so GitHub adds success(): a failed key job skips it. - assert.doesNotMatch(gate, /always\(\)|failure\(\)|cancelled\(\)|success\(\)/); + // !cancelled() keeps a gate-skipped key job from skipping this one, so the + // key job's success is spelled out: a failed key job skips it. + assert.doesNotMatch(gate, /always\(\)|failure\(\)|success\(\)/); + const run = (key, probe = { outputs: { "skip-key": "" } }) => admits(gate, withNeeds({ [KEY]: key, [PROBE]: probe })); + const handedOff = { handoff: "true", "key-leg-clean": "true", "credential-present": "true" }; + assert.ok(run({ result: "success", outputs: handedOff })); + assert.ok(run({ result: "success", outputs: { "credential-present": "false" } })); + assert.ok(!run({ result: "success", outputs: { ...handedOff, "key-leg-clean": "" } })); + assert.ok(!run({ result: "success", outputs: { "credential-present": "true" } })); + assert.ok(!run({ result: "failure", outputs: handedOff })); + assert.ok(run({ result: "skipped", outputs: {} }, { result: "success", outputs: { "skip-key": "true" } })); + // A key job skipped for any other reason (the trigger gate) judges nothing. + assert.ok(!run({ result: "skipped", outputs: {} }, { result: "skipped", outputs: {} })); + assert.ok(!run({ result: "skipped", outputs: {} }, { result: "success", outputs: { "skip-key": "false" } })); assert.match(job, /^ {4}runs-on: ubuntu-24\.04$/m); // The key job's outputs reach only that if, the resolver's env and the // record's fallback reason, which they pick between three literals; never a @@ -414,21 +474,26 @@ test("the subscription job runs only on a clean handoff or an empty key, on a fi test("the fallback job runs only on the subscription job's clean handoff, on a fixed runner", () => { const job = jobText(FB); assert.match(job, /^ {4}needs: check-intent-subscription$/m); - const gate = /^ {4}if: >-\n((?: {6}.*\n)+)/m.exec(job)?.[1].replace(/\s+/g, " ").trim(); + const gate = gateOf(FB); // One hop: an empty subscription token is the subscription job's red, // never a reason to run the fallback (the reusables gate on handoff alone). assert.equal( gate, [ - "github.ref == 'refs/heads/main'", - "&& github.event.workflow_run.event == 'pull_request'", - "&& (github.event.workflow_run.conclusion == 'success' || github.event.workflow_run.conclusion == 'failure')", - "&& github.event.workflow_run.head_repository.full_name == github.repository", + "!cancelled()", + "&& needs.check-intent-subscription.result == 'success'", + `&& ${TRIGGER_GATE}`, "&& needs.check-intent-subscription.outputs.handoff == 'true'", "&& needs.check-intent-subscription.outputs.leg-clean == 'true'", ].join(" "), ); - assert.doesNotMatch(gate, /always\(\)|failure\(\)|cancelled\(\)|success\(\)/); + // The implicit success() needs every ancestor, so a key job the credit gate + // skipped would skip this job; the subscription job's success is spelled out. + assert.doesNotMatch(gate, /always\(\)|failure\(\)|success\(\)/); + const handedOff = { handoff: "true", "leg-clean": "true" }; + assert.ok(admits(gate, withNeeds({ [SUB]: { result: "success", outputs: handedOff } }))); + assert.ok(!admits(gate, withNeeds({ [SUB]: { result: "failure", outputs: handedOff } }))); + assert.ok(!admits(gate, withNeeds({ [SUB]: { result: "success", outputs: { ...handedOff, "leg-clean": "" } } }))); assert.match(job, /^ {4}runs-on: ubuntu-24\.04$/m); const steps = job.slice(job.indexOf("\n steps:\n")); assert.deepEqual( @@ -499,7 +564,14 @@ test("the subscription job repeats the key job's steps but for the credential", .join("\n"); const normalize = (blocks) => blocks - .filter((s) => !s.name.startsWith("Check the key leg") && !s.name.startsWith("Check the subscription leg") && s.name !== "Hand the reads on") + .filter( + (s) => + !s.name.startsWith("Check the key leg") && + !s.name.startsWith("Check the subscription leg") && + s.name !== "Hand the reads on" && + // The key's credit record: only a key leg's credit stop says the key is out. + s.name !== "Record the key's credit stop in run ${{ github.run_id }}", + ) .map((s) => lines(s.block) .replace(`\n${CARRY_ENV.join("\n")}`, "") @@ -677,7 +749,8 @@ test("a handoff leaves one lane record per leg, the leg that handed off marked h // name and says why in fallback-reason, one of report-lane-outcome's // credit-stop, auth or no-key. assert.match(jobText(SUB), /needs\.check-intent\.outputs\.key-leg-clean == 'true'/); - assert.deepEqual([...FALLBACK_REASON.matchAll(/&& '([a-z-]+)'/g)].map((m) => m[1]), ["credit-stop", "auth", "no-key"]); + // The last credit-stop is the credit gate's skip of the key job. + assert.deepEqual([...FALLBACK_REASON.matchAll(/&& '([a-z-]+)'/g)].map((m) => m[1]), ["credit-stop", "auth", "no-key", "credit-stop"]); assert.match(FALLBACK_REASON, /lane-class == 'auth' && 'auth'/); // A clean subscription handoff skips the subscription job's report the // same way; the fallback job, which runs exactly when leg-clean is true, diff --git a/.github/workflows/pr-review-main.yml b/.github/workflows/pr-review-main.yml index 62dd71b967e..00918fd06b6 100644 --- a/.github/workflows/pr-review-main.yml +++ b/.github/workflows/pr-review-main.yml @@ -25,7 +25,9 @@ name: pr-review-main # lanes' order (pr-review-hosted.yml's secrets): the review job on the API # key; review-subscription on the role's subscription token for a lane whose # key leg handed off (run-claude-cli's handoff: a credit stop, a 401 or 402) -# or whose key is empty; review-fallback on FALLBACK_CLAUDE_CODE_OAUTH_TOKEN +# or whose key is empty, and for every lane while review-handoffs-credit +# finds the key's credit stop recent, which skips the review job; +# review-fallback on FALLBACK_CLAUDE_CODE_OAUTH_TOKEN # for a lane the subscription leg handed off (a limit stop, a credit stop, a # 401 or 402, or an empty token, with the fallback token set). Each leg is # its own job on a new VM holding one credential, and repeats every step from @@ -106,6 +108,7 @@ jobs: head-sha: ${{ steps.gate.outputs.head-sha }} base-sha: ${{ steps.gate.outputs.base-sha }} judged: ${{ steps.judged.outputs.judged }} + main-sha: ${{ steps.main.outputs.main-sha }} steps: - name: Check out the default branch tip uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -114,6 +117,12 @@ jobs: path: .base persist-credentials: false + # The main commit a lane's later leg checks out when the credit gate + # skipped its key leg, which would have recorded its own. + - name: Record the main commit + id: main + run: echo "main-sha=$(git -C .base rev-parse --verify HEAD)" >>"$GITHUB_OUTPUT" + - name: Set up Node uses: ./.base/.github/actions/setup-pinned-node # zizmor: ignore[self-repository] with: @@ -325,13 +334,100 @@ jobs: with: tokens: github-token + # Whether the reviews role's key, which both lanes' key legs share, is still + # out of credit: a key-leg credit stop in this workflow inside the window + # (vars.KEY_CREDIT_WINDOW_HOURS, 1 hour unset) skips the review job, and + # review-handoffs hands both lanes to review-subscription as credit-stop + # (ci-workflows check-key-credit, gate.cjs). The key's record is in + # review-handoffs, the job that routes a key leg's credit stop, so the + # gate reads that job and this one, `review-handoffs-credit`. Its own + # record carries the origin run forward in its name, so the window counts + # from the credit stop, not the last skip, and a topped-up or rotated key + # is tried once it ends. It runs only where this attempt's trigger job + # would start the key legs, holds no secret, checks nothing out and reads + # only this workflow's runs. Every failure runs the key: the review job's + # `if` starts with !cancelled(). + review-handoffs-credit: + needs: trigger + if: >- + github.ref == 'refs/heads/main' + && github.event.workflow_run.event == 'pull_request' + && github.event.workflow_run.conclusion == 'success' + && github.event.workflow_run.head_repository.full_name == github.repository + && needs.trigger.outputs.run-attempt == github.run_attempt + && needs.trigger.outputs.proceed == 'true' + && needs.trigger.outputs.review == 'true' + && needs.trigger.outputs.judged != 'true' + runs-on: ubuntu-24.04 + timeout-minutes: 10 + cache-mode: none + permissions: + actions: read # this workflow's runs and their jobs + 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: melodic-software/ci-workflows/.github/actions/check-key-credit@2476d5dde8979eab34374150b7e4b0a2b99f4e88 # 2476d5d 2026-10-29 + with: + key-job: review-handoffs + window-hours: ${{ vars.KEY_CREDIT_WINDOW_HOURS || '1' }} + + # Read by the next runs' gate; its name is the record (check-key-credit + # RECORD_PATTERN). + - 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 check-key-credit read" + + # Each skipped key leg's lane record: outcome skip, class credit-stop, + # handed off, so leg counts before and after the gate compare. + - name: Report the skipped claude-review key leg + if: steps.gate.outputs.skip-key == 'true' && vars.CLAUDE_REVIEW_DISABLED != 'true' + continue-on-error: true + uses: melodic-software/ci-workflows/.github/actions/report-lane-outcome@6b9c640bf209c0c7e76f1111c5d18c96707348a2 # 6b9c640 2026-11-02 + with: + outcome: skipped + lane: pr-review-main + lane-id: pr-review-main-claude-review + credential: REVIEWS_CLAUDE_API_KEY + known-failure-class: credit-stop + handoff: "true" + + - name: Report the skipped claude-security-review key leg + if: steps.gate.outputs.skip-key == 'true' && vars.CLAUDE_SECURITY_REVIEW_DISABLED != 'true' + continue-on-error: true + uses: melodic-software/ci-workflows/.github/actions/report-lane-outcome@6b9c640bf209c0c7e76f1111c5d18c96707348a2 # 6b9c640 2026-11-02 + with: + outcome: skipped + lane: pr-review-main + lane-id: pr-review-main-claude-security-review + credential: REVIEWS_CLAUDE_API_KEY + known-failure-class: credit-stop + handoff: "true" + + - name: Log the API budget + if: always() + continue-on-error: true + timeout-minutes: 1 + uses: melodic-software/ci-workflows/.github/actions/report-api-budget@2476d5dde8979eab34374150b7e4b0a2b99f4e88 # 2476d5d 2026-10-29 + with: + tokens: github-token + # No leg starts when the trigger job of this attempt found no lane source # or a head both lanes already reviewed: the leg would review nothing. A - # leg re-run alone in a later attempt starts and gates itself. + # leg re-run alone in a later attempt starts and gates itself. With the + # key out of credit, review-handoffs-credit skips this job. review: - needs: trigger + needs: [trigger, review-handoffs-credit] if: >- - github.ref == 'refs/heads/main' + needs.review-handoffs-credit.outputs.skip-key != 'true' + && !cancelled() + && needs.trigger.result == 'success' + && github.ref == 'refs/heads/main' && github.event.workflow_run.event == 'pull_request' && github.event.workflow_run.conclusion == 'success' && github.event.workflow_run.head_repository.full_name == github.repository @@ -1140,13 +1236,16 @@ jobs: # The lanes the key leg handed on, as review-subscription's matrix: fixed # lane rows, each reason checked against the closed set the key leg hands - # on with. It holds no secret, runs no model and checks nothing out. + # on with. With the review job skipped by review-handoffs-credit, every + # enabled lane is handed on as credit-stop, at the trigger job's main + # commit. It holds no secret, runs no model and checks nothing out. review-handoffs: - needs: review + needs: [trigger, review-handoffs-credit, review] if: >- !cancelled() && github.ref == 'refs/heads/main' - && (needs.review.outputs.claude-review != '' || needs.review.outputs.claude-security-review != '') + && (needs.review.outputs.claude-review != '' || needs.review.outputs.claude-security-review != '' + || needs.review.result == 'skipped' && needs.review-handoffs-credit.outputs.skip-key == 'true') runs-on: ubuntu-24.04 timeout-minutes: 5 permissions: {} @@ -1156,10 +1255,10 @@ jobs: - name: Route the handed-off lanes id: route env: - CODE: ${{ needs.review.outputs.claude-review }} - SECURITY: ${{ needs.review.outputs.claude-security-review }} - CODE_BASE: ${{ needs.review.outputs.claude-review-base-sha }} - SECURITY_BASE: ${{ needs.review.outputs.claude-security-review-base-sha }} + CODE: ${{ needs.review.outputs.claude-review || needs.review.result == 'skipped' && needs.review-handoffs-credit.outputs.skip-key == 'true' && vars.CLAUDE_REVIEW_DISABLED != 'true' && 'credit-stop' || '' }} + SECURITY: ${{ needs.review.outputs.claude-security-review || needs.review.result == 'skipped' && needs.review-handoffs-credit.outputs.skip-key == 'true' && vars.CLAUDE_SECURITY_REVIEW_DISABLED != 'true' && 'credit-stop' || '' }} + CODE_BASE: ${{ needs.review.outputs.claude-review-base-sha || needs.trigger.outputs.main-sha }} + SECURITY_BASE: ${{ needs.review.outputs.claude-security-review-base-sha || needs.trigger.outputs.main-sha }} REASONS: credit-stop auth no-key run: | matrix=$(jq -cn --arg code "$CODE" --arg security "$SECURITY" --arg code_base "$CODE_BASE" --arg security_base "$SECURITY_BASE" --arg reasons "$REASONS" ' @@ -1172,6 +1271,16 @@ jobs: echo "matrix=$matrix" >>"$GITHUB_OUTPUT" echo "handed on: $(jq -r '[.include[].lane] | join(", ")' <<<"$matrix")" + # Read by the next runs' review-handoffs-credit gate: a step named for + # this run that succeeds only when a key leg that ran handed on with a + # credit stop, never on a lane the gate itself handed on. Its name is + # the record; keep it equal to check-key-credit's RECORD_PATTERN. + - name: Record the key's credit stop in run ${{ github.run_id }} + if: ${{ !cancelled() && (needs.review.outputs.claude-review == 'credit-stop' || needs.review.outputs.claude-security-review == 'credit-stop') }} + continue-on-error: true + timeout-minutes: 1 + run: echo "the key ended in a credit stop" + # The subscription leg, one per lane the key leg handed on, on a new VM: # the key leg's processes, files and environment do not reach it. Its steps # repeat the review job's from a fresh main checkout but for the @@ -2810,8 +2919,9 @@ jobs: # write-verdict.mjs checked, from this run's own artifacts. record: needs: [review, review-subscription, review-fallback] - # A skipped key leg reviewed nothing, so no later leg ran: nothing to record. - if: ${{ !cancelled() && github.ref == 'refs/heads/main' && needs.review.result != 'skipped' }} + # A key leg skipped by its own gate reviewed nothing, so no later leg ran: + # nothing to record. One the credit gate skipped handed every lane on. + if: ${{ !cancelled() && github.ref == 'refs/heads/main' && (needs.review.result != 'skipped' || needs.review-subscription.result != 'skipped') }} runs-on: ubuntu-24.04 timeout-minutes: 5 permissions: diff --git a/.github/workflows/pr-verify.yml b/.github/workflows/pr-verify.yml index 74498fa1fef..deb1e4c7aa1 100644 --- a/.github/workflows/pr-verify.yml +++ b/.github/workflows/pr-verify.yml @@ -29,7 +29,8 @@ name: pr-verify # when check-intent's key leg handed off (run-claude-cli's handoff output: # a credit stop, or class auth with API status 401 or 402) with no permission # denial, or its key is empty; then check-intent writes no verdict and does -# not fail. With the org secret FALLBACK_CLAUDE_CODE_OAUTH_TOKEN set, the +# not fail. It also runs when check-intent-credit skipped check-intent because +# the key's credit stop is recent. With the org secret FALLBACK_CLAUDE_CODE_OAUTH_TOKEN set, the # subscription leg hands off the same way (a limit stop, a credit stop, or a # 401 or 402) and the check-intent-fallback job judges once on that token, on # a third VM; with it empty, nothing hands off past the subscription job. So @@ -88,13 +89,77 @@ defaults: shell: bash jobs: - check-intent: + # Whether the key is still out of credit: a check-intent credit stop in this + # workflow inside the window (vars.KEY_CREDIT_WINDOW_HOURS, 1 hour unset) + # skips check-intent, and check-intent-subscription judges instead + # (ci-workflows check-key-credit, gate.cjs). Its record step carries the + # origin run forward in its name, so the window counts from the credit stop, + # not the last skip, and a topped-up or rotated key is tried once it ends. + # It holds no secret, checks nothing out and reads only this workflow's runs. + # Every failure runs the key: check-intent's `if` starts with !cancelled(). + check-intent-credit: if: >- github.ref == 'refs/heads/main' && github.event.workflow_run.event == 'pull_request' && (github.event.workflow_run.conclusion == 'success' || github.event.workflow_run.conclusion == 'failure') && github.event.workflow_run.head_repository.full_name == github.repository runs-on: ubuntu-24.04 + timeout-minutes: 10 + cache-mode: none + permissions: + actions: read # this workflow's runs and their jobs + 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: melodic-software/ci-workflows/.github/actions/check-key-credit@2476d5dde8979eab34374150b7e4b0a2b99f4e88 # 2476d5d 2026-10-29 + with: + key-job: check-intent + window-hours: ${{ vars.KEY_CREDIT_WINDOW_HOURS || '1' }} + + # Read by the next runs' gate; its name is the record (check-key-credit + # RECORD_PATTERN). + - 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 check-key-credit read" + + # The skipped key leg's lane record: outcome skip, class credit-stop, + # handed off, so leg counts before and after the gate compare. + - name: Report the skipped key leg + if: steps.gate.outputs.skip-key == 'true' + continue-on-error: true + uses: melodic-software/ci-workflows/.github/actions/report-lane-outcome@6b9c640bf209c0c7e76f1111c5d18c96707348a2 # 6b9c640 2026-11-02 + with: + outcome: skipped + lane: pr-verify + lane-id: pr-verify + credential: PR_VERIFY_CLAUDE_API_KEY + known-failure-class: credit-stop + handoff: "true" + + - name: Log the API budget + if: always() + continue-on-error: true + timeout-minutes: 1 + uses: melodic-software/ci-workflows/.github/actions/report-api-budget@2476d5dde8979eab34374150b7e4b0a2b99f4e88 # 2476d5d 2026-10-29 + with: + tokens: github-token + + check-intent: + needs: check-intent-credit + if: >- + needs.check-intent-credit.outputs.skip-key != 'true' + && !cancelled() + && github.ref == 'refs/heads/main' + && github.event.workflow_run.event == 'pull_request' + && (github.event.workflow_run.conclusion == 'success' || github.event.workflow_run.conclusion == 'failure') + && github.event.workflow_run.head_repository.full_name == github.repository + runs-on: ubuntu-24.04 timeout-minutes: 301 cache-mode: read permissions: @@ -1915,6 +1980,16 @@ jobs: echo '```' } >>"$GITHUB_STEP_SUMMARY" + # Read by the next runs' check-intent-credit gate: a step named for this + # run that succeeds only after a judge step's credit-stop handoff, which + # on this key leg means the key is out of credit. Its name is the record; + # keep it equal to check-key-credit's RECORD_PATTERN. + - name: Record the key's credit stop in run ${{ github.run_id }} + if: always() && (steps.judge.outputs.handoff-reason == 'credit-stop' || steps.judge-2.outputs.handoff-reason == 'credit-stop' || steps.judge-3.outputs.handoff-reason == 'credit-stop' || steps.judge-4.outputs.handoff-reason == 'credit-stop' || steps.judge-5.outputs.handoff-reason == 'credit-stop' || steps.judge-6.outputs.handoff-reason == 'credit-stop' || steps.judge-7.outputs.handoff-reason == 'credit-stop' || steps.judge-8.outputs.handoff-reason == 'credit-stop' || steps.judge-9.outputs.handoff-reason == 'credit-stop' || steps.judge-10.outputs.handoff-reason == 'credit-stop' || steps.judge-11.outputs.handoff-reason == 'credit-stop' || steps.judge-12.outputs.handoff-reason == 'credit-stop' || steps.judge-spec.outputs.handoff-reason == 'credit-stop') + continue-on-error: true + timeout-minutes: 1 + run: echo "the key ended in a credit stop" + - name: Log the API budget if: always() continue-on-error: true @@ -1938,18 +2013,23 @@ jobs: # key-leg check and the reads it takes from check-intent's `reads` output # (workflow.test.mjs holds them equal), so the trigger gate, the kill # switch and the hardening run again. Only this job references the - # subscription token. needs adds success(): a check-intent that failed, a - # denial on its key leg included, leaves this job skipped. + # subscription token. It runs after a check-intent that succeeded, so one + # that failed, a denial on its key leg included, leaves this job skipped; + # or after check-intent-credit skipped check-intent, with no reads to take. check-intent-subscription: - needs: check-intent + needs: [check-intent-credit, check-intent] if: >- - github.ref == 'refs/heads/main' + !cancelled() + && github.ref == 'refs/heads/main' && github.event.workflow_run.event == 'pull_request' && (github.event.workflow_run.conclusion == 'success' || github.event.workflow_run.conclusion == 'failure') && github.event.workflow_run.head_repository.full_name == github.repository + && ((needs.check-intent.result == 'success' && ((needs.check-intent.outputs.handoff == 'true' && needs.check-intent.outputs.key-leg-clean == 'true') - || needs.check-intent.outputs.credential-present == 'false') + || needs.check-intent.outputs.credential-present == 'false')) + || (needs.check-intent.result == 'skipped' + && needs.check-intent-credit.outputs.skip-key == 'true')) runs-on: ubuntu-24.04 timeout-minutes: 301 cache-mode: read @@ -2304,7 +2384,7 @@ jobs: known-failure-class: ${{ steps.judge.outputs.lane-class }} handoff: ${{ steps.judge.outputs.handoff == 'true' && steps.sub-leg.outcome != 'failure' }} # Why this job ran, for its lane record. - fallback-reason: ${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || '' }} + fallback-reason: ${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || needs.check-intent-credit.outputs.skip-key == 'true' && 'credit-stop' || '' }} # The leg's transcript, failed legs included, sealed to the transcript # key and kept 7 days for the puller; a failed upload never fails the job. @@ -2414,7 +2494,7 @@ jobs: max-turns: ${{ steps.models.outputs.max-turns }} known-failure-class: ${{ steps.judge-2.outputs.lane-class }} handoff: ${{ steps.judge-2.outputs.handoff == 'true' && steps.sub-leg-2.outcome != 'failure' }} - fallback-reason: ${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || '' }} + fallback-reason: ${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || needs.check-intent-credit.outputs.skip-key == 'true' && 'credit-stop' || '' }} - name: Upload chunk 2's sealed transcript if: always() && steps.judge-2.outputs.execution-file != '' @@ -2512,7 +2592,7 @@ jobs: max-turns: ${{ steps.models.outputs.max-turns }} known-failure-class: ${{ steps.judge-3.outputs.lane-class }} handoff: ${{ steps.judge-3.outputs.handoff == 'true' && steps.sub-leg-3.outcome != 'failure' }} - fallback-reason: ${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || '' }} + fallback-reason: ${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || needs.check-intent-credit.outputs.skip-key == 'true' && 'credit-stop' || '' }} - name: Upload chunk 3's sealed transcript if: always() && steps.judge-3.outputs.execution-file != '' @@ -2611,7 +2691,7 @@ jobs: max-turns: ${{ steps.models.outputs.max-turns }} known-failure-class: ${{ steps.judge-4.outputs.lane-class }} handoff: ${{ steps.judge-4.outputs.handoff == 'true' && steps.sub-leg-4.outcome != 'failure' }} - fallback-reason: ${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || '' }} + fallback-reason: ${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || needs.check-intent-credit.outputs.skip-key == 'true' && 'credit-stop' || '' }} - name: Upload chunk 4's sealed transcript if: always() && steps.judge-4.outputs.execution-file != '' @@ -2712,7 +2792,7 @@ jobs: max-turns: ${{ steps.models.outputs.max-turns }} known-failure-class: ${{ steps.judge-5.outputs.lane-class }} handoff: ${{ steps.judge-5.outputs.handoff == 'true' && steps.sub-leg-5.outcome != 'failure' }} - fallback-reason: ${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || '' }} + fallback-reason: ${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || needs.check-intent-credit.outputs.skip-key == 'true' && 'credit-stop' || '' }} - name: Upload chunk 5's sealed transcript if: always() && steps.judge-5.outputs.execution-file != '' @@ -2813,7 +2893,7 @@ jobs: max-turns: ${{ steps.models.outputs.max-turns }} known-failure-class: ${{ steps.judge-6.outputs.lane-class }} handoff: ${{ steps.judge-6.outputs.handoff == 'true' && steps.sub-leg-6.outcome != 'failure' }} - fallback-reason: ${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || '' }} + fallback-reason: ${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || needs.check-intent-credit.outputs.skip-key == 'true' && 'credit-stop' || '' }} - name: Upload chunk 6's sealed transcript if: always() && steps.judge-6.outputs.execution-file != '' @@ -2915,7 +2995,7 @@ jobs: max-turns: ${{ steps.models.outputs.max-turns }} known-failure-class: ${{ steps.judge-7.outputs.lane-class }} handoff: ${{ steps.judge-7.outputs.handoff == 'true' && steps.sub-leg-7.outcome != 'failure' }} - fallback-reason: ${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || '' }} + fallback-reason: ${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || needs.check-intent-credit.outputs.skip-key == 'true' && 'credit-stop' || '' }} - name: Upload chunk 7's sealed transcript if: always() && steps.judge-7.outputs.execution-file != '' @@ -3018,7 +3098,7 @@ jobs: max-turns: ${{ steps.models.outputs.max-turns }} known-failure-class: ${{ steps.judge-8.outputs.lane-class }} handoff: ${{ steps.judge-8.outputs.handoff == 'true' && steps.sub-leg-8.outcome != 'failure' }} - fallback-reason: ${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || '' }} + fallback-reason: ${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || needs.check-intent-credit.outputs.skip-key == 'true' && 'credit-stop' || '' }} - name: Upload chunk 8's sealed transcript if: always() && steps.judge-8.outputs.execution-file != '' @@ -3122,7 +3202,7 @@ jobs: max-turns: ${{ steps.models.outputs.max-turns }} known-failure-class: ${{ steps.judge-9.outputs.lane-class }} handoff: ${{ steps.judge-9.outputs.handoff == 'true' && steps.sub-leg-9.outcome != 'failure' }} - fallback-reason: ${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || '' }} + fallback-reason: ${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || needs.check-intent-credit.outputs.skip-key == 'true' && 'credit-stop' || '' }} - name: Upload chunk 9's sealed transcript if: always() && steps.judge-9.outputs.execution-file != '' @@ -3227,7 +3307,7 @@ jobs: max-turns: ${{ steps.models.outputs.max-turns }} known-failure-class: ${{ steps.judge-10.outputs.lane-class }} handoff: ${{ steps.judge-10.outputs.handoff == 'true' && steps.sub-leg-10.outcome != 'failure' }} - fallback-reason: ${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || '' }} + fallback-reason: ${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || needs.check-intent-credit.outputs.skip-key == 'true' && 'credit-stop' || '' }} - name: Upload chunk 10's sealed transcript if: always() && steps.judge-10.outputs.execution-file != '' @@ -3333,7 +3413,7 @@ jobs: max-turns: ${{ steps.models.outputs.max-turns }} known-failure-class: ${{ steps.judge-11.outputs.lane-class }} handoff: ${{ steps.judge-11.outputs.handoff == 'true' && steps.sub-leg-11.outcome != 'failure' }} - fallback-reason: ${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || '' }} + fallback-reason: ${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || needs.check-intent-credit.outputs.skip-key == 'true' && 'credit-stop' || '' }} - name: Upload chunk 11's sealed transcript if: always() && steps.judge-11.outputs.execution-file != '' @@ -3440,7 +3520,7 @@ jobs: max-turns: ${{ steps.models.outputs.max-turns }} known-failure-class: ${{ steps.judge-12.outputs.lane-class }} handoff: ${{ steps.judge-12.outputs.handoff == 'true' && steps.sub-leg-12.outcome != 'failure' }} - fallback-reason: ${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || '' }} + fallback-reason: ${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || needs.check-intent-credit.outputs.skip-key == 'true' && 'credit-stop' || '' }} - name: Upload chunk 12's sealed transcript if: always() && steps.judge-12.outputs.execution-file != '' @@ -3613,7 +3693,7 @@ jobs: max-turns: ${{ steps.models.outputs.max-turns }} known-failure-class: ${{ steps.judge-spec.outputs.lane-class }} handoff: ${{ steps.judge-spec.outputs.handoff == 'true' && steps.sub-leg-spec.outcome != 'failure' }} - fallback-reason: ${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || '' }} + fallback-reason: ${{ needs.check-intent.outputs.lane-class == 'credit-stop' && 'credit-stop' || needs.check-intent.outputs.lane-class == 'auth' && 'auth' || needs.check-intent.outputs.credential-present == 'false' && 'no-key' || needs.check-intent-credit.outputs.skip-key == 'true' && 'credit-stop' || '' }} - name: Upload the spec judgment's sealed transcript if: always() && steps.judge-spec.outputs.execution-file != '' @@ -3762,10 +3842,14 @@ jobs: # to a fixed string in its `if` or the fallback-reason expression, never # written into a `run:` script. It is the last leg and never hands off: an # empty subscription token fails the subscription job instead. + # !cancelled(): the implicit success() also needs every ancestor, so a + # check-intent the credit gate skipped would skip this job too. check-intent-fallback: needs: check-intent-subscription if: >- - github.ref == 'refs/heads/main' + !cancelled() + && needs.check-intent-subscription.result == 'success' + && github.ref == 'refs/heads/main' && github.event.workflow_run.event == 'pull_request' && (github.event.workflow_run.conclusion == 'success' || github.event.workflow_run.conclusion == 'failure') && github.event.workflow_run.head_repository.full_name == github.repository From 606e5313408cbae533337b29306d46e82f27fa43 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Mon, 2 Nov 2026 21:35:15 -0500 Subject: [PATCH 2/2] fix(pr-pipeline): carry pr-review-main's credit record on every run check-key-credit reads only the newest three completed, unskipped runs. Most pr-review-main runs end success with no lane source to review, so a gate that ran only on lane-source runs left those runs without the carried record, and the origin fell out of the gate's reads before the window ended. review-handoffs-credit now runs on every run of this attempt's trigger job. Only a run whose trigger job starts the key legs reports them skipped or lets review-handoffs hand the lanes on, so a run with no lane source still routes nothing. Refs: #12526 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VcupojVgxFqBnrah9DwFLE --- .../scripts/pr-review-main/workflow.test.mjs | 34 +++++++++++++------ .github/workflows/pr-review-main.yml | 21 ++++++------ 2 files changed, 35 insertions(+), 20 deletions(-) diff --git a/.github/scripts/pr-review-main/workflow.test.mjs b/.github/scripts/pr-review-main/workflow.test.mjs index a8200bfb650..49ef47c1d82 100644 --- a/.github/scripts/pr-review-main/workflow.test.mjs +++ b/.github/scripts/pr-review-main/workflow.test.mjs @@ -1027,12 +1027,18 @@ const RECORD_IF = /^ {4}if: \$\{\{ !cancelled\(\) && github\.ref == 'refs\/heads test("no leg starts, and nothing records, for a head with no lane source or one both lanes reviewed", () => { assert.ok(/^ {4}if: >-\n((?: {6}.*\n)+)/m.exec(jobText(KEY))[1].endsWith(KEY_SKIP)); assert.match(jobText("record"), RECORD_IF); - // The credit gate runs only where this attempt's trigger job starts the key leg. - for (const term of ["needs.trigger.outputs.run-attempt == github.run_attempt", "needs.trigger.outputs.proceed == 'true'", "needs.trigger.outputs.review == 'true'", "needs.trigger.outputs.judged != 'true'"]) { - assert.ok(jobGate(PROBE).includes(`&& ${term}`), term); + // A skipped key leg is reported, and a lane handed on, only where this + // attempt's trigger job starts the key leg. + for (const report of ["Report the skipped claude-review key leg", "Report the skipped claude-security-review key leg"]) { + const condition = /^ {8}if: (.*)$/m.exec(step(report, PROBE))[1]; + for (const term of GO_AHEAD.slice(1)) assert.ok(condition.includes(` && ${term} && `), `${report}: ${term}`); } + for (const term of GO_AHEAD) assert.ok(jobGate("review-handoffs").includes(`&& ${term}`), term); }); +// This attempt's trigger job starts the key leg: a lane-source head not yet judged. +const GO_AHEAD = ["needs.trigger.outputs.run-attempt == github.run_attempt", "needs.trigger.outputs.proceed == 'true'", "needs.trigger.outputs.review == 'true'", "needs.trigger.outputs.judged != 'true'"]; + const jobGate = (id) => /^ {4}if: >-\n((?: {6}.*\n)+)/m.exec(jobText(id))?.[1].replace(/\s+/g, " ").trim(); const upstreamRun = { ref: "refs/heads/main", repository: "o/r", run_attempt: "1", event: { workflow_run: { event: "pull_request", conclusion: "success", head_repository: { full_name: "o/r" } } } }; const triggered = { result: "success", outputs: { "run-attempt": "1", proceed: "true", review: "true", judged: "" } }; @@ -1051,12 +1057,15 @@ test("the credit gate skips the key leg only on skip-key true, and a gate failur // The trigger job's own result still gates the key leg, as the implicit success() did. assert.ok(!runs({ result: "failure", outputs: {} }, { ...triggered, result: "failure" })); assert.ok(!runs({ result: "success", outputs: {} }, triggered, { cancelled: true })); - // The gate itself: the trigger job's checks only, then this attempt's go-ahead. + // The gate runs on every run of this attempt's trigger job, a head with no + // lane source or one already judged included, so each completed run the + // gate reads next (only the newest few) carries the record forward. const probeRuns = (trigger) => admits(jobGate(PROBE), { github: upstreamRun, needs: { trigger } }); - assert.ok(probeRuns(triggered)); - for (const outputs of [{ proceed: "" }, { review: "false" }, { judged: "true" }, { "run-attempt": "2" }]) { - assert.ok(!probeRuns({ ...triggered, outputs: { ...triggered.outputs, ...outputs } }), JSON.stringify(outputs)); + for (const outputs of [{}, { proceed: "" }, { review: "false" }, { judged: "true" }]) { + assert.ok(probeRuns({ ...triggered, outputs: { ...triggered.outputs, ...outputs } }), JSON.stringify(outputs)); } + assert.ok(!probeRuns({ ...triggered, outputs: { ...triggered.outputs, "run-attempt": "2" } })); + assert.ok(!admits(jobGate(PROBE), { github: { ...upstreamRun, ref: "refs/heads/x" }, needs: { trigger: triggered } })); }); test("the credit gate holds no secret, checks nothing out and reads only this workflow's runs", () => { @@ -1076,7 +1085,7 @@ test("the credit gate holds no secret, checks nothing out and reads only this wo // Each skipped key leg leaves its lane record, so leg counts compare. for (const [lane, sw] of [["claude-review", "CLAUDE_REVIEW_DISABLED"], ["claude-security-review", "CLAUDE_SECURITY_REVIEW_DISABLED"]]) { const report = step(`Report the skipped ${lane} key leg`, PROBE); - assert.match(report, new RegExp(`^ {8}if: steps\\.gate\\.outputs\\.skip-key == 'true' && vars\\.${sw} != 'true'$`, "m")); + assert.match(report, new RegExp(`^ {8}if: steps\\.gate\\.outputs\\.skip-key == 'true' && ${GO_AHEAD.slice(1).join(" && ").replace(/[.()|]/g, "\\$&")} && vars\\.${sw} != 'true'$`, "m")); assert.equal(input(report, "outcome"), "skipped"); assert.equal(input(report, "lane-id"), `pr-review-main-${lane}`); assert.equal(input(report, "credential"), "REVIEWS_CLAUDE_API_KEY"); @@ -1105,13 +1114,18 @@ test("with the key leg skipped by the credit gate, every enabled lane goes to th const route = jobText("review-handoffs"); assert.equal(needsOf("review-handoffs"), `[trigger, ${PROBE}, ${KEY}]`); const routeIf = jobGate("review-handoffs"); - const handsOn = (review, probe, options) => admits(routeIf, { github: upstreamRun, needs: { review, [PROBE]: probe } }, options); + const handsOn = (review, probe, trigger = triggered) => admits(routeIf, { github: upstreamRun, needs: { trigger, review, [PROBE]: probe } }); const gated = { result: "success", outputs: { "skip-key": "true" } }; assert.ok(handsOn({ result: "skipped", outputs: {} }, gated)); assert.ok(handsOn({ result: "success", outputs: { "claude-review": "credit-stop" } }, { result: "success", outputs: { "skip-key": "false" } })); // A key leg skipped for any other reason, or that handed nothing on, routes nothing. assert.ok(!handsOn({ result: "skipped", outputs: {} }, { result: "skipped", outputs: {} })); assert.ok(!handsOn({ result: "success", outputs: {} }, { result: "success", outputs: { "skip-key": "false" } })); + // With the key out of credit, a run whose trigger job starts no key leg + // (no lane source, a judged head, a refused gate, another attempt) routes nothing. + for (const outputs of [{ review: "false" }, { judged: "true" }, { proceed: "" }, { "run-attempt": "2" }]) { + assert.ok(!handsOn({ result: "skipped", outputs: {} }, gated, { ...triggered, outputs: { ...triggered.outputs, ...outputs } }), JSON.stringify(outputs)); + } for (const [env, lane, sw] of [["CODE", "claude-review", "CLAUDE_REVIEW_DISABLED"], ["SECURITY", "claude-security-review", "CLAUDE_SECURITY_REVIEW_DISABLED"]]) { assert.match(route, new RegExp(`^ {10}${env}: \\$\\{\\{ needs\\.review\\.outputs\\.${lane} \\|\\| needs\\.review\\.result == 'skipped' && needs\\.${PROBE}\\.outputs\\.skip-key == 'true' && vars\\.${sw} != 'true' && 'credit-stop' \\|\\| '' \\}\\}$`, "m")); } @@ -1484,7 +1498,7 @@ test("a later leg runs only for the lanes the previous leg handed on", () => { // The key route also reads the trigger job's commit and the credit gate's // skip, which hands every lane on (its own test above). assert.equal(needsOf(route), route === "review-handoffs" ? `[trigger, ${PROBE}, ${from}]` : from); - const gated = route === "review-handoffs" ? `\\n {6}\\|\\| needs\\.review\\.result == 'skipped' && needs\\.${PROBE}\\.outputs\\.skip-key == 'true'` : ""; + const gated = route === "review-handoffs" ? `\\n {6}\\|\\| needs\\.review\\.result == 'skipped' && needs\\.${PROBE}\\.outputs\\.skip-key == 'true'\\n[^)]*` : ""; assert.match(routeJob, new RegExp(`&& \\(needs\\.${from}\\.outputs\\.claude-review != '' \\|\\| needs\\.${from}\\.outputs\\.claude-security-review != ''${gated}\\)`)); // !cancelled(): one lane's failed leg does not drop the other lane's // handoff; the route still passes on only closed-set reasons. diff --git a/.github/workflows/pr-review-main.yml b/.github/workflows/pr-review-main.yml index 00918fd06b6..e0491574ee0 100644 --- a/.github/workflows/pr-review-main.yml +++ b/.github/workflows/pr-review-main.yml @@ -343,10 +343,12 @@ jobs: # gate reads that job and this one, `review-handoffs-credit`. Its own # record carries the origin run forward in its name, so the window counts # from the credit stop, not the last skip, and a topped-up or rotated key - # is tried once it ends. It runs only where this attempt's trigger job - # would start the key legs, holds no secret, checks nothing out and reads - # only this workflow's runs. Every failure runs the key: the review job's - # `if` starts with !cancelled(). + # is tried once it ends. It runs on every run of this attempt's trigger + # job, lane source or not, so each run the gate reads carries the record; + # only a run whose trigger job starts the key legs reports them skipped or + # hands them on. It holds no secret, checks nothing out and reads only this + # workflow's runs. Every failure runs the key: the review job's `if` starts + # with !cancelled(). review-handoffs-credit: needs: trigger if: >- @@ -355,9 +357,6 @@ jobs: && github.event.workflow_run.conclusion == 'success' && github.event.workflow_run.head_repository.full_name == github.repository && needs.trigger.outputs.run-attempt == github.run_attempt - && needs.trigger.outputs.proceed == 'true' - && needs.trigger.outputs.review == 'true' - && needs.trigger.outputs.judged != 'true' runs-on: ubuntu-24.04 timeout-minutes: 10 cache-mode: none @@ -386,7 +385,7 @@ jobs: # Each skipped key leg's lane record: outcome skip, class credit-stop, # handed off, so leg counts before and after the gate compare. - name: Report the skipped claude-review key leg - if: steps.gate.outputs.skip-key == 'true' && vars.CLAUDE_REVIEW_DISABLED != 'true' + if: steps.gate.outputs.skip-key == 'true' && needs.trigger.outputs.proceed == 'true' && needs.trigger.outputs.review == 'true' && needs.trigger.outputs.judged != 'true' && vars.CLAUDE_REVIEW_DISABLED != 'true' continue-on-error: true uses: melodic-software/ci-workflows/.github/actions/report-lane-outcome@6b9c640bf209c0c7e76f1111c5d18c96707348a2 # 6b9c640 2026-11-02 with: @@ -398,7 +397,7 @@ jobs: handoff: "true" - name: Report the skipped claude-security-review key leg - if: steps.gate.outputs.skip-key == 'true' && vars.CLAUDE_SECURITY_REVIEW_DISABLED != 'true' + if: steps.gate.outputs.skip-key == 'true' && needs.trigger.outputs.proceed == 'true' && needs.trigger.outputs.review == 'true' && needs.trigger.outputs.judged != 'true' && vars.CLAUDE_SECURITY_REVIEW_DISABLED != 'true' continue-on-error: true uses: melodic-software/ci-workflows/.github/actions/report-lane-outcome@6b9c640bf209c0c7e76f1111c5d18c96707348a2 # 6b9c640 2026-11-02 with: @@ -1245,7 +1244,9 @@ jobs: !cancelled() && github.ref == 'refs/heads/main' && (needs.review.outputs.claude-review != '' || needs.review.outputs.claude-security-review != '' - || needs.review.result == 'skipped' && needs.review-handoffs-credit.outputs.skip-key == 'true') + || needs.review.result == 'skipped' && needs.review-handoffs-credit.outputs.skip-key == 'true' + && needs.trigger.outputs.run-attempt == github.run_attempt && needs.trigger.outputs.proceed == 'true' + && needs.trigger.outputs.review == 'true' && needs.trigger.outputs.judged != 'true') runs-on: ubuntu-24.04 timeout-minutes: 5 permissions: {}