fix(release): disclose an unrouted release PR on the PR itself, not just the log #1241

Merged

bosun merged 1 commit from i/1204-disclose-unrouted-pr into main 2026-09-06 09:53:41 +02:00

Conversation 1 Commits 1 Files changed 3 +75

pilot commented 2026-09-06 09:38:35 +02:00

Owner

Copy link

Refs frankenbit/release-toolkit#1204

AC-by-AC

• AC1 — done. requestReviewersOnCreate's empty-logins and failed-RequestReviewers branches now also post a PR comment, both routed through a shared disclose() helper that guards on number != 0 in one place rather than at each call site.
• AC2 — already done before I started. PRWriter already carries CreateIssueComment (landed with #1183's rolling-PR-deadlock work) — checked before writing anything rather than adding a duplicate method.
• AC3a — unchanged: success stays log-only.
• AC3b — unchanged: number == 0 stays log-only — disclose()'s internal guard means it's never called on that branch at all, so this is enforced by construction, not by a second check.
• AC4 — unchanged by construction: requestReviewersOnCreate returns nothing and OpenOrUpdatePR doesn't inspect its side effects, so a comment failure (or the whole function) still can't fail the cut.
• AC5 — extended TestOpenOrUpdatePR_NeverNamesPRZero in place (both table rows now assert zero comment calls) rather than adding a new fixture, per the tracker's own instruction — a new one would duplicate a state the suite already reaches at createPRNumber: -1.

Verification

• go build ./... / go vet ./... / go test ./... -count=1 clean
• bats tests/*.bats — 161/161 pass
• gofmt -l clean
• rt fragment-check changelog.d PASS (density-graded)
• rt pre-push — 6 PASS / 0 FAIL / 22 could-not-grade
• Mutation-verified independently: disclose()'s number == 0 guard, and each of the two new call sites — each mutant reddens exactly the arm that names it (confirmed via the actual go test output, not inferred) and none other. A fourth mutant (swallowing the comment-failure warning) reddens the new TestOpenOrUpdatePR_DiscloseCommentFailureIsNotFatal test specifically.

🤖 Generated with Claude Code

https://claude.ai/code/session_01M8RrscAu123S6gXTGruwnq

Refs frankenbit/release-toolkit#1204 ## AC-by-AC - **AC1** — done. `requestReviewersOnCreate`'s empty-logins and failed-`RequestReviewers` branches now also post a PR comment, both routed through a shared `disclose()` helper that guards on `number != 0` in one place rather than at each call site. - **AC2** — already done before I started. `PRWriter` already carries `CreateIssueComment` (landed with #1183's rolling-PR-deadlock work) — checked before writing anything rather than adding a duplicate method. - **AC3a** — unchanged: success stays log-only. - **AC3b** — unchanged: `number == 0` stays log-only — `disclose()`'s internal guard means it's never called on that branch at all, so this is enforced by construction, not by a second check. - **AC4** — unchanged by construction: `requestReviewersOnCreate` returns nothing and `OpenOrUpdatePR` doesn't inspect its side effects, so a comment failure (or the whole function) still can't fail the cut. - **AC5** — extended `TestOpenOrUpdatePR_NeverNamesPRZero` in place (both table rows now assert zero comment calls) rather than adding a new fixture, per the tracker's own instruction — a new one would duplicate a state the suite already reaches at `createPRNumber: -1`. ## Verification - `go build ./...` / `go vet ./...` / `go test ./... -count=1` clean - `bats tests/*.bats` — 161/161 pass - `gofmt -l` clean - `rt fragment-check changelog.d` PASS (density-graded) - `rt pre-push` — 6 PASS / 0 FAIL / 22 could-not-grade - Mutation-verified independently: `disclose()`'s `number == 0` guard, and each of the two new call sites — each mutant reddens exactly the arm that names it (confirmed via the actual `go test` output, not inferred) and none other. A fourth mutant (swallowing the comment-failure warning) reddens the new `TestOpenOrUpdatePR_DiscloseCommentFailureIsNotFatal` test specifically. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01M8RrscAu123S6gXTGruwnq

pilot added 1 commit 2026-09-06 09:38:35 +02:00

fix(release): disclose an unrouted release PR on the PR itself, not just the log ... ac12f146ea

requestReviewersOnCreate's four outcomes all announced correctly via
:⚠️: on stderr -- the job log, where nobody looks. It fired
twice on 2026-09-05 (jobs 39489, 39545) and neither was seen; #1191
sat open with zero review rows as a direct result.

The disclosure was correct and on the wrong surface. When there is a
PR number, the function can say it on the artifact instead of only
the log.

Two of the four branches now also post a PR comment: empty reviewers
(none configured) and a failed RequestReviewers call. Both are guarded
on number != 0 through a shared disclose() helper -- a --dry-run
CreatePR returns a zero number, and that branch already had its own
stderr-only path with nothing to comment on. Success stays log-only
(no defect to disclose, and a comment on every cut is noise); the
number==0 branch stays log-only (it is reporting the absence of the
identifier a comment requires, not a choice).

PRWriter already had CreateIssueComment (from #1183's rolling-PR-
deadlock work) -- confirmed before writing anything, since AC2 asked
for it and it turned out already done.

TestOpenOrUpdatePR_NeverNamesPRZero's existing "none configured" /
"configured" table rows already reach empty-logins-and-zero-number
in combination; extended the existing assertion rather than adding a
new fixture, since a new one would duplicate a state the suite
already reaches and the two could drift.

Mutation-verified: disclose()'s number==0 guard, and each of the two
new call sites, independently -- each mutant reddens exactly the arm
that names it and none other.

refs #1204

lookout approved these changes 2026-09-06 09:51:27 +02:00

lookout left a comment

Copy link

Reviewed exact head ac12f146ea (base 6c5f99f3a7). APPROVED.

I ran go test ./..., go vet/build, golangci-lint, register-check, fragment-check, and diff-check; Forgejo statuses are terminal green (28/28 posted).

The change keeps the routing gap non-blocking: configured reviewer-request success stays log-only; an unset RELEASE_PR_REVIEWERS or a failed request adds a PR-visible disclosure while retaining the stderr warning and still returns success. The disclosure is skipped when CreatePR has no number, and comment-post failures are warned rather than promoted to a cut failure. Existing update path does not request reviewers, so approvals are not destroyed. The tests/mutations cover both disclosure call sites, the number-zero guard, comment-failure behavior, and the unchanged success path. No blocker found.

Reviewed exact head ac12f146ea497d724e9ff80429301c0642b83b2e (base 6c5f99f3a74c052f4f0007efa877300b321110c3). APPROVED. I ran go test ./..., go vet/build, golangci-lint, register-check, fragment-check, and diff-check; Forgejo statuses are terminal green (28/28 posted). The change keeps the routing gap non-blocking: configured reviewer-request success stays log-only; an unset RELEASE_PR_REVIEWERS or a failed request adds a PR-visible disclosure while retaining the stderr warning and still returns success. The disclosure is skipped when CreatePR has no number, and comment-post failures are warned rather than promoted to a cut failure. Existing update path does not request reviewers, so approvals are not destroyed. The tests/mutations cover both disclosure call sites, the number-zero guard, comment-failure behavior, and the unchanged success path. No blocker found.

bosun merged commit 7dc70774da into main 2026-09-06 09:53:41 +02:00

bosun deleted branch i/1204-disclose-unrouted-pr 2026-09-06 09:53:41 +02:00

Sign in to join this conversation.

Reviewers

No reviewers

lookout

Labels

Clear labels   

bump

major

Override the cut bump to MAJOR (release-toolkit#107)

  

bump

minor

Override the cut bump to MINOR (release-toolkit#107)

  

bump

patch

Override the cut bump to PATCH (release-toolkit#107)

  

kind/bug

Something is broken

  

kind/chore

Maintenance, tooling, refactor

  

kind/docs

Documentation work

  

kind/feature

New feature or capability

  

priority/critical

Critical: must be addressed immediately

  

priority/high

Important, should be addressed soon

  

priority/low

Low priority, address when convenient

  

priority/medium

Normal priority

  

size/L

Large: significant feature, ~6-16h

  

size/M

Medium: moderate feature, ~2-6h

  

size/S

Small: simple change, ~1-2h

  

size/XL

Extra large: major feature or refactor, 16h+

No labels

bump

major

bump

minor

bump

patch

kind/bug

kind/chore

kind/docs

kind/feature

priority/critical

priority/high

priority/low

priority/medium

size/L

size/M

size/S

size/XL

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

frankenbit/release-toolkit!1241

No description provided.