diff --git a/.github/workflows/auto-ready.yml b/.github/workflows/auto-ready.yml new file mode 100644 index 000000000..4e878c48a --- /dev/null +++ b/.github/workflows/auto-ready.yml @@ -0,0 +1,113 @@ +# Promote a draft PR to ready-for-review once CI is green. +# +# This automates the draft-first discipline documented in claude-pr-review.yml: +# open as a draft, let CI and any fixups land while draft, mark ready once +# green. The marking-ready step is the only manual link in that chain, and this +# workflow closes it — a draft whose CI run succeeds is flipped to ready, which +# hands it to the automated review. The full pipeline is: +# +# open draft PR -> CI (ci.yml, runs on drafts) -> green -> THIS workflow +# -> PR marked ready -> Claude PR Review (via explicit dispatch, see below) +# +# `workflow_run` rather than a job appended to ci.yml, for three reasons: +# - ci.yml's `ci-green` audit step hand-verifies its own gate; a downstream +# promote job there would need an EXEMPT entry and would tangle promotion +# into the required check's job graph. +# - the promotion needs `pull-requests: write`; ci.yml is least-privilege +# `contents: read` at the top and PR jobs there run on untrusted-input +# paths. This workflow checks out nothing and runs no project code, so the +# write scope never coexists with anything a PR author controls. +# - run-level `conclusion == 'success'` is exactly "the whole CI run passed" +# (it subsumes `ci-green`; it is marginally stricter — a red report-only +# `coverage` job also holds the draft, which is a safe default). +# +# The GITHUB_TOKEN no-retrigger rule: events caused by the default token do NOT +# start new workflow runs, so `gh pr ready` below fires the `ready_for_review` +# event but claude-pr-review.yml would never see it. `workflow_dispatch` is the +# documented exception to that rule, so the promote step invokes the review +# explicitly after flipping the PR. Do not "simplify" this to rely on the event. +# +# Scope — a draft is promoted only when ALL of these hold: +# - the completed CI run is a `pull_request` run that succeeded; +# - the head branch lives in THIS repo (fork drafts are never touched: their +# authors' draft state is not ours to change, and `workflow_run` payloads +# don't carry fork PR associations reliably anyway); +# - the PR is still open, still a draft, and its head SHA still equals the +# SHA that went green (a push racing the promotion keeps the PR a draft — +# the newer run will promote it if it also goes green); +# - it is not a Dependabot PR (consistent with the review workflow's filter); +# - it does not carry the `keep-draft` label — the opt-out for work that +# should stay draft while green (long-running spikes, stacked PRs). Apply +# the label before pushing, or re-convert to draft after; a promoted PR is +# not promoted again unless it is re-drafted and goes green again. +# +# Like every `workflow_run` workflow, this fires only once THIS file is on the +# default branch — it cannot be exercised from its own PR. +name: Auto-ready draft PRs + +on: + workflow_run: + workflows: [CI] + types: [completed] + +# One promotion at a time per head branch; a newer CI completion supersedes an +# in-flight promotion for the same branch. +concurrency: + group: auto-ready-${{ github.event.workflow_run.head_branch }} + cancel-in-progress: true + +# `pull-requests: write` marks the PR ready (a GraphQL mutation, via `gh pr +# ready`); `actions: write` dispatches the review workflow. Nothing is checked +# out, so no contents scope at all. +permissions: + pull-requests: write + actions: write + +jobs: + promote: + name: Mark green draft ready for review + if: >- + github.event.workflow_run.conclusion == 'success' && + github.event.workflow_run.event == 'pull_request' && + github.event.workflow_run.head_repository.full_name == github.repository + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + # `workflow_run.pull_requests` is unreliable (empty for some payloads), so + # resolve the PR ourselves: open PRs whose head is the run's branch, then + # filter to drafts still pointing at the exact SHA that went green. All + # untrusted-ish values (branch name, SHA) travel via env and jq --arg, + # never interpolated into the script or a URL. + - name: Promote draft PR(s) at the green SHA + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + HEAD_BRANCH: ${{ github.event.workflow_run.head_branch }} + HEAD_SHA: ${{ github.event.workflow_run.head_sha }} + DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} + run: | + owner="${REPO%%/*}" + numbers="$(gh api -X GET "repos/$REPO/pulls" \ + -f state=open -f head="$owner:$HEAD_BRANCH" | + jq -r --arg sha "$HEAD_SHA" ' + .[] + | select(.draft) + | select(.head.sha == $sha) + | select(.user.login != "dependabot[bot]") + | select([.labels[].name] | index("keep-draft") | not) + | .number')" + + if [ -z "$numbers" ]; then + echo "Nothing to promote at $HEAD_SHA (no open draft, superseded by a newer push, or held with keep-draft)." + exit 0 + fi + + for n in $numbers; do + echo "CI is green at $HEAD_SHA - marking PR #$n ready for review." + gh pr ready "$n" --repo "$REPO" + # See the header: the GITHUB_TOKEN-caused ready_for_review event + # cannot start the review workflow, so dispatch it explicitly. + gh workflow run claude-pr-review.yml \ + --repo "$REPO" --ref "$DEFAULT_BRANCH" -f pr_number="$n" + echo "Dispatched claude-pr-review.yml for PR #$n." + done diff --git a/.github/workflows/claude-pr-review.yml b/.github/workflows/claude-pr-review.yml index 6e1ca3ca0..bf06960ef 100644 --- a/.github/workflows/claude-pr-review.yml +++ b/.github/workflows/claude-pr-review.yml @@ -10,6 +10,17 @@ # (the `design/pending/.md` increment file, ADR conventions, the test # gate) rather than reviewing the diff in isolation. # +# Two ways in: +# - `ready_for_review`: a human marks the draft ready. +# - `workflow_dispatch` with a `pr_number` input: auto-ready.yml promotes a +# green draft and dispatches the review here. The dispatch exists because +# the promotion runs on the default GITHUB_TOKEN, whose events never start +# workflow runs — its `ready_for_review` event fires but cannot trigger +# this workflow, so auto-ready.yml invokes it by name instead +# (`workflow_dispatch` is the documented exception to the no-retrigger +# rule). The input also gives maintainers a manual re-review button in the +# Actions tab, which the run-once discipline above otherwise lacks. +# # NOTE on fork PRs: `pull_request` from a fork gets a read-only token, so review # comments will NOT post on external contributions. If you need that, switch to # `pull_request_target` and check out the *base* ref to stay safe from untrusted @@ -24,11 +35,18 @@ name: Claude PR Review on: pull_request: types: [ready_for_review] + workflow_dispatch: + inputs: + pr_number: + description: Pull request number to review + required: true + type: string -# Belt-and-suspenders: nothing re-triggers this per-PR now, but a manual -# re-run (or the trigger types above growing again) shouldn't stack. +# Belt-and-suspenders: the ready_for_review transition and the auto-ready +# dispatch shouldn't stack if both ever fire for the same PR, and neither +# should a manual re-run. concurrency: - group: claude-pr-${{ github.event.pull_request.number }} + group: claude-pr-${{ github.event.pull_request.number || inputs.pr_number }} cancel-in-progress: true # Least privilege: read the tree, write the PR review/comments. @@ -47,22 +65,33 @@ jobs: # case the trigger types above ever grow again. Never Dependabot's: those # are dependency bumps we don't want reviewed (and run with a read-only # token that couldn't post anyway). + # + # A `workflow_dispatch` run skips both checks: auto-ready.yml has already + # applied them (draft state, Dependabot, keep-draft) before dispatching, + # and a human dispatching from the Actions tab is making a deliberate call + # — including, legitimately, on a still-draft PR. if: >- - github.event.pull_request.draft == false && - github.event.pull_request.user.login != 'dependabot[bot]' + github.event_name == 'workflow_dispatch' || + (github.event.pull_request.draft == false && + github.event.pull_request.user.login != 'dependabot[bot]') runs-on: ubuntu-latest timeout-minutes: 30 steps: + # On dispatch there is no PR context, so name the PR's merge ref + # explicitly — the same merge commit a `pull_request` checkout gets by + # default. (If the PR has conflicts the merge ref doesn't exist and this + # fails loudly, which beats silently reviewing the default branch.) - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v4 with: fetch-depth: 0 # full history so Claude can diff against the base branch + ref: ${{ github.event_name == 'workflow_dispatch' && format('refs/pull/{0}/merge', inputs.pr_number) || '' }} - uses: anthropics/claude-code-action@1623c36729ac1cd5895198cded705a287de7db79 # v1 with: claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} prompt: | REPO: ${{ github.repository }} - PR NUMBER: ${{ github.event.pull_request.number }} + PR NUMBER: ${{ github.event.pull_request.number || inputs.pr_number }} Review this pull request. The PR branch is checked out in the working directory; work primarily from the diff.