Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .changes/pr-pipeline/fix-7732-workflow-modified-skip-5fd5f8c0.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
bump: patch
---

### Fixed

- A write lane `pull_request` run on a PR that changes a workflow file the token broker compares no
longer fails red. When the broker refuses the mint as `workflow-modified` and GitHub's compare of
the default branch with the gated head lists the named file, the run job ends green with no token,
its summary names the reason and file, and the check is neutral with the new skip reason
`workflow-modified`
([pr-run-activity.md](https://github.com/melodic-software/claude-code-plugins/blob/main/docs/conventions/pr-pipeline/pr-run-activity.md#a-workflow-file-the-pr-changes)).
Every other denial, and a refusal of a file the PR does not change, stays red. `version` stays 2.
11 changes: 11 additions & 0 deletions .github/actions/report-check-run/report-check-run.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,17 @@ test("a skip reason in the base schema's enum POSTs neutral", async () => {
assert.match(calls[0].body.output.summary, /not-applicable-paths/);
});

// #7732: request-lane-token's skip for a workflow file the PR changes ends the
// run job green with the act step skipped and this skip reason.
test("a workflow-modified skip from a run job that succeeded POSTs neutral", async () => {
const verdict = { ...JSON.parse(readFileSync(fixture("verdict-skipped.json"), "utf8")), "skip-reason": "workflow-modified", outcome: "skipped" };
const verdictPath = path.join(dir, "verdict.json");
writeFileSync(verdictPath, JSON.stringify(verdict));
const { calls } = await run({ VERDICT_PATH: verdictPath, ACT_OUTCOME: "skipped" });
assert.equal(calls[0].body.conclusion, "neutral");
assert.equal(calls[0].body.output.summary, "Skip reason: workflow-modified");
});

test("the enum is read from base-path: a base schema without the reason POSTs failure", async () => {
const schemaPath = path.join(basePath, SCHEMA_REL);
const schema = JSON.parse(readFileSync(schemaPath, "utf8"));
Expand Down
31 changes: 30 additions & 1 deletion .github/actions/request-lane-token/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ run, and mints nothing ([Check mode](#check-mode)).
| `dispatcher-run-id` | `token` mode: the `pr-unblock-dispatch.yml` run that dispatched this one; sent as `dispatcher_run_id` when set. The broker needs it to trust a run sent by `github-actions[bot]` |
| `effect`, `contents`, `pull-requests`, `issues` | `token` mode: this job's own `resolve-config` outputs, compared with the broker's answer |
| `superseded-ok` | `true` ends green, with a `lane-token-superseded` warning and no token, on a 403 whose reason is `workflow-modified` (a newer copy of the caller workflow on the default branch, whose run does the work). Default `false` |
| `pr-workflow-change-ok` | `token` mode with `pr-number`: `true` ends green, with no token and `skip-reason` `workflow-modified`, on a 403 `workflow-modified` naming a workflow file the PR itself changes ([below](#a-workflow-file-the-pr-changes)). Default `false` |
| `head-sha`, `default-branch`, `github-token` | Required with `pr-workflow-change-ok`: the gated head, the default branch, and a token that reads the repository's contents, for the compare |

## Outputs

Expand All @@ -33,6 +35,7 @@ run, and mints nothing ([Check mode](#check-mode)).
| `expires-at` | The expiry the broker returned |
| `mint-id` | `<check_run_id>-<run_attempt>`, the broker's audit key for this mint |
| `failure-reason` | The one-line reason the step failed with (`lane-token-denied: ...`); empty on a success |
| `skip-reason` | `workflow-modified` when `pr-workflow-change-ok` skipped; empty otherwise |

## What it does

Expand Down Expand Up @@ -61,7 +64,9 @@ run, and mints nothing ([Check mode](#check-mode)).
4. Any status but 200 fails red with `lane-token-denied`, the HTTP status, and the broker's
`reason` and `detail`, printed as one line of data. The reasons are listed in the contract.
With `superseded-ok: true`, a 403 with reason `workflow-modified` warns and ends green instead,
setting no output, so steps gated on the `token` output skip.
setting no output, so steps gated on the `token` output skip. With `pr-workflow-change-ok: true`,
such a 403 for a workflow file the PR changes ends green with `skip-reason` set
([below](#a-workflow-file-the-pr-changes)).
5. On 200, masks the token, then compares the broker's `effect`, `permissions`, `lane`, `activity`
and `repository_ids` with this job's values (`pull-requests` is `pull_requests` in the API). Any
difference revokes the token and fails red with `effect-mismatch`. That happens when the PR's
Expand Down Expand Up @@ -95,6 +100,30 @@ Run it from the base checkout before any head checkout, after `resolve-config`:

The job needs `permissions: id-token: write`, and so does the caller job that grants it.

## A workflow file the PR changes

The broker refuses every mint as `workflow-modified` while a PR changes the caller workflow or
`pr-run-activity-write.yml`, and the refusal does not say whether the PR or the default branch
changed the file. With `pr-workflow-change-ok: true` the step skips only when:

1. the answer is 403 with reason `workflow-modified`;
2. its `detail` is exactly `<file> differs from refs/heads/<default-branch>`, where `<file>` is a
`.github/workflows/*.yml` or `*.yaml` path; and
3. `GET /repos/{owner}/{repo}/compare/<default-branch>...<head-sha>`, read with `github-token`,
lists `<file>` as a `filename` or a renamed file's `previous_filename`. The three-dot compare
lists what the head changed since its merge base with the default branch; GitHub lists at most
300 changed files, so a file past that is not found and the step stays red.

It then writes `lane-token-skipped: ...` with the reason and file to the job summary as a notice,
sets `skip-reason=workflow-modified`, sets no token and exits 0. Every
other case fails red as step 4 says, with `; not skipped: <why>` appended for a `workflow-modified`
refusal: a detail that does not parse, a file the compare does not list, or a compare that cannot
be read. The compare is GitHub's, at the gated head, and the caller passes the input only on a
`pull_request` run, which executes the PR's own workflow files: a dispatched run executes the
default branch's files, so its refusal means the default branch moved and must stay red. Text the
PR's code writes never reaches the decision. A PR that changes the workflow running the job already
controls that job, so this adds nothing it could not already do.

## Check mode

With `mode: check` the action takes only `broker-url`, `audience`, `lane` and `activity`; a
Expand Down
80 changes: 78 additions & 2 deletions .github/actions/request-lane-token/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ description: >
a 503 `github-unavailable`, with backoff, or `github-rate-limited`, after its
`Retry-After`, masks the token, and fails red, after
revoking the token, unless the broker's effect, permissions, lane, activity
and repository equal what this job resolved. In `check` mode it asks the
and repository equal what this job resolved; with `pr-workflow-change-ok` a
`workflow-modified` refusal of a workflow file the PR changes skips instead
of failing. In `check` mode it asks the
broker's check endpoint, which mints nothing, whether a read lane may run,
and fails red on anything but an allow. Contract:
docs/conventions/pr-pipeline/pr-run-activity.md, run job step 6 and the
Expand Down Expand Up @@ -80,6 +82,29 @@ inputs:
work. Any other denial still fails red. Default `false`.
required: false
default: "false"
pr-workflow-change-ok:
description: >-
`true` ends the step green, with no token and the `skip-reason` output
`workflow-modified`, when the broker answers 403 `workflow-modified`
naming a `.github/workflows/` file that the pull request itself changes:
the GitHub compare from `default-branch` to `head-sha` lists it. Any
other denial, a refusal that does not parse, a file the compare does not
list, or a failed compare read still fails red. `token` mode with
`pr-number` only. Default `false`.
required: false
default: "false"
head-sha:
description: The gated head SHA the compare reads; required with `pr-workflow-change-ok`.
required: false
default: ""
default-branch:
description: The repository's default branch, the compare's base; required with `pr-workflow-change-ok`.
required: false
default: ""
github-token:
description: A token that can read the repository's contents, for the compare; required with `pr-workflow-change-ok`.
required: false
default: ""

outputs:
token:
Expand All @@ -96,6 +121,10 @@ outputs:
The one-line reason the step failed with, such as `lane-token-denied: ...`;
empty on a success.
value: ${{ steps.request.outputs.failure-reason }}
skip-reason:
description: >-
`workflow-modified` when `pr-workflow-change-ok` skipped the activity; empty otherwise.
value: ${{ steps.request.outputs.skip-reason }}

runs:
using: composite
Expand All @@ -118,6 +147,10 @@ runs:
PULL_REQUESTS: ${{ inputs.pull-requests }}
ISSUES: ${{ inputs.issues }}
SUPERSEDED_OK: ${{ inputs.superseded-ok }}
PR_WORKFLOW_CHANGE_OK: ${{ inputs.pr-workflow-change-ok }}
HEAD_SHA: ${{ inputs.head-sha }}
DEFAULT_BRANCH: ${{ inputs.default-branch }}
API_TOKEN: ${{ inputs.github-token }}
run: |
set -euo pipefail
umask 077
Expand All @@ -140,6 +173,14 @@ runs:
[[ $LANE =~ $name && $ACTIVITY =~ $name ]] || fail "the lane and activity must be kebab-case names"
SUPERSEDED_OK=${SUPERSEDED_OK:-false}
[[ $SUPERSEDED_OK == true || $SUPERSEDED_OK == false ]] || fail "superseded-ok must be true or false"
PR_WORKFLOW_CHANGE_OK=${PR_WORKFLOW_CHANGE_OK:-false}
[[ $PR_WORKFLOW_CHANGE_OK == true || $PR_WORKFLOW_CHANGE_OK == false ]] ||
fail "pr-workflow-change-ok must be true or false"
if [ "$PR_WORKFLOW_CHANGE_OK" = true ]; then
[[ $MODE == token && -n ${PR_NUMBER:-} && ${HEAD_SHA:-} =~ ^[0-9a-f]{40}$ &&
${DEFAULT_BRANCH:-} =~ ^[A-Za-z0-9._-]+$ && -n ${API_TOKEN:-} ]] ||
fail "pr-workflow-change-ok needs token mode, a PR number, a head SHA, the default branch and a GitHub token"
fi
case "$MODE" in
token)
[[ $EFFECT =~ $name ]] || fail "the effect must be a kebab-case name"
Expand Down Expand Up @@ -173,6 +214,7 @@ runs:

response="$RUNNER_TEMP/lane-token-response.json"
headers="$RUNNER_TEMP/lane-token-headers.txt"
compare="$RUNNER_TEMP/lane-token-compare.json"
issued=""
revoke() {
printf 'Authorization: Bearer %s\n' "$1" |
Expand All @@ -184,7 +226,7 @@ runs:
# prints its status, so only a token that passed every check reaches the job.
cleanup() {
local rc=$?
rm -f "$response" "$headers"
rm -f "$response" "$headers" "$compare"
if [ "$rc" -ne 0 ] && [ -n "$issued" ]; then
echo "Revoke attempt returned $(revoke "$issued")"
fi
Expand Down Expand Up @@ -284,6 +326,40 @@ runs:
printf -- '- %s\n' "$msg" >>"$GITHUB_STEP_SUMMARY"
exit 0
fi
# The refusal does not say whether the PR or the default branch
# changed the file, so the skip needs GitHub's own three-dot compare
# at the gated head to list it; any doubt stays red.
if [[ $PR_WORKFLOW_CHANGE_OK == true && $status == 403 && $reason == workflow-modified ]]; then
raw=$(jq -r 'if type == "object" and (.detail | type) == "string" then .detail else "" end' "$response" 2>/dev/null) || raw=""
file="" why=""
if [[ $raw =~ ^(\.github/workflows/[A-Za-z0-9._-]+\.ya?ml)\ differs\ from\ refs/heads/([A-Za-z0-9._-]+)$ &&
${BASH_REMATCH[2]} == "$DEFAULT_BRANCH" ]]; then
file=${BASH_REMATCH[1]}
else
why="the refusal names no workflow file against refs/heads/$DEFAULT_BRANCH"
fi
if [ -n "$file" ]; then
if printf 'Authorization: Bearer %s\n' "$API_TOKEN" |
curl -sS --fail --proto '=https' --max-time 30 -o "$compare" -H @- \
-H 'Accept: application/vnd.github+json' -H 'X-GitHub-Api-Version: 2022-11-28' \
"$GITHUB_API_URL/repos/$GITHUB_REPOSITORY/compare/$DEFAULT_BRANCH...$HEAD_SHA"; then
touched=$(jq --arg f "$file" \
'type == "object" and (.files | type) == "array" and any(.files[]; .filename == $f or .previous_filename == $f)' \
"$compare" 2>/dev/null) || touched=false
[ "$touched" = true ] || why="the pull request's diff at $HEAD_SHA does not change $file"
else
why="the compare of $DEFAULT_BRANCH...$HEAD_SHA could not be read"
fi
fi
if [ -z "$why" ]; then
msg="lane-token-skipped: the broker refused the token as workflow-modified because this pull request changes $file; no lane run gets a token while it does, so the activity is skipped"
echo "::notice::$msg"
printf -- '- %s\n' "$msg" >>"$GITHUB_STEP_SUMMARY"
echo "skip-reason=workflow-modified" >>"$GITHUB_OUTPUT"
exit 0
fi
detail="$detail; not skipped: $why"
fi
fail "$denied: the broker answered $status with reason $reason${detail:+: $detail}"
fi

Expand Down
Loading
Loading