ci: route pr-quality and auto-merge-deps through shared netresearch/.github reusables#605
Merged
Conversation
Replace the hand-rolled auto-approve job with a single `uses:` of
netresearch/.github/.github/workflows/pr-quality.yml@main. The workflow now
has zero step-level `uses:`.
`auto-approve-require-write: true` is required: the reusable's default
authorization test is `author_association`, which is not a permission
(COLLABORATOR is returned for read/triage collaborators too). The opt-in
replaces that pre-filter with the same
`GET /repos/{repo}/collaborators/{author}/permission` lookup this repo used
before and approves only on `admin`/`write`.
`ready_for_review` is added to the trigger types to compensate for the
reusable's `draft == false` guard — otherwise a PR opened as a draft would
stay unapproved until its next push.
Top-level permissions are `{}`; the calling job grants exactly the union of
the reusable's job permissions (contents: read, pull-requests: write).
Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
Dependency ReviewThe following issues were found:
License Issues.github/workflows/pr-quality.yml
OpenSSF Scorecard
Scanned Files
|
…ed reusable The release-please arm (author github-actions[bot] + label 'autorelease: pending') is dead code: no release-please PR has ever been opened in this repo, and releases are cut manually from a signed tag via release.when-tagged.yml. Its approve step was additionally a deliberate no-op, and the comment pointing at an APPROVE_TOKEN path in pr-quality.yml described a capability that no longer exists. With that arm removed the job does nothing the shared reusable does not already do, so the workflow becomes a caller of netresearch/.github/.github/workflows/auto-merge-deps.yml@main. merge-strategy is pinned to "rebase": the reusable auto-detects squash-first, but this repo has allow_squash_merge=false / allow_merge_commit=true / allow_rebase_merge=true, so the detector would pick --merge whereas the inline job ran `gh pr merge --auto --rebase`. The caller job grants contents: write + pull-requests: write, matching the reusable's job-level permissions exactly (a smaller set would startup_failure). Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
…se-please arm Corrects the previous commit message, which claimed no release-please PR had ever been opened here. Measured with `gh pr list --state all --limit 300` filtered on head branch: exactly one genuine release-please PR exists (#466, 2026-02-16, merged), and `gh run list` shows auto-merge-deps.yml did run on that branch. The other two "release-please" head-branch matches are Renovate PRs bumping the release-please action. The approval half of the removed arm was still a no-op by construction, and #466 was approved manually. Nothing has come from release-please in the five months since. The removal decision is unchanged; only the justification is now stated accurately, in the file itself. Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
CybotTM
changed the title
|
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
1 participant
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.
Routes two workflows through shared reusables in
netresearch/.github:.github/workflows/pr-quality.ymlpr-quality.yml@main.github/workflows/auto-merge-deps.ymlauto-merge-deps.yml@mainBoth callers now have zero step-level
uses:— each has a single job that is auses:of its reusable..github/workflows/AGENTS.mdis updated for the changedauto-merge-deps.ymlscope.release.when-tagged.ymlandrelease-please.ymlare deliberately untouched.1.
pr-quality.ymlHow the original blocker was proved resolved
This migration was refused once before: this repo authorizes auto-approval on the author's repository permission (
admin/write), while the hub reusable's default test isauthor_association— which is not a permission (COLLABORATORis returned for read/triage collaborators as well). The hub has since gainedauto-approve-require-write. Proof it actually delivers the property, traced from requirement to API call:ifends in( inputs.auto-approve-require-write || contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.pull_request.author_association) ). With the inputtruethe||short-circuits andauthor_associationis never evaluated.gh api "repos/$REPO/collaborators/$PR_AUTHOR/permission" --jq '.permission' 2>/dev/null || echo "none"— byte-identical to the removed line 44 of this repo's old workflow, including the fail-closed|| echo "none".inputs.auto-approve-require-write == false || contains(fromJSON('["admin","write"]'), steps.check-permission.outputs.permission). With the inputtruethe left operand isfalse, soadmin/writeis mandatory — equivalent to the removed line 48.grep -n "createReview\|pr review\|APPROVE"over the reusable returns only the onecreateReviewblock at lines 205-211. Nothing else can widen the gate downstream.CybotTM, same-repo branches) carryAPPROVEDreviews fromgithub-actions[bot], whilegh run listshows Auto-merge bot PRs = skipped on both of those branches and PR Quality Gates = success. Only the oldpr-quality.ymlcould have produced those approvals — soGET /collaborators/{author}/permissionalready resolves toadmin/writeunderpull_request_targetwithGITHUB_TOKENand this repo'spull-requests: writegrant. The migrated job grants a superset (contents: read+pull-requests: write).Before / after behaviour
admin/writerepo permissiongh pr review --approve)pulls.createReview, same lookup)read/triage/no permissionnone)none, fails closed)author_associationaloneauto-approve-require-write: true)dependabot[bot]/renovate[bot]auto-merge-deps.ymlauto-merge-deps.ymlstill approves themgithub-actions[bot](release-please)synchronize(needed:dismiss_stale_reviews: true)auto-approvecarriesneeds: quality-gatedraft == false)ready_for_reviewhead.repo.full_name == github.repository)quality-gatejob){controls}placeholder so no dead linkpermissions:pull-requests: write{}+ job-level unionubuntu-24.04ubuntu-latest(reusable)ci / …only)Approval is now coupled to the quality gate — stated plainly
The reusable's
auto-approvejob declaresneeds: quality-gate. Consequences, which an earlier revision of this description glossed as merely "additive":quality-gatefails (e.g.pulls.listFileserrors, or the checkout step fails),auto-approveis skipped and the PR is not auto-approved. That failure mode did not exist before.quality-gateis itself guarded bydraft == false, so on a draft PR both jobs are skipped — theready_for_reviewtrigger below is what recovers from that.core.warning()for a large PR does not fail the job, so an oversized PR still gets approved; only a genuine job failure blocks approval.This is accepted, not refuted: the shared standard runs the size check ahead of approval in every consuming repo, and this repo now gets the same ordering.
Why dropping the bot branches loses nothing
auto-merge-deps.ymlalso approvesdependabot[bot]andrenovate[bot]— before this PR in its own inlineAuto-approve PRstep, after this PR in the reusable'sApprove PRstep, whose jobifadmits exactly those two logins.gh pr view <n> --json reviewsshows PRs chore(deps): update actions/checkout digest to 3d3c42e #602, chore(deps): bump actions/setup-node from 6.4.0 to 7.0.0 in the github-actions group #597 and chore(deps): bump step-security/harden-runner from 2.19.4 to 2.20.0 in the github-actions group #587 each carrying 2APPROVEDreviews fromgithub-actions[bot], and chore(deps): update github/codeql-action action to v4.37.1 #601 carrying 4 — one per workflow, per run. Removing one of two duplicates leaves the approval intact.Auto-approve (bot via APPROVE_TOKEN)step was already a no-op:APPROVE_TOKENexists neither as a repo secret (gh secret list→ empty) nor among the org secrets visible to this repo, so it fell back toGITHUB_TOKEN, which cannot approve its own bot's PRs, and the failure was swallowed by|| true. Empirical confirmation: release-please PR chore(main): release 4.1.0 #466 (github-actions[bot], labelautorelease: pending) was approved manually byCybotTM, not by the workflow.Trigger change
ready_for_reviewis added totypes. The reusable guards both jobs withdraft == false; marking a PR ready fires neitheropenednorsynchronize, so without this a draft PR would stay unapproved until its next push — a regression the migration itself would introduce. The spelling is validated byactionlint, which rejects an unknown activity type forpull_request_targetand listsready_for_reviewamong the accepted ones (negative control below). All pre-existing types (opened,synchronize,reopened) are preserved, as are the workflownameand the (absent)concurrency.actions/checkoutunderpull_request_targetThe old file carried a comment forbidding
actions/checkout; the reusable'squality-gatejob contains one. The invariant that comment protects — untrusted PR code executing with a write-scoped token — is preserved:actions/github-scriptwith an inline script; nothing from the working tree is executed, so the checked-out ref is not a code-execution vector either way.actions/checkoutis called withoutref, so it resolvesgithub.ref, which forpull_request_targetis the base branch, not the PR head.persist-credentials: false, so no token is left in.git/config.2.
auto-merge-deps.ymlThe release-please arm is removed
The old job's
ifhad three arms; the third matchedgithub-actions[bot]carrying the labelautorelease: pending, i.e. a release-please PR. Its scope, measured rather than assumed (gh pr list --state all --limit 300, filtered on head branch):release-please--branches--main--components--node-magento-eqp, 2026-02-16, merged). The two other "release-please" head-branch matches are Renovate PRs bumping the release-please action (chore(deps): update googleapis/release-please-action action to v4.4.1 #529, chore(deps): update googleapis/release-please-action action to v5 #540), which thedependabot/renovatearms handle anyway.release-please.ymlkept runningsuccess(most recently 2026-07-20). Releases here are cut by pushing a signedv*tag, handled byrelease.when-tagged.yml.googleapis/release-please-actionmatches only this repo and its sibling, out of 191 — nothing else in the org uses it.The arm's approval half was a no-op by construction:
Auto-approve PRwas explicitly skipped forgithub-actions[bot], and the comment deferred approval to "pr-quality.ymlusingAPPROVE_TOKEN" — a path that does not exist (no such secret, see above) and that thepr-quality.ymlmigration in this same PR removes outright, so that comment pointed at a capability that is gone. On #466 the approval came manually fromCybotTM.Its merge half did fire, once:
gh run listshowsauto-merge-deps.ymlrunning on that branch on 2026-02-16 (threesuccess, one earlierfailure). So the honest statement is not "this code never ran" but "this code has run for one PR in the repo's history, five months ago, on a release path the repo no longer uses".Removing it is a deliberate owner decision, not an oversight, and it is not reimplemented in the shared workflow — netresearch/.github#261 ("RECOMMEND CLOSE: allow-release-please arm is unreachable") was closed for that reason. If release-please is ever revived here,
--autoon its PRs comes back as an explicit change rather than as an unused branch nobody remembers.release-please.ymlitself is out of scope for this PR and is untouched.With that arm gone the remaining logic — approve
dependabot[bot]/renovate[bot], thengh pr merge --auto— is exactly what the shared reusable does, so the file becomes a caller.Merge strategy is pinned, not auto-detected
The reusable auto-detects squash-first:
gh api repos/netresearch/node-magento-eqpreturns{"merge":true,"squash":false,"rebase":true}, so the detector would resolve to--merge, whereas the inline job rangh pr merge --auto --rebase. The caller therefore passesmerge-strategy: rebase, which the reusable turns intoSTRATEGY="--rebase"before the detector branch is reached.Before / after behaviour
dependabot[bot]/renovate[bot]ifadmits exactly those two logins)github-actions[bot]+autorelease: pendingif, approval deliberately skipped, thengh pr merge --auto --rebase--rebase(hardcoded)--rebase(merge-strategy: rebase)ifmatchedsafe-updates-onlyleft at its defaultfalse)dependabot/fetch-metadatastepsteps.metadatawas never referencedsafe-updates-only, i.e. not herepull_request_target:withtypes: [opened, synchronize, reopened]pull_request_target:— and those three types are exactly GitHub's default activity setpermissions:contents: write,pull-requests: write{}+ job-levelcontents: write+pull-requests: writeubuntu-24.04, no timeoutubuntu-latest,timeout-minutes: 5(reusable)nameAuto-merge bot PRsAuto-merge dependency PRs(matches the org templates; it no longer handles non-dependency bots)ci / …contextsactions/checkoutunderpull_request_targetPermissions
The reusable's job declares
contents: write+pull-requests: write. A caller granting less startup-fails before any job runs, so the caller job grants exactly that set.Verification
Run in this branch's working tree, and nothing beyond this is claimed:
actionlint .github/workflows/auto-merge-deps.yml .github/workflows/pr-quality.yml→ exit 0.actionlinton a copy withready_for_reviewmisspelled → exit 1,invalid activity type … available types are … "ready_for_review" ….yq '[.jobs[].steps[]?.uses]' .github/workflows/auto-merge-deps.yml→[](no step-leveluses);yq '.on, .permissions, .jobs'confirms the single job, itspermissions, andwith: {merge-strategy: rebase}.gh api repos/netresearch/node-magento-eqp --jq '{merge:.allow_merge_commit,squash:.allow_squash_merge,rebase:.allow_rebase_merge}'→{"merge":true,"squash":false,"rebase":true}— the basis for pinning the strategy.gh api repos/netresearch/node-magento-eqp/branches/main/protection→ required contexts areci / TS Check / Lint,ci / TS Check / Check types,ci / Test / test (node 24),ci / Build / build;required_approving_review_count: 1,dismiss_stale_reviews: true. No job renamed or removed here is a required context.gh pr view 602|601|597|587 --json reviews→ 2, 4, 2, 2APPROVEDreviews fromgithub-actions[bot](the duplicate-approval claim above).gh pr list --state all --limit 300 --json number,headRefNamefiltered on head branch → three matches, of which one (chore(main): release 4.1.0 #466) is a genuine release-please branch and two (chore(deps): update googleapis/release-please-action action to v4.4.1 #529, chore(deps): update googleapis/release-please-action action to v5 #540) are Renovate action bumps;gh run list --workflow=auto-merge-deps.ymlfiltered the same way → four runs, all on chore(main): release 4.1.0 #466's branch, 2026-02-16.netresearch/.github@main(84499c7), plus this repo'srelease-please.ymlandbranches/main/protection.Correction carried forward: the first commit message on the
auto-merge-deps.ymlchange asserted that no release-please PR had ever been opened here. That is wrong — #466 exists, and the arm's merge half ran on it. The follow-up commit7da85b4states the accurate scope in the file itself; the removal decision is unchanged.Not verified: neither migrated file can be exercised by this PR.
pull_request_targetalways runs the workflow definition from the base branch, so the runs on this PR execute the old files frommain. The new definitions first run after merge. The remoteuses:references and their inputs were therefore checked by readingnetresearch/.github@maindirectly —auto-approve-require-write(boolean),auto-approve-message(string) andmerge-strategy(string) all exist with those names and types — not by execution.