Skip to content

fix(ci): exempt release-please PRs from no-mistakes gate - #2616

Closed
kunchenguid wants to merge 2 commits into
mainfrom
fm/nm-required-exempt-release-please-r1
Closed

kunchenguid wants to merge 2 commits into
mainfrom
fm/nm-required-exempt-release-please-r1

Conversation

@kunchenguid

Copy link
Copy Markdown
Owner

Intent

Exempt release-please's own release PRs from firstmate's required no-mistakes enforcement check (.github/workflows/no-mistakes-required.yml).

Problem: the workflow requires every PR to main to carry the no-mistakes pipeline signature in its body, exempting only github-actions[bot] and dependabot[bot]. release-please now authors its release PRs under the kunchenguid identity via a personal access token, not the exempt bot, so automated release PRs FAIL the required check and need a manual owner override on every release. This was reproduced first, as the brief required: kunchenguid/sshhip PR #293 ('chore(main): release sshhip 1.22.0', head branch release-please--branches--main--components--sshhip, user kunchenguid) has check run 'PR must be raised via no-mistakes' with conclusion failure; running the pre-fix step script locally with that PR's environment also exits 1.

CRITICAL DESIGN CONSTRAINT stated by the requester: do NOT exempt the kunchenguid human identity broadly, because that would also exempt the captain's own hand-authored PRs and defeat the gate. The release PRs must be identified by a reliable release-please-specific signal determined from how release-please actually opens these PRs.

Deliberate decisions made while doing the work:

  • Exempt only when THREE release-please facts hold together: (1) head branch under release-please's reserved prefix (release-please--* for the current manifest layout, plus release-please/* for the older layout), (2) the head repository equals the base repository, so a fork can never claim it, and (3) the generated 'This PR was generated with Release Please' footer is present in the PR body. Requiring all three means a hand-written human PR cannot claim the exemption with a borrowed branch name or a copied body, since only a writer to the repository can push that branch here.
  • Deliberately did NOT use the release-please label (autorelease: pending): release-please labels the PR after creating it, so an 'opened' event can race the label and make the gate flaky. Author login is deliberately not used at all for release PRs.
  • Deliberately moved the existing github-actions[bot] / dependabot[bot] exemptions out of the job-level 'if' expression into the same step shell script, so the whole exemption decision is one executable contract in one place rather than split between a GitHub expression and a script. Consequence: exempt bot PRs now report a successful check instead of a skipped one, which is equivalent or better for a required check.
  • The workflow is a shared template copied verbatim into consumer repositories (SSHHIP carries a byte-verbatim copy), so the decision deliberately stays inline in the workflow and must NOT be factored out into a bin/ helper script that consumers would also have to copy, and that PR-head content could tamper with.

