Skip to content

fix(bump-dev-version): warn when auto-merge can't complete without WORKFLOW_TOKEN - #433

Merged
d-morrison merged 3 commits into
mainfrom
fix/bump-dev-version-auto-merge-token-warning
Aug 7, 2026
Merged

fix(bump-dev-version): warn when auto-merge can't complete without WORKFLOW_TOKEN#433
d-morrison merged 3 commits into
mainfrom
fix/bump-dev-version-auto-merge-token-warning

Conversation

@d-morrison

@d-morrison d-morrison commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Closes #409.

Problem

bump-dev-version's auto-merge default cannot complete in a repo with required status checks. The bump PR is authored by the integrated GITHUB_TOKEN, and GitHub does not trigger pull_request workflow runs for a GITHUB_TOKEN-authored PR (it suppresses them to prevent recursion). So the required checks never report, and native auto-merge waits forever — the PR sits open on every merge until a human intervenes. The failure is silent: nothing at run time distinguishes it from success (the fail-fast shape).

WORKFLOW_TOKEN already fixes this when set (it authors the PR as a real user, whose pull_request runs are not suppressed), but the docs framed it as needed only "to push to a protected branch," so a consumer reasonably omits it and gets a permanently stuck PR.

Changes

  1. Fail-fast warning (.github/workflows/bump-dev-version.yml): a new first step emits a ::warning:: (plus a step-summary note) when auto-merge is on and WORKFLOW_TOKEN is unset, so the misconfiguration is visible up front instead of as a PR that never merges.
  2. Docs (examples/bump-dev-version.yml, website/reference/bump-dev-version.qmd, and the auto-merge input description): state that WORKFLOW_TOKEN is effectively required when the repo has required status checks and auto-merge is on, and explain why.

Warn, not hard-fail

The auto-merge + no-WORKFLOW_TOKEN combination is only unreachable when the repo has required checks. A repo without them auto-merges fine on GITHUB_TOKEN, so a hard failure would break that legitimate config. Detecting "has required checks" would need an API call and extra permissions; a loud warning covers the case the issue names without that complexity or that false-positive risk. The reasoning is recorded in a comment on the step.

Notes on verification

The warning step lives in the reusable workflow, which _selftest.yml's dev-version job can't exercise (it calls the bump-dev-version composite directly; the reusable workflow's own @v2 refs don't resolve until the tag slides — the bootstrapping gap in CLAUDE.md). Validated by actionlint (which runs shellcheck on the run: block, exit 0) and by reading; the warning logic is a two-line shell conditional matching the existing "Enable auto-merge" step's env pattern.

@d-morrison
d-morrison marked this pull request as ready for review August 7, 2026 01:41
@d-morrison
d-morrison requested a lite review from Copilot August 7, 2026 01:41
@github-actions
github-actions Bot removed the request for review from Copilot August 7, 2026 01:41
Comment thread .github/workflows/bump-dev-version.yml
Comment thread examples/bump-dev-version.yml Outdated
Comment thread website/reference/bump-dev-version.qmd Outdated
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

…rge:false is not an escape); sweep README + permissions
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.

2026-08-07 02:44 UTC

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Claude finished reviewView run


All three prior inline findings and the README gap were addressed by commit b1c57e75 ("review round 1: fix WORKFLOW_TOKEN doc accuracy"). I verified each fix directly against the current file contents and did a full-repo grep sweep to confirm no other stale sites remain.

Code review: #433

Verification of round-1 fixes (against my prior review, posted here, verdict "Needs more work"):

  1. .github/workflows/bump-dev-version.yml:92-118 — the warning message no longer suggests auto-merge: false as a working fallback. It now correctly states "Required checks block manual merges too, so turning off auto-merge does not help," and directs the user to set WORKFLOW_TOKEN. ✅ Fixed.
  2. examples/bump-dev-version.yml:24-38 — the comment now documents both reasons for WORKFLOW_TOKEN additively (protected-branch push and required-checks/auto-merge), explicitly credits the sibling open-sync-pr stubs for the first reason, and drops the misleading auto-merge: false remedy in favor of "Omit the token only when neither applies." ✅ Fixed.
  3. website/reference/bump-dev-version.qmd:34-61 — same additive two-reasons treatment, now lists the genuinely-working remedies (WORKFLOW_TOKEN, bypass-rights force-merge, or re-triggering checks via a real-user event). Also updated the auto-merge input row and WORKFLOW_TOKEN secret row to point at each other. ✅ Fixed.
  4. README.md:150-157 (raised in the summary, not inline, since it wasn't in the prior diff) — the stale "only to push to a protected branch" claim is now replaced with the additive two-reasons phrasing. website/permissions.qmd was also swept with a matching update and a working hyperlink to the reference page. ✅ Fixed.

Full-repo grep sweep (per CLAUDE.md's re-grep guidance): confirmed the only remaining "needed only to push to a protected branch" occurrences are in bump-submodule.qmd, sync-upstream.qmd, sync-shared-fragments.qmd, and claude.qmd — all of which are still accurate as-is, since those workflows don't have bump-dev-version's native-auto-merge/required-checks failure mode. No other stale site found.

New content check: I read the full diff of the round-1 fix commit and the resulting files in their current state. The new prose is factually consistent with GitHub's documented behavior (required status checks block manual merge button too, absent bypass rights), internally consistent across all four touched files, and doesn't introduce new inaccuracies. One trivial, non-blocking nit: README.md:156 says "see the reference page" without a hyperlink, while the parallel website/permissions.qmd:62-63 addition does link [reference page](reference/bump-dev-version.qmd). Not flagging this as blocking — README.md has no established convention of linking to reference pages elsewhere in that section, so this isn't a regression, just a minor missed opportunity for consistency with the permissions.qmd sibling edit made in the same commit.

No new bugs, syntax errors, or CLAUDE.md violations found in the round-1 fix commit.

Verdict

Ready for merge — all three inline findings and the summary finding from the prior review round have been correctly and accurately addressed; no new issues found.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

💰 Cost: $1.5027 (review) — run

@d-morrison
d-morrison merged commit 59ece54 into main Aug 7, 2026
32 checks passed
@d-morrison
d-morrison deleted the fix/bump-dev-version-auto-merge-token-warning branch August 7, 2026 02:43
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.

bump-dev-version: auto-merge can never complete without WORKFLOW_TOKEN when the repo has required checks

1 participant