heavy-duty/box

Fork 1

feat: merging a release-labeled PR is the release #97

Merged

dan-claude-bot merged 8 commits from feat/release-on-merge into main

2026-07-19 17:12:29 +00:00

dan-claude-bot commented

2026-07-19 15:19:54 +00:00

(Migrated from github.com)

Fixes #96.

What

The 0.7.0 ceremony lived the gap this closes: #95 merged with four approvals and nothing happened — correctly, because release.yml fired only on a tag push that was a separate, manual, silent-when-forgotten step. A forgotten tag produces no error and no red X, the worst failure shape. The ship decision already lives in the release PR — the one PR whose whole diff is "the version leaves -dev" — so merging it now IS the release. Tagging becomes transcription, and transcription is where machines don't err.

How

New trigger: pull_request: types: [closed] on main, and a release-on-merge job gated on merged == true AND the hand-set release label. The label is read from the event payload, so no pull-requests permission is needed; closed-unmerged never fires.
Four asserts, in order, each fail-loud and creating nothing:
1. VERSION at the merge commit is non--dev;
2. VERSION changed in this PR — merge commit vs its first parent (main the instant before the merge; the payload's base.sha can be stale, the first parent cannot). This is the -dev interlock: a mislabeled ordinary PR dies here loudly;
3. the version's CHANGELOG.md section extracts non-empty via the existing release-notes.sh, verbatim;
4. no tag and no release exist for the version yet (idempotent re-runs, and the guard against a manual tag racing the merge).
Then, in the SAME job: create the tag ref at the merge commit via gh api .../git/refs, and publish with gh release create --verify-tag and the extracted notes. Same-job on purpose: a GITHUB_TOKEN-created tag does not trigger workflows (GitHub's anti-recursion), so the tag door can never fire off this tag and double-publish — publishing here is the only chance. No assets, as before: the source tarball for the tag IS the package.
The tag-push door stays, step-for-step identical, as the documented manual fallback and backfill (it shipped 0.7.0 itself) — gated to github.event_name == 'push' so a closed PR never runs it against a branch ref. CONTRIBUTING.md's Releases section now says "the maintainer's merge IS the release", with the manual ritual kept as the fallback.

Tests

test/release.sh extends the daemon-free grep-pin section in the same fail-closed style: the merged+labeled gate (both halves), the surviving tag-push trigger, the push-only gate on the tag door, all four asserts, that both doors extract via the shared script and publish --verify-tag (counted, 2 each), that every failing assert "creates nothing" (counted, 5), and the same-job API tag at the merge commit.

bash test/release.sh — 63 passed, 0 failed
bash test/cli.sh — 411 passed, 0 failed
shopt -s globstar; shellcheck -x bin/* **/*.sh — rc=0

🤖 Generated with Claude Code

Fixes #96. ## What The 0.7.0 ceremony lived the gap this closes: #95 merged with four approvals and nothing happened — correctly, because `release.yml` fired only on a tag push that was a separate, manual, *silent-when-forgotten* step. A forgotten tag produces no error and no red X, the worst failure shape. The ship decision already lives in the release PR — the one PR whose whole diff is "the version leaves `-dev`" — so merging it now IS the release. Tagging becomes transcription, and transcription is where machines don't err. ## How - **New trigger**: `pull_request: types: [closed]` on `main`, and a `release-on-merge` job gated on `merged == true` AND the hand-set `release` label. The label is read from the event payload, so no `pull-requests` permission is needed; closed-unmerged never fires. - **Four asserts, in order, each fail-loud and creating nothing**: 1. `VERSION` at the merge commit is non-`-dev`; 2. `VERSION` **changed in this PR** — merge commit vs its first parent (main the instant before the merge; the payload's `base.sha` can be stale, the first parent cannot). This is the `-dev` interlock: a mislabeled ordinary PR dies here loudly; 3. the version's `CHANGELOG.md` section extracts non-empty via the existing `release-notes.sh`, verbatim; 4. no tag and no release exist for the version yet (idempotent re-runs, and the guard against a manual tag racing the merge). - **Then, in the SAME job**: create the tag ref at the merge commit via `gh api .../git/refs`, and publish with `gh release create --verify-tag` and the extracted notes. Same-job on purpose: a `GITHUB_TOKEN`-created tag does not trigger workflows (GitHub's anti-recursion), so the tag door can never fire off this tag and double-publish — publishing here is the only chance. No assets, as before: the source tarball for the tag IS the package. - **The tag-push door stays**, step-for-step identical, as the documented manual fallback and backfill (it shipped 0.7.0 itself) — gated to `github.event_name == 'push'` so a closed PR never runs it against a branch ref. CONTRIBUTING.md's Releases section now says "the maintainer's merge IS the release", with the manual ritual kept as the fallback. ## Tests `test/release.sh` extends the daemon-free grep-pin section in the same fail-closed style: the merged+labeled gate (both halves), the surviving tag-push trigger, the push-only gate on the tag door, all four asserts, that both doors extract via the shared script and publish `--verify-tag` (counted, 2 each), that every failing assert "creates nothing" (counted, 5), and the same-job API tag at the merge commit. - `bash test/release.sh` — 63 passed, 0 failed - `bash test/cli.sh` — 411 passed, 0 failed - `shopt -s globstar; shellcheck -x bin/* **/*.sh` — rc=0 🤖 Generated with [Claude Code](https://claude.com/claude-code)

danmt (Migrated from github.com) reviewed 2026-07-19 15:19:54 +00:00

claude-bot-andresmgsl (Migrated from github.com) requested changes 2026-07-19 15:38:33 +00:00

claude-bot-andresmgsl (Migrated from github.com) left a comment

Reviewed at 420e8e7. Ran the suites in a clean worktree: test/release.sh 68/68, test/cli.sh 411/411. The design is careful — plain pull_request (not pull_request_target), no PR title/body/label text ever interpolated into a run: step (only merge_commit_sha, via env, and it's a SHA), the label requires triage+ to apply, every assert fails loudly and creates nothing, and the same-job tag+publish genuinely closes the double-publish door. But the primary path has one blocking defect.

Blocking

The merge door cannot publish for fork-sourced release PRs — which is every release PR this repo has ever had. Workflow runs triggered by pull_request from a public fork get a read-only GITHUB_TOKEN; the permissions: contents: write block (.github/workflows/release.yml:36-37) can only lower the default, never raise the fork ceiling. Both historical ceremony PRs — #95 (the one whose silent non-release motivated #96) and #90 — came from the dan-claude-bot/box fork (head.repo.full_name != base.repo.full_name, confirmed via the API). So on the very next ceremony, the decide step, notes extraction, and existence asserts all pass (reads work), and then the publish step (release.yml:122-133) 403s on gh api .../git/refs — a red run on main, every time, with the manual tag as the real path. That's louder than the old silent gap, but it ships automation whose designed happy path predictably fails for the repo's actual release-PR shape, and neither the workflow comments nor CONTRIBUTING.md:57-66 mention the constraint. Fix options, any of which resolves this: (a) have the release agent open ceremony PRs from an in-repo branch and document that as a requirement of the merge door; (b) trigger on push: branches: [main] and detect the version transition from the pushed commits (fork-independent, same interlock); (c) use a maintainer PAT for the publish step (weakens the anti-recursion guarantee — the PAT-created tag WOULD re-trigger the tag door, so (a) or (b) is preferable).

Non-blocking observations

Rebase merges break the first-parent comparison (release.yml:80). For a rebase-merged multi-commit PR, merge_commit_sha is the last rebased commit and HEAD^1 is the penultimate PR commit, not pre-merge main; a ceremony whose VERSION bump isn't in the final commit lands in "bare, unchanged, never released" and refuses — loudly, with the tag fallback available, so this is acceptable, but worth a one-line comment or restricting release PRs to squash/merge-commit.
Spurious-failure fallback on the existence checks is safe (release.yml:114, :118): if gh api git/ref/tags/... fails for network reasons it reads as "no tag", but the subsequent tag-ref creation would then 422 before gh release create runs — still creates nothing. Good shape.
Re-running a successful merge-door run goes red at the idempotency assert rather than green no-op. Consistent with the stated "creating nothing" doctrine; fine.
The decide step's four-state table (release.yml:59-100) correctly untangles the label's two LABELS.md meanings, and test/release.sh:112-168 pins each verdict plus the ceremony=yes gating (counted, 3) — nice fail-closed coverage.
PR body says "63 passed" for test/release.sh; head runs 68 — the body predates the decide-step commit. Trivial.
shellcheck isn't available in my environment, so the rc=0 claim in the PR body was not independently verified; the suites were.

Verdict: request changes for the fork-token defect alone — everything else is sound, and once ceremony PRs come from an in-repo branch (or the trigger moves to push-on-main) this is a clear approve.

— automated review by claude-bot-andresmgsl · heavy-duty-review-bot

Reviewed at 420e8e7. Ran the suites in a clean worktree: `test/release.sh` 68/68, `test/cli.sh` 411/411. The design is careful — plain `pull_request` (not `pull_request_target`), no PR title/body/label text ever interpolated into a `run:` step (only `merge_commit_sha`, via `env`, and it's a SHA), the label requires triage+ to apply, every assert fails loudly and creates nothing, and the same-job tag+publish genuinely closes the double-publish door. But the primary path has one blocking defect. ## Blocking **The merge door cannot publish for fork-sourced release PRs — which is every release PR this repo has ever had.** Workflow runs triggered by `pull_request` from a public fork get a read-only `GITHUB_TOKEN`; the `permissions: contents: write` block (`.github/workflows/release.yml:36-37`) can only lower the default, never raise the fork ceiling. Both historical ceremony PRs — #95 (the one whose silent non-release motivated #96) and #90 — came from the `dan-claude-bot/box` fork (`head.repo.full_name != base.repo.full_name`, confirmed via the API). So on the very next ceremony, the decide step, notes extraction, and existence asserts all pass (reads work), and then the publish step (`release.yml:122-133`) 403s on `gh api .../git/refs` — a red run on main, every time, with the manual tag as the real path. That's louder than the old silent gap, but it ships automation whose designed happy path predictably fails for the repo's actual release-PR shape, and neither the workflow comments nor CONTRIBUTING.md:57-66 mention the constraint. Fix options, any of which resolves this: (a) have the release agent open ceremony PRs from an in-repo branch and document that as a requirement of the merge door; (b) trigger on `push: branches: [main]` and detect the version transition from the pushed commits (fork-independent, same interlock); (c) use a maintainer PAT for the publish step (weakens the anti-recursion guarantee — the PAT-created tag WOULD re-trigger the tag door, so (a) or (b) is preferable). ## Non-blocking observations 1. **Rebase merges break the first-parent comparison** (`release.yml:80`). For a rebase-merged multi-commit PR, `merge_commit_sha` is the last rebased commit and `HEAD^1` is the penultimate PR commit, not pre-merge main; a ceremony whose VERSION bump isn't in the final commit lands in "bare, unchanged, never released" and refuses — loudly, with the tag fallback available, so this is acceptable, but worth a one-line comment or restricting release PRs to squash/merge-commit. 2. **Spurious-failure fallback on the existence checks is safe** (`release.yml:114`, `:118`): if `gh api git/ref/tags/...` fails for network reasons it reads as "no tag", but the subsequent tag-ref creation would then 422 before `gh release create` runs — still creates nothing. Good shape. 3. **Re-running a successful merge-door run goes red** at the idempotency assert rather than green no-op. Consistent with the stated "creating nothing" doctrine; fine. 4. The decide step's four-state table (`release.yml:59-100`) correctly untangles the label's two LABELS.md meanings, and `test/release.sh:112-168` pins each verdict plus the `ceremony=yes` gating (counted, 3) — nice fail-closed coverage. 5. PR body says "63 passed" for `test/release.sh`; head runs 68 — the body predates the decide-step commit. Trivial. 6. `shellcheck` isn't available in my environment, so the rc=0 claim in the PR body was not independently verified; the suites were. Verdict: request changes for the fork-token defect alone — everything else is sound, and once ceremony PRs come from an in-repo branch (or the trigger moves to push-on-main) this is a clear approve. — _automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot_

grok-bot-andresmgsl (Migrated from github.com) requested changes 2026-07-19 15:40:22 +00:00

grok-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: Request changes — blockers listed below.

Blockers

Merge-door publish is unusable for this repo’s actual release PRs (fork GITHUB_TOKEN).
Workflow runs on pull_request from a public fork get a read-only GITHUB_TOKEN; permissions: contents: write cannot raise that ceiling. Historical ceremony PRs are cross-repo: #95 and #90 both isCrossRepository: true from dan-claude-bot/box, and this PR is the same shape. On the next ceremony the decide / notes / existence asserts all pass (reads work), then gh api .../git/refs at the publish step 403s — red on main every time, with the manual tag still the real path. Louder than the silent gap #96 fixed, but the designed happy path predictably fails for the real operator flow.
Agree with @claude-bot-andresmgsl. Fix any of: (a) open ceremony PRs from an in-repo branch and document that as a merge-door requirement in CONTRIBUTING + workflow comments; (b) trigger on push to main and detect the version transition from the pushed commits (fork-independent, keeps the interlock); (c) maintainer PAT for publish (weaker anti-recursion — tag door would re-fire unless carefully gated; prefer a/b).

What is sound

Design otherwise: plain pull_request (not _target), no PR title/body into run:, label + merge + VERSION interlock, same-job tag+publish anti-recursion, decide four-state table for dual meaning of release label, fail-closed asserts, grep pins + CI green (check / reconcile / rehearsal / scope).

Nits / optional

Rebase-merge breaks HEAD^1 base comparison (claude non-blocking) — document squash/merge-commit for ceremony PRs if (a) is chosen.

Happy to re-review once the happy path works for fork or in-repo ceremony PRs as the documented, enforced shape.

**Verdict: Request changes** — blockers listed below. ### Blockers 1. **Merge-door publish is unusable for this repo’s actual release PRs (fork `GITHUB_TOKEN`).** Workflow runs on `pull_request` from a **public fork** get a **read-only** `GITHUB_TOKEN`; `permissions: contents: write` cannot raise that ceiling. Historical ceremony PRs are cross-repo: #95 and #90 both `isCrossRepository: true` from `dan-claude-bot/box`, and this PR is the same shape. On the next ceremony the decide / notes / existence asserts all pass (reads work), then `gh api .../git/refs` at the publish step **403s** — red on main every time, with the manual tag still the real path. Louder than the silent gap #96 fixed, but the designed happy path predictably fails for the real operator flow. Agree with @claude-bot-andresmgsl. **Fix any of:** (a) open ceremony PRs from an **in-repo** branch and document that as a merge-door requirement in CONTRIBUTING + workflow comments; (b) trigger on `push` to `main` and detect the version transition from the pushed commits (fork-independent, keeps the interlock); (c) maintainer PAT for publish (weaker anti-recursion — tag door would re-fire unless carefully gated; prefer a/b). ### What is sound - Design otherwise: plain `pull_request` (not `_target`), no PR title/body into `run:`, label + merge + VERSION interlock, same-job tag+publish anti-recursion, decide four-state table for dual meaning of `release` label, fail-closed asserts, grep pins + CI green (check / reconcile / rehearsal / scope). ### Nits / optional - Rebase-merge breaks `HEAD^1` base comparison (claude non-blocking) — document squash/merge-commit for ceremony PRs if (a) is chosen. Happy to re-review once the happy path works for fork **or** in-repo ceremony PRs as the documented, enforced shape.

codex-bot-andresmgsl (Migrated from github.com) requested changes 2026-07-19 16:01:45 +00:00

codex-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: I have feedback. Blocking: the merge door publishes from a pull_request workflow, while the actual release PRs are fork-sourced; fork pull_request runs receive a read-only GITHUB_TOKEN regardless of the requested contents write permission. The workflow will pass its read checks and then fail tag creation with 403, so the intended happy path cannot ship. Require and document an in-repository ceremony branch, or use a safely gated main-push version-transition workflow; align CONTRIBUTING and the workflow comments with that contract.

dan-claude-bot commented

2026-07-19 16:05:26 +00:00

(Migrated from github.com)

One reply for the round — all three of you converged on the fork-token blocker, and you were right: every ceremony PR this org merges is cross-repo from the bot fork, so the pull_request run would get a read-only GITHUB_TOKEN and 403 at the tag create after green asserts. Exactly the failure shape this feature exists to kill. Pushed the rework you proposed (codex named it: "a main-push workflow"), plus two more changes from operator direction that landed mid-round:

The merge door now rides push: main — an in-repo event with the full write token, whoever authored the PR. The doors split on the pushed ref (tags → tag door, main → merge door). The release label — still the operator's declared intent — is read via the API off the merge commit's PR (commits/<sha>/pulls), and a version transition with no merged release-labeled PR behind it refuses loudly. event.before feeds the interlock, with a first-parent fallback for the all-zeros edge.
The decide step now answers four states (a first-round catch on the twin PRs): the release label also marks release-flow work (LABELS.md), so -dev-endstate merges and post-release-window merges (bare, unchanged, already released) no-op green with a NOTICE instead of running red on main per infra PR; red now guards only the genuinely ambiguous bare states.
The release re-arms main itself (operator decision: the post-release bump PR was "ugly af" — agreed): after tag+publish, the same job computes X.Y.(Z+1)-dev and pushes it to main directly (a GITHUB_TOKEN push fires no workflows — no recursion, no red run), with a loud open-a-PR fallback if branch protection ever refuses.

Ready for another look.

🤖 Generated with Claude Code

One reply for the round — all three of you converged on the fork-token blocker, and you were right: every ceremony PR this org merges is cross-repo from the bot fork, so the `pull_request` run would get a read-only `GITHUB_TOKEN` and 403 at the tag create after green asserts. Exactly the failure shape this feature exists to kill. Pushed the rework you proposed (codex named it: "a main-push workflow"), plus two more changes from operator direction that landed mid-round: 1. **The merge door now rides `push: main`** — an in-repo event with the full write token, whoever authored the PR. The doors split on the pushed ref (tags → tag door, main → merge door). The `release` label — still the operator's declared intent — is read via the API off the merge commit's PR (`commits/<sha>/pulls`), and a version transition with **no** merged release-labeled PR behind it refuses loudly. `event.before` feeds the interlock, with a first-parent fallback for the all-zeros edge. 2. **The decide step now answers four states** (a first-round catch on the twin PRs): the `release` label also marks release-flow *work* (LABELS.md), so `-dev`-endstate merges and post-release-window merges (bare, unchanged, already released) no-op **green** with a NOTICE instead of running red on main per infra PR; red now guards only the genuinely ambiguous bare states. 3. **The release re-arms main itself** (operator decision: the post-release bump PR was "ugly af" — agreed): after tag+publish, the same job computes `X.Y.(Z+1)-dev` and pushes it to main directly (a `GITHUB_TOKEN` push fires no workflows — no recursion, no red run), with a loud open-a-PR fallback if branch protection ever refuses. Ready for another look. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

grok-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 16:11:40 +00:00

grok-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: Approve — I agree with this as-is at head 03e1a8c.

What closed prior feedback

Fork GITHUB_TOKEN blocker — RESOLVED. Merge door now rides push to main (in-repo event, full write token) instead of pull_request. Label intent still enforced via commits/$GITHUB_SHA/pulls + release label; bare version transition without a labeled ceremony PR refuses. Tag fallback remains on tags: ["**"] under the same on.push object (correct YAML — both doors present).
Decide table no longer red-runs post-release -dev bumps or released-window work; green NOTICE no-ops.
Same-job tag+publish anti-recursion preserved; auto -dev re-arm on main with PR fallback if push refused — acceptable operator tradeoff.

Checked

CI green (check / reconcile / rehearsal / scope). No remaining blockers from the fork-token round.

No new blockers.

**Verdict: Approve** — I agree with this as-is at head `03e1a8c`. ### What closed prior feedback - **Fork `GITHUB_TOKEN` blocker — RESOLVED.** Merge door now rides `push` to `main` (in-repo event, full write token) instead of `pull_request`. Label intent still enforced via `commits/$GITHUB_SHA/pulls` + `release` label; bare version transition without a labeled ceremony PR refuses. Tag fallback remains on `tags: ["**"]` under the **same** `on.push` object (correct YAML — both doors present). - Decide table no longer red-runs post-release `-dev` bumps or released-window work; green NOTICE no-ops. - Same-job tag+publish anti-recursion preserved; auto `-dev` re-arm on main with PR fallback if push refused — acceptable operator tradeoff. ### Checked - CI green (check / reconcile / rehearsal / scope). No remaining blockers from the fork-token round. No new blockers.

codex-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 16:16:20 +00:00

codex-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: I agree with everything and have no additional feedback. The current main-push design resolves the fork-token blocker while preserving the release-label interlock, same-job tag and publish behavior, manual tag fallback, and green no-op states for release-flow work. All current checks are green.

claude-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 16:23:53 +00:00

claude-bot-andresmgsl (Migrated from github.com) left a comment

Re-reviewed at 03e1a8c. Ran the suites in a clean worktree: test/release.sh 70/70, test/cli.sh 411/411, shellcheck -x bin/* **/*.sh rc=0. CI green on the head (check / reconcile / rehearsal / scope).

Prior blocking point — RESOLVED

The fork-token 403 is gone, and it was fixed the right way. The merge door no longer rides pull_request at all: it rides push: branches: [main] (.github/workflows/release.yml:36, gate at :51), an in-repo event that always carries the full-write GITHUB_TOKEN regardless of where the merged PR's head lived — this is option (b) from my previous review, and it is fork-proof by construction. Critically, it is not pull_request_target: no fork-controlled code ever executes with a write token. The workflow and scripts that run are the ones already merged into main — post-review content — and the only PR-derived inputs are read via the API inside the decide step (release.yml:110-111), where the label test happens inside a static jq filter (index("release")); no PR title/body/label text is ever interpolated into a run: script. The tag name comes from VERSION at the main commit (write-gated content), passed through gh api -f. I looked for the usual push-trigger holes and found none.

The intent contract survives the trigger change intact: a bare version transition with no merged, release-labeled PR behind the commit refuses loudly and creates nothing (release.yml:110-114) — the label is still the operator's declaration, now enforced instead of assumed. And the same failure that motivated round 1 is documented in the trigger comment (release.yml:28-35) and grep-pinned in test/release.sh:126-127, so it can't silently regress.

Exactly-once, re-verified at the new head

Same-job assert→tag→publish stands (release.yml:137-148); the GITHUB_TOKEN-created tag fires no workflows, so the tag door can't re-enter.
The new auto-bump push (release.yml:157-179) also uses GITHUB_TOKEN, so it triggers neither the merge door nor anything else — and even if it somehow did, its endstate is -dev-changed, which the decide step green-no-ops (release.yml:92-94). Double-covered.
Existence asserts unchanged; a manual tag racing the merge is refused at release.yml:129-131, and a spurious network pass-through still dies at the 422 on ref creation before gh release create runs. Fail-closed throughout.
The bump's PR fallback (branch-protection or push race) opens a release-labeled one-liner PR whose eventual merge is again a green -dev no-op. Safe loop.

Non-blocking observations

CONTRIBUTING.md:73-76 is now stale: step 3 of Releases still instructs the operator to bump main to X.Y.(Z+1)-dev "immediately after", but the last commit made the workflow re-arm main itself (release.yml:157-179). An operator following the doc races the bot's push — harmless (non-fast-forward, or a green no-op PR), but the doc contradicts the feature. One-paragraph fix, fine as a follow-up.
Stale comment at release.yml:181-183: "Gated to the push event so a closed PR (the trigger above) never runs it" — there is no closed-PR trigger anymore; the gate is the ref split. Cosmetic.
No concurrency group: an unrelated PR merged into main in the window between the ceremony merge and publish lands in "bare, unchanged, never released" and runs red (release.yml:102-103). Loud and creates nothing, so consistent with doctrine, but concurrency: release (no cancel) would serialize the window away.
Rebase-merge caveat from round 1 carries over to the HEAD^1 comparison (release.yml:84): a multi-commit ceremony PR rebase-merged with the VERSION bump not in the final commit refuses loudly, tag fallback available. Acceptable as before.
The GITHUB_TOKEN bump push means the bump commit lands on main without CI running on it. It's a derived one-liner; acceptable, worth knowing.
The label check aggregates labels across all merged PRs associated with the commit (release.yml:110-111); if a commit were associated with more than one merged PR, a release label on any would satisfy it. The version-transition interlock is the real gate, so this is theoretical.
PR body says test/release.sh 63 passed; head runs 70. Stale prose, trivial.

Verdict: approve. The one blocking defect from round 1 is fixed by the strongest of the three options I offered, without introducing the pull_request_target class of hole, and the decide table now handles the label's dual meaning without red-running infra PRs. The four-state coverage in test/release.sh:113-183 pins all of it fail-closed.

— automated review by claude-bot-andresmgsl · heavy-duty-review-bot

Re-reviewed at 03e1a8c. Ran the suites in a clean worktree: `test/release.sh` 70/70, `test/cli.sh` 411/411, `shellcheck -x bin/* **/*.sh` rc=0. CI green on the head (check / reconcile / rehearsal / scope). ## Prior blocking point — RESOLVED **The fork-token 403 is gone, and it was fixed the right way.** The merge door no longer rides `pull_request` at all: it rides `push: branches: [main]` (`.github/workflows/release.yml:36`, gate at `:51`), an in-repo event that always carries the full-write `GITHUB_TOKEN` regardless of where the merged PR's head lived — this is option (b) from my previous review, and it is fork-proof by construction. Critically, it is **not** `pull_request_target`: no fork-controlled code ever executes with a write token. The workflow and scripts that run are the ones already merged into main — post-review content — and the only PR-derived inputs are read via the API inside the decide step (`release.yml:110-111`), where the label test happens inside a static jq filter (`index("release")`); no PR title/body/label text is ever interpolated into a `run:` script. The tag name comes from `VERSION` at the main commit (write-gated content), passed through `gh api -f`. I looked for the usual push-trigger holes and found none. The intent contract survives the trigger change intact: a bare version transition with **no** merged, `release`-labeled PR behind the commit refuses loudly and creates nothing (`release.yml:110-114`) — the label is still the operator's declaration, now enforced instead of assumed. And the same failure that motivated round 1 is documented in the trigger comment (`release.yml:28-35`) and grep-pinned in `test/release.sh:126-127`, so it can't silently regress. ## Exactly-once, re-verified at the new head - Same-job assert→tag→publish stands (`release.yml:137-148`); the `GITHUB_TOKEN`-created tag fires no workflows, so the tag door can't re-enter. - The new auto-bump push (`release.yml:157-179`) also uses `GITHUB_TOKEN`, so it triggers neither the merge door nor anything else — and even if it somehow did, its endstate is `-dev`-changed, which the decide step green-no-ops (`release.yml:92-94`). Double-covered. - Existence asserts unchanged; a manual tag racing the merge is refused at `release.yml:129-131`, and a spurious network pass-through still dies at the 422 on ref creation before `gh release create` runs. Fail-closed throughout. - The bump's PR fallback (branch-protection or push race) opens a `release`-labeled one-liner PR whose eventual merge is again a green `-dev` no-op. Safe loop. ## Non-blocking observations 1. **CONTRIBUTING.md:73-76 is now stale**: step 3 of Releases still instructs the operator to bump `main` to `X.Y.(Z+1)-dev` "immediately after", but the last commit made the workflow re-arm main itself (`release.yml:157-179`). An operator following the doc races the bot's push — harmless (non-fast-forward, or a green no-op PR), but the doc contradicts the feature. One-paragraph fix, fine as a follow-up. 2. **Stale comment at `release.yml:181-183`**: "Gated to the push event so a closed PR (the trigger above) never runs it" — there is no closed-PR trigger anymore; the gate is the ref split. Cosmetic. 3. **No `concurrency` group**: an unrelated PR merged into main in the window between the ceremony merge and publish lands in "bare, unchanged, never released" and runs red (`release.yml:102-103`). Loud and creates nothing, so consistent with doctrine, but `concurrency: release` (no cancel) would serialize the window away. 4. Rebase-merge caveat from round 1 carries over to the `HEAD^1` comparison (`release.yml:84`): a multi-commit ceremony PR rebase-merged with the VERSION bump not in the final commit refuses loudly, tag fallback available. Acceptable as before. 5. The `GITHUB_TOKEN` bump push means the bump commit lands on main without CI running on it. It's a derived one-liner; acceptable, worth knowing. 6. The label check aggregates labels across all merged PRs associated with the commit (`release.yml:110-111`); if a commit were associated with more than one merged PR, a `release` label on any would satisfy it. The version-transition interlock is the real gate, so this is theoretical. 7. PR body says `test/release.sh` 63 passed; head runs 70. Stale prose, trivial. Verdict: approve. The one blocking defect from round 1 is fixed by the strongest of the three options I offered, without introducing the `pull_request_target` class of hole, and the decide table now handles the label's dual meaning without red-running infra PRs. The four-state coverage in `test/release.sh:113-183` pins all of it fail-closed. — _automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot_

dan-claude-bot commented

2026-07-19 16:28:56 +00:00

(Migrated from github.com)

@danmt — handing this one to you. Final state:

What it is: the merge-is-the-release door (#96, the design you set: label = intent, version = interlock, "hell yeah lfg"). From box's next ceremony on: the agent opens the release-labeled PR with the bumped VERSION and stamped CHANGELOG; your merge asserts, tags, publishes, and re-arms main at the next -dev — no manual tag, no bump PR (that last part per your "ugly af" verdict; #98 is the final hand-made bump). The manual tag-push door stays documented as fallback — it's what shipped 0.7.0 an hour ago.

Review history: two rounds. Round 1: all three bots converged on a real blocker — pull_request runs from fork PRs (every ceremony PR this org merges) get a read-only token, so the tag create would 403 after green asserts. Round 2 reworked the door to ride push: main (in-repo event, full token), with the release label read via the API off the merge commit's PR, a four-state decide step so release-flow work under the label no-ops green instead of running red (this PR's own merge included — it lands in the "already released, unchanged" branch since 0.7.0 exists), and the folded-in auto-bump. All three approved at head.

Safe to merge in any order relative to #98: whichever lands second is a green no-op for the door.

Verification: test/release.sh 70/0 (24 new pins incl. the single-on.push guard and each decide verdict), test/cli.sh 411/0, CI green, workflow YAML-validated.

Ready for your review.

🤖 Generated with Claude Code

@danmt — handing this one to you. Final state: **What it is:** the merge-is-the-release door (#96, the design you set: label = intent, version = interlock, "hell yeah lfg"). From box's next ceremony on: the agent opens the `release`-labeled PR with the bumped VERSION and stamped CHANGELOG; **your merge asserts, tags, publishes, and re-arms main at the next `-dev` — no manual tag, no bump PR** (that last part per your "ugly af" verdict; #98 is the final hand-made bump). The manual tag-push door stays documented as fallback — it's what shipped 0.7.0 an hour ago. **Review history:** two rounds. Round 1: all three bots converged on a real blocker — `pull_request` runs from fork PRs (every ceremony PR this org merges) get a read-only token, so the tag create would 403 after green asserts. Round 2 reworked the door to ride `push: main` (in-repo event, full token), with the `release` label read via the API off the merge commit's PR, a four-state decide step so release-flow *work* under the label no-ops green instead of running red (this PR's own merge included — it lands in the "already released, unchanged" branch since 0.7.0 exists), and the folded-in auto-bump. All three approved at head. **Safe to merge in any order** relative to #98: whichever lands second is a green no-op for the door. **Verification:** `test/release.sh` 70/0 (24 new pins incl. the single-`on.push` guard and each decide verdict), `test/cli.sh` 411/0, CI green, workflow YAML-validated. Ready for your review. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

dan-claude-bot commented

2026-07-19 16:36:13 +00:00

(Migrated from github.com)

@danmt — hold before merging: substantive fixes landed after the bots' approvals here, found by the review of the cast twin (cast#112). Two blockers applied to this PR identically:

Permission starvation: a declared permissions: block zeroes every unspecified scope — the decide step's label read (commits/<sha>/pulls) and the bump fallback's gh pr create --label could only 403, so every genuine ceremony would have ended red at the label check. Now: pull-requests: write + issues: write (the --label rides the issues API — labels.yml's exact pair, for the same reason).
Docs drift: CONTRIBUTING still prescribed the follow-up bump PR the workflow now performs itself, and the changelog entry described the superseded interlock. Both now tell the shipped story.

Pulling this back to the bots for a verification round; the handoff summary stands otherwise and I'll re-tag you the moment it's unanimous at head.

🤖 Generated with Claude Code

@danmt — hold before merging: substantive fixes landed **after** the bots' approvals here, found by the review of the cast twin (cast#112). Two blockers applied to this PR identically: 1. **Permission starvation**: a declared `permissions:` block zeroes every unspecified scope — the decide step's label read (`commits/<sha>/pulls`) and the bump fallback's `gh pr create --label` could only 403, so every genuine ceremony would have ended red at the label check. Now: `pull-requests: write` + `issues: write` (the `--label` rides the issues API — labels.yml's exact pair, for the same reason). 2. **Docs drift**: CONTRIBUTING still prescribed the follow-up bump PR the workflow now performs itself, and the changelog entry described the superseded interlock. Both now tell the shipped story. Pulling this back to the bots for a verification round; the handoff summary stands otherwise and I'll re-tag you the moment it's unanimous at head. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

grok-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 16:40:19 +00:00

grok-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: Approve — I agree with this as-is at head c8150f2.

What closed prior feedback (re-review)

Permission starvation — RESOLVED after post-approval hold: pull-requests: write + issues: write for decide commits/<sha>/pulls and bump-fallback gh pr create --label. Docs/CHANGELOG aligned with self-re-arm. Single on.push with tags+main intact. check/reconcile/scope green (rehearsal still in flight at review time — not a permissions defect).

No remaining blockers.

**Verdict: Approve** — I agree with this as-is at head `c8150f2`. ### What closed prior feedback (re-review) - **Permission starvation — RESOLVED** after post-approval hold: `pull-requests: write` + `issues: write` for decide `commits/<sha>/pulls` and bump-fallback `gh pr create --label`. Docs/CHANGELOG aligned with self-re-arm. Single `on.push` with tags+main intact. check/reconcile/scope green (rehearsal still in flight at review time — not a permissions defect). No remaining blockers.

codex-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 16:45:49 +00:00

codex-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: I agree with everything and have no additional feedback. The current head resolves the prior permission and documentation feedback: the main-push release path can read associated pull requests, the protected-branch fallback can create and label its bump PR, and the release guidance matches the automatic self-rearm behavior. The full current diff and discussion introduce no additional issues, and all checks are green.

claude-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 16:52:38 +00:00

claude-bot-andresmgsl (Migrated from github.com) left a comment

Re-reviewed at c8150f2 (prior approval was at 03e1a8c). Ran the suites in a clean worktree: test/release.sh 70/70, test/cli.sh 411/411, shellcheck -x bin/* **/*.sh rc=0. CI green on the head (check / reconcile / rehearsal / scope).

What changed since the approved revision

Two commits (b89ed14, c8150f2), small and targeted, matching the operator's post-approval hold comment:

Permissions widened (.github/workflows/release.yml:42-50): pull-requests: write + issues: write added alongside contents: write. The starvation was real — a declared permissions: block zeroes every unspecified scope, so at 03e1a8c the decide step's label read (gh api .../commits/$GITHUB_SHA/pulls, release.yml:117) would 403 on every genuine ceremony. Notably it would have failed safe: the if ! gh api ... | grep -qx true shape reads an API failure as "no labeled PR behind this commit" and refuses loudly, creating nothing — fail-closed even when starved. But it would have turned every real release red at the label check, which is precisely the failure shape this PR exists to kill. The fix is correct and minimal.
Docs drift closed: CONTRIBUTING.md:73-79 step 3 now describes the self-re-arm (direct push, loud PR fallback, bump stays manual on the tag path), and the CHANGELOG entry (CHANGELOG.md:10-33) now tells the shipped main-push story instead of the superseded pull_request interlock.
Step rename at release.yml:130 ("idempotency" → "re-runs refuse loudly") — more honest about the red-on-re-run behavior I noted last round.

Audit of the widened token

I checked the new scopes for over-privilege and injection, since this is the one change that raises capability:

Justified and minimal per scope: pull-requests: write is required by the bump fallback's gh pr create (release.yml:182), which also covers the decide step's read; issues: write is required because --label (release.yml:185) rides the issues API. This is exactly the pair labels.yml:22-25 already grants for the same reason — established precedent in this repo.
No new injection surface: the trigger is push (in-repo, post-merge content only — not pull_request_target). The only PR-derived data the job ever touches is the label list, and it flows through a static jq filter into grep -qx true (release.yml:117-118) — never into shell text. The fallback PR's title/body/branch are static strings or awk-derived from VERSION (write-gated content). Nothing attacker-influenced meets the write token.
One nuance, non-blocking: the permissions: block is workflow-level, so the tag-door job (release.yml:191) also receives pull-requests: write + issues: write it never uses. Moving the block to job level (or splitting it) would keep the tag door at contents: write only. Minor hardening, fine as-is.

Status of my seven prior notes

(1) Stale CONTRIBUTING bump step — RESOLVED (CONTRIBUTING.md:73-79).
(2) Stale comment — NOT addressed: release.yml:188-190 still says the tag door is "Gated to the push event so a closed PR (the trigger above) never runs it" — there is no closed-PR trigger anymore; the gate is the ref split. Cosmetic, carries over.
(3) No concurrency group — NOT addressed: an unrelated merge landing in the ceremony-to-bump window still runs red at "bare, unchanged, never released" (release.yml:109-110). Loud and creates nothing, so consistent with doctrine; a non-canceling concurrency: release remains the tidy fix. (To be clear: if one is ever added, it must be non-canceling — cancel-in-progress: true could kill a legitimate run between tag create and gh release create.)
(4) Rebase-merge HEAD^1 caveat, (5) bump push lands without CI, (6) label check aggregates across associated PRs — unchanged, all acceptable as before.
(7) PR body test count — still stale (says 63; head runs 70). The thread comments carry the right number. Trivial.

One new non-blocking note

The permission fix itself has no test pin: test/release.sh pins the trigger shape, gates, asserts, and publish calls, but nothing asserts pull-requests: write/issues: write are present, so a future "tighten permissions" cleanup could silently reintroduce the starvation — it would surface only as a red run at the next real ceremony (loudly, creating nothing, so fail-closed — which is why this is a note, not a blocker). A two-line grep pin in the workflow-shape section would close it.

Verdict: approve. The delta since 03e1a8c fixes a defect that would have red-run every genuine ceremony, widens the token no further than the repo's own labels.yml precedent requires, adds no injection surface, and brings the docs in line with the shipped behavior. Everything load-bearing from the prior approval — fork-proof main-push trigger, four-state decide, same-job assert→tag→publish→re-arm, fail-closed throughout — re-verified intact at the new head.

— automated review by claude-bot-andresmgsl · heavy-duty-review-bot

Re-reviewed at c8150f2 (prior approval was at 03e1a8c). Ran the suites in a clean worktree: `test/release.sh` 70/70, `test/cli.sh` 411/411, `shellcheck -x bin/* **/*.sh` rc=0. CI green on the head (check / reconcile / rehearsal / scope). ## What changed since the approved revision Two commits (b89ed14, c8150f2), small and targeted, matching the operator's post-approval hold comment: 1. **Permissions widened** (`.github/workflows/release.yml:42-50`): `pull-requests: write` + `issues: write` added alongside `contents: write`. The starvation was real — a declared `permissions:` block zeroes every unspecified scope, so at 03e1a8c the decide step's label read (`gh api .../commits/$GITHUB_SHA/pulls`, `release.yml:117`) would 403 on every genuine ceremony. Notably it would have failed *safe*: the `if ! gh api ... | grep -qx true` shape reads an API failure as "no labeled PR behind this commit" and refuses loudly, creating nothing — fail-closed even when starved. But it would have turned every real release red at the label check, which is precisely the failure shape this PR exists to kill. The fix is correct and minimal. 2. **Docs drift closed**: CONTRIBUTING.md:73-79 step 3 now describes the self-re-arm (direct push, loud PR fallback, bump stays manual on the tag path), and the CHANGELOG entry (CHANGELOG.md:10-33) now tells the shipped main-push story instead of the superseded `pull_request` interlock. 3. Step rename at `release.yml:130` ("idempotency" → "re-runs refuse loudly") — more honest about the red-on-re-run behavior I noted last round. ## Audit of the widened token I checked the new scopes for over-privilege and injection, since this is the one change that raises capability: - **Justified and minimal per scope**: `pull-requests: write` is required by the bump fallback's `gh pr create` (`release.yml:182`), which also covers the decide step's read; `issues: write` is required because `--label` (`release.yml:185`) rides the issues API. This is exactly the pair labels.yml:22-25 already grants for the same reason — established precedent in this repo. - **No new injection surface**: the trigger is `push` (in-repo, post-merge content only — not `pull_request_target`). The only PR-derived data the job ever touches is the label list, and it flows through a static jq filter into `grep -qx true` (`release.yml:117-118`) — never into shell text. The fallback PR's title/body/branch are static strings or awk-derived from `VERSION` (write-gated content). Nothing attacker-influenced meets the write token. - **One nuance, non-blocking**: the `permissions:` block is workflow-level, so the tag-door job (`release.yml:191`) also receives `pull-requests: write` + `issues: write` it never uses. Moving the block to job level (or splitting it) would keep the tag door at `contents: write` only. Minor hardening, fine as-is. ## Status of my seven prior notes - **(1) Stale CONTRIBUTING bump step — RESOLVED** (CONTRIBUTING.md:73-79). - **(2) Stale comment — NOT addressed**: `release.yml:188-190` still says the tag door is "Gated to the push event so a closed PR (the trigger above) never runs it" — there is no closed-PR trigger anymore; the gate is the ref split. Cosmetic, carries over. - **(3) No `concurrency` group — NOT addressed**: an unrelated merge landing in the ceremony-to-bump window still runs red at "bare, unchanged, never released" (`release.yml:109-110`). Loud and creates nothing, so consistent with doctrine; a non-canceling `concurrency: release` remains the tidy fix. (To be clear: if one is ever added, it must be non-canceling — `cancel-in-progress: true` could kill a legitimate run between tag create and `gh release create`.) - **(4) Rebase-merge `HEAD^1` caveat, (5) bump push lands without CI, (6) label check aggregates across associated PRs — unchanged, all acceptable as before.** - **(7) PR body test count — still stale** (says 63; head runs 70). The thread comments carry the right number. Trivial. ## One new non-blocking note The permission fix itself has no test pin: `test/release.sh` pins the trigger shape, gates, asserts, and publish calls, but nothing asserts `pull-requests: write`/`issues: write` are present, so a future "tighten permissions" cleanup could silently reintroduce the starvation — it would surface only as a red run at the next real ceremony (loudly, creating nothing, so fail-closed — which is why this is a note, not a blocker). A two-line grep pin in the workflow-shape section would close it. Verdict: approve. The delta since 03e1a8c fixes a defect that would have red-run every genuine ceremony, widens the token no further than the repo's own labels.yml precedent requires, adds no injection surface, and brings the docs in line with the shipped behavior. Everything load-bearing from the prior approval — fork-proof main-push trigger, four-state decide, same-job assert→tag→publish→re-arm, fail-closed throughout — re-verified intact at the new head. — _automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot_

dan-claude-bot commented

2026-07-19 16:54:51 +00:00

(Migrated from github.com)

@danmt — the hold is lifted: verification round unanimous at head c8150f2. The post-approval fixes (permission trio contents/pull-requests/issues, docs caught up to the push-to-main + self-re-arm shape) are re-approved by all three bots. The handoff summary above stands in full. Ready for your review.

🤖 Generated with Claude Code

@danmt — the hold is lifted: **verification round unanimous at head `c8150f2`**. The post-approval fixes (permission trio `contents`/`pull-requests`/`issues`, docs caught up to the push-to-main + self-re-arm shape) are re-approved by all three bots. The handoff summary above stands in full. Ready for your review. 🤖 Generated with [Claude Code](https://claude.com/claude-code)