Test: tests/fm-no-mistakes-required-workflow.test.sh loads the workflow through a real YAML parser (ruby/psych, the same approach tests/fm-test-run.test.sh already uses for ci.yml) and EXECUTES the extracted step script with the environment GitHub supplies, asserting observable exit status and output - it never asserts workflow source text. Cases: a real release-please release PR (using the exact body of sshhip#293) is exempt; the older release-please branch layout is exempt; both bot authors stay exempt; a human PR without the pipeline signature still FAILS; a release-please-shaped branch name alone without the generated body FAILS; a fork copying a release-please body FAILS. It also asserts, through the parsed semantic model, that the job carries no job-level 'if', so the decision cannot silently move back out of the executable step and escape the test. The new test is classified into the pure-contract-unit family in bin/fm-test-run.sh; it was deliberately NOT added to the proven-isolated set, which requires a new concurrent isolation proof archive.

Acceptance criteria from the requester: a release-please release PR passes without an owner override; a non-exempt human PR to main without the no-mistakes signature still fails; the failure was reproduced first; a test asserts both the exemption and the still-fails-for-humans behavior; the change stays adoptable by consumer repositories and the PR notes that consumers carrying a verbatim copy (SSHHIP) must re-sync this workflow.

What Changed

  • Exempt same-repository release-please PRs only when their reserved branch prefix and generated footer both match, while unsigned human and fork PRs still fail.
  • Move existing GitHub Actions and Dependabot exemptions into the executable gate script so exempt automation reports success.
  • Add parser-backed contract coverage and documentation for the exemption; consumers with verbatim workflow copies, including SSHHIP, must re-sync the workflow.

Risk Assessment

✅ Low: The change is narrowly scoped and the three-factor release-please exemption preserves failure behavior for ordinary unsigned human and fork PRs while retaining existing bot exemptions.

Testing

Inspected the workflow change, ran its focused executable contract test directly and through the classified test runner, then captured an end-user-style CI transcript proving the pre-fix failure, successful release-please exemption, and continued rejection of unsigned human and fork PRs; all checks behaved as intended and the worktree remained clean.

Evidence: Required-check behavior before and after the release-please exemption

Source: Required-check behavior before and after the release-please exemption

No-mistakes required-check behavior transcript
Scripts were parsed from workflow YAML with Ruby Psych and executed with pull-request environment variables.

=== PRE-FIX: real release-please-shaped PR is rejected ===
exit_status=1
::error::This PR was not raised through no-mistakes.

Contributions to this repository must be submitted via 'git push no-mistakes'.
That pipeline runs the required review/test/lint/CI steps and writes a
deterministic '## Pipeline' section into the PR body containing:

    Updates from [git push no-mistakes](https://github.com/kunchenguid/no-mistakes)

See CONTRIBUTING.md for setup and the full workflow.

PR author: kunchenguid

=== FIXED: same release-please-shaped PR is exempt ===
exit_status=0
PR #293 is a release-please release PR (release-please--branches--main--components--sshhip); release automation is exempt.

=== FIXED: ordinary human PR remains rejected ===
exit_status=1
::error::This PR was not raised through no-mistakes.

Contributions to this repository must be submitted via 'git push no-mistakes'.
That pipeline runs the required review/test/lint/CI steps and writes a
deterministic '## Pipeline' section into the PR body containing:

    Updates from [git push no-mistakes](https://github.com/kunchenguid/no-mistakes)

See CONTRIBUTING.md for setup and the full workflow.

PR author: kunchenguid

=== FIXED: fork cannot claim release exemption ===
exit_status=1
::error::This PR was not raised through no-mistakes.

Contributions to this repository must be submitted via 'git push no-mistakes'.
That pipeline runs the required review/test/lint/CI steps and writes a
deterministic '## Pipeline' section into the PR body containing:

    Updates from [git push no-mistakes](https://github.com/kunchenguid/no-mistakes)

See CONTRIBUTING.md for setup and the full workflow.

PR author: someone

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

✅ **Review** - passed

✅ No issues found.

✅ **Test** - passed

✅ No issues found.

  • Inspected git diff 03bb1d8b78a8632ae2d9cea4c10868eb100e885e..607f5d52051b2f65a3d87a9a7472daf44fd3d2a4.
  • bash tests/fm-no-mistakes-required-workflow.test.sh
  • bin/fm-test-run.sh tests/fm-no-mistakes-required-workflow.test.sh
  • Parsed the pre-fix and fixed workflow scripts with Ruby Psych and executed them using release-please, ordinary-human, and fork PR environments; confirmed pre-fix rejection, fixed exemption, and retained rejection safeguards.
  • git status --short confirmed testing left the worktree clean.
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

release-please opens its release PRs with a personal access token, so the
author login is the repository owner rather than github-actions[bot]. The
required "PR must be raised via no-mistakes" check therefore failed on every
release PR (kunchenguid/sshhip#293 is a real one) and each release needed a
manual owner override.

Identify those PRs by what release-please itself produces rather than by
author: a branch under its reserved release-please prefix, pushed to this
repository rather than a fork, whose body carries the generated Release Please
footer. All three must hold, so a hand-written human PR cannot claim the
exemption with a borrowed branch name or a copied body, and a human PR without
the pipeline signature still fails.

Move the bot exemptions out of the job-level `if` into the same step script so
the whole decision is one executable contract, and add a test that loads the
workflow through a YAML parser and runs that script over the exempt and
non-exempt cases.
@kunchenguid

Copy link
Copy Markdown
Owner Author

Closing unmerged: misscoped. The firstmate repo does not use release-please and has no release-please PRs; firstmate is NOT a template for SSHHIP release-please/no-mistakes-required. The release-please exemption belongs in SSHHIP directly, where release-please actually runs. Re-scoping the fix to SSHHIP.

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.

1 participant