* review-checklist: ping GitHub assignee(s) once the checklist is fully signed off
Adds computeAssigneePing(), a pure function that posts a ready-to-merge
comment mentioning the PR's assignees the moment every CODEOWNERS area
transitions from pending to signed-off. Gated on the false->true
transition so it fires once, not on every later workflow run while the
PR stays fully approved, and posted as its own comment since editing
an existing comment to add a mention isn't a reliable notification.
* review-checklist: sync a checklist-complete label with the all-done state
Toggles the "checklist-complete" label (pre-created in the repo, green
#0E8A16) on and off to track whether every CODEOWNERS area is currently
signed off, so it's visible as a badge on the /pulls list without
opening each PR. Unlike the assignee ping, this is level-triggered: the
label comes off again if a later change request breaks the sign-off,
and reattaches once the PR is fully approved again.
* Fix release-progress badges to read native issue-level Priority field
The Project Priority board field was consolidated into a Priority field
managed at the issue level (GitHub's org-wide custom issue fields), which
the GraphQL API mirrors into projects as ProjectV2ItemIssueFieldValue
instead of ProjectV2ItemFieldSingleSelectValue. The badge script's query
had no fragment for that type, so it silently found zero priority values
and would raise ProjectFieldMissingError on every run.
* Show TBD on the Next Release badge when no milestone due date is set
Previously the badge silently omitted any due-date annotation when the
milestone had none, which read as if the badge just hadn't picked one up.
* Update release-schedule image and plantuml source from add-next-release-badge branch
* Don't flag CODEOWNERS auto-assignment as a manual reviewer add on a PR's first pass
GitHub's own CODEOWNERS engine fires an identical review_requested webhook
(sender type User, not Bot) when it auto-assigns owners at PR-open time. If
one of those survives the cancel-in-progress race as the run that actually
executes, it was mistaken for a human deliberately requesting that reviewer,
permanently flagging them "manually added" and requiring separate approval —
e.g. a catch-all "*" owner also named on a touched area's CODEOWNERS line got
flagged on every PR touching that area, even though no human ever picked them.
Gate the manual-add detection and the sticky per-area assignment on
!isFirstCoordinationPass (no checklist comment posted yet), since that's
exactly the window where GitHub's own auto-assignment is indistinguishable
from a real human pick.
Checkout deliberately uses persist-credentials: false so the workflow
token never lands in git config, but the add-and-commit step never got
a token of its own to push with -- it silently relied on credentials
checkout no longer leaves behind. Every push to any branch failed with
"could not read Username for 'https://github.com'" once clang-format
found something to fix, regardless of the repo's Actions write-permission
setting.
The hdf5_vol_rest_fetchcontent job fails at "Install HSDS dependencies"
because HSDS pins numpy==2.4.6, which publishes no distributions for
Python 3.10:
ERROR: Could not find a version that satisfies the requirement
numpy==2.4.6 (from versions: ... 2.2.5, 2.2.6)
numpy 2.4.6 declares requires_python >= 3.11, and HSDS's pyproject.toml
now says requires-python = ">=3.11". Move the matrix to Python 3.12,
which is what HSDS uses for its own unix-socket test job - the same
configuration this job exercises.
Also drop the "Fix requests version" step that force-downgraded to
requests==2.31.0. HSDS now pins requests-unixsocket==0.4.1, which fixed
the requests 2.32 breakage that step worked around, so the downgrade only
fights HSDS's own pins. Upstream HSDS has already commented the same step
out of its socket-test job.
Fixes#6657
Claude-Session: https://claude.ai/code/session_011qUw9C11arB1WftiT56EA4
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* review-checklist: don't lose a reviewer who only left comments
GitHub un-requests a reviewer the instant they submit any review,
including a comment-only one from batching several inline comments into
a single submission — not just Approve/Request-changes. Unlike a stale
approval, this never produces a DISMISSED transition, so the checklist
had no way to tell "abandoned the area" apart from "still reviewing,
just hasn't finished yet."
This silently dropped the reviewer's mention from the checklist display
the moment they left comments, and risked the next push's additive-fill
picker handing their area to a completely different load-balanced
reviewer (observed on PR #6645: jhendersonHDF's batched review comments
repeatedly vanished him from src/test's rows mid-review).
Track a sticky area assignee as still engaged whenever they have no
APPROVED/CHANGES_REQUESTED/DISMISSED review on record, regardless of
whether GitHub currently lists them as requested, and use that in the
read-only display path, resolveAreaPicks, and the additive-fill picker.
* draft-pr-policy: a checked keepalive checkbox is real activity too
lastRealActivityAt() deliberately ignores bot comments so a metadata-only
bump can't dodge the staleness check forever. But checking the "Still
working on this" checkbox is an edit to the bot's own keepalive comment,
not a new comment of the human's own — its author stays
github-actions[bot], so the edit was invisible to the activity check too.
Confirming via the checkbox removed the label and posted "Thanks for
confirming" without ever moving the underlying 60-day clock, so the very
next scheduled run saw the same stale last-activity timestamp and
immediately re-flagged it — contradicting the checkbox's own promise
that checking it "resets this".
Observed on #6326: once its true last activity fell behind the 60-day
window, and only the checkbox (never a new commit or comment) was used
to confirm it, the label thrashed on and off on a roughly 1-2 day loop.
Now a checked keepalive checkbox counts via the comment's updated_at
(when it was toggled), same as any other real activity signal.
The issue_comment trigger let every PR comment boot a full runner just
to make one API call checking draft status, even though the only
comments that matter are ones on PRs already labeled draft-stale. That
label snapshot is already present in the webhook payload, so gate the
job on it directly and drop the now-redundant draft-status API step.
Every CODEOWNERS "avalanche" (multiple owners of one area simultaneously
requested — on PR creation, on ready_for_review, or when a push first
touches an already-covered area) was resolved by re-running the
load-balancer from scratch, with no memory of who was already the
settled reviewer for that area. Since open-PR review load drifts over
time, this made the pick non-deterministic across repeated events and
caused three related symptoms:
- marking a draft ready for review could swap out reviewers who were
already assigned (manually or from an earlier pass) for different
people
- a routine follow-up push could re-avalanche an area and bump the
already-engaged reviewer for whoever currently has a lighter queue
- a manually re-requested reviewer survived the triggering event (the
existing forced-pick handling already covers that) but got silently
removed again on a later event, since two now-requested owners look
identical to an unpruned avalanche without that memory
Add a persisted per-area "sticky assignment" record (ASSIGNED_PREFIX,
alongside the existing exclusion and manually-added markers) and a
resolveAreaPicks() helper that prefers a valid sticky pick, then a lone
already-requested owner, before ever falling back to a fresh
load-balanced pick. Manual review_requested actions now write to this
record too, so a deliberate reviewer choice holds up across future
runs, not just the run it was made on.
Co-authored-by: H. Joe Lee <hyoklee@hdfgroup.org>
Address all zizmor 1.25.2 findings in the Maven/Java-themed workflows
with no behavioral change:
- template-injection: move attacker-controllable ${{ }} expressions
(matrix.*, runner.workspace, github.actor, github.base_ref,
steps.*.outputs.*, needs.*.outputs.*) out of run: script bodies into
step-level env: blocks referenced as shell/pwsh variables.
- artipacked: add persist-credentials: false to all actions/checkout
steps (none of these workflows push to git).
- excessive-permissions: add/tighten explicit permissions. Reduce
overly broad workflow-level packages:/pull-requests: grants to
contents: read, granting minimal packages: read/write only to the
specific jobs that need it.
- secrets-inherit (release.yml): remove secrets: inherit on the
test-maven-packages.yml call, which declares no workflow_call secrets.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Larry Knox <lrknox@hdfgroup.org>
* Harden workflow checkout credential hygiene + permissions
Address zizmor static-analysis findings:
- artipacked: add `persist-credentials: false` to actions/checkout
steps in codeql.yml (2), test-binary-installation.yml (2),
clang-format-check.yml, freebsd.yml, h5py.yml, openbsd.yml,
signed-plugins.yml, and update-progress.yml. None of these jobs
push to git, so dropping the persisted checkout credential is safe.
- excessive-permissions: restrict call-workflows.yml top-level
permissions to `contents: read`, and grant `packages: write` /
`pull-requests: write` only on the two jobs that call the
maven-staging reusable workflow (which declares those needs).
No behavioral change.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Removed extra blank lines.
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Larry Knox <lrknox@hdfgroup.org>
* Harden scheduled/downstream build workflows against zizmor findings
Fix all zizmor static-analysis findings in the scheduled/downstream build
workflows without changing behavior:
- template-injection: move ${{ }} expressions out of run: script bodies
into step-level env: blocks referenced as shell variables
- artipacked: add persist-credentials: false to actions/checkout steps
(none of these jobs push to git)
- excessive-permissions: add top-level 'permissions: contents: read' to
macos-26-matrix.yml (other files already restrict permissions)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Address Copilot review comments on PR #6544
- Drop unnecessary command substitution in daily-schedule.yml's
FILE_NAME_BASE assignment
- Name the two env-block steps in daily-build.yml that previously ran
unnamed (easier to scan in logs)
- Quote $GITHUB_OUTPUT in the getinputs step
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Remove extra blank lines.
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Larry Knox <lrknox@hdfgroup.org>
Address GitHub Actions static-analysis (zizmor/CodeQL) findings in the
S3 publishing workflows:
- publish-release.yml: hoist every inputs/secrets/vars value used inside
run blocks into job-level env vars and reference plain shell variables
($USE_TAG, $FILE_NAME, $TARGET_DIR, $S3_BUCKET, $TARGET_PATH, $DRY_RUN),
eliminating 41 template-injection findings. Add persist-credentials:
false to the checkout (artipacked).
- publish-branch.yml: add persist-credentials: false to the checkout
(artipacked); its S3 sync already used env vars.
No behavioral change: the same values are used, only via the shell
environment instead of direct ${{ }} expansion into the script body.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
GitHub-hosted runners have no real InfiniBand, and the Open MPI bundled
with NVIDIA HPC-X intermittently has UCX select a non-functional verbs
interface (e.g. ud_verbs/mana_0), causing:
ucp_worker.c:1487 UCX ERROR uct_iface_open(ud_verbs/mana_0:1) failed
pml_ucx.c:319 Error: Failed to create UCP worker
MPI_INIT has failed because at least one MPI process is unreachable
which fails h5_api_test_parallel_* tests in the "Run Tests" step. This
only affects nvhpc.yml (HPC-X's bundled Open MPI ships UCX); main-par.yml
uses Ubuntu's openmpi-bin package, which doesn't use UCX and is unaffected.
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* Add Fortran ABI compatibility check to abi-report workflow
Mirrors the existing C/HL/C++ abi-dumper + abi-compliance-checker
steps for libhdf5_fortran.so, plus a supplementary nm-based exported
symbol diff since gfortran's mangled names don't encode argument
lists and DWARF-based diffing has not been validated against
Fortran-specific constructs (array descriptors, derived types).
* Auto-detect ABI reference release instead of hardcoding it
file_ref was pinned to '2.0.0' in both daily-build.yml and release.yml
and had gone stale across two releases (2.1.0, 2.1.1) without being
bumped, silently comparing against a two-versions-old baseline. Have
abi-report.yml query the GitHub API for the latest published
HDFGroup/hdf5 release when file_ref isn't explicitly given, so callers
no longer need to remember to update a pinned tag after every release.
Verified the resolution command returns 2.1.1 against the live API.
Also documents the expected false-positive pattern in the new Fortran
ABI check: abi-dumper misreads gfortran's DWARF encoding of
assumed-shape array descriptors as fixed array bounds, producing bulk
Low-severity noise on the KIND/RANK-generated H5_gen.F90 procedures
that isn't a real interface change.
Generate the Doxygen tag file inside the html output directory
(hdf5lib_docs/html/hdf5.tag) instead of at the build root. The S3
documentation publishing (daily "latest", release zip, and
publish-release) all derive from the docs-doxygen artifact, which is
uploaded from the build-tree html directory. Placing the tag file there
makes it flow to every destination, e.g.
https://support.hdfgroup.org/documentation/hdf5/latest/hdf5.tag
Since the tag file now lives inside html/, the existing DIRECTORY install
already installs it to ${HDF5_INSTALL_DOC_DIR}/html/hdf5.tag, so the
redundant standalone install(FILES ...) is removed (it would otherwise
reference a file that no longer exists at the build root).
* Add Next Release badge and Medium/Low priority progress badges
Adds a "Next Release" badge (in-development version from H5public.h)
and extends the existing Critical/High priority progress badges with
Medium (P2) and Low (P3) priority tracking, all fed from the same
GitHub Project #39 gist-based badge pipeline.
* Remove in-progress 2.0 entry from release schedule diagram
The Release Schedule chart is meant to show only past releases that
have reached end of life; the in-development series is now tracked
separately by the Release Progress badges. Regenerate the PNG from
the updated PlantUML source and clarify the README wording.
* Add Latest Release badge and milestone target date to Next Release
Fetches the most recent published release in the current major
version series from the GitHub Releases API (e.g. "2.1.1
(2026-03-23)") and exposes it as a new "Latest Release" badge.
Also looks up the target due date of the matching GitHub milestone
(e.g. "HDF5 2.2.0") and appends it to the existing Next Release
badge when one is set. Both lookups are independent of the
project-board query and degrade gracefully to "N/A" on failure.
* Add change-request and manually-added-CODEOWNER lines to review checklist
Reviewers who request changes now get their own line under each area
their inline comments touch, whether or not they're a CODEOWNER or a
requested reviewer. CODEOWNERS who are review-requested directly by a
human (rather than auto-picked) get a separate required-approval line,
and their approval is now required for that area's sign-off in addition
to the usual reviewer's.
* Prune reviewers whose area dropped out of scope
A CODEOWNER requested for an area the PR used to touch, before a later
push narrowed the diff, was never removed: touchedAreaOwners only
reflects areas touched right now, so none of the existing avalanche
pruning ever considered them. Reproduced live on PR #6528, where the
branch's diff shrank to just .github/ but gheber/fortnern/mattjala
(owners of unrelated areas) stayed as requested reviewers.
Manually-added CODEOWNERS are exempt, since a human's deliberate
request shouldn't be silently undone by a later push.
* review-checklist: fix avalanche exemption regressing genuine avalanches
#6524 exempted a review_requested login's whole area from avalanche
detection to stop a manual re-request from being undone. But a genuine
CODEOWNERS avalanche's own review_requested sub-events look identical
to that case, and one of them can survive the ready_for_review vs.
review_requested concurrency race (#6530: draft PR marked ready for
review left all 3 CODEOWNERS owners requested instead of pruning to
one, because the surviving run's action was one of the avalanche's own
review_requested events, exempting the whole area).
Replace the exemption with a forced pick: the directly-requested login
still always survives, but now by replacing the rest of that area's
avalanche instead of exempting the area from pruning — so a genuine
avalanche still collapses to one person. Also gate justRequestedLogin
on !isBotSender, matching updatedManuallyAdded's existing reasoning
just above it: the bot's own requestReviewers calls fire this same
event and aren't a human decision.
A direct review_requested (e.g. manually re-requesting a reviewer via
the GitHub UI) adds a second currently-requested owner to that
reviewer's area — indistinguishable from an unpruned CODEOWNERS
avalanche, so the avalanche-detection pass immediately removed them
again on the same run. Carve out the login this run's review_requested
action names before running avalanche detection.
* ci: skip draft-pr-policy checkout on issue-only comments
The issue_comment event fires for comments on both issues and PRs;
draft-pr-policy only cares about PR draft-staleness, so short-circuit
before checkout when the comment is on a plain issue. Also switch all
three jobs to a sparse, shallow checkout since they only need
.github/scripts to load the github-script payload.
* ci: also skip draft-pr-policy on bot-authored PR comments
draft-pr-policy.js already excludes Bot comments when deciding whether
a draft was revived, so a bot comment (e.g. review-checklist posting
its checklist) can never do anything meaningful here — it still ran
checkout + API calls only to no-op. Filter it out at the same if:
using the free github.event.comment.user.type field.
* ci: skip checkout for draft-pr-policy on non-draft PR comments
Even after filtering out issue-only and bot comments, a human comment
on any non-draft PR still paid for a full checkout just to have the
script itself discover pr.draft is false and return. Add a cheap
pulls.get-only pre-check step (no checkout required) and gate the real
checkout + script load on its result.
These workflows use pull_request_target / workflow_run with elevated
permissions (pull-requests: write, issues: write). Because GitHub
copies workflow files into forks, any fork with Actions enabled was
independently running these privileged workflows against its own
PRs, e.g. https://github.com/sp26-hdfgroup/hdf5-sandbox/pull/2.
Add the same github.repository == 'HDFGroup/hdf5' guard already used
in review-checklist-test.yml so the job-level if short-circuits
before doing anything in a fork's copy of the workflow.
actions/checkout v7.0.0 (bumped in #6500) added a blanket check that
blocks checkout on pull_request_target/workflow_run events, even when
no fork ref is checked out. This workflow only ever checks out the
base branch (develop), so opt in via allow-unsafe-pr-checkout.
Co-authored-by: H. Joe Lee <hyoklee@hdfgroup.org>
- action's hash pin has mismatched or missing version comment
- credential persistence through GitHub Actions artifacts: does not set
persist-credentials: false
The HDF-EOS5 compatibility test downloads a source tarball from
git.earthdata.nasa.gov, an external host that has proven unreliable and
frequently times out, causing spurious PR-check failures unrelated to the
code under review.
Move the test off the pull_request/push triggers onto a daily schedule
(plus workflow_dispatch) so transient network failures no longer block PR
merges, while still catching genuine HDF-EOS5 regressions on a daily
cadence. Guard the job with an owner check so it does not run on forks.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Fix runExecute.cmake: restore ERROR_APPEND and fix zero-or-more mask patterns
Three bugs were introduced when runTest.cmake was refactored into runExecute.cmake:
1. ERROR_APPEND=1 support was dropped. Tests that redirect their error-stack
output to stderr and rely on ERROR_APPEND to combine it with stdout before
comparison (e.g. H5TEST-err_compat, H5TEST-error_test) fail silently because
the reference file never matches the truncated actual output.
2. " line [0-9]*" uses zero-or-more digits, so it matches " line " (with zero
digits) in already-masked reference text like " line (number)", prepending
another "(number)" and producing " line (number)(number)".
3. "HDF5 [1-9]*[.][0-9]*..." has the "HDF5 " prefix and uses [1-9]* (zero-or-
more), so it does not match "2.2.0" in actual output (blocked by the leading
"("), and double-masks "version (number)" in the reference.
"H5Eget_auto[1-2]*" / "H5Eset_auto[1-2]*" have the same zero-or-more issue,
double-masking "(1 or 2)" already present in reference files.
Fix: restore ERROR_APPEND handling; change * to + for all digit/char class
quantifiers that can match zero occurrences in already-masked reference text;
remove the "HDF5 " prefix from the version pattern. Same fix applied to
HDF5Examples/config/cmake/runExecute.cmake which has identical bugs.
* Fix AOCC CI: source setenv_AOCC.sh from its actual install location
install.sh generates setenv_AOCC.sh inside the aocc-compiler-VERSION/
directory (where it runs from), not in the repo root one level up.
* Fix AOCC CI: replace hardcoded runner paths with GITHUB_WORKSPACE
All /home/runner/work/hdf5/hdf5/ paths were hardcoded to the HDFGroup/hdf5
repo name. Running the workflow in any fork (e.g. brtnfld/hdf5_swmr_2)
puts the checkout under a different path, breaking AOCC/OpenMPI install,
configure, and build steps. Use $GITHUB_WORKSPACE / ${{ github.workspace }}
so the paths resolve correctly regardless of repo name.
* fix(aocc): source setenv_AOCC.sh from workspace root, not compiler subdir
install.sh places setenv_AOCC.sh in the workspace root, not inside the
aocc-compiler-X.X.X/ directory.
* Fix runExecute.cmake: anchor HDF5 version regex to full token
Replace the over-broad version pattern with one that matches the complete
'HDF5 (x.y.z)' token, preventing over-matching of IP addresses and other
version-like strings, and avoiding nested substitution artifacts in .err
reference comparisons. Applied identically to both copies of the file.
Three distinct but related bugs caused the review-checklist bot to
@ mention more reviewers than intended.
**Bug 1 — bot-sender guard (sticky exclusion)**
The bot's own removeRequestedReviewers API calls fire
review_request_removed events that self-trigger the workflow. Without a
guard, that self-triggered run treated its own bookkeeping removal as a
deliberate human decision and added the login to the persisted
exclusion set permanently. Fix: check sender.type === 'Bot' and skip
the exclusion update for bot-originated removals.
**Bug 2 — cancel-in-progress race on opened/ready_for_review**
GitHub fires one review_requested event per CODEOWNERS auto-assigned
owner; each triggers a workflow run. With cancel-in-progress, the
surviving run may be a review_requested rather than the opened event,
bypassing the avalanche-prune branch entirely and leaving all CODEOWNERS
requested. Fix: track hasExistingComment as a proxy for "first
coordination pass" — if no checklist comment exists yet, prune regardless
of which action survives the race.
**Bug 3 — per-area avalanche on synchronize (PR #6484)**
GitHub's CODEOWNERS engine re-fires when a commit first touches a new
CODEOWNERS-covered area, not only on PR open but also on synchronize.
The surviving run fell through to additive fill, saw the area as
"already has owners, skip", and listed all auto-assigned CODEOWNERS.
Fix: before the synchronize-swap and additive-fill paths, detect any
area whose owner-list ∩ existingRequested > 1 (per-area avalanche) and
prune that area to the single load-balanced pick.
84 tests (was 82).
* ci: replace actions/stale with bot-aware mark-stale script
actions/stale uses updatedAt to measure inactivity, so any bot event
(e.g. the /remove-reviewer acknowledgment comment) resets the stale
countdown even when there has been no meaningful human activity for
months. PR #6332 was last touched by a human on 2026-04-02 but was
not flagged because a reviewer-removal on 2026-06-11 refreshed the
timestamp.
Replace the actions/stale step with a custom mark-stale.js script
(same pattern as alert-stale.js) that only counts non-bot comments,
non-bot review submissions, and commits as meaningful activity. The
script also removes the stale label if such activity occurs after the
label was applied.
* ci: fix draft-stale keepalive for external contributors
Two bugs with the keepalive checkbox on draft-stale PRs:
1. External contributors (fork authors) lack write access to edit the
bot's comment, so clicking the checkbox silently fails for them.
Fix: also treat a new non-bot comment posted after the keepalive
comment as a sufficient keepalive signal.
2. The stale label could take up to 24 hours to be removed (daily cron
only). Fix: add an issue_comment.created trigger so draft-pr-policy
fires immediately when someone comments on a stale draft PR.
mark-stale and alert-stale are guarded to only run on
schedule/workflow_dispatch, not on every comment.
Also fix lastRealActivityAt to exclude bot comments (matching
mark-stale.js), so the keepalive and "Thanks for confirming" bot
comments don't count as real activity when measuring staleness.
* fix(review-checklist): re-request dismissed reviewer on fixup push
When a new commit dismisses a prior reviewer's approval, GitHub's
CODEOWNERS engine auto-assigns a fresh (possibly different) owner for
the changed area. The synchronize handler now detects dismissed
area-owners and swaps them back in, removing the fresh CODEOWNERS pick.
Adds planSynchronizeSwaps() pure helper (exported) with 7 unit tests
covering the PR 6475 scenario and edge cases.
* fix(review-checklist): don't remove a fresh pick still needed by another area
planSynchronizeSwaps could remove a fresh CODEOWNERS pick to restore a
dismissed reviewer even when that pick also covered a different,
unrelated touched area — removeRequestedReviewers strips them from the
whole PR, silently uncovering the other area. Now skips removal when
the candidate owns any other touched area.
Also dedupes the consuming loop so a login needed by two areas isn't
requested/removed twice.
5 new tests covering the cross-area guard and its boundaries.
* fix(review-checklist): don't let bot's own reviewer removal create a sticky exclusion
The bot's own removeUnselected/removeRequestedReviewers calls (draft-opened
CODEOWNERS cleanup, stale-exclusion enforcement) fire review_request_removed,
which the workflow also listens on — self-triggering another run. That run
previously read its own bookkeeping removal as a deliberate human decision
and added the login to the persisted exclusion set, permanently blocking
that owner from ever being auto-assigned to the PR again. Guard on
sender.type !== 'Bot' so only human-driven removals become sticky.
* fix(review-checklist): prune CODEOWNERS avalanche regardless of which event wins the race
GitHub's CODEOWNERS engine fires one review_requested event per auto-assigned
owner on PR creation, and each re-triggers this workflow. With concurrency:
cancel-in-progress, whichever run starts last wins — and that's just as
likely to be one of those review_requested runs as the opened run itself.
A surviving review_requested run fell through to the additive-fill branch,
saw every area already "covered" by the avalanche, and pruned nothing.
This hit PR #6479 itself: all 4 CODEOWNERS for .github/ stayed requested.
Branch on whether a checklist comment exists yet instead of which action
survived — that signal is race-resistant: no comment means this is the PR's
first coordination pass no matter which event got here. Threads
hasExistingComment through from run() (defaulting to true, i.e. additive-fill,
on a comment-fetch failure so an API hiccup can't be mistaken for a fresh PR).
2 new tests: the #6479 race itself, and a contrast case confirming a routine
review_requested on an already-established PR still uses additive-fill.
---------
Co-authored-by: H. Joe Lee <hyoklee@hdfgroup.org>
pr.updated_at is bumped by metadata-only changes (reviewer
requested/removed, labels, milestone, assignee), letting an
abandoned draft dodge the staleness check indefinitely. Use the
latest commit/comment/review/review-comment timestamp instead.
* CI: prune CODEOWNERS avalanche on non-draft PR open
GitHub auto-assigns every touched-area CODEOWNER when a non-draft PR
opens. The previous additive-only logic saw all areas already covered
and returned the full auto-assigned set as confirmedRequested, so the
checklist @-mentioned every owner before the final reviewer set was
known.
On opened/reopened (non-draft), now behave like the draft case: prune
to a single load-balanced pick per area via removeUnselected before
posting the checklist, requesting any pick not already on the PR.
* CI: guard reviewer-exclusion update against bot-triggered removals
The bot's own CODEOWNERS cleanup and draft-handling calls fire
review_request_removed events that self-trigger another workflow run.
Without this guard, that run would interpret the bot's own removal as a
deliberate human decision and permanently add the login to the exclusion
set, blocking that owner from ever being auto-assigned to the PR again.
Add PullRequest inline fragment to the project items GraphQL query
so PRs in the GitHub Project are counted alongside issues when
calculating release blocker/must-do progress.
review-checklist.js previously tried to enforce a single load-balanced
reviewer per area by actively stripping anyone else who showed up, using a
checklist-comment-existence heuristic (isOpeningRace) to decide when it was
safe to do so. Walking through real PR timelines surfaced several cases
where that heuristic and that stripping behavior did the wrong thing:
- An old PR's first encounter with the bot (or one whose checklist comment
was deleted) was misdiagnosed as a brand-new PR, forcibly reselecting a
human-curated reviewer list.
- A CODEOWNER for areas this PR doesn't touch (e.g. a project lead added by
hand for their judgment) was swept by the same logic as junk
auto-assignment, with no protection for an existing approval.
- A reviewer who isn't an owner of any touched area was invisible in the
checklist and untouched by any cleanup path either way.
- A PR that picked up reviewers while non-draft, then was converted to
draft, had those reviewers wiped out by the next push — converted_to_draft
isn't in this workflow's trigger list, so there's no event at the actual
transition to distinguish "noise from this PR's creation" from "a real
assignment from before it became a draft."
- A reviewer removed via the PR UI could be silently re-added by GitHub's
own CODEOWNERS engine on a later push, since there was no memory of the
removal across runs.
Redesign around one principle: the bot should never automatically strip a
reviewer who's already on the PR, manually added or auto-assigned, except
where the policy explicitly calls for it. Concretely:
- coordinateReviewers is now purely additive for non-draft PRs: chooseReviewers
runs against the *real* existingRequested, so it naturally skips any area
that already has someone and only fills gaps. This is idempotent and safe
on every event, eliminating the need for the old race-detection heuristic
entirely (isOpeningRace, the new-PR/synchronize distinction, and the
post-request 15s delayed re-clear are all removed as dead weight).
- Pruning — used only where the bot still clears reviewers — is scoped to
touchedAreaOwners (owners of areas this PR touches, plus catch-all "*"
owners) instead of the repo-wide allCodeOwners, so a CODEOWNER for
unrelated areas is never touched.
- Draft handling clears auto-assigned reviewers only at the literal
opened/reopened-as-draft moment — the one point where "CODEOWNERS noise
from this PR's creation" and "this PR's actual reviewer state" are
mechanically the same thing. Every other event while draft leaves existing
reviewers alone.
- An explicit review_request_removed persists that login to an exclusion
list embedded as a second hidden marker in the checklist comment (the only
durable storage available), and every run strips anyone in that list who
reappears — covering GitHub re-assigning them on a later push. A direct
review_requested for that exact login overrides the exclusion, since it's
the strongest available signal that someone individually decided that
person should be back right now.
- buildBody adds a catch-all "Additional reviewers" line for anyone
requested who isn't an owner of any touched area, so they're visible and
their approval is shown (informationally — it doesn't gate any area's
sign-off), and accepts any owner's approval for an area's sign-off rather
than only the specifically-assigned one's.
12 new tests cover the buildBody changes and the parseExcluded/serializeExcluded
round-trip; all existing pure-function tests are unchanged.
Co-authored-by: H. Joe Lee <hyoklee@hdfgroup.org>
* Add stale PR/issue policy with assignee alerts instead of auto-close
Ready PRs/issues use actions/stale to label inactivity (30/60 days).
Draft PRs get a longer 90-day window and only reset on an explicit
"still working on this" comment, since pushes/CI activity alone
shouldn't make an abandoned draft look fresh. Nothing is auto-closed:
once a staleness label has persisted past its alert threshold, a
custom script pings the assignee (falling back to requested
reviewers, then the author) to decide whether to keep it open or
close it.
* Scope stale policy to PRs only, not issues
Issue staleness is disabled (days-before-issue-stale: -1) and the
alert script now skips any non-PR item defensively, since this
workflow is meant to address PRs sitting unmerged/unreviewed, not
issue triage.
* Draft stale window 60 days (was 90); drop dead pull_request branch
alert-stale.js's pickAlertTargets is now only ever called with PR
items (filtered upstream in runAlertStale), so the pull_request
check around the requested-reviewers lookup was dead code.
* Set persist-credentials: false on checkout steps
Fixes two zizmor notes: these checkouts only need to read local
script files for github-script's require(), so there's no reason
to persist the GITHUB_TOKEN in git config afterward.
* Use a keep-alive checkbox instead of a magic comment phrase for drafts
Checking a box in the bot's own stale-notice comment is more
discoverable than requiring an exact phrase, and the live checkbox
state can be read straight off that comment's current body on each
run instead of scanning new comments for a regex match.
---------
Co-authored-by: H. Joe Lee <hyoklee@hdfgroup.org>
* CI: fix buildBody to accept any owner approval, not only the assigned reviewer
PR #6446 narrowed the approver lookup to effectiveReviewers (the confirmed-
requested subset), inadvertently breaking the case where a non-assigned owner
approves. Restore the original intent: any CODEOWNER's approval signs off an
area; fall back to effectiveReviewers only when no CODEOWNER is assigned.
* text updates
* CI: add tests for non-CODEOWNER reviewer sign-off path (#6446)
Three tests cover the nonOwnerReviewers fallback introduced in #6446:
- pending mention when a non-owner is manually assigned
- non-owner approval signs off an area with no CODEOWNER assigned
- non-owner approval does NOT sign off when a CODEOWNER was assigned
* CI: restore review_requested/review_request_removed triggers
These were dropped in #6453 when ready_for_review was added, breaking
automatic checklist updates on manual reviewer changes.
Both events fall through to the preserve-existing path in
coordinateReviewers when a checklist already exists, so the checklist
body is simply rebuilt with the current requested_reviewers set.
Also extend the opening-race detection (previously called isFirstSyncRace)
to cover review_requested: for any PR, CODEOWNERS auto-assignment fires
review_requested shortly after opened, and with cancel-in-progress: true
that run can cancel the opened run. If no checklist exists yet, treat the
review_requested event as a new-PR event and run full selection.
* CI: show only one reviewer per checklist area
Each area's pending mention now shows the primary (first) effective reviewer
rather than all confirmed-requested owners joined with commas.
* CI: revert to showing all confirmed reviewers per checklist area
A manually added person who is also a CODEOWNER for that area should be
shown alongside the load-balanced pick, not hidden. "One reviewer per area"
only describes chooseReviewers' default selection, not a display constraint.
* CI: skip reviewer removal when they have already submitted a review
GitHub's API rejects removeRequestedReviewers for users who have already
reviewed; detect that case upfront via listReviews and report it clearly
instead of surfacing a cryptic API error.
* CI: defer reviewer assignment for draft PRs until ready for review
Add ready_for_review to the pull_request_target trigger so the workflow
fires when a draft is promoted. In the script, skip requestReviewers
(and the CODEOWNERS auto-assignment cleanup second pass) while the PR is
a draft; still assign the PR author and strip any reviewers GitHub
auto-assigned. When the PR is marked ready, treat it like a fresh open
and run the full load-balanced reviewer selection.
* CI: fix synchronize race — treat first synchronize as new PR when opened was cancelled
When a fork PR is created, GitHub fires both opened and synchronize.
With cancel-in-progress: true the synchronize run often wins, and the
opened run (which clears CODEOWNERS auto-assignments) is cancelled before
it completes. Detect this: if synchronize fires but no checklist comment
exists yet, run the full new-PR path (enforceSelection + load-balanced
reviewer assignment) instead of silently carrying forward all auto-assigned
reviewers.
* CI: enforce reviewer selection before posting @mentions
On synchronize, run enforceSelection against the ideal load-balanced
pick before building the checklist body so the @mentions are never
sent until the reviewer list is actually correct. Re-fetch the PR
after cleanup so confirmedRequested reflects reality, not the pre-
cleanup snapshot.
For workflow_run (review submitted), reviewer assignment is intentionally
left unchanged but @mentions are filtered to the ideal selection so
CODEOWNERS extras don't generate spurious notifications on comment edits.
* CI: preserve manually added reviewers on synchronize
Reverting the enforceSelection call on the non-first-run synchronize
path. There is no API way to distinguish CODEOWNERS auto-assignments
from manually added reviewers, so enforcing the load-balanced selection
on every synchronize would silently remove intentional additions.
The initial cleanup (opened or first-synchronize via openedWasSkipped)
already produces a correct reviewer list; subsequent synchronize and
workflow_run events carry it forward unchanged.
* CI: pin actions/github-script to commit hash in remove-reviewer.yml
zizmor requires actions to be pinned to a commit hash rather than a tag.
Use the same pinned hash (ed597411d8f924073f98dfc5c65a23a2325f34cd, v8.0.0)
already used in review-checklist.yml.
* CI: refactor review-checklist.js for readability
Extract the reviewer coordination logic out of the monolithic run()
function into named module-level helpers:
checklistExists() — single responsibility: does a checklist
comment already exist on the PR?
removeUnselected() — remove CODEOWNERS auto-assignments not in
the load-balanced selection set
requestReviewers() — request each reviewer individually so one
bad login cannot block the rest
removeUnselectedAfterDelay() — 15-second wait + re-fetch + re-enforce
for GitHub's async auto-assignment race
coordinateReviewers() — top-level dispatcher; the four event paths
(read-only, synchronize-normal, new-PR/draft,
new-PR/non-draft) are now explicit branches
with labelled comments instead of nested ifs
run() is now a straight pipeline of 8 numbered steps with no nested
async functions. Behaviour is unchanged.
* CI: extract convertGlobToRegex and use github.paginate consistently
Extract glob-to-regex conversion into a standalone helper and replace
manual pagination loops for listFiles and listReviews with github.paginate,
matching the existing style used for listComments and pulls.list.
* CI: restrict remove-reviewer workflow to main repo only
Adds a github.repository guard so the job does not run in forks,
preventing unintended resource consumption and command execution
against fork PRs.
* CI: restrict test-maven-packages workflow to main repo only
Adds github.repository guards to all three jobs so the workflow
does not evaluate in forks, preventing spurious "workflow file issue"
failures on push events in forked repositories.
* Revert "CI: restrict test-maven-packages workflow to main repo only"
This reverts commit cc1208226d.
* CI: fix template injection in workflow_dispatch run: blocks
Move workflow_dispatch inputs that appeared directly in run: scripts
into step-level env: blocks, then reference them as plain shell
variables. This is the pattern zizmor recommends and eliminates the
injection vector.
Only workflow_dispatch inputs are fixed here — workflow_call inputs
in reusable files are not attacker-controlled (they come from the
calling workflow) so no changes are needed there.
Files changed:
- publish-branch.yml: local_dir, target_dir in aws s3 sync
- java-implementation-test.yml: java_versions, platforms, test_mode
- maven-staging.yml: test_maven_deployment, java_implementation,
platforms, use_snapshot_version
- maven-build-test.yml: test_deployment, java_implementation,
platforms, test_examples
- test-maven-packages.yml: version, repository_url throughout
- test-binary-installation.yml: maven_repository, maven_version,
install_method
* CI: move remaining GHA expressions from run: blocks to env: vars
* CI: exclude pure reusable workflows from zizmor template-injection scan
Pure reusable workflows (workflow_call only, no workflow_dispatch/pull_request/
push/schedule/release triggers) can never be triggered directly by external
users. Template-injection findings against inputs.* in those files are false
positives — inputs arrive from the trusted calling workflow, not from attackers.
Scanning only files with user-facing triggers keeps Security tab alerts
meaningful and prevents developers from dismissing real findings.
bmribler
Larry Knoxandlrknox
Scot Breitenfeld
H. Joe LeeandClaude Opus 5
dependabot[bot]
jhendersonHDF