From a113121d067b51b2bfbacd2e1a17755e1e61d2b0 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Wed, 16 Sep 2026 12:01:51 +0200 Subject: [PATCH 1/5] sweep-open-prs: skill for triaging the whole open PR queue Codifies the pass a maintainer makes repeatedly: gather state for every open PR, bucket each one into merge-now / approve+merge / needs-triage / blocked, and act only on what is confirmed. Two `gh pr list` calls cover the queue -- one for state, one for a check rollup collapsed to only non-green results -- which avoids the per-PR loop that trips gh's spurious 401. Records the judgement rules that decide most of the queue: BLOCKED + REVIEW_REQUIRED means "needs one approval" rather than "broken", self-authored PRs can never be self-approved, carried-over warnings are not regressions, and a held bump is often superseded rather than fixed. Routes anything red, or any bump being approved on substance, to analyze-action-pr rather than duplicating its failure taxonomy. Generated-by: Claude Opus 5 --- .claude/skills/sweep-open-prs/SKILL.md | 170 +++++++++++++++++++++++++ 1 file changed, 170 insertions(+) create mode 100644 .claude/skills/sweep-open-prs/SKILL.md diff --git a/.claude/skills/sweep-open-prs/SKILL.md b/.claude/skills/sweep-open-prs/SKILL.md new file mode 100644 index 000000000..005ab81a6 --- /dev/null +++ b/.claude/skills/sweep-open-prs/SKILL.md @@ -0,0 +1,170 @@ + + +--- +name: sweep-open-prs +description: Use when the maintainer asks what can be landed across the open PR queue - "check if I can merge any PRs", "which PRs can be approved and merged", "sweep the PRs", "what's ready to land", "anything new on the PRs". Also for a routine pass over the queue after an overnight dependabot batch. Not for a deep review of one named PR. +--- + +# Sweep the open PR queue + +Answer "what can I land right now?" for every open PR in one pass, then act +only on what the maintainer confirms. + +The queue is mostly dependabot: allowlist bumps under +`.github/actions/for-dependabot-triggered-reviews/`, plus workflow and +`uv.lock` bumps. A sweep is cheap because two `gh` calls cover the whole +queue; the expensive part is reviewing whatever the sweep surfaces as red or +as needing a human read. + +## Environment + +`gh` reads its config and auth keyring from `~/.config/gh`, which the sandbox +denies. Every `gh` call in this skill needs `dangerouslyDisableSandbox: true`. +The first failure looks like this and is not an auth problem: + +``` +failed to load config: open /Users//.config/gh/config.yml: operation not permitted +``` + +Two more traps, both of which produce a bogus `HTTP 401`: + +| Trap | Fix | +|---|---| +| Batching `gh` calls in a shell `for` loop | One `gh` call per Bash invocation | +| `gh` invoked from a python/sh child process | Call `gh` directly from Bash | + +## Step 1: gather + +Two calls cover the queue. Run them as written - the per-PR alternative burns +a call per PR and hits the loop trap above. + +State: + +``` +gh pr list --repo apache/infrastructure-actions --limit 50 \ + --json number,title,author,isDraft,mergeable,mergeStateStatus,reviewDecision \ + --jq '.[] | "\(.number)\t@\(.author.login)\tmergeable=\(.mergeable)\tstate=\(.mergeStateStatus)\treview=\(.reviewDecision)\t\(.title)"' +``` + +Checks, collapsed to only what is NOT green: + +``` +gh pr list --repo apache/infrastructure-actions --limit 50 \ + --json number,statusCheckRollup \ + --jq '.[] | "\(.number)\t" + ([.statusCheckRollup[] + | select((.conclusion//.state) != "SUCCESS" + and (.conclusion//.state) != "NEUTRAL" + and (.conclusion//.state) != "SKIPPED") + | "\(.name//.context)=\(.conclusion//.state)"] + | if length==0 then "ALL GREEN" else join(" ") end)' +``` + +`mergeStateStatus: UNKNOWN` means GitHub is still recomputing mergeability, +usually right after another PR merged. It is not a verdict - re-query that PR +before classifying it. + +## Step 2: classify + +Bucket each PR on the observable fields alone: + +| Bucket | Signature | Action | +|---|---|---| +| **Merge now** | `state=CLEAN`, `review=APPROVED`, all green | Merge on confirmation | +| **Approve + merge** | all green, `review=REVIEW_REQUIRED`, author is dependabot or another person | Review, then approve + merge | +| **Needs triage** | any check not green | Read the failing job before judging | +| **Blocked on reviewer** | all green, `review=REVIEW_REQUIRED`, **maintainer is the author** | Cannot self-approve - nudge, do not attempt | +| **Blocked on author** | `review=CHANGES_REQUESTED` | Waiting on the bump or an upstream fix | +| **Conflicting** | `mergeable=CONFLICTING` | Needs a rebase first | + +Two rules that decide most of the queue: + +- **You cannot approve your own PR.** A maintainer-authored PR with + `review=REVIEW_REQUIRED` is never in an actionable bucket no matter how + green it is. Say so plainly rather than offering to approve it. +- **`state=BLOCKED` with `review=REVIEW_REQUIRED` just means "needs an + approval"** - it is not a failure. `BLOCKED` alongside a red check is the + failure. + +## Step 3: act, per bucket + +### Allowlist bumps (`action-allowlist-review:` titles) + +The `verify` check is the gate. Green `verify` means the tool reconciled the +published action against a rebuild from its own lock file. + +**REQUIRED SUB-SKILL:** for anything red, or for any bump you are about to +approve on substance rather than on a green tick, use `analyze-action-pr`. It +owns the failure taxonomy (pipe-to-shell, unverified download, in-tree +binaries, verify-script gaps) and the upstream-issue workflow. + +Before treating a warning as a finding, **diff it against the approved +version**. A warning the already-approved version also carries is a standing +property of the action, not a regression the bump introduced. Carried-over +warnings do not block. + +### Lock-file and workflow bumps + +`uv.lock` / `package-lock.json` only, or a workflow action SHA bump: read the +diff, confirm it touches no action surface, approve. + +### Red checks + +Read the failing job before forming a view: + +``` +gh pr checks --repo apache/infrastructure-actions --json name,state,link \ + --jq '.[] | select(.state=="FAILURE") | .link' +gh run view --repo apache/infrastructure-actions --job --log-failed \ + | sed 's/\x1b\[[0-9;]*[a-zA-Z]//g' +``` + +Always strip the ANSI escapes or the summary table is unreadable. + +**A red check on a stale branch may be a fixed bug.** Before reporting a +finding, check whether the failure predates something already merged to main - +rebase or re-run rather than filing against it. + +### Superseded PRs + +A bump held for an upstream defect is often replaced rather than fixed: +dependabot opens a new PR at the newer version and closes the old one. Before +digging back into a held PR, check whether a higher-numbered PR for the same +action already exists. + +## Reporting + +Lead with what the maintainer can act on, not with the full queue. A table +keyed by bucket beats a PR-by-PR narrative once the queue is more than a +handful. + +**Draft every outbound message and wait for explicit confirmation** - reviews, +PR comments, upstream issues. Approving is a message in the maintainer's name. +Merging a PR they just approved in the same turn is the mid-flow exception. + +## Common mistakes + +| Mistake | Consequence | +|---|---| +| Reading `state=BLOCKED` as "broken" | Most of a healthy queue is `BLOCKED` pending one approval | +| Classifying on a `mergeStateStatus: UNKNOWN` | Misses a PR that is actually ready; re-query first | +| Offering to approve the maintainer's own PR | Impossible on GitHub; wastes a round-trip | +| Treating a carried-over warning as a regression | Blocks a bump that is no worse than the approved version | +| Filing upstream before searching the target repo | Duplicate issues; search existing issues, including the maintainer's own | +| Looping `gh` over PR numbers | Spurious `HTTP 401` on every call after the first | From cd648f9f81387e175a783c55ad3e4c65464aa88e Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Wed, 16 Sep 2026 12:07:45 +0200 Subject: [PATCH 2/5] README: document the agent skills Adds an Agent Skills section covering analyze-action-pr and sweep-open-prs, with a TOC entry. Points readers at Batch-Reviewing Dependabot PRs as the non-agent equivalent of the queue sweep. analyze-action-pr had never been documented; it is included here rather than left invisible now that the section exists. Generated-by: Claude Opus 5 --- README.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/README.md b/README.md index 8e50da541..576e8292a 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,7 @@ This repository hosts GitHub Actions developed by the ASF community and approved - [Checking the Action Usage in an ASF Project](#checking-the-action-usage-in-an-asf-project) - [Submitting an Action](#submitting-an-action) +- [Agent Skills](#agent-skills) - [Available GitHub Actions](#available-github-actions) - [Versioning and Pinning Actions](#versioning-and-pinning-actions) - [Organization-wide GitHub Actions Allow List](#management-of-organization-wide-github-actions-allow-list) @@ -93,6 +94,28 @@ action, even if they're not committers on _this_ project: you're especially qualified to judge and vouch for the safety and correctness of the action. +## Agent Skills + +Skills under `.claude/skills/` package the review workflows a maintainer repeats, so an +agent runs them the same way each time. They are agent-facing and need no setup beyond +an authenticated `gh` CLI; a human can follow the same steps by reading the `SKILL.md`. + + - [`analyze-action-pr`](/.claude/skills/analyze-action-pr/SKILL.md): Triage a single PR + that adds or bumps an action. Runs `verify-action-build`, classifies each failing + action (pipe-to-shell, unverified download, unverified in-tree binaries, gaps in the + verify script itself) and proposes the next step -- recommend approval, open an + upstream issue, or fix `verify-action-build` with a regression test. Its "Recent + precedents" table records how earlier PRs in each shape were resolved. + - [`sweep-open-prs`](/.claude/skills/sweep-open-prs/SKILL.md): Triage the whole open PR + queue in one pass. Buckets every open PR into merge-now, approve-and-merge, + needs-triage or blocked, and routes anything red to `analyze-action-pr`. Useful after + an overnight dependabot batch. + +Use `sweep-open-prs` to find what needs attention and `analyze-action-pr` to go deep on +one PR. For the non-agent equivalent of the sweep, see +[Batch-Reviewing Dependabot PRs](#batch-reviewing-dependabot-prs), which drives the same +verification from `verify-action-build` directly. + ## Available GitHub Actions - [ASF Infrastructure Pelican Action](/pelican/README.md): Generate and publish project websites with GitHub Actions From 39f12e3f3f36f75e3e25ec7ac5f12083ea0b70e4 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Fri, 18 Sep 2026 06:08:43 +0200 Subject: [PATCH 3/5] sweep-open-prs: report unfinished checks instead of blanking them A CheckRun carries conclusion: null until status reaches COMPLETED, and its state is always null -- state belongs to the StatusContext shape. Reaching straight for (.conclusion // .state) printed a bare "name=" for every check still running, which reads as a finding rather than "ask again in a minute". It misreported a PR whose ten checks were simply mid-flight. Read status first and report QUEUED / IN_PROGRESS, and record alongside the UNKNOWN mergeability case that neither is a verdict. Generated-by: Claude Opus 5 --- .claude/skills/sweep-open-prs/SKILL.md | 31 ++++++++++++++++++++------ 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/.claude/skills/sweep-open-prs/SKILL.md b/.claude/skills/sweep-open-prs/SKILL.md index 005ab81a6..d194c15ac 100644 --- a/.claude/skills/sweep-open-prs/SKILL.md +++ b/.claude/skills/sweep-open-prs/SKILL.md @@ -69,16 +69,32 @@ Checks, collapsed to only what is NOT green: gh pr list --repo apache/infrastructure-actions --limit 50 \ --json number,statusCheckRollup \ --jq '.[] | "\(.number)\t" + ([.statusCheckRollup[] - | select((.conclusion//.state) != "SUCCESS" - and (.conclusion//.state) != "NEUTRAL" - and (.conclusion//.state) != "SKIPPED") - | "\(.name//.context)=\(.conclusion//.state)"] + | . as $c + | (if ($c.status // "COMPLETED") != "COMPLETED" then $c.status + elif ($c.conclusion // "") != "" then $c.conclusion + elif ($c.state // "") != "" then $c.state + else "PENDING" end) as $s + | select($s != "SUCCESS" and $s != "NEUTRAL" and $s != "SKIPPED") + | "\($c.name // $c.context)=\($s)"] | if length==0 then "ALL GREEN" else join(" ") end)' ``` -`mergeStateStatus: UNKNOWN` means GitHub is still recomputing mergeability, -usually right after another PR merged. It is not a verdict - re-query that PR -before classifying it. +**Read `status` before `conclusion`.** A `CheckRun` carries `conclusion: null` +until its `status` reaches `COMPLETED`, and its `state` is always null - `state` +belongs to the `StatusContext` shape instead. A query that reaches straight for +`(.conclusion // .state)` therefore prints a bare `name=` for every check still +running, which reads like a finding rather than "ask again in a minute". The +query above reports `QUEUED` or `IN_PROGRESS` instead. + +Two states that are *not* verdicts, and must be re-queried rather than +classified: + +- `mergeStateStatus: UNKNOWN` - GitHub is recomputing mergeability, usually + right after another PR merged. It also appears mid-merge-storm when several + PRs touch one file; the state settles back to `CLEAN` on its own. +- A check reported `QUEUED` or `IN_PROGRESS` - not yet a result. Checks on a + fresh PR routinely take minutes, so a sweep run the moment a PR opens will + see them unfinished. ## Step 2: classify @@ -164,6 +180,7 @@ Merging a PR they just approved in the same turn is the mid-flow exception. |---|---| | Reading `state=BLOCKED` as "broken" | Most of a healthy queue is `BLOCKED` pending one approval | | Classifying on a `mergeStateStatus: UNKNOWN` | Misses a PR that is actually ready; re-query first | +| Reading an unfinished check as a failure | A running check has no conclusion yet; re-query rather than triaging it | | Offering to approve the maintainer's own PR | Impossible on GitHub; wastes a round-trip | | Treating a carried-over warning as a regression | Blocks a bump that is no worse than the approved version | | Filing upstream before searching the target repo | Duplicate issues; search existing issues, including the maintainer's own | From 9a5fdba1b3a894557c2a5c4736f6de7730d777d9 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Sat, 17 Oct 2026 01:51:05 +0200 Subject: [PATCH 4/5] sweep-open-prs: run gh bare so the sandbox exclusion applies With excludedCommands "gh *", gh needs no sandbox bypass, but only as a single bare command; a pipe or redirect re-sandboxes it. Update the Environment section, drop the sed pipe from the red-check recipe, and add the mistake to the table. Generated-by: Claude Opus 5 --- .claude/skills/sweep-open-prs/SKILL.md | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/.claude/skills/sweep-open-prs/SKILL.md b/.claude/skills/sweep-open-prs/SKILL.md index d194c15ac..bd9aabb36 100644 --- a/.claude/skills/sweep-open-prs/SKILL.md +++ b/.claude/skills/sweep-open-prs/SKILL.md @@ -35,14 +35,24 @@ as needing a human read. ## Environment -`gh` reads its config and auth keyring from `~/.config/gh`, which the sandbox -denies. Every `gh` call in this skill needs `dangerouslyDisableSandbox: true`. -The first failure looks like this and is not an auth problem: +`gh` reads its config and auth keyring from `~/.config/gh` and needs +api.github.com, so it does not work inside the sandbox. The usual setup +excludes it with `sandbox.excludedCommands: ["gh *"]`, and then every `gh` +call runs unsandboxed with no bypass needed. Without that exclusion, each call +needs `dangerouslyDisableSandbox: true`, and the first failure looks like this +(it is not an auth problem): ``` failed to load config: open /Users//.config/gh/config.yml: operation not permitted ``` +**The exclusion only matches a bare `gh` command.** A pipe, redirect, `;` or +`&&` puts the whole command back in the sandbox, where it fails with +`deny network-outbound api.github.com:443`, or with +`tls: failed to verify certificate: x509: OSStatus -26276` once the host is +allowed. Filter with `--jq` / `--template` instead of piping, and read output +from the tool result instead of redirecting it to a file. + Two more traps, both of which produce a bogus `HTTP 401`: | Trap | Fix | @@ -147,11 +157,11 @@ Read the failing job before forming a view: ``` gh pr checks --repo apache/infrastructure-actions --json name,state,link \ --jq '.[] | select(.state=="FAILURE") | .link' -gh run view --repo apache/infrastructure-actions --job --log-failed \ - | sed 's/\x1b\[[0-9;]*[a-zA-Z]//g' +gh run view --repo apache/infrastructure-actions --job --log-failed ``` -Always strip the ANSI escapes or the summary table is unreadable. +Run each as its own Bash call, with no pipe (see Environment). The log keeps +its ANSI colour escapes; read past them to the summary table. **A red check on a stale branch may be a fixed bug.** Before reporting a finding, check whether the failure predates something already merged to main - @@ -185,3 +195,4 @@ Merging a PR they just approved in the same turn is the mid-flow exception. | Treating a carried-over warning as a regression | Blocks a bump that is no worse than the approved version | | Filing upstream before searching the target repo | Duplicate issues; search existing issues, including the maintainer's own | | Looping `gh` over PR numbers | Spurious `HTTP 401` on every call after the first | +| Piping or redirecting `gh` output | The command runs sandboxed and fails with a network deny or `x509` TLS error | From 040dc36ef7fde810385108505e0ecb0b40d4ad7b Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Sat, 17 Oct 2026 01:53:12 +0200 Subject: [PATCH 5/5] sweep-open-prs: keep gh commands on one line A multi-line gh command (backslash continuation, multi-line --jq or --body) does not match the "gh *" sandbox exclusion. Collapse the recipes to single lines and say so in Environment. Generated-by: Claude Opus 5 --- .claude/skills/sweep-open-prs/SKILL.md | 29 +++++++++----------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/.claude/skills/sweep-open-prs/SKILL.md b/.claude/skills/sweep-open-prs/SKILL.md index bd9aabb36..26d557985 100644 --- a/.claude/skills/sweep-open-prs/SKILL.md +++ b/.claude/skills/sweep-open-prs/SKILL.md @@ -51,7 +51,10 @@ failed to load config: open /Users//.config/gh/config.yml: operation not p `deny network-outbound api.github.com:443`, or with `tls: failed to verify certificate: x509: OSStatus -26276` once the host is allowed. Filter with `--jq` / `--template` instead of piping, and read output -from the tool result instead of redirecting it to a file. +from the tool result instead of redirecting it to a file. Keep the command on +one line too: a multi-line argument (a `--body` with newlines, a `\` +continuation) does not match the exclusion either, so pass long text with +`--body-file`. Two more traps, both of which produce a bogus `HTTP 401`: @@ -62,31 +65,20 @@ Two more traps, both of which produce a bogus `HTTP 401`: ## Step 1: gather -Two calls cover the queue. Run them as written - the per-PR alternative burns -a call per PR and hits the loop trap above. +Two calls cover the queue. Run them as written, each on a single line (see +Environment) - the per-PR alternative burns a call per PR and hits the loop +trap above. State: ``` -gh pr list --repo apache/infrastructure-actions --limit 50 \ - --json number,title,author,isDraft,mergeable,mergeStateStatus,reviewDecision \ - --jq '.[] | "\(.number)\t@\(.author.login)\tmergeable=\(.mergeable)\tstate=\(.mergeStateStatus)\treview=\(.reviewDecision)\t\(.title)"' +gh pr list --repo apache/infrastructure-actions --limit 50 --json number,title,author,isDraft,mergeable,mergeStateStatus,reviewDecision --jq '.[] | "\(.number)\t@\(.author.login)\tmergeable=\(.mergeable)\tstate=\(.mergeStateStatus)\treview=\(.reviewDecision)\t\(.title)"' ``` Checks, collapsed to only what is NOT green: ``` -gh pr list --repo apache/infrastructure-actions --limit 50 \ - --json number,statusCheckRollup \ - --jq '.[] | "\(.number)\t" + ([.statusCheckRollup[] - | . as $c - | (if ($c.status // "COMPLETED") != "COMPLETED" then $c.status - elif ($c.conclusion // "") != "" then $c.conclusion - elif ($c.state // "") != "" then $c.state - else "PENDING" end) as $s - | select($s != "SUCCESS" and $s != "NEUTRAL" and $s != "SKIPPED") - | "\($c.name // $c.context)=\($s)"] - | if length==0 then "ALL GREEN" else join(" ") end)' +gh pr list --repo apache/infrastructure-actions --limit 50 --json number,statusCheckRollup --jq '.[] | "\(.number)\t" + ([.statusCheckRollup[] | . as $c | (if ($c.status // "COMPLETED") != "COMPLETED" then $c.status elif ($c.conclusion // "") != "" then $c.conclusion elif ($c.state // "") != "" then $c.state else "PENDING" end) as $s | select($s != "SUCCESS" and $s != "NEUTRAL" and $s != "SKIPPED") | "\($c.name // $c.context)=\($s)"] | if length==0 then "ALL GREEN" else join(" ") end)' ``` **Read `status` before `conclusion`.** A `CheckRun` carries `conclusion: null` @@ -155,8 +147,7 @@ diff, confirm it touches no action surface, approve. Read the failing job before forming a view: ``` -gh pr checks --repo apache/infrastructure-actions --json name,state,link \ - --jq '.[] | select(.state=="FAILURE") | .link' +gh pr checks --repo apache/infrastructure-actions --json name,state,link --jq '.[] | select(.state=="FAILURE") | .link' gh run view --repo apache/infrastructure-actions --job --log-failed ```