Skip to content

ci: add weekly stale PR reminder - #283

Open
lisazacarias wants to merge 1 commit into
slaclab:mainfrom
lisazacarias:ci/stale-pr-reminder
Open

ci: add weekly stale PR reminder#283
lisazacarias wants to merge 1 commit into
slaclab:mainfrom
lisazacarias:ci/stale-pr-reminder

Conversation

@lisazacarias

Copy link
Copy Markdown
Collaborator

What this changes

Adds a scheduled workflow that lists open PRs with no approving review older
than five days and posts them to Slack on Monday mornings. Advisory only: it
never comments on a PR, requests a review, or closes anything. Drafts and
CHANGES_REQUESTED are excluded because neither is waiting on a reviewer.

Safe to merge before the webhook exists — without SLACK_STALE_PR_WEBHOOK
the job prints the list to the run log and exits clean.

Operator-visible

None.

Scope

Single-purpose: one workflow plus the script it calls. 288 added lines, no
existing file touched, nothing to split.

Decisions worth recording

Filtering logic in .github/scripts/stale_prs.py, not a heredoc in the
workflow.
The first draft had ~70 lines of Python embedded in the YAML. CI
runs flake8 . and black --check . from the repo root with no .github
exclusion, so a script gets linted where embedded YAML Python does not, and
it can be imported and unit-tested. This mirrors operator_notes.py, which
already splits the same way: real logic in a script, the small Slack post
inline. Cost is a checkout step the workflow otherwise would not need.

The review filter excludes rather than includes. It skips APPROVED and
CHANGES_REQUESTED instead of testing for a specific "unreviewed" value. This
matters because reviewDecision is not one value: against main (protected,
required_approving_review_count = 1) unreviewed PRs return
REVIEW_REQUIRED, but a PR opened against an unprotected base returns "",
and the field can be absent. Excluding the two known-not-waiting states keeps
all three cases reported, and keeps working if GitHub adds a fourth state.

Kept the $GITHUB_OUTPUT heredoc guard even though it cannot currently
fire.
Everything interpolated into the note — PR titles, author logins, team
names — is single-line, so no line can equal the delimiter. Unlike
operator_notes.py, where PR bodies make it load-bearing, here it is purely
defensive. Retained because it costs nothing and stops being inert the moment
anyone renders a PR body into this note.

continue-on-error: true on the Slack step matches release.yml:135. A
failed nudge should not turn the repo's Actions tab red. The tradeoff is that
a broken webhook fails quietly; the find step does not share this, so an
API or permissions failure still fails the job loudly.

Learning reviewer

None specifically. CODEOWNERS will auto-request both reviewers, but there is
no machine behavior here — no PV writes, no hardware assumptions, nothing that
changes what the linac does. Reviewing the reviewDecision reasoning above is
the only part that needs a second opinion.

Testing

Verified with a harness that parses this workflow, extracts each step's run:
block, executes it under bash with gh stubbed to captured API output, and
parses $GITHUB_OUTPUT using the runner's own delimiter rules — so the Slack
step receives exactly what it would receive in CI.

Covered: real API output from this repo (5 open PRs → 2 correctly listed);
the nothing-stale path; reviewRequests containing both User (login) and
Team (name/slug) entries, plus slug-only and unknown-__typename
fallbacks; reviewDecision as REVIEW_REQUIRED, "", and absent; a PR title
containing STALE_EOF, $(id), backticks, quotes and ; rm -rf / (all
inert); the day-count boundary; the query-limit truncation warning; and a
simulated 403 confirming a permissions failure fails the step visibly.

black --check ., flake8 . and actionlint are all clean repo-wide.

Not verified, and worth knowing before this is trusted: whether
GITHUB_TOKEN with pull-requests: read is sufficient for
--json reviewDecision,reviewRequests. workflow_dispatch requires the file
on the default branch, so no pre-merge run is possible. The reasoning that it
should suffice is that these are pull-request resources and GITHUB_TOKEN
cannot be granted org-level scopes at all. If it is wrong, gh exits
non-zero, the step fails red, and the run log shows GitHub's own message —
nothing silently under-reports. Recommend dispatching once by hand after merge
before relying on the Monday cron.

  • pytest passes — 2905 passed, 3 skipped in 114s. The three skips are
    pre-existing and unrelated (test_tune_cavity, test_backend_cavity,
    test_fault).
  • Coverage still clears 80% — 85% total, unchanged by this PR: addopts
    scopes coverage to --cov=sc_linac_physics, and this adds nothing to
    that package.
  • Checked against simulation — n/a, no EPICS or PyDM code involved.

Lists open PRs with no approving review that are older than five days and
posts them to Slack on Monday mornings. Advisory only: it never comments on
a PR, requests a review, or closes anything. Drafts and CHANGES_REQUESTED
are excluded because neither is blocked on a reviewer.

The filtering logic lives in .github/scripts/stale_prs.py rather than a
heredoc in the workflow, matching operator_notes.py. CI runs `flake8 .` and
`black --check .` from the repo root, so a script is linted where embedded
YAML Python is not.

Safe to merge before SLACK_STALE_PR_WEBHOOK exists: without the secret the
job prints the list to the run log and exits clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

🟢 PR size: 294 lines — within target

+294 / -0 in production code.
0 test and asset files excluded.

Largest files Change
.github/scripts/stale_prs.py +206/-0
.github/workflows/stale-prs.yml +88/-0

Target is 400 lines excluding tests, hard stop around 800 — see
"Scope and shipping" in CLAUDE.md. If this PR is over, the description
should say why it could not be split. Advisory only; this check never
fails the build.

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.

2 participants