ci: fix pr-queue-hygiene mergeable check to use per-PR API fetch - #240
Open
mrbobbytables wants to merge 1 commit into
Open
mrbobbytables wants to merge 1 commit into
mrbobbytables wants to merge 1 commit into
Conversation
Fixes #217: scripts/pr-queue-hygiene.mjs re-fetches each PR's mergeability individually via GET /repos/{owner}/{repo}/pulls/{number} instead of relying on the pulls-list endpoint's 'mergeable' field, which GitHub computes asynchronously and frequently returns as UNKNOWN on list responses -- silently letting conflicting PRs evade the 48h hygiene check. Also fixes two related bugs caught in review of the underlying, still-open PR #115 (which operationalizes this GOVERNANCE.md rule but could not land the workflow file itself due to workflow-scope credential limits on its fork): - Paginates through every open PR instead of relying on 'gh pr list' default --limit 30, which silently drops PRs past one page. - Persists a first-conflict-observed marker comment on each PR and measures the 48h window from that timestamp, since 'updatedAt' reflects any PR update (commit, label, comment), not how long the PR has actually been conflicting. Adds a pr-queue-hygiene npm script and tests, and documents the operational status in GOVERNANCE.md. The .github/workflows/ wiring (.github/workflows/pr-queue-hygiene.yml calling this script on a schedule) is included in the PR description for a maintainer to apply, since this environment's push credentials lack the GitHub 'workflow' OAuth scope required to push changes under .github/workflows/. Signed-off-by: mrbobbytables <mrbobbytables@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #217: the mergeability check for the GOVERNANCE.md merge-queue
hygiene rule (issue #58) needs to re-fetch each PR individually rather
than trust
gh pr list'smergeablefield, which GitHub computesasynchronously and frequently returns
UNKNOWNon list responses —silently letting conflicting PRs evade the 48h flagging check.
This PR adds:
scripts/pr-queue-hygiene.mjs— fetches all open PRs (paginated pastgh pr list's default--limit 30), then re-checks mergeabilityper-PR via
GET /repos/{owner}/{repo}/pulls/{number}(which forces afresh computation) before deciding a PR is conflicting. Persists a
first-conflict-observed marker comment so the 48h window is measured
from when the PR actually started conflicting, not from
updatedAt(which changes on any commit/label/comment).
tests/pr-queue-hygiene.test.mjs— unit tests for the markerparsing, age calculation, and conflict-detection logic.
pr-queue-hygienenpm script for manual/local runs.Context: related, still-open PR #115
PR #115 proposed this same feature but shipped it as inline bash in
.github/workflows/pr-queue-hygiene.ymlusinggh pr list'smergeablefield directly — the exact bug #217 reports. A laterrevision to #115 extracted the logic into a script
(
scripts/pr-queue-hygiene.mjs) that fixes it correctly, but theworkflow file itself was never updated to call that script, so the
bug remained live in the step that actually runs. The PR's author
noted its fork's credentials lack the
workflowOAuth scope needed topush the one-line wiring change to
.github/workflows/*.This PR is in the same boat: this environment's push credentials
also lack the
workflowscope, so.github/workflows/pr-queue-hygiene.ymlitself could not be pushed here. A maintainer with that scope will
need to add it directly — the full contents are below (also mirrors
what #115 needed):
Once this workflow file is added (either by a maintainer here, or by
rebasing/merging #115 with its workflow step updated to
run: node scripts/pr-queue-hygiene.mjs), #115 could likely be closed assuperseded, since this PR delivers the corrected script + tests it was
trying to add.
Testing
node --test— full suite passes (63 tests, including the 8 newones in
tests/pr-queue-hygiene.test.mjs).npx prettier --check— passes on all changed files.js-yaml.— hive: backend=copilot