Repository navigation
feat(pr-pipeline): trust a reconciler-recorded re-run of a lost pr-verify run - #8562
Conversation
…rify run The lost-run reconciler could re-run CI workflows but only list lost lane runs for a person, because check-trusted-trigger refuses every re-run github-actions[bot] starts. pr-verify runs lost to an outage therefore still needed a manual re-run. Trust rule: with reconciler-workflow set (pr-verify only), a re-run whose triggering actor is github-actions[bot] proceeds only when it is attempt 2 of this run, started by github-actions[bot] (numeric id), and an unexpired artifact named reconcile-rerun-<run id>-2-<head sha> exists, created no later than that attempt started, by a run of the reconciler workflow in this repository on the default branch, started by its schedule or a listed human's dispatch, whose head is the default branch tip or behind it with no same-named tag, and that started at most 10 minutes before this attempt. The artifact's contents are never read. Every refusal is untrusted-rerunner; the original event's actors and the PR author are still checked as before. The reconciler now plans first, uploads one record per pr-verify run it will re-run, and only then re-runs them, re-reading each run (and, for PR CI, the PR head) before the write. Hardening from the security review (F1-F4, N1-N2): - the record binds run id, attempt 2 and head sha, and must come from a default-branch reconciler run started by its schedule or a listed human; - a record whose run cannot be read does not vouch, and the check reads at most 20 candidate runs before refusing, so forged same-named records cannot drain the token quota; - a pr-verify run whose first attempt started its judge step is never re-run, so a head is never judged twice; - a run none of whose jobs started (a pending duplicate concurrency cancelled) is never re-run; - the plan's record names are validated before upload, and verify re-runs share the in-flight cap with PR CI sets; - a pr-verify run that ended startup_failure with no jobs is held for a person. Review, refine and feedback-dispatch lanes still refuse these re-runs. Refs: #8468 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xk7Lz7d2twxvA3LfRxnLkT
…equest The record name carries the pr-verify run's own head_sha, which is main's tip; the live gate binds the re-run to the pull request. Say so where the record is built, so nobody relies on its SHA for that. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xk7Lz7d2twxvA3LfRxnLkT
|
Interaction found while building #8577 (pr-verify two-job credentials): |
…scription token only on a credit stop (#8577) No related issue: run-claude-cli v0.54.0 (ci-workflows 7b36242) takes exactly one credential, so pr-verify needs the two-job pattern before its pin can move. ## Summary ci-workflows v0.54.0 makes the run-claude-cli action take exactly one of `api-key` or `subscription-token`, fail closed if given both, and set `lane-class=credit-stop` on a key credit stop instead of falling back inside the step. pr-verify pins v0.53.0, which fell back in-step. This moves pr-verify to two jobs and repins to v0.54.0 (`7b362428`). ## Fix - `check-intent` runs the model on the automations API key. New outputs: `lane-class`, `credential-present`, `key-leg-clean`. - `check-intent-subscription` (`needs: check-intent`, `ubuntu-24.04`, fresh checkout, every step repeated, nothing carried from the first job) runs only when the key job hit a credit stop with a clean key leg, or no key is set (`if:` at :364; with no status function GitHub adds `success()`, so a failed key job skips it). - `secrets.AUTOMATIONS_CLAUDE_CODE_OAUTH_TOKEN` is now only in the subscription job (:490, :563). Permissions unchanged, read-only in both jobs. - Lane-funding conditions kept: key first (automations credits spent first); each job echoes `credential:` in report-lane-outcome; a new "Check the key leg" step (:300) fails the key job on any permission denial, and `write-verdict.mjs` still catches denials in the subscription job, so a denial never yields a verdict; exactly one verdict per run (a credit stop always fails the key job's judge step). Step names "Judge the pull request" and "Upload the verdict" kept. - Job count: a skipped job uses no runner, so the steady state adds one skipped job per run; on a credit stop the jobs run one after the other. Known interaction: the held reconciler change (#8562) counts a started judge step in any job, so it would hold rather than re-run a run whose subscription job was lost. That fails safe; the fix belongs in #8562. ## Verification - `workflow.test.mjs` rewritten per job: hand-off condition, each secret only in its own job, the subscription job's steps equal the key job's apart from the credential (a mutation check fails on drift). New tests for `checkKeyLeg` in `write-verdict.mjs`. resolve-pr, reconcile, callers and workflow-gates tests pass; actionlint, offline zizmor and runner-policy pass. - Needs GitHub Security & Cost's pass before merge (secrets, `needs`, `if` changes). ## Related - ci-workflows v0.54.0; #8510 (v0.53.0 pin); #8562. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Xk7Lz7d2twxvA3LfRxnLkT --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 609af18353
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
claude-security-review reviewed Security review (incremental, head 74d0dbd, since 609af18)Findings: 0. The only change since the last review is one line in This turns on a trust path that was off before. With the input empty, every re-run started by
The workflow's trusted-trigger design is unchanged: it runs on |
|
claude-review reviewed No findings. The only change since 609af18 is one added line in I checked the following myself with tools:
I did not run the tests. The 29/29 and 262/262 results and the claim about artifact scoping are the author's and I have not confirmed them. I did not re-review the other files, which are unchanged since the last review. |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xk7Lz7d2twxvA3LfRxnLkT
Refs: #8468
Draft: owner decision — this changes who can trigger the credentialed pr-verify lane (GitHub Security & Cost rule: such a change goes to the owner rather than merging overnight).
Summary
The outage reconciler (#8505, merged) re-runs CI runs lost to a GitHub outage, but a
github-actions[bot]re-run of pr-verify stops in check-trusted-trigger withuntrusted-actor, so lost pr-verify runs still need a person. This lets check-trusted-trigger trust one specific re-run: attempt 2 of a pr-verify run started bygithub-actions[bot]that carries a reconciler record artifact bound to that run id and head.Fix
.github/actions/check-trusted-trigger/): accepts the re-run only when the record artifactreconcile-rerun-<run id>-2-<head sha>exists and the run that uploaded it is the reconciler workflow on main (schedule or a listed human's dispatch), judged by the run GitHub attributes the artifact to, never the artifact's contents. Every read fails closed; a candidate that can't be read does not vouch; at most 20 candidate runs are read..github/scripts/maintenance-reconcile-lost-runs/reconcile.mjs): records and re-runs only pr-verify runs that never started their judge step (a re-run whose head already has a counted verdict is stopped by resolve-pr, and same-head runs queue on the pr-verify concurrency group), skips runs whose first-attempt jobs never started (startup_failure is held for a person), and re-reads the PR head before re-running PR CI (kept from feat(pr-pipeline): re-run PR CI runs lost to a GitHub outage #8505).Security review (fresh-context reviewer, then GitHub Security & Cost): no path lets an attacker-controlled run reach pr-verify's credentials. Residuals accepted: an
actions: writeholder can start the recorded re-run first (same payload and main code); run/job scoping of artifact uploads is server-enforced but undocumented (UNVERIFIED).Covers pr-verify only; review, refine and feedback-dispatch re-runs need broker and ci-workflows changes: #8524.
Verification
reconcile.test.mjs29/29,check-trusted-trigger.test.mjs262/262; new tests failed before their fixes.scripts/check-affected.sh --tests: 10 passed, 4 skipped.Related
Hold: a human merges this
🤖 Generated with Claude Code
https://claude.ai/code/session_01Xk7Lz7d2twxvA3LfRxnLkT