docs: extract RELEASING.md, add approval tooling - #91
Merged
Merged
Conversation
A merged release PR does not ship on its own: release-tag.yml is gated on the protected `release` environment and sits in `waiting` until a maintainer approves it. Nothing times out and nothing sends a reminder, so a prepared release can sit indefinitely while Release PR's own guard refuses to prepare the next one. v0.1.2 waited an hour for exactly this. Finding that run in the Actions UI was the only step between a merged PR and a signed tag. `make release-approve` finds it, prints the tag it would mint and the commit that tag would point at, and asks before POSTing the approval; `make release-status` reports where a release stands without touching anything. The gate itself is unchanged. It is what keeps the CI release key from being usable by anyone who can merge a `release/*` PR, so this makes the approval easy to find and press rather than automatic. The branch name is parsed through the shared parse-version.sh for the same reason the workflows use it: it decides what gets tagged, so `release/v1.0.0; rm -rf /` has to be rejected rather than interpolated. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The release material was lines 70-153 of CONTRIBUTING, about 45% of a contributor-facing document given over to a process only maintainers can run. It moves out largely verbatim; every claim in it was checked against the workflows and against the v0.1.0-v0.1.2 run history before the move, and all of them held. Two changes while moving it. The approval becomes its own numbered step rather than a consequence explained in prose after the list, because that is the one step a human has to take and burying it is how v0.1.2 came to wait an hour. And the flow now names `make release-status` and `make release-approve`. CONTRIBUTING keeps a `#### Creating a release` stub pointing here, so the `#creating-a-release` anchor still resolves for any link not updated below, and the heading is added to a table of contents that had been missing it. README, AGENTS.md, CLAUDE.md and copilot-instructions.md all now point at RELEASING; the copilot one had been linking `CONTRIBUTING.md` from inside `.github/`, which never resolved, and described the flow as Release PR then Release Publish with Release Tag left out entirely. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The link was relative, so it resolved to a file in this repository that does not exist. The code of conduct is an account-level community health file in michen00/.github: GitHub surfaces it on the community profile, but a relative link from here still 404s, so point at where it lives. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #91 +/- ##
==========================================
+ Coverage 51.57% 53.85% +2.27%
==========================================
Files 31 33 +2
Lines 2092 2427 +335
==========================================
+ Hits 1079 1307 +228
- Misses 1013 1120 +107
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
PR Summary by QodoDocument release flow and add approval/status commands
AI Description
Diagram
High-Level Assessment
Files changed (10)
|
RELEASING said both RELEASE_GPG_PRIVATE_KEY and RELEASE_GPG_PASSPHRASE
were required, and that Release Tag and Release Publish fail at the GPG
import step without both. `gh secret list` shows only the private key,
and v0.1.2 published forty minutes ago, so that was wrong in the
direction that costs the most: it invites someone to "fix" a missing
secret by setting it to an empty string.
The release key has no passphrase. sign-artifacts.sh already branches on
`[ -n "${GPG_PASSPHRASE-}" ]` and the import action accepts an empty
passphrase, so omitting the secret is the supported path, not an
oversight. Say which secret is actually required, say why the other is
absent, and keep the setup snippet's passphrase advice for anyone
generating a new key.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Qodo reviewed 7bc2f1c and raised seven findings. Four are real. The high one is a genuine bug. approve.sh printed the waiting run's headSha as the commit to be tagged, but release-tag.yml checks out `pull_request.merge_commit_sha`, and this repo squash-merges, so the two always differ. Verified against the release that just shipped: the run reported 3179e83 while v0.1.2 landed on 62c098a. A prompt whose whole job is showing what you are approving was naming a commit the tag never points at. It now resolves the merged PR for the release branch and shows that merge commit, refusing to approve when it cannot be resolved rather than falling back to the branch tip. The test could reach the real GitHub CLI. Setup was unchecked, so a failed mktemp or chmod would leave `gh` resolving to the caller's authenticated binary and the --yes cases would POST an approval against whatever release was actually waiting. Setup is now fail-fast and ends by asserting the stub is what `gh` resolves to. `gh release view` exits non-zero both when no release exists and when the call fails, so `|| echo none` reported an outage as an empty repository; `release list` separates them. `gh pr list` used the default 30-item page, so a release PR behind thirty open PRs read as absent. The approval path now exits 0 explicitly. Three findings are declined, all style claims contradicted by the repo's own conventions: the bash shebang, BASH_SOURCE, `local` and `[[ ]]` match every other file in tests/; PASSED/FAILED match tests/test-parse-version.sh; and that same file ends on a bare `[ "$FAILED" -eq 0 ]`. CLAUDE.md's POSIX rule covers scripts/, which shellcheck --shell=sh enforces separately. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Owner
Author
|
/agentic_review |
Owner
Author
|
[Review-Convergence] Round 1: active
Qodo raised 7 findings on 7bc2f1c. 4 fixed in 4d8edb7 — including a real one: the approval prompt showed the run's Quorum for this PR is 1 clean Qodo pass, per operator instruction. |
|
Code review by qodo was updated up to the latest commit 4d8edb7 |
Qodo is right, and this is a claim I introduced one commit ago while
fixing a different wrong claim about the same secret.
RELEASING said omitting RELEASE_GPG_PASSPHRASE "is what the branch
tests for". It is not. Both workflows pass the secret through
unconditionally, an absent secret expands to an empty string, and
sign-artifacts.sh branches on `[ -n "${GPG_PASSPHRASE-}" ]` — empty
versus nonempty, never absent versus present. An empty secret and no
secret sign identically.
The advice to omit it still holds, but the reason had to change: not
that signing can tell the difference, but that a secret which exists
and means nothing invites someone to later fill it with a passphrase
the key does not have. The setup snippet now guards the `gh secret
set` on a non-empty passphrase instead of telling the reader to set
it conditionally while unconditionally setting it.
Also captured the exit status in the status-mode test that asserted
on output alone, so a regression that prints the right text and then
fails cannot pass.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Owner
Author
|
/agentic_review |
Owner
Author
|
[Review-Convergence] Round 2: active
Qodo's fresh pass on 4d8edb7 found 2 more, both real and both fixed in 3a1cc5c: a status-mode test that asserted on output while ignoring the exit status, and a false claim in RELEASING.md about how the passphrase secret is detected. The second was text I added last round while fixing a different wrong claim about the same secret — an absent secret and an empty one sign identically, so "omitting it is what the branch tests for" was not true. |
|
Code review by qodo was updated up to the latest commit 3a1cc5c |
Owner
Author
|
[Review-Convergence] Round 3: converged-merge-blocked
Bot-review convergence is complete. Qodo confirms it reviewed up to 3a1cc5c and reports zero findings, which meets the quorum of 1 clean Qodo pass set for this PR. Remaining blocker is human, not bot: Three rounds, nine Qodo findings: six fixed, three declined as style claims the repo's own conventions contradict. The one worth a second look before merging is the GPG passphrase paragraph in RELEASING.md — it was corrected twice, in two different ways, and is the only part of this PR whose accuracy rests on my reading rather than on a test. |
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
2 participants
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.
Why
Merging #83 prepared v0.1.2, and then nothing happened for an hour. Nothing was broken —
release-tag.ymlwas sitting inwaitingon the protectedreleaseenvironment, which is exactly what CONTRIBUTING described. The step was documented; it just wasn't findable, and there was no reminder, no timeout, and no command to run.What
RELEASING.md— the release material was lines 70–153 of CONTRIBUTING, about 45% of a contributor-facing doc given over to a maintainer-only process. Every substantive claim was verified against the workflows and the v0.1.0–v0.1.2 run history before the move; the ones that did not survive that check are listed under Doc accuracy fixes. Two changes while moving:make release-statusandmake release-approve.make release-approve/make release-status—release-approvefinds the waiting run, prints the tag it would mint and the commit that tag will point at, and asks before POSTing.release-statusreports where a release stands and changes nothing.The approval gate itself is unchanged. It's what keeps the CI release key from being usable by anyone who can merge a
release/*PR, so this makes the click easy to find and press rather than removing it.Doc accuracy fixes
RELEASE_GPG_PRIVATE_KEYandRELEASE_GPG_PASSPHRASEwere required and that both workflows fail without both.gh secret listshows only the private key, and v0.1.2 published anyway. The first correction then overstated the mechanism — it claimed signing distinguishes an omitted secret from an empty one. It does not:release-publish.ymlpasses the secret through unconditionally, an absent secret expands to an empty string, andsign-artifacts.shbranches on[ -n "${GPG_PASSPHRASE-}" ]. Absent and empty sign identically. The guidance to omit it survives with an accurate reason, and the setup snippet now guards itsgh secret setinstead of setting the secret unconditionally while telling the reader not to.Creating a releaseandOne-time release key setup— the two sections everything deep-links to.CODE_OF_CONDUCT.mdlink was relative, so it 404'd from this repo; the file is an account-level health file inmichen00/.github..github/copilot-instructions.mdlinkedCONTRIBUTING.mdfrom inside.github/, which never resolved, and described the flow as Release PR → Release Publish with Release Tag left out.CONTRIBUTING keeps a
#### Creating a releasestub, so the#creating-a-releaseanchor still resolves for any link not updated here.Review-round fixes
Qodo raised nine findings across three passes; six were fixed here and three declined as style claims the repo's own conventions contradict (per-finding replies are on the threads).
approve.shdisplayed the workflow run'sheadSha, butrelease-tag.ymlchecks outpull_request.merge_commit_sha, and this repo squash-merges, so the two always differ — v0.1.2's run reported 3179e83 while the tag landed on 62c098a. A confirmation whose only job is showing what you are approving was naming a commit the tag would never point at. It now resolves the merged PR's merge commit and refuses to approve when it cannot.mktemporchmodleftghresolving to the caller's authenticated binary, so the--yescases could have POSTed a real approval against a genuinely waiting release. Setup is now fail-fast and asserts the stub resolves first.gh release viewexits non-zero both when no release exists and when the call fails, so a lookup error printed as "no releases";gh release listseparates the two. The open-PR lookup used the default 30-item page, which could hide a pending release PR.Verification
make test— all suites pass, including a newapprove.shsuite driven by a stubbedgh(no network) covering status mode, the refusal paths, branch-name validation, and a regression asserting the summary names the merge commit rather than the branch tip.shellcheck --shell=shacrossscripts— clean.make run-pre-commit— clean.make release-statussmoke-tested against this repo: reports v0.1.2, no open release PR, nothing awaiting approval.A
docs:squash subject doesn't clear the worthiness gate, so this won't propose its own release — it'll ride along in the next one.🤖 Generated with Claude Code
Closes #92 (review-convergence bulletin)