From ed05e2a7ca49d0c0f6c721cfa5b7b87a6595df2e Mon Sep 17 00:00:00 2001 From: Blake Bertuccelli-Booth <46652+bbertucc@users.noreply.github.com> Date: Sun, 4 Oct 2026 13:33:06 -0400 Subject: [PATCH 1/3] feat(ci): the maintainer works only on issues an owner labelled `maintainer` issue-to-pr.yml now keeps only issues whose last `maintainer` label event was added by a login in .github/owners. A dispatch by number is held to the same rule, and the check fails closed. issue-triage.yml now tags the owners once on each open issue that no owner has approved yet. .github/owners is on the auto-PR forbidden paths. Co-Authored-By: Claude Opus 5.5 --- .github/owners | 6 +++ .github/scripts/ask-owners.sh | 33 ++++++++++++ .github/scripts/owner-approved.sh | 29 +++++++++++ .github/workflows/issue-to-pr.yml | 27 ++++++++-- .github/workflows/issue-triage.yml | 20 +++++++ CONTRIBUTING.md | 7 +-- docs/ci.md | 10 +++- test/owner-gate.test.ts | 83 ++++++++++++++++++++++++++++++ 8 files changed, 206 insertions(+), 9 deletions(-) create mode 100644 .github/owners create mode 100755 .github/scripts/ask-owners.sh create mode 100755 .github/scripts/owner-approved.sh create mode 100644 test/owner-gate.test.ts diff --git a/.github/owners b/.github/owners new file mode 100644 index 00000000..189efa11 --- /dev/null +++ b/.github/owners @@ -0,0 +1,6 @@ +# Owners: the people who decide what the maintainer works on. One GitHub login per line. +# +# The maintainer agent and `issue-to-pr.yml` work only on an issue an owner has labelled +# `maintainer`. `issue-triage.yml` tags these owners when a new issue needs that decision. +# Read from `main`, and on `issue-to-pr.yml`'s forbidden paths, so a PR cannot add itself. +bbertucc diff --git a/.github/scripts/ask-owners.sh b/.github/scripts/ask-owners.sh new file mode 100755 index 00000000..a69904f2 --- /dev/null +++ b/.github/scripts/ask-owners.sh @@ -0,0 +1,33 @@ +#!/usr/bin/env bash +# Usage: ask-owners.sh +# +# Tags the owners in .github/owners once on an open issue that no owner has labelled `maintainer`, +# so they can decide whether the maintainer should work on it. Posts nothing on a closed issue, on +# one labelled `maintainer`, `duplicate`, `wontfix` or `invalid`, or on one it already asked on. +set -euo pipefail + +n="$1" +here="$(dirname "$0")" +marker='' + +gh issue view "$n" --json state,labels,comments > /tmp/ask-owners.json +if [ "$(jq -r .state /tmp/ask-owners.json)" != "OPEN" ]; then + echo "#$n is not open — not asking." + exit 0 +fi +skip=$(jq -r '[.labels[].name] | map(select(. == "maintainer" or . == "duplicate" or . == "wontfix" or . == "invalid")) | join(", ")' /tmp/ask-owners.json) +if [ -n "$skip" ]; then + echo "#$n is labelled $skip — not asking." + exit 0 +fi +if jq -e --arg m "$marker" 'any(.comments[]; .body | contains($m))' /tmp/ask-owners.json >/dev/null; then + echo "#$n was already asked on — not asking again." + exit 0 +fi + +owners=$(grep -v '^[[:space:]]*#' "$here/../owners" | grep -v '^[[:space:]]*$' | sed 's/^/@/' | paste -sd' ' -) +{ + printf '%s: this issue is ready for an owner to review. The maintainer works on it only after an owner adds the `maintainer` label.\n\n' "$owners" + printf '%s\n' "$marker" +} > /tmp/ask-owners.md +gh issue comment "$n" --body-file /tmp/ask-owners.md diff --git a/.github/scripts/owner-approved.sh b/.github/scripts/owner-approved.sh new file mode 100755 index 00000000..e67ec1f7 --- /dev/null +++ b/.github/scripts/owner-approved.sh @@ -0,0 +1,29 @@ +#!/usr/bin/env bash +# Usage: owner-approved.sh +# +# Exits 0 and prints the owner's login when an owner in .github/owners put the `maintainer` label +# on the issue and it is still there. Exits 1 otherwise, including when the events can't be read. +# +# Only accounts with triage access or higher can label an issue. The label event's actor is checked +# too, because that access is wider than the owners list. +set -euo pipefail + +n="$1" +repo="${GITHUB_REPOSITORY:-$(gh repo view --json nameWithOwner --jq .nameWithOwner)}" +owners_file="$(dirname "$0")/../owners" + +last=$(gh api --paginate "repos/$repo/issues/$n/events" \ + | jq -r '.[] | select((.event == "labeled" or .event == "unlabeled") and .label.name == "maintainer") + | "\(.event) \(.actor.login)"' \ + | tail -n 1) + +case "$last" in + "labeled "*) login="${last#labeled }" ;; + *) exit 1 ;; +esac + +if grep -v '^[[:space:]]*#' "$owners_file" | grep -qxF "$login"; then + echo "$login" + exit 0 +fi +exit 1 diff --git a/.github/workflows/issue-to-pr.yml b/.github/workflows/issue-to-pr.yml index 153809a1..518dcbe9 100644 --- a/.github/workflows/issue-to-pr.yml +++ b/.github/workflows/issue-to-pr.yml @@ -25,6 +25,9 @@ name: Issue to PR # "Claims" means a closing keyword or an `issue-` branch, not a passing `#` # in prose; see `Pick an issue` for why, and for every reason a run is skipped. # +# It works only on an issue an owner in .github/owners has labelled `maintainer`, +# a dispatch by number included. issue-triage.yml tags the owners for that call. +# # Requires the same two repo settings as code-review.yml, and nothing new: # - secret AWS_BEDROCK_ROLE_ARN — the OIDC role. Its trust policy covers this # repo's `refs/heads/*` subjects, which is what a `schedule` or @@ -316,7 +319,8 @@ jobs: # already made the "is this worth doing" call, so skip labels and a # past rejection do not veto them — asking again by hand is exactly # how you overrule a rejection. A second PR for an issue that is - # still in the queue is just a mess, so that one still applies. + # still in the queue is just a mess, so that one still applies. So + # does the owner's `maintainer` label, checked below for both paths. if grep -qxF "$FORCED_ISSUE" /tmp/open-auto-issues.txt; then echo "should_run=false" >> "$GITHUB_OUTPUT" printf 'Skipped: issue #%s already has an open `iris-auto` PR.\n' \ @@ -366,6 +370,21 @@ jobs: echo "forced=false" >> "$GITHUB_OUTPUT" fi + # Only issues an owner approved: an owner in .github/owners added the `maintainer` label. + # This applies to a dispatch by number too, and fails closed if the events can't be read. + jq -r '.[].number' /tmp/candidates.json | while read -r n; do + if .github/scripts/owner-approved.sh "$n" >/dev/null; then echo "$n"; fi + done | jq -R 'tonumber' | jq -s . > /tmp/approved.json + jq --slurpfile ok /tmp/approved.json '[.[] | select(.number as $n | $ok[0] | index($n))]' \ + /tmp/candidates.json > /tmp/candidates-approved.json + mv /tmp/candidates-approved.json /tmp/candidates.json + if [ -n "${FORCED_ISSUE:-}" ] && [ "$(jq length /tmp/candidates.json)" -eq 0 ]; then + echo "should_run=false" >> "$GITHUB_OUTPUT" + printf 'Skipped: no owner has labelled issue #%s `maintainer`.\n' "$FORCED_ISSUE" >> "$GITHUB_STEP_SUMMARY" + echo "::error::Issue #$FORCED_ISSUE needs the \`maintainer\` label from an owner in .github/owners." + exit 1 + fi + COUNT=$(jq length /tmp/candidates.json) echo "candidate issues: $COUNT" if [ "$COUNT" -eq 0 ]; then @@ -379,8 +398,8 @@ jobs: echo echo 'Every open issue is linked by an open pull request (anyone'"'"'s — see the table' echo 'above if there is one), already attempted by this workflow (open or' - echo 'previously-rejected PR), or carries a skip label. Nothing was run — no model' - echo 'call, no PR, no comment.' + echo 'previously-rejected PR), carries a skip label, or has no `maintainer` label' + echo 'from an owner. Nothing was run — no model call, no PR, no comment.' } >> "$GITHUB_STEP_SUMMARY" echo "::notice::No eligible open issue — nothing to do." exit 0 @@ -902,7 +921,7 @@ jobs: # an issue closes — into `.github/scripts/triage-decide.sh`. A privilege # boundary drawn at a directory name follows the privilege, not the name, # so anywhere a workflow's own logic lives belongs in this pattern. - FORBIDDEN='^(\.github/workflows/|\.github/scripts/|\.github/CODEOWNERS|LICENSE|infra/|\.env)' + FORBIDDEN='^(\.github/workflows/|\.github/scripts/|\.github/CODEOWNERS|\.github/owners|LICENSE|infra/|\.env)' RESULT=/tmp/iris-auto-result.json CLAIMED_PR="" diff --git a/.github/workflows/issue-triage.yml b/.github/workflows/issue-triage.yml index 1eadb6ec..a71cd162 100644 --- a/.github/workflows/issue-triage.yml +++ b/.github/workflows/issue-triage.yml @@ -1015,3 +1015,23 @@ jobs: # `.github/scripts/` is also in `issue-to-pr.yml`'s forbidden paths and in # `code-review.yml`'s CI-security gate, for the same reason. run: .github/scripts/triage-decide.sh + + # Tags the owners in .github/owners on an issue triage left open, so they can decide whether the + # maintainer should work on it. Its own job so that none of triage's early exits skip it. + ask-owners: + needs: triage + if: ${{ always() && inputs.dry_run != true }} + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + issues: write + steps: + - uses: actions/checkout@v7 + with: + persist-credentials: false + - name: Ask the owners + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + NUMBER: ${{ github.event.issue.number || inputs.issue_number }} + run: .github/scripts/ask-owners.sh "$NUMBER" diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1e2b0ec7..499446e1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -21,13 +21,14 @@ By participating you agree to our [Code of Conduct](CODE_OF_CONDUCT.md). one per content type". `chartDataAgent.md` is the shape that earns its place. - **Code** — bug fixes and improvements via pull request. -A well-written issue may get a pull request without you doing anything else. A scheduled workflow -ranks the open issues Sun–Wed and opens one PR for the most pressing one it can finish well +A well-written issue may get a pull request without you doing anything else. An owner first labels +it `maintainer` (a bot tags them when it's time). A scheduled workflow then ranks those issues +Sun–Wed and opens one PR for the most pressing one it can finish well ([details](docs/ci.md#scheduled-issue-triage)) — accessibility barriers rank first, and small user-visible fixes reported against the demo rank well because they review cleanly. It never touches an issue labelled `no-auto-pr`, never files a second PR for an issue it has already tried, and stops entirely when nothing is eligible. If you'd rather own the fix yourself, say so on the -issue and add that label. +issue and ask for the `no-auto-pr` label. **You get the credit for it.** A PR from that workflow names you in its body and carries a `Co-authored-by` trailer for your account on the commit, so the merged commit is attributed to you diff --git a/docs/ci.md b/docs/ci.md index 4a181937..7a38a329 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -112,6 +112,8 @@ that human is reading, not whether they read it. opened or reopened**. It reads the new issue, finds the open issue it most resembles, and — only when a second, independent session fails to refute the claim — closes the new one as a duplicate with a comment naming the survivor. Anything short of that is commented and reported, never closed. +An issue it leaves open gets one comment tagging the owners in `.github/owners`, asking for the +`maintainer` label. It exists because the dedupe already in the app cannot do this, and was never trying to. `src/github/issue.ts` refuses to file an `Agent update proposal:` whose title exactly matches an @@ -261,6 +263,10 @@ UTC**. It reads the open issues, ranks them by what most improves Iris, and open **one** pull request for the top issue it can finish well, with a review requested from **@bbertucc**. +It works only on an issue an owner listed in [`.github/owners`](../.github/owners) has labelled +`maintainer`, a dispatch by number included. [Closing duplicate issues](#closing-duplicate-issues) +tags the owners on each new issue it leaves open, so they can make that call. + [Automated code review](#automated-code-review) raised the ceiling on how much review this maintainership can absorb; this spends some of that headroom on the other side of the same bottleneck — issues that are correct, small, and never picked up. A reported barrier that sits open @@ -378,8 +384,8 @@ A deployment nobody else runs must never be able to turn this project's `main` r [`.github/workflows/quality-report.yml`](../.github/workflows/quality-report.yml) runs **Saturdays at 20:00 UTC**. It reads `GET /v1/quality` on a live deployment, compares a handful of rates against thresholds held in that workflow file, and opens one issue per crossed threshold. -[Scheduled issue triage](#scheduled-issue-triage) then ranks those issues with everything else and -may open a PR against one. +Once an owner labels one `maintainer`, [Scheduled issue triage](#scheduled-issue-triage) ranks it +with everything else and may open a PR against it. Everything before this depended on somebody typing. An issue, or a session's feedback — the loop is good, but a person has to start it. Meanwhile Iris grades itself on every single run: how many diff --git a/test/owner-gate.test.ts b/test/owner-gate.test.ts new file mode 100644 index 00000000..addbf56b --- /dev/null +++ b/test/owner-gate.test.ts @@ -0,0 +1,83 @@ +// The maintainer works on an issue only after an owner labels it `maintainer`. These run the two +// scripts that enforce and announce that against a fake `gh`, and pin the workflow lines that call them. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync, chmodSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { parse } from "yaml"; + +const ROOT = join(import.meta.dirname, ".."); +const SCRIPTS = join(ROOT, ".github", "scripts"); + +const FAKE_GH = `#!/usr/bin/env bash +set -euo pipefail +case "$1 $2" in + "api --paginate") [ -n "\${FAKE_FAIL:-}" ] && exit 1; cat "$FAKE_DIR/events.json" ;; + "issue view") cat "$FAKE_DIR/issue.json" ;; + "issue comment") cp "$5" "$FAKE_DIR/posted.md" ;; + *) echo "fake gh: unexpected $*" >&2; exit 2 ;; +esac +`; + +function run(script: string, files: Record, env: Record = {}) { + const dir = mkdtempSync(join(tmpdir(), "owner-gate-")); + try { + writeFileSync(join(dir, "gh"), FAKE_GH); + chmodSync(join(dir, "gh"), 0o755); + for (const [name, value] of Object.entries(files)) writeFileSync(join(dir, name), JSON.stringify(value)); + const r = spawnSync(join(SCRIPTS, script), ["7"], { + encoding: "utf8", + env: { ...process.env, PATH: `${dir}:${process.env.PATH}`, FAKE_DIR: dir, GITHUB_REPOSITORY: "o/r", ...env }, + }); + const posted = existsSync(join(dir, "posted.md")) ? readFileSync(join(dir, "posted.md"), "utf8") : null; + return { status: r.status, stdout: r.stdout.trim(), posted }; + } finally { + rmSync(dir, { recursive: true, force: true }); + } +} + +const ev = (event: string, login: string, label = "maintainer") => ({ event, actor: { login }, label: { name: label } }); + +test("an issue is approved only while an owner's `maintainer` label is on it", () => { + const approved = (events: unknown[], env?: Record) => run("owner-approved.sh", { "events.json": events }, env); + assert.deepEqual(approved([ev("labeled", "bbertucc")]), { status: 0, stdout: "bbertucc", posted: null }); + assert.equal(approved([ev("labeled", "someone-else")]).status, 1, "not an owner"); + assert.equal(approved([ev("labeled", "bbertucc"), ev("unlabeled", "bbertucc")]).status, 1, "removed again"); + assert.equal(approved([ev("labeled", "bbertucc", "bug")]).status, 1, "another label"); + assert.equal(approved([ev("labeled", "someone-else"), ev("unlabeled", "x"), ev("labeled", "bbertucc")]).status, 0); + assert.equal(approved([ev("labeled", "bbertucc"), ev("labeled", "someone-else")]).status, 1, "the last label event decides"); + assert.equal(approved([]).status, 1); + assert.equal(approved([ev("labeled", "bbertucc")], { FAKE_FAIL: "1" }).status, 1, "fails closed"); +}); + +test("triage tags the owners once on an open issue no owner has approved", () => { + const ask = (issue: unknown) => run("ask-owners.sh", { "issue.json": issue }); + const open = { state: "OPEN", labels: [], comments: [] }; + const first = ask(open); + assert.equal(first.status, 0); + assert.match(first.posted ?? "", /^@bbertucc: .*`maintainer` label/); + assert.match(first.posted ?? "", //); + assert.equal(ask({ ...open, state: "CLOSED" }).posted, null); + for (const name of ["maintainer", "duplicate", "wontfix", "invalid"]) { + assert.equal(ask({ ...open, labels: [{ name }] }).posted, null, name); + } + assert.equal(ask({ ...open, labels: [{ name: "bug" }] }).posted !== null, true); + assert.equal(ask({ ...open, comments: [{ body: first.posted }] }).posted, null, "already asked"); +}); + +test("issue-to-pr keeps only approved issues, and triage asks after every outcome", () => { + const itp = parse(readFileSync(join(ROOT, ".github", "workflows", "issue-to-pr.yml"), "utf8")); + const pick: string = itp.jobs.propose.steps.find((s: { id?: string }) => s.id === "triage").run; + const gate = pick.indexOf(".github/scripts/owner-approved.sh"); + assert.ok(gate > pick.lastIndexOf("> /tmp/candidates.json"), "after both paths build the candidates"); + assert.ok(gate < pick.indexOf("COUNT=$(jq length /tmp/candidates.json)"), "before they are counted"); + assert.match(readFileSync(join(ROOT, ".github", "workflows", "issue-to-pr.yml"), "utf8"), /FORBIDDEN='[^']*\\\.github\/owners/); + + const triage = parse(readFileSync(join(ROOT, ".github", "workflows", "issue-triage.yml"), "utf8")); + const job = triage.jobs["ask-owners"]; + assert.equal(job.needs, "triage"); + assert.match(job.if, /always\(\)/); + assert.match(job.steps.at(-1).run, /^\.github\/scripts\/ask-owners\.sh /); +}); From 384b6894103ddbb58f9f0041e2c669e8591e52c8 Mon Sep 17 00:00:00 2001 From: Blake Bertuccelli-Booth <46652+bbertucc@users.noreply.github.com> Date: Sun, 4 Oct 2026 13:53:12 -0400 Subject: [PATCH 2/3] fix(ci): address review notes on the owner gate - ask-owners.sh skips every one of issue-to-pr.yml's SKIP_LABELS, which includes no-auto-pr. A test keeps the two lists in step. - issue-to-pr.yml also requires `maintainer` among the current labels, so deleting the label from the repo revokes approval. Corrected the script comment to say what it checks. - code-review.yml's CI-security item now covers `.github/owners`. - ask-owners waits for triage to succeed, so a failed triage doesn't use up the one ask. - An owners file with no entries skips the ask instead of failing. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/ask-owners.sh | 10 +++++++--- .github/scripts/owner-approved.sh | 5 +++-- .github/workflows/code-review.yml | 3 ++- .github/workflows/issue-to-pr.yml | 3 ++- .github/workflows/issue-triage.yml | 5 +++-- test/owner-gate.test.ts | 25 ++++++++++++++++++++----- 6 files changed, 37 insertions(+), 14 deletions(-) diff --git a/.github/scripts/ask-owners.sh b/.github/scripts/ask-owners.sh index a69904f2..7102eef9 100755 --- a/.github/scripts/ask-owners.sh +++ b/.github/scripts/ask-owners.sh @@ -3,7 +3,7 @@ # # Tags the owners in .github/owners once on an open issue that no owner has labelled `maintainer`, # so they can decide whether the maintainer should work on it. Posts nothing on a closed issue, on -# one labelled `maintainer`, `duplicate`, `wontfix` or `invalid`, or on one it already asked on. +# one labelled `maintainer` or with one of issue-to-pr.yml's SKIP_LABELS, or on one it already asked on. set -euo pipefail n="$1" @@ -15,7 +15,7 @@ if [ "$(jq -r .state /tmp/ask-owners.json)" != "OPEN" ]; then echo "#$n is not open — not asking." exit 0 fi -skip=$(jq -r '[.labels[].name] | map(select(. == "maintainer" or . == "duplicate" or . == "wontfix" or . == "invalid")) | join(", ")' /tmp/ask-owners.json) +skip=$(jq -r '[.labels[].name] | map(select(IN("maintainer", "wontfix", "invalid", "duplicate", "question", "no-auto-pr"))) | join(", ")' /tmp/ask-owners.json) if [ -n "$skip" ]; then echo "#$n is labelled $skip — not asking." exit 0 @@ -25,7 +25,11 @@ if jq -e --arg m "$marker" 'any(.comments[]; .body | contains($m))' /tmp/ask-own exit 0 fi -owners=$(grep -v '^[[:space:]]*#' "$here/../owners" | grep -v '^[[:space:]]*$' | sed 's/^/@/' | paste -sd' ' -) +owners=$(grep -v -e '^[[:space:]]*#' -e '^[[:space:]]*$' "$here/../owners" | sed 's/^/@/' | paste -sd' ' -) || true +if [ -z "$owners" ]; then + echo ".github/owners lists no one — not asking." + exit 0 +fi { printf '%s: this issue is ready for an owner to review. The maintainer works on it only after an owner adds the `maintainer` label.\n\n' "$owners" printf '%s\n' "$marker" diff --git a/.github/scripts/owner-approved.sh b/.github/scripts/owner-approved.sh index e67ec1f7..4581019c 100755 --- a/.github/scripts/owner-approved.sh +++ b/.github/scripts/owner-approved.sh @@ -1,8 +1,9 @@ #!/usr/bin/env bash # Usage: owner-approved.sh # -# Exits 0 and prints the owner's login when an owner in .github/owners put the `maintainer` label -# on the issue and it is still there. Exits 1 otherwise, including when the events can't be read. +# Exits 0 and prints the owner's login when the last `maintainer` label event on the issue is an +# add by an owner in .github/owners. Exits 1 otherwise, including when the events can't be read. +# It does not read the current labels; issue-to-pr.yml checks those. # # Only accounts with triage access or higher can label an issue. The label event's actor is checked # too, because that access is wider than the owners list. diff --git a/.github/workflows/code-review.yml b/.github/workflows/code-review.yml index e33847a4..fa67773d 100644 --- a/.github/workflows/code-review.yml +++ b/.github/workflows/code-review.yml @@ -1038,7 +1038,8 @@ jobs: 9. **CI and workflow security — conditional, and when it applies it comes FIRST.** Skip this item entirely unless the diff touches `.github/workflows/**`, - `.github/scripts/**` or `.github/actions/**`. `.github/scripts/` counts + `.github/scripts/**`, `.github/actions/**` or `.github/owners`, which decides + whose `maintainer` label sends an issue to `issue-to-pr.yml`. `.github/scripts/` counts because a workflow may keep part of itself there — GitHub refuses a `run:` block past 21000 characters, so `issue-triage.yml` holds its enforcement step in `.github/scripts/triage-decide.sh`, and that file decides whether an diff --git a/.github/workflows/issue-to-pr.yml b/.github/workflows/issue-to-pr.yml index 518dcbe9..f08b47bc 100644 --- a/.github/workflows/issue-to-pr.yml +++ b/.github/workflows/issue-to-pr.yml @@ -375,7 +375,8 @@ jobs: jq -r '.[].number' /tmp/candidates.json | while read -r n; do if .github/scripts/owner-approved.sh "$n" >/dev/null; then echo "$n"; fi done | jq -R 'tonumber' | jq -s . > /tmp/approved.json - jq --slurpfile ok /tmp/approved.json '[.[] | select(.number as $n | $ok[0] | index($n))]' \ + jq --slurpfile ok /tmp/approved.json \ + '[.[] | select(any(.labels[]; .name == "maintainer")) | select(.number as $n | $ok[0] | index($n))]' \ /tmp/candidates.json > /tmp/candidates-approved.json mv /tmp/candidates-approved.json /tmp/candidates.json if [ -n "${FORCED_ISSUE:-}" ] && [ "$(jq length /tmp/candidates.json)" -eq 0 ]; then diff --git a/.github/workflows/issue-triage.yml b/.github/workflows/issue-triage.yml index a71cd162..3922a5d6 100644 --- a/.github/workflows/issue-triage.yml +++ b/.github/workflows/issue-triage.yml @@ -1017,10 +1017,11 @@ jobs: run: .github/scripts/triage-decide.sh # Tags the owners in .github/owners on an issue triage left open, so they can decide whether the - # maintainer should work on it. Its own job so that none of triage's early exits skip it. + # maintainer should work on it. Its own job so that none of triage's early exits skip it, and it + # waits for triage to succeed, so a failed triage doesn't use up the one ask. ask-owners: needs: triage - if: ${{ always() && inputs.dry_run != true }} + if: ${{ needs.triage.result == 'success' && inputs.dry_run != true }} runs-on: ubuntu-latest timeout-minutes: 5 permissions: diff --git a/test/owner-gate.test.ts b/test/owner-gate.test.ts index addbf56b..27c3bf77 100644 --- a/test/owner-gate.test.ts +++ b/test/owner-gate.test.ts @@ -3,7 +3,7 @@ import { test } from "node:test"; import assert from "node:assert/strict"; import { spawnSync } from "node:child_process"; -import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync, chmodSync } from "node:fs"; +import { copyFileSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync, chmodSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { parse } from "yaml"; @@ -21,13 +21,21 @@ case "$1 $2" in esac `; -function run(script: string, files: Record, env: Record = {}) { +// `owners` swaps in an owners file by running a copy of the script from the temp dir. +function run(script: string, files: Record, env: Record = {}, owners?: string) { const dir = mkdtempSync(join(tmpdir(), "owner-gate-")); try { writeFileSync(join(dir, "gh"), FAKE_GH); chmodSync(join(dir, "gh"), 0o755); for (const [name, value] of Object.entries(files)) writeFileSync(join(dir, name), JSON.stringify(value)); - const r = spawnSync(join(SCRIPTS, script), ["7"], { + let path = join(SCRIPTS, script); + if (owners !== undefined) { + mkdirSync(join(dir, "scripts")); + path = join(dir, "scripts", script); + copyFileSync(join(SCRIPTS, script), path); + writeFileSync(join(dir, "owners"), owners); + } + const r = spawnSync(path, ["7"], { encoding: "utf8", env: { ...process.env, PATH: `${dir}:${process.env.PATH}`, FAKE_DIR: dir, GITHUB_REPOSITORY: "o/r", ...env }, }); @@ -60,11 +68,14 @@ test("triage tags the owners once on an open issue no owner has approved", () => assert.match(first.posted ?? "", /^@bbertucc: .*`maintainer` label/); assert.match(first.posted ?? "", //); assert.equal(ask({ ...open, state: "CLOSED" }).posted, null); - for (const name of ["maintainer", "duplicate", "wontfix", "invalid"]) { + for (const name of ["maintainer", "duplicate", "wontfix", "invalid", "question", "no-auto-pr"]) { assert.equal(ask({ ...open, labels: [{ name }] }).posted, null, name); } assert.equal(ask({ ...open, labels: [{ name: "bug" }] }).posted !== null, true); assert.equal(ask({ ...open, comments: [{ body: first.posted }] }).posted, null, "already asked"); + const none = run("ask-owners.sh", { "issue.json": open }, {}, "# no one\n\n"); + assert.deepEqual([none.status, none.posted], [0, null], "no owners to ask"); + assert.match(run("ask-owners.sh", { "issue.json": open }, {}, "# x\na\n \nb\n").posted ?? "", /^@a @b: /); }); test("issue-to-pr keeps only approved issues, and triage asks after every outcome", () => { @@ -74,10 +85,14 @@ test("issue-to-pr keeps only approved issues, and triage asks after every outcom assert.ok(gate > pick.lastIndexOf("> /tmp/candidates.json"), "after both paths build the candidates"); assert.ok(gate < pick.indexOf("COUNT=$(jq length /tmp/candidates.json)"), "before they are counted"); assert.match(readFileSync(join(ROOT, ".github", "workflows", "issue-to-pr.yml"), "utf8"), /FORBIDDEN='[^']*\\\.github\/owners/); + assert.match(pick.slice(gate), /select\(any\(\.labels\[\]; \.name == "maintainer"\)\)/, "and the label is on it now"); + const skip = pick.match(/SKIP_LABELS="([^"]*)"/)![1]!.split(" "); + const asks = readFileSync(join(SCRIPTS, "ask-owners.sh"), "utf8"); + for (const name of skip) assert.ok(asks.includes(`"${name}"`), `ask-owners skips ${name} too`); const triage = parse(readFileSync(join(ROOT, ".github", "workflows", "issue-triage.yml"), "utf8")); const job = triage.jobs["ask-owners"]; assert.equal(job.needs, "triage"); - assert.match(job.if, /always\(\)/); + assert.match(job.if, /needs\.triage\.result == 'success'/); assert.match(job.steps.at(-1).run, /^\.github\/scripts\/ask-owners\.sh /); }); From 76abab36764d7b742924d292e97e7f945ade4363 Mon Sep 17 00:00:00 2001 From: Blake Bertuccelli-Booth <46652+bbertucc@users.noreply.github.com> Date: Sun, 4 Oct 2026 14:13:48 -0400 Subject: [PATCH 3/3] fix(ci): filter on the label before the per-issue events calls The scheduled path no longer pays one paginated events call per open issue. Also renamed the test to match what it asserts, and the docs line now says skip-labelled issues get no comment. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/issue-to-pr.yml | 9 +++++---- docs/ci.md | 4 ++-- test/owner-gate.test.ts | 10 ++++++---- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/.github/workflows/issue-to-pr.yml b/.github/workflows/issue-to-pr.yml index f08b47bc..3175aa43 100644 --- a/.github/workflows/issue-to-pr.yml +++ b/.github/workflows/issue-to-pr.yml @@ -372,12 +372,13 @@ jobs: # Only issues an owner approved: an owner in .github/owners added the `maintainer` label. # This applies to a dispatch by number too, and fails closed if the events can't be read. - jq -r '.[].number' /tmp/candidates.json | while read -r n; do + # The label filter goes first, so only labelled issues cost an events call. + jq '[.[] | select(any(.labels[]; .name == "maintainer"))]' /tmp/candidates.json > /tmp/labelled.json + jq -r '.[].number' /tmp/labelled.json | while read -r n; do if .github/scripts/owner-approved.sh "$n" >/dev/null; then echo "$n"; fi done | jq -R 'tonumber' | jq -s . > /tmp/approved.json - jq --slurpfile ok /tmp/approved.json \ - '[.[] | select(any(.labels[]; .name == "maintainer")) | select(.number as $n | $ok[0] | index($n))]' \ - /tmp/candidates.json > /tmp/candidates-approved.json + jq --slurpfile ok /tmp/approved.json '[.[] | select(.number as $n | $ok[0] | index($n))]' \ + /tmp/labelled.json > /tmp/candidates-approved.json mv /tmp/candidates-approved.json /tmp/candidates.json if [ -n "${FORCED_ISSUE:-}" ] && [ "$(jq length /tmp/candidates.json)" -eq 0 ]; then echo "should_run=false" >> "$GITHUB_OUTPUT" diff --git a/docs/ci.md b/docs/ci.md index 7a38a329..d584513c 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -112,8 +112,8 @@ that human is reading, not whether they read it. opened or reopened**. It reads the new issue, finds the open issue it most resembles, and — only when a second, independent session fails to refute the claim — closes the new one as a duplicate with a comment naming the survivor. Anything short of that is commented and reported, never closed. -An issue it leaves open gets one comment tagging the owners in `.github/owners`, asking for the -`maintainer` label. +An issue it leaves open, with no skip label, gets one comment tagging the owners in +`.github/owners`, asking for the `maintainer` label. It exists because the dedupe already in the app cannot do this, and was never trying to. `src/github/issue.ts` refuses to file an `Agent update proposal:` whose title exactly matches an diff --git a/test/owner-gate.test.ts b/test/owner-gate.test.ts index 27c3bf77..9ec43a71 100644 --- a/test/owner-gate.test.ts +++ b/test/owner-gate.test.ts @@ -78,14 +78,16 @@ test("triage tags the owners once on an open issue no owner has approved", () => assert.match(run("ask-owners.sh", { "issue.json": open }, {}, "# x\na\n \nb\n").posted ?? "", /^@a @b: /); }); -test("issue-to-pr keeps only approved issues, and triage asks after every outcome", () => { +test("issue-to-pr keeps only approved issues, and triage asks once triage succeeds", () => { const itp = parse(readFileSync(join(ROOT, ".github", "workflows", "issue-to-pr.yml"), "utf8")); const pick: string = itp.jobs.propose.steps.find((s: { id?: string }) => s.id === "triage").run; + const built = pick.lastIndexOf("> /tmp/candidates.json"); + const label = pick.indexOf('select(any(.labels[]; .name == "maintainer"))'); const gate = pick.indexOf(".github/scripts/owner-approved.sh"); - assert.ok(gate > pick.lastIndexOf("> /tmp/candidates.json"), "after both paths build the candidates"); - assert.ok(gate < pick.indexOf("COUNT=$(jq length /tmp/candidates.json)"), "before they are counted"); + const counted = pick.indexOf("COUNT=$(jq length /tmp/candidates.json)"); + assert.ok(built < label && label < gate, "the current label is checked first, after both paths build the candidates"); + assert.ok(gate < counted, "before they are counted"); assert.match(readFileSync(join(ROOT, ".github", "workflows", "issue-to-pr.yml"), "utf8"), /FORBIDDEN='[^']*\\\.github\/owners/); - assert.match(pick.slice(gate), /select\(any\(\.labels\[\]; \.name == "maintainer"\)\)/, "and the label is on it now"); const skip = pick.match(/SKIP_LABELS="([^"]*)"/)![1]!.split(" "); const asks = readFileSync(join(SCRIPTS, "ask-owners.sh"), "utf8"); for (const name of skip) assert.ok(asks.includes(`"${name}"`), `ask-owners skips ${name} too`);