Skip to content

feat(ci): the maintainer works only on issues an owner labelled maintainer - #512

Merged
bbertucc merged 3 commits into
mainfrom
worktree-owner-gate
Oct 4, 2026
Merged

bbertucc merged 3 commits into
mainfrom
worktree-owner-gate

Conversation

@bbertucc

@bbertucc bbertucc commented Oct 4, 2026 •

Copy link
Copy Markdown
Member

Iris Maintainer Agent here.

The maintainer now acts on an issue only after an owner approves it, following the pattern in UIC-OSF/software-factory-blocks.

  • Gate: .github/scripts/owner-approved.sh <n> passes only if the last maintainer label event on the issue was an add, made by a login in .github/owners. If the events can't be read, it fails. issue-to-pr.yml filters its candidates through this check and also requires the label to still be on the issue. A dispatch by number is checked the same way.
  • Ask: triage still processes every issue. Once triage succeeds, a new ask-owners job tags the owners once on any open issue that is not approved and has none of the skip labels.
  • Owners file: .github/owners lists bbertucc. It is on the auto-PR forbidden paths.
  • Label: I created the maintainer label.
  • Tests: test/owner-gate.test.ts runs both scripts against a fake gh. All 18 mutations were killed.

Limitation: the maintainer agent posts as bbertucc, so the gate can't tell my label from an owner's. The rule is that I never add maintainer myself. A separate bot account would close this gap.

🤖 Generated with Claude Code

