From f306a8977bde895e657f8e3c35ecbc855fed6501 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Fri, 23 Oct 2026 03:44:21 -0400 Subject: [PATCH 1/2] fix(pr-pipeline): start the address-feedback and review-dependency fallback calls from trusted values address-feedback-subscription and review-dependency-subscription started on the first call's lane-class and credential-present, which the read runner's run job sets after head code runs. Give each leg a handoff job, as fix-ci has, that reads this run's jobs through the Actions API once, and start the fallback call on its posted output. Every fallback call with a handoff job, fix-ci's included, now passes fallback: handoff instead of a label from the first call's lane-class; the read runner accepts it and labels the lane record "posted no check". Closes #9365 Co-Authored-By: Claude Opus 5.5 --- ...r-unblock-subscription-handoff-3c9e17a4.md | 8 + .../actions/resolve-config/callers.test.mjs | 147 +++++++++++++++--- .../pr-unblock/workflow-gates.test.mjs | 53 ++++++- .github/workflows/pr-run-activity-read.yml | 14 +- .github/workflows/pr-unblock.yml | 96 +++++++++--- .../pr-pipeline/pr-run-activity.md | 23 +-- 6 files changed, 285 insertions(+), 56 deletions(-) create mode 100644 .changes/pr-pipeline/fix-pr-unblock-subscription-handoff-3c9e17a4.md diff --git a/.changes/pr-pipeline/fix-pr-unblock-subscription-handoff-3c9e17a4.md b/.changes/pr-pipeline/fix-pr-unblock-subscription-handoff-3c9e17a4.md new file mode 100644 index 0000000000..c203626a9e --- /dev/null +++ b/.changes/pr-pipeline/fix-pr-unblock-subscription-handoff-3c9e17a4.md @@ -0,0 +1,8 @@ +--- +bump: patch +--- + +### Fixed + +- address-feedback's and review-dependency's fallback calls no longer start on the read runner's run-job outputs, which head code can forge. As for fix-ci, `address-feedback-handoff` and `review-dependency-handoff`, base jobs with only `actions: read`, read this run's jobs through the Actions API once, and the fallback call runs when the first call's report job posted no check ([#9365](https://github.com/melodic-software/claude-code-plugins/issues/9365)). +- Every fallback call with a handoff job, fix-ci's among them, passes `fallback: handoff`, which the read runner now accepts, instead of a label taken from the first call's `lane-class`. Its lane record reads "posted no check" in place of "hit a credit stop" or "is empty" ([#9365](https://github.com/melodic-software/claude-code-plugins/issues/9365)). diff --git a/.github/actions/resolve-config/callers.test.mjs b/.github/actions/resolve-config/callers.test.mjs index 55eda6cd06..ef28f59ea0 100644 --- a/.github/actions/resolve-config/callers.test.mjs +++ b/.github/actions/resolve-config/callers.test.mjs @@ -1271,9 +1271,20 @@ test("pr-run-activity-read.yml: the fallback call's lane record names why it ran assert.equal(lane(""), "pr-unblock/fix-ci"); assert.equal(lane("credit-stop"), "pr-unblock/fix-ci (key AUTOMATIONS_CLAUDE_API_KEY hit a credit stop)"); assert.equal(lane("no-key"), "pr-unblock/fix-ci (key AUTOMATIONS_CLAUDE_API_KEY is empty)"); + assert.equal(lane("handoff"), "pr-unblock/fix-ci (key AUTOMATIONS_CLAUDE_API_KEY posted no check)"); assert.equal(record.with["step-timeout-minutes"], 50, "the launcher step's timeout"); }); +// A caller whose handoff job read only that the first call posted no check +// passes fallback handoff; the run job's input check must let it through. +test("pr-run-activity-read.yml: the fallback input takes empty, credit-stop, no-key or handoff, and nothing else", () => { + const step = workflow("pr-run-activity-read.yml").jobs.run.steps.find((s) => s.env?.FALLBACK === "${{ inputs.fallback }}"); + const check = /^ *case "\$FALLBACK" in\n[\s\S]*?^ *esac$/m.exec(step.run)[0]; + const status = (fallback) => spawnSync("bash", ["-c", check], { env: { FALLBACK: fallback }, encoding: "utf8" }).status; + for (const fallback of ["", "credit-stop", "no-key", "handoff"]) assert.equal(status(fallback), 0, fallback); + for (const fallback of ["handoff ", "posted", "credit-stop\nno-key"]) assert.equal(status(fallback), 1, JSON.stringify(fallback)); +}); + // Each patch producer run on run-claude-cli, as [file, first job, // subscription job, the subscription job's other needs, the outputs of those // needs it must also see, the push job]. Each call holds one Claude secret @@ -1340,13 +1351,13 @@ const PATCH_OUTPUTS = { "patch-state": "patch-state", "patch-sha256": "patch-sha const SCAN_ONLY = new Set(["fix-ci", "address-feedback", "review-dependency"]); // The base-code job whose jobs-API read starts a lane's fallback call, in // place of the first call's own handoff outputs. -const HANDOFF = { "fix-ci": "fix-ci-handoff" }; +const HANDOFF = { "fix-ci": "fix-ci-handoff", "address-feedback": "address-feedback-handoff", "review-dependency": "review-dependency-handoff" }; const PATCH_OUTPUTS_OF = (firstId) => SCAN_ONLY.has(firstId) ? Object.fromEntries(Object.entries(PATCH_OUTPUTS).map(([input, output]) => [input, `scan-${output}`])) : PATCH_OUTPUTS; const WRITE_RUNNER = "./.github/workflows/pr-run-activity-write.yml"; for (const [file, firstId, subId, extraNeeds, extraProceed, applyId] of PATCH_LANES) { - test(`${file}: ${subId} runs only on ${firstId}'s credit stop or empty key and reads only those two outputs`, () => { + test(`${file}: ${subId} runs only after ${firstId} handed off, read ${HANDOFF[firstId] ? `by ${HANDOFF[firstId]}` : "from its credit stop or empty key alone"}`, () => { const { jobs } = workflow(file); const first = jobs[firstId]; const sub = jobs[subId]; @@ -1362,11 +1373,12 @@ for (const [file, firstId, subId, extraNeeds, extraProceed, applyId] of PATCH_LA const { fallback, ...same } = sub.with; assert.deepEqual(same, first.with, "the fallback call runs the same activity on the same inputs"); // The first call's outputs reach the fallback call only through two exact - // compares, and a lane with a handoff job only through fallback's label. + // compares, and a lane with a handoff job not at all: its fallback is the + // one fact the handoff job's read attests, that the first call posted no check. const outputsOf = new RegExp(`needs\\.${firstId}\\.outputs\\.`); - const text = JSON.stringify(HANDOFF[firstId] ? { ...sub, if: "" } : sub); + const text = JSON.stringify(sub); const compares = [...text.matchAll(new RegExp(`needs\\.${firstId}\\.outputs\\.([\\w-]+) == '([^']*)'`, "g"))].map((m) => `${m[1]} == ${m[2]}`); - assert.deepEqual([...new Set(compares)].sort(), HANDOFF[firstId] ? ["lane-class == credit-stop"] : ["credential-present == false", "lane-class == credit-stop"]); + assert.deepEqual([...new Set(compares)].sort(), HANDOFF[firstId] ? [] : ["credential-present == false", "lane-class == credit-stop"]); assert.doesNotMatch( text.replace(new RegExp(`needs\\.${firstId}\\.outputs\\.(lane-class == 'credit-stop'|credential-present == 'false')`, "g"), ""), outputsOf, @@ -1388,8 +1400,12 @@ for (const [file, firstId, subId, extraNeeds, extraProceed, applyId] of PATCH_LA assert.equal(runs("credit-stop", "true", { ...extraProceed, [name]: "false" }), false, `${name} false`); } } - assert.equal(evaluate(fallback, { [`needs.${firstId}.outputs.lane-class`]: "credit-stop" }), "credit-stop"); - assert.equal(evaluate(fallback, { [`needs.${firstId}.outputs.lane-class`]: "" }), "no-key"); + if (HANDOFF[firstId]) { + assert.equal(fallback, "handoff"); + } else { + assert.equal(evaluate(fallback, { [`needs.${firstId}.outputs.lane-class`]: "credit-stop" }), "credit-stop"); + assert.equal(evaluate(fallback, { [`needs.${firstId}.outputs.lane-class`]: "" }), "no-key"); + } // No run: script in the lane reads the first call's outputs. for (const [id, job] of Object.entries(jobs)) { for (const step of job.steps ?? []) { @@ -3217,11 +3233,11 @@ test("leak scan: the split-line pass leaves out the R2 key pair pattern, so adja // --- address-feedback on run-claude-cli: a select job, two model calls, a push job, a reply job --- // The outputs of a model call (the read runner) each address-feedback job may -// read: the subscription call its fallback compares, the push job the patch -// the scan job uploaded, the reply job the replies it uploaded. No job takes a -// gate, a head or a reply input from anything else a model call reports. +// read: the push job the patch the scan job uploaded, the reply job the +// replies it uploaded. No job takes a gate, a head or a reply input from +// anything else a model call reports; the subscription call starts on +// address-feedback-handoff's read and reads none. const MODEL_CALL_READS = { - "address-feedback-subscription": ["credential-present", "lane-class"], "apply-feedback-patch": ["feedback-attempt", "scan-patch-attempt", "scan-patch-sha256", "scan-patch-state", "scan-reply-attempt"], "publish-feedback": ["feedback-attempt", "feedback-sha256", "scan-patch-state"], }; @@ -3290,13 +3306,12 @@ test("pr-unblock.yml: the apply and reply jobs take every model-call input from // --- review-dependency on run-claude-cli: a select job, two model calls, a push job, a verdict job --- -// The outputs of a model call each review-dependency job may read: the -// subscription call its fallback compares; the push job the patch, its report -// and the verdict the scan job uploaded; the verdict job the verdict, the -// patch state and the scan's refusal. No job takes a gate or a head from a -// model call. +// The outputs of a model call each review-dependency job may read: the push +// job the patch, its report and the verdict the scan job uploaded; the +// verdict job the verdict, the patch state and the scan's refusal. No job +// takes a gate or a head from a model call; the subscription call starts on +// review-dependency-handoff's read and reads none. const DEPENDENCY_CALL_READS = { - "review-dependency-subscription": ["credential-present", "lane-class"], "apply-dependency-patch": ["dependency-attempt", "dependency-sha256", "dependency-verdict", "scan-patch-attempt", "scan-patch-sha256", "scan-patch-state", "scan-reply-attempt"], "apply-dependency-verdict": ["dependency-attempt", "dependency-sha256", "scan-patch-state", "scan-refused"], }; @@ -3916,9 +3931,9 @@ const FIX_CI_CALLS = ["fix-ci", "fix-ci-subscription", "fix-ci-rung-up", "fix-ci // `reportSkipped` the calls whose report job was skipped, as a handoff, forged // or not, leaves it; `unallowed` the calls whose broker step did not succeed // (skipped on a kill switch, failed on a refusal). -function jobListing(posted, { reportSkipped = [], unallowed = [] } = {}) { +function jobListing(posted, { reportSkipped = [], unallowed = [], calls = FIX_CI_CALLS } = {}) { const jobs = [{ name: "fix-ci-context", conclusion: "success", steps: [] }]; - for (const call of FIX_CI_CALLS) { + for (const call of calls) { jobs.push({ name: `${call} / broker-check`, conclusion: "success", @@ -4200,3 +4215,97 @@ test("pr-run-activity-read.yml: a trigger gate that read another head than the c const { jobs } = workflow("pr-unblock.yml"); for (const id of FIX_CI_CALLS) assert.equal(jobs[id].with["head-sha"], "${{ needs.fix-ci-context.outputs.head-sha }}", id); }); + +// --- pr-unblock subscription legs: every start from a trusted value --- + +// The read runner's outputs its run job sets. The run job runs head code +// (rule h16), so a caller's start may read none of them. +const RUN_JOB_OUTPUTS = () => + Object.entries(workflow("pr-run-activity-read.yml").on.workflow_call.outputs) + .filter(([, output]) => /\bjobs\.run\.outputs\./.test(output.value)) + .map(([name]) => name); +// Automation Lanes' PR-B (#9332) gives fix-docs its handoff job; until then +// its subscription leg still starts on the run job's outputs. +const SUBSCRIPTION_START_PENDING = new Set(["pr-unblock.yml fix-docs-subscription"]); + +// #9365: a forged run job could start or suppress a subscription leg. +test("no subscription leg's if: reads a read runner run-job output", () => { + const runOutputs = RUN_JOB_OUTPUTS(); + assert.ok(["lane-class", "credential-present", "handed-off"].every((name) => runOutputs.includes(name)), runOutputs.join(" ")); + const legs = []; + for (const file of workflowFiles()) { + const { jobs } = workflow(file); + const calls = new Set(Object.entries(jobs ?? {}).filter(([, job]) => job?.uses === READ_RUNNER).map(([id]) => id)); + for (const id of calls) { + if (jobs[id].with?.fallback === undefined) continue; + const leg = `${file} ${id}`; + legs.push(leg); + const read = [...String(jobs[id].if).matchAll(/needs\.([\w-]+)\.outputs\.([\w-]+)/g)] + .filter(([, job, output]) => calls.has(job) && runOutputs.includes(output)) + .map(([ref]) => ref); + if (SUBSCRIPTION_START_PENDING.has(leg)) assert.notDeepEqual(read, [], `${leg} no longer reads a run-job output: drop it from SUBSCRIPTION_START_PENDING`); + else assert.deepEqual(read, [], leg); + } + } + assert.deepEqual(legs.sort(), [ + "pr-unblock.yml address-feedback-subscription", + "pr-unblock.yml fix-ci-rung-up-subscription", + "pr-unblock.yml fix-ci-subscription", + "pr-unblock.yml fix-docs-subscription", + "pr-unblock.yml review-dependency-subscription", + ]); +}); + +// #9365: address-feedback's and review-dependency's fallback calls start as +// fix-ci's do, on a base job's jobs-API read of the first call's report. +test("pr-unblock.yml: address-feedback's and review-dependency's fallback calls start on a handoff job's jobs-API read, never on a model call's output", (t) => { + const { jobs } = workflow("pr-unblock.yml"); + const helper = jobs["fix-ci-handoff"].steps[0]; + for (const [firstId, subId, handoffId, selectId] of [ + ["address-feedback", "address-feedback-subscription", "address-feedback-handoff", "address-feedback-select"], + ["review-dependency", "review-dependency-subscription", "review-dependency-handoff", "review-dependency-select"], + ]) { + const sub = jobs[subId]; + const handoff = jobs[handoffId]; + const proceed = `needs.${selectId}.outputs.proceed`; + // The evaluator refuses any reference left unset: the start reads only these. + const starts = (posted, proceeds = "true") => evaluate(sub.if, { [proceed]: proceeds, [`needs.${handoffId}.outputs.posted`]: posted }); + assert.equal(starts("false"), true, `${subId} after a call that posted no check`); + assert.equal(starts(""), true, `${subId} after a failed read: a second leg, never a lost check`); + assert.equal(starts("true"), false, `${subId} after a call that posted its check`); + assert.equal(starts("false", "false"), false); + assert.equal(sub.with.fallback, "handoff"); + // The handoff job: fix-ci-handoff's one shell step on a read-only token. + assert.deepEqual(handoff.needs, [selectId, firstId]); + assert.equal(evaluate(handoff.if, { [proceed]: "true" }), true); + assert.equal(evaluate(handoff.if, { [proceed]: "false" }), false); + assert.deepEqual(handoff.permissions, { actions: "read" }); + assert.deepEqual(handoff.outputs, { posted: "${{ steps.posted.outputs.posted }}" }); + assert.equal(handoff.steps.length, 1); + assert.deepEqual(stepSecrets(handoff), []); + assert.doesNotMatch(JSON.stringify(handoff), new RegExp(`needs\\.${firstId}\\.outputs\\.`)); + const step = handoff.steps[0]; + assert.equal(step.uses, undefined); + assert.equal(step.id, "posted"); + assert.equal(step.run, helper.run, "fix-ci-handoff's read"); + assert.deepEqual(step.env, { GH_TOKEN: "${{ github.token }}", CALLS: firstId }); + // Its read over this lane's calls: the first call's report posted, or not. + const listing = (posted, options = {}) => jobListing(posted, { calls: [firstId, subId], ...options }); + const read = readPosted(t, step, listing([firstId])); + assert.equal(read.status, 0); + assert.equal(read.outputs.posted, "true"); + assert.match(read.calls, /^api --paginate repos\/o\/r\/actions\/runs\/99\/jobs\?filter=latest&per_page=100 --jq /); + for (const [name, value] of Object.entries({ + "a skipped report job": listing([], { reportSkipped: [firstId] }), + "a report that posted nothing": listing([]), + "only the fallback call posted": listing([subId]), + })) { + const other = readPosted(t, step, value); + assert.equal(other.status, 0, name); + assert.equal(other.outputs.posted, "false", `${handoffId}: ${name}`); + } + const failed = readPosted(t, step, listing([firstId]), { fail: true }); + assert.notEqual(failed.status, 0); + assert.deepEqual(failed.outputs, {}); + } +}); diff --git a/.github/scripts/pr-unblock/workflow-gates.test.mjs b/.github/scripts/pr-unblock/workflow-gates.test.mjs index dcb41a065d..be6cd750f8 100644 --- a/.github/scripts/pr-unblock/workflow-gates.test.mjs +++ b/.github/scripts/pr-unblock/workflow-gates.test.mjs @@ -146,14 +146,22 @@ test("pr-feedback-dispatch.yml posts verifier notes only for a pr-verify run of }); const UNBLOCK = "pr-unblock.yml"; -const JOBS = ["address-feedback-select", "address-feedback-not-run", "address-feedback", "address-feedback-subscription", "apply-feedback-patch", "publish-feedback"]; +const JOBS = [ + "address-feedback-select", + "address-feedback-not-run", + "address-feedback", + "address-feedback-handoff", + "address-feedback-subscription", + "apply-feedback-patch", + "publish-feedback", +]; // Run 39783964241's shape: an API-key run skips the subscription call, and a // job that needs a skipped job and lacks !cancelled() (or always()) is skipped // too, so the push and the reply would never run. test("pr-unblock.yml: every address-feedback job after a model call starts its if: with !cancelled()", () => { const after = JOBS.filter((job) => jobGate(UNBLOCK, job).needs.some((need) => need === "address-feedback" || need === "address-feedback-subscription")); - assert.deepEqual(after, ["address-feedback-subscription", "apply-feedback-patch", "publish-feedback"]); + assert.deepEqual(after, ["address-feedback-handoff", "address-feedback-subscription", "apply-feedback-patch", "publish-feedback"]); for (const job of [...after, "address-feedback", "address-feedback-not-run"]) { assert.match(jobGate(UNBLOCK, job).gate, /^\$\{\{ !cancelled\(\) && /, job); } @@ -193,6 +201,7 @@ const DEPENDENCY_JOBS = [ "review-dependency-select", "review-dependency-not-run", "review-dependency", + "review-dependency-handoff", "review-dependency-subscription", "apply-dependency-patch", "apply-dependency-verdict", @@ -202,7 +211,7 @@ const DEPENDENCY_JOBS = [ // lacks !cancelled() is skipped too, so the push and the verdict would never run. test("pr-unblock.yml: every review-dependency job after a model call starts its if: with !cancelled()", () => { const after = DEPENDENCY_JOBS.filter((job) => jobGate(UNBLOCK, job).needs.some((need) => need === "review-dependency" || need === "review-dependency-subscription")); - assert.deepEqual(after, ["review-dependency-subscription", "apply-dependency-patch", "apply-dependency-verdict"]); + assert.deepEqual(after, ["review-dependency-handoff", "review-dependency-subscription", "apply-dependency-patch", "apply-dependency-verdict"]); for (const job of [...after, "review-dependency", "review-dependency-not-run"]) { assert.match(jobGate(UNBLOCK, job).gate, /^\$\{\{ !cancelled\(\) && /, job); } @@ -327,3 +336,41 @@ test("pr-unblock.yml: apply-patch runs on a written or empty patch, or a refused assert.equal(runs("success", [], ["", "true"]), false, "nor its refusal"); assert.equal(runs("skipped", [], ["", ""]), false, "a call that left no patch and refused none"); }); + +// #9365: a hostile model job can forge lane-class, credential-present, +// handed-off or any output of its own. address-feedback's and +// review-dependency's fallback calls start on their handoff jobs' reads, so +// no forged output starts or suppresses them. +test("pr-unblock.yml: address-feedback's and review-dependency's fallback calls start on base jobs alone, whatever a model call reports", () => { + for (const [first, handoff, sub, select] of [ + ["address-feedback", "address-feedback-handoff", "address-feedback-subscription", "address-feedback-select"], + ["review-dependency", "review-dependency-handoff", "review-dependency-subscription", "review-dependency-select"], + ]) { + const forged = new RegExp(`needs\\.${first}(?:-subscription)?\\.outputs\\.`); + assert.doesNotMatch(jobGate(UNBLOCK, sub).gate, forged, sub); + // Every forged pair a run job can report: a credit stop, an empty key, a + // clean run. The start follows the handoff job's read alone. + const forgeries = [ + { "lane-class": "credit-stop", "credential-present": "true", "handed-off": "true" }, + { "lane-class": "", "credential-present": "false", "handed-off": "true" }, + { "lane-class": "", "credential-present": "true", "handed-off": "false" }, + ]; + for (const outputs of forgeries) { + const context = (posted, proceed = "true") => ({ + ...Object.fromEntries(Object.entries(outputs).map(([name, value]) => [`needs.${first}.outputs.${name}`, value])), + [`needs.${select}.outputs.proceed`]: proceed, + [`needs.${handoff}.outputs.posted`]: posted, + }); + const label = JSON.stringify(outputs); + assert.equal(evaluate(jobGate(UNBLOCK, sub).gate, context("true")), false, `${sub} after a posted check, ${label}`); + assert.equal(evaluate(jobGate(UNBLOCK, sub).gate, context("false")), true, `${sub} after no posted check, ${label}`); + assert.equal(evaluate(jobGate(UNBLOCK, sub).gate, context("false", "false")), false, `${sub} with no proceed, ${label}`); + } + assert.deepEqual(jobGate(UNBLOCK, sub).needs, [select, first, handoff]); + assert.doesNotMatch(jobGate(UNBLOCK, handoff).gate, forged, handoff); + // The handoff job runs whenever the select job proceeded. + for (const [proceed, runs] of [["true", true], ["false", false], ["", false]]) { + assert.equal(evaluate(jobGate(UNBLOCK, handoff).gate, { [`needs.${select}.outputs.proceed`]: proceed }), runs, `${handoff} ${JSON.stringify(proceed)}`); + } + } +}); diff --git a/.github/workflows/pr-run-activity-read.yml b/.github/workflows/pr-run-activity-read.yml index ed1ab5cd22..8f224c58c4 100644 --- a/.github/workflows/pr-run-activity-read.yml +++ b/.github/workflows/pr-run-activity-read.yml @@ -76,9 +76,11 @@ on: fallback: description: >- PR mode: empty for the first call, which runs on claude-api-key and - hands off on a credit stop or an empty key; `credit-stop` or - `no-key` for the second, which runs on claude-code-oauth-token and - names why in its lane record. + hands off on a credit stop or an empty key; `credit-stop`, `no-key` + or `handoff` for the second, which runs on claude-code-oauth-token + and names why in its lane record. `handoff` says only that the + first call posted no check, for a caller that reads that from this + run's jobs rather than from the first call's outputs. required: false type: string default: "" @@ -505,9 +507,9 @@ jobs: fi fi case "$FALLBACK" in - "" | credit-stop | no-key) ;; + "" | credit-stop | no-key | handoff) ;; *) - echo "::error::the fallback input must be empty, credit-stop or no-key" + echo "::error::the fallback input must be empty, credit-stop, no-key or handoff" exit 1 ;; esac @@ -1595,6 +1597,8 @@ jobs: && format('{0}/{1} (key {2} hit a credit stop)', inputs.lane, inputs.activity, steps.launch.outputs.key-name) || inputs.fallback == 'no-key' && format('{0}/{1} (key {2} is empty)', inputs.lane, inputs.activity, steps.launch.outputs.key-name) + || inputs.fallback == 'handoff' + && format('{0}/{1} (key {2} posted no check)', inputs.lane, inputs.activity, steps.launch.outputs.key-name) || format('{0}/{1}', inputs.lane, inputs.activity) }} lane-id: ${{ inputs.lane }} step-started-at: ${{ steps.key-present.outputs.started-at || steps.token-present.outputs.started-at }} diff --git a/.github/workflows/pr-unblock.yml b/.github/workflows/pr-unblock.yml index f76767e6e2..5731c47fc7 100644 --- a/.github/workflows/pr-unblock.yml +++ b/.github/workflows/pr-unblock.yml @@ -302,16 +302,45 @@ jobs: claude-api-key: ${{ secrets.AUTOMATIONS_CLAUDE_API_KEY }} scan-carrier-private-key: ${{ secrets.SCAN_CARRIER_PRIVATE_KEY }} + # Whether address-feedback's report job posted its check, read from this run's + # jobs as fix-ci-handoff reads fix-ci's, never from an address-feedback + # output: the run job runs head code and can forge them. A failed read + # leaves posted empty, which starts the fallback call. + address-feedback-handoff: + needs: [address-feedback-select, address-feedback] + if: ${{ !cancelled() && needs.address-feedback-select.outputs.proceed == 'true' }} + runs-on: ubuntu-24.04 + timeout-minutes: 10 + permissions: + actions: read # this run's jobs and their steps + outputs: + posted: ${{ steps.posted.outputs.posted }} + steps: + - name: Read whether address-feedback posted its check + id: posted + env: + GH_TOKEN: ${{ github.token }} + CALLS: address-feedback + run: | + names=$(gh api --paginate "repos/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID/jobs?filter=latest&per_page=100" \ + --jq '.jobs[] | (select(any(.steps[]?; .name == "Post the check run" and .conclusion == "success")) | "posted \(.name)"), + (select(any(.steps[]?; .name == "Ask the broker whether the lane may run" and .conclusion == "success")) | "allowed \(.name)")') + posted=false + allowed=false + for call in $CALLS; do + if grep -qxF "posted $call / report" <<<"$names"; then posted=true; fi + if grep -qxF "allowed $call / broker-check" <<<"$names"; then allowed=true; fi + done + printf 'posted=%s\nallowed=%s\n' "$posted" "$allowed" >>"$GITHUB_OUTPUT" + # The subscription fallback, as fix-ci-subscription is for fix-ci: it runs - # only when address-feedback's key leg hit a credit stop or its key is - # empty, reads only those two outputs, each compared to a fixed string, - # and holds the OAuth token alone. A forged value only adds a second leg. + # only when address-feedback-handoff read no check posted, which a credit + # stop or an empty key leaves, and holds the OAuth token alone. address-feedback-subscription: - needs: [address-feedback-select, address-feedback] + needs: [address-feedback-select, address-feedback, address-feedback-handoff] if: >- ${{ !cancelled() && needs.address-feedback-select.outputs.proceed == 'true' - && (needs.address-feedback.outputs.lane-class == 'credit-stop' - || needs.address-feedback.outputs.credential-present == 'false') }} + && needs.address-feedback-handoff.outputs.posted != 'true' }} permissions: contents: read pull-requests: read @@ -326,7 +355,7 @@ jobs: role: automations lane: pr-unblock activity: address-feedback - fallback: ${{ needs.address-feedback.outputs.lane-class == 'credit-stop' && 'credit-stop' || 'no-key' }} + fallback: handoff pr-number: ${{ inputs.pr-number }} dispatcher-run-id: ${{ inputs.dispatcher-run-id }} trusted-base: default-tip @@ -640,16 +669,45 @@ jobs: claude-api-key: ${{ secrets.AUTOMATIONS_CLAUDE_API_KEY }} scan-carrier-private-key: ${{ secrets.SCAN_CARRIER_PRIVATE_KEY }} + # Whether review-dependency's report job posted its check, read from this run's + # jobs as fix-ci-handoff reads fix-ci's, never from an review-dependency + # output: the run job runs head code and can forge them. A failed read + # leaves posted empty, which starts the fallback call. + review-dependency-handoff: + needs: [review-dependency-select, review-dependency] + if: ${{ !cancelled() && needs.review-dependency-select.outputs.proceed == 'true' }} + runs-on: ubuntu-24.04 + timeout-minutes: 10 + permissions: + actions: read # this run's jobs and their steps + outputs: + posted: ${{ steps.posted.outputs.posted }} + steps: + - name: Read whether review-dependency posted its check + id: posted + env: + GH_TOKEN: ${{ github.token }} + CALLS: review-dependency + run: | + names=$(gh api --paginate "repos/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID/jobs?filter=latest&per_page=100" \ + --jq '.jobs[] | (select(any(.steps[]?; .name == "Post the check run" and .conclusion == "success")) | "posted \(.name)"), + (select(any(.steps[]?; .name == "Ask the broker whether the lane may run" and .conclusion == "success")) | "allowed \(.name)")') + posted=false + allowed=false + for call in $CALLS; do + if grep -qxF "posted $call / report" <<<"$names"; then posted=true; fi + if grep -qxF "allowed $call / broker-check" <<<"$names"; then allowed=true; fi + done + printf 'posted=%s\nallowed=%s\n' "$posted" "$allowed" >>"$GITHUB_OUTPUT" + # The subscription fallback, as fix-ci-subscription is for fix-ci: it runs - # only when review-dependency's key leg hit a credit stop or its key is - # empty, reads only those two outputs, each compared to a fixed string, - # and holds the OAuth token alone. A forged value only adds a second leg. + # only when review-dependency-handoff read no check posted, which a credit + # stop or an empty key leaves, and holds the OAuth token alone. review-dependency-subscription: - needs: [review-dependency-select, review-dependency] + needs: [review-dependency-select, review-dependency, review-dependency-handoff] if: >- ${{ !cancelled() && needs.review-dependency-select.outputs.proceed == 'true' - && (needs.review-dependency.outputs.lane-class == 'credit-stop' - || needs.review-dependency.outputs.credential-present == 'false') }} + && needs.review-dependency-handoff.outputs.posted != 'true' }} permissions: contents: read pull-requests: read @@ -664,7 +722,7 @@ jobs: role: automations lane: pr-unblock activity: review-dependency - fallback: ${{ needs.review-dependency.outputs.lane-class == 'credit-stop' && 'credit-stop' || 'no-key' }} + fallback: handoff pr-number: ${{ inputs.pr-number }} dispatcher-run-id: ${{ inputs.dispatcher-run-id }} trusted-base: default-tip @@ -954,9 +1012,9 @@ jobs: # The subscription fallback, as a second call on new VMs: it runs only when # fix-ci posted no check, which a credit stop or an empty key leaves as a # handoff with no upload, lane record or check. Its start reads only - # fix-ci-handoff, and it holds the OAuth token alone. fallback, the lane - # record's label, picks between two literals on fix-ci's lane-class: a - # forged one only mislabels the record. + # fix-ci-handoff, and it holds the OAuth token alone. fallback is handoff, + # the one fact that read attests: the lane record says the key posted no + # check, not why, since only the run job knows. fix-ci-subscription: needs: [fix-ci-context, fix-ci, fix-ci-handoff] if: >- @@ -976,7 +1034,7 @@ jobs: role: automations lane: pr-unblock activity: fix-ci - fallback: ${{ needs.fix-ci.outputs.lane-class == 'credit-stop' && 'credit-stop' || 'no-key' }} + fallback: handoff pr-number: ${{ inputs.pr-number }} dispatcher-run-id: ${{ inputs.dispatcher-run-id }} trusted-base: default-tip @@ -1123,7 +1181,7 @@ jobs: role: automations lane: pr-unblock activity: fix-ci - fallback: ${{ needs.fix-ci-rung-up.outputs.lane-class == 'credit-stop' && 'credit-stop' || 'no-key' }} + fallback: handoff pr-number: ${{ inputs.pr-number }} dispatcher-run-id: ${{ inputs.dispatcher-run-id }} trusted-base: default-tip diff --git a/docs/conventions/pr-pipeline/pr-run-activity.md b/docs/conventions/pr-pipeline/pr-run-activity.md index be79047c6b..0f0d619881 100644 --- a/docs/conventions/pr-pipeline/pr-run-activity.md +++ b/docs/conventions/pr-pipeline/pr-run-activity.md @@ -73,7 +73,7 @@ permissions, and passes no App key. | `dependency-verdict` | `false` | Write file only: for the verdict script of a [`prepare: dependency-review`](#prepare-dependency-review) round, download the select job's `context.json` and the scanned verdict | | `dependency-context-attempt`, `dependency-run-sha256`, `dependency-head-sha` | `''` | Write file only: with `dependency-verdict`, the select job's `context-attempt`, `run-sha256` and `head-sha` outputs | | `dependency-run-attempt`, `dependency-verdict-sha256`, `dependency-patch-state`, `dependency-scan-refused` | `''` | Write file only: with `dependency-verdict`, the same read call's `dependency-attempt`, `dependency-sha256`, `scan-patch-state` and `scan-refused` outputs | -| `fallback` | `''` | Read file only, PR mode: `''` for the caller's first call, on `claude-api-key`; `credit-stop` or `no-key` for its fallback call, on `claude-code-oauth-token` ([Two calls on one credential each](#two-calls-on-one-credential-each)); any other value fails red in step 1 | +| `fallback` | `''` | Read file only, PR mode: `''` for the caller's first call, on `claude-api-key`; `credit-stop`, `no-key` or `handoff` for its fallback call, on `claude-code-oauth-token` ([Two calls on one credential each](#two-calls-on-one-credential-each)); any other value fails red in step 1 | | `model-rung` | `''` | Read file only: the model rung the activity's profile runs at, resolve-config's `model-rung` (`1` takes its `escalate-to`) | | `patch-model-rung` | `''` | Write file only: for [`prepare: apply-patch`](#prepare-apply-patch), the rung the patch's producer ran at; at `0` (or empty) an empty fix-ci patch may skip `rung-up` | | `ci-context-attempt`, `ci-context-sha256` | `''` | Read file only: for [`prepare: ci-context`](#prepare-ci-context), the attempt naming the `ci-context-` artifact and its file's SHA-256 | @@ -642,15 +642,18 @@ Read file, PR mode: a skill runs on ci-workflows' job. The caller calls the read file twice, each call with one secret, named for its `role`: `pr-unblock.yml`'s `fix-ci` passes only `claude-api-key` (`AUTOMATIONS_CLAUDE_API_KEY`), and `fix-ci-subscription` passes only `claude-code-oauth-token` -(`AUTOMATIONS_CLAUDE_CODE_OAUTH_TOKEN`); its `fix-docs` and `fix-docs-subscription`, and its -`address-feedback` and `address-feedback-subscription`, do the same. The second runs only when the first's `lane-class` output -is `credit-stop` or its `credential-present` is `false`, each compared to that fixed string, and -passes `fallback: credit-stop` or `no-key`. Both queue in `pr-pipeline-write-`. fix-ci's -fallback calls start on no first-call output: `fix-ci-handoff` (and `fix-ci-rung-up-handoff` for -the rung-up call), a job with only `actions: read` that checks out nothing, reads this run's jobs -through the Actions API, and the fallback runs unless the first call's ` / report` job has -a `Post the check run` step that succeeded. A failed read runs the fallback too. Only `fallback`, -the lane record's label, still reads `lane-class`; a forged one mislabels the record. +(`AUTOMATIONS_CLAUDE_CODE_OAUTH_TOKEN`); its `fix-docs`, `address-feedback` and +`review-dependency` calls and their `-subscription` calls do the same. Both queue in +`pr-pipeline-write-`. Each fallback call but fix-docs's starts on no first-call output: a +handoff job (`fix-ci-handoff`, `fix-ci-rung-up-handoff`, `address-feedback-handoff`, +`review-dependency-handoff`) with only `actions: read` that checks out nothing reads this run's +jobs through the Actions API once, and the fallback runs unless the first call's +` / report` job has a `Post the check run` step that succeeded. A failed read runs the +fallback too. These fallback calls pass `fallback: handoff`, so the lane record says the key +posted no check, not why: only the run job, which head code can forge, knows whether it was a +credit stop or an empty key. `fix-docs-subscription` still runs when `fix-docs`'s `lane-class` +output is `credit-stop` or its `credential-present` is `false`, each compared to that fixed +string, and passes `fallback: credit-stop` or `no-key`. 1. Before any head code, the run job fails red unless the `role` is lower-case kebab, the model route resolves an alias (`default` resolves to none) and the skill is advisory. While sudo From 88fc5fcf45dc30341ed0e88f3bd2857d1bc9729b Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Fri, 23 Oct 2026 03:47:46 -0400 Subject: [PATCH 2/2] test(pr-pipeline): cite the run that shows address-feedback's report step names Co-Authored-By: Claude Opus 5.5 --- .github/actions/resolve-config/callers.test.mjs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/actions/resolve-config/callers.test.mjs b/.github/actions/resolve-config/callers.test.mjs index fa16c74f25..3996a6ab81 100644 --- a/.github/actions/resolve-config/callers.test.mjs +++ b/.github/actions/resolve-config/callers.test.mjs @@ -4385,7 +4385,10 @@ test("no subscription leg's if: reads a read runner run-job output", () => { }); // #9365: address-feedback's and review-dependency's fallback calls start as -// fix-ci's do, on a base job's jobs-API read of the first call's report. +// fix-ci's do, on a base job's jobs-API read of the first call's report. The +// read runner's report job is one job for every activity: run 39894065797's +// listing names "address-feedback / report" with its "Post the check run" +// step and "address-feedback / broker-check" with its broker step. test("pr-unblock.yml: address-feedback's and review-dependency's fallback calls start on a handoff job's jobs-API read, never on a model call's output", (t) => { const { jobs } = workflow("pr-unblock.yml"); const helper = jobs["fix-ci-handoff"].steps[0];