…tainer`

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 <noreply@anthropic.com>

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All six checks pass. The gate itself is sound: owner-approved.sh fails closed on an unreadable
event list, tail -n 1 makes the last label event decisive, the filter runs after both candidate
paths build /tmp/candidates.json and before COUNT, GH_TOKEN is already on the Pick an issue
step so the subprocess inherits it, .github/owners is on FORBIDDEN, and the new ask-owners job
passes the issue number through env: and quotes it. I verified the
jq -r … | while read … | jq -R tonumber | jq -s . pipeline under set -euo pipefail with every
candidate rejected: it yields [] and the step continues rather than aborting. No blocking finding.

Non-blocking notes

1. ask-owners.sh skip list disagrees with issue-to-pr.yml's, so no-auto-pr gets tagged anyway.
.github/scripts/ask-owners.sh:16 skips maintainer, duplicate, wontfix, invalid.
issue-to-pr.yml:124 skips wontfix invalid duplicate question no-auto-pr. no-auto-pr is the
documented "I'd rather own this myself" opt-out (CONTRIBUTING.md, same hunk this PR edits), so an
issue carrying it still gets @bbertucc tagged and asked for the maintainer label — and if that
label is then added, SKIP_LABELS still drops the issue, silently. Reachable today on a reopened
event (a filer cannot label at opened time, a maintainer can before reopening). Harm is one noisy
comment plus an approval that does nothing; adding question and no-auto-pr to the select on
line 16 closes both halves.

2. owner-approved.sh's comment overstates what it checks.
.github/scripts/owner-approved.sh:4 — "put the maintainer label on the issue and it is still
there
". The script never reads the issue's current labels; it infers presence from the last
labeled/unlabeled event. Deleting the maintainer label from the repo's label set removes it
from every issue without emitting unlabeled events, and the gate would keep approving. Latent: it
needs someone to delete the repo label rather than remove it from the issue. /tmp/candidates.json
already carries each issue's current labels from gh issue list --json … labels, so the
confirming select is one line in the workflow.

3. .github/owners is a privilege grant that the review workflow's CI-security item does not cover.
The file decides which issues the maintainer agent writes code and opens PRs for. It is not in
code-review.yml:457's ALWAYS_INCLUDE_PATHS, and — the part that matters — not in the item-9
trigger list at code-review.yml:1040: "Skip this item entirely unless the diff touches
.github/workflows/**, .github/scripts/** or .github/actions/**". A future PR that only adds a
login to .github/owners therefore gets a review with the whole CI-security item explicitly
skipped. Context inclusion is fine (6 lines, under the byte floor); it is the trigger that misses
it. Branch protection and FORBIDDEN still hold, which is why this is a note.

4. always() means a failed triage still produces "ready for an owner to review", once and only once.
.github/workflows/issue-triage.yml:1023 — if: ${{ always() && inputs.dry_run != true }}. The
rationale comment above it covers triage's early exits; it also catches a triage job that failed
mid-run. The owner then gets a comment saying the issue is ready for their decision when no
duplicate analysis was ever posted, and the <!-- iris-ask-owners:v1 --> marker makes it one-shot,
so the issue is never asked about again. success() || failure() would not help — the distinction
you want is "triage reached a verdict", which is needs.triage.result != 'failure'.

5. An owners file with no entries fails the step instead of skipping it.
.github/scripts/ask-owners.sh:28 — under pipefail, if every line is a comment the second
grep -v exits 1 and the script dies before gh issue comment. Latent (the file has one entry);
reachable the day the last owner is removed, and it reads as a broken workflow rather than "no
owners to ask".

Accessibility impact: none — this changes only which issues CI acts on; no Iris source, agent prompt or delivered HTML is touched.

- 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 <noreply@anthropic.com>
@bbertucc

bbertucc commented Oct 4, 2026

Copy link
Copy Markdown
Member Author

Iris Maintainer Agent here.

Fixed all five round-1 notes:

  1. ask-owners.sh now skips every one of SKIP_LABELS, including question and no-auto-pr. A test fails if the two lists diverge.
  2. The workflow now also requires maintainer among the issue's current labels. The script comment now says it reads events only.
  3. Item 9 in code-review.yml now triggers on .github/owners.
  4. ask-owners runs only when needs.triage.result == 'success'.
  5. An owners file with no entries now prints a line and exits 0. This path is tested.

All six new lines pass mutation checks.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

This PR changes code-review.yml — its review runs on the fallback token

The OIDC exchange for a Claude App token refuses while this file differs from
the copy on the default branch, so the job hands the action GITHUB_TOKEN
instead. The review still runs; it posts as github-actions[bot] rather than
claude[bot]. Every other check (install, typecheck, unit, e2e, actionlint) runs
against this PR's head as usual.

Read the workflow diff yourself anyway. The reviewer has a CI-security
checklist for it, but it is reviewing the file that decides whether anything
gets reviewed. Check in particular whether this diff lets PR-authored code run
with secrets, widens permissions:, interpolates attacker-controlled
${{ github.event.* }} into a run: block, or weakens the step that
fails the job when no review was posted.

Run: https://github.com/EqualifyEverything/equalify-iris/actions/runs/37223616669

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All six checks pass. Every note from my review of ed05e2a is addressed:

  1. ask-owners.sh now skips question and no-auto-pr as well — fixed, and the test at test/owner-gate.test.ts:769 pins the two lists together so they cannot drift again.
  2. The owner-approved.sh comment no longer claims to check current labels, and issue-to-pr.yml:378 confirms the label is still on the issue — fixed on both halves.
  3. .github/owners is in code-review.yml's item-9 trigger list — fixed.
  4. always() is now needs.triage.result == 'success' — fixed; a failed triage no longer spends the one-shot ask.
  5. The || true on .github/scripts/ask-owners.sh:28 turns an empty owners file into a skip instead of a step failure — fixed.

I re-checked the one genuinely new piece of logic, the gate block at issue-to-pr.yml:373-387. Both paths that write /tmp/candidates.json derive it from /tmp/open-issues.json, which carries labels (line 199), so any(.labels[]; ...) cannot hit a null; an unlabelled issue yields false rather than an error. The exit 1 on the forced path matches the two forced-path exits above it (lines 329, 338), and nothing has been mutated by line 386, so the always() verify step at line 899 is correctly skipped via should_run=false. No blocking finding.

Non-blocking notes

1. The gate pays for an event list on every candidate, including the ones the label filter is about to drop. .github/workflows/issue-to-pr.yml:375:

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(any(.labels[]; .name == "maintainer")) | select(.number as $n | $ok[0] | index($n))]' \

The maintainer-label select is free — candidates.json already holds each issue's labels — but it runs after the loop. So on the scheduled path every candidate gets a gh api --paginate repos/.../issues/N/events call, and gh issue list --state open --limit 100 (line 198) means that is up to 100 paginated calls per run against the 1,000/hour GITHUB_TOKEN budget, when in the steady state exactly one issue carries the label. Hoisting the select(any(.labels[]; .name == "maintainer")) above the loop gives the same result for one jq. Not blocking: nothing fails today, and the gate's verdict is identical either way.

2. test/owner-gate.test.ts:761 is named for the behaviour the same test now asserts against. The title is "issue-to-pr keeps only approved issues, and triage asks after every outcome", and line 776 asserts /needs\.triage\.result == 'success'/ — which is precisely not every outcome, and is the fix for note 4. A reader hitting this failure later reads the title as the contract.

3. docs/ci.md:115 overstates which issues get the comment. "An issue it leaves open gets one comment tagging the owners in .github/owners, asking for the maintainer label." .github/scripts/ask-owners.sh:16 posts nothing on an issue labelled maintainer, wontfix, invalid, duplicate, question or no-auto-pr, so a triaged-and-left-open question gets no comment. "An issue it leaves open and no skip label covers" would match the script.

Accessibility impact: none — this changes only which issues CI acts on; no Iris source, agent prompt or delivered HTML is touched.

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 <noreply@anthropic.com>
@bbertucc

bbertucc commented Oct 4, 2026

Copy link
Copy Markdown
Member Author

Iris Maintainer Agent here.

Fixed all three round-2 notes:

  1. The maintainer label filter now runs before the loop, so only labelled issues get an events call. The test pins the order: candidates built, label filter, owner check, count. All three mutations were killed.
  2. The test is renamed to "…triage asks once triage succeeds".
  3. docs/ci.md now says the comment goes to an issue left open "with no skip label".

One full npm test run locally had 1 failure out of 1,779, and I didn't capture which test. Three reruns passed, and owner-gate.test.ts passed on its own every time.

🤖 Generated with Claude Code

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All six checks pass. Every note from my review of 384b689 is fixed, and the final commit
introduced no new defect I can find.

  1. Events call per candidate — fixed. .github/workflows/issue-to-pr.yml:376 now hoists
    select(any(.labels[]; .name == "maintainer")) into /tmp/labelled.json before the loop, so
    the scheduled path makes one issues/N/events call per labelled issue instead of up to 100.
    The verdict is unchanged: both the label filter and the gate still read candidates built by
    either path, and test/owner-gate.test.ts:802 now pins the new order
    (built < label && label < gate).
  2. Test title vs assertion — fixed: test/owner-gate.test.ts:795 reads "triage asks once
    triage succeeds".
  3. docs/ci.md overstating who gets the comment — fixed: "An issue it leaves open, with no
    skip label, gets one comment…".

Re-checked the rewritten gate block for regressions from the hoist:
/tmp/labelled.json holds the same objects as candidates.json (just filtered), everything
downstream reads candidates.json after the mv (lines 390, 413, 476, 513, 560), so the model's
candidate list and the /tmp/candidate-nums.txt validation at line 476 both see only approved
issues. With no labelled candidate, jq -r … | while … | jq -R tonumber | jq -s . still yields
[] under set -euo pipefail and the step continues; $ok[0] | index($n) on [] is null, so an
empty approval list filters everything rather than erroring. The forced-path exit 1 at line 387
is still before any mutation, matching the exits at 329 and 338.

No blocking finding, and no new non-blocking note.

Accessibility impact: none — this changes only which issues CI acts on; no Iris source, agent
prompt or delivered HTML is touched.

@bbertucc
bbertucc merged commit f60e1d7 into main Oct 4, 2026
7 checks passed
@bbertucc
bbertucc deleted the worktree-owner-gate branch October 4, 2026 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant