fix(prep): print the [Unreleased] refusal's lines without a CRLF file's \r (#1507) #1508

Merged

claude merged 2 commits from claude/1507-prep-refusal-crlf into main 2026-10-02 13:23:04 +02:00

Conversation 5 Commits 2 Files changed 6 +90 -3

claude commented 2026-10-02 12:43:40 +02:00

Member

• Copy link
• Copy Markdown

Refs #1507

Why

On a CRLF CHANGELOG.md, rt prep's [Unreleased] refusal (prep.UnreleasedDropError, #1486) printed each UncarriedLine.Text verbatim, \r included. In a terminal the \r returns the cursor to column 0, so the reason is drawn over the file line and text the refusal exists to point at. Detection was already right. #1493 fixed the same thing in the fold-back refusal with a private unCR, so the two refusals rendered the same line two different ways.

Decision tree

Where the \r is stripped

• At render time is right when the struct must keep each record verbatim (AC 2) and every refusal should print it the same way. Taken.
• At read time (strip \r from the body before UncarriedLines) would be right only if nothing needed the raw record. UncarriedLine.Text is documented as verbatim, and #1493 kept it that way. Mutant M4 below shows the new test refuses this option.

Shape of the shared helper

• UncarriedLine.Render(where, reason), which renders the whole <where>: <text> <- <reason> line, is right when the thing that drifted is the line format: both refusals carried their own copy of the same format literal. Taken. Each caller keeps its own indent, where and reason, because those differ. rt prep numbers lines within the file or within the body. The fold-back refusal overrides the reason for a pending line above its first heading.
• An exported unCR would be right if callers needed to strip some field other than the line text. It leaves each caller to remember the call, which is how rt prep missed it. unCR stays unexported; the fold-back refusal still uses it for PendingHeading.

What is stripped

• Only a trailing \r (#1493's strings.TrimRight(s, "\r")) is right because LF output must stay byte-identical (AC 3). Trailing spaces or tabs the operator wrote print as before. Taken.
• All trailing awk white space (the trim Reason() applies to Heading) would change LF output for a line ending in spaces. Mutant M3 shows the new unit test refuses it.

What's in the PR

• internal/changelog/merge.go: adds UncarriedLine.Render. The Text doc now says the field keeps a CRLF file's \r and that Render strips it.
• internal/prep/prep.go: UnreleasedDropError.Error prints each line through Render.
• internal/changelog/foldback.go: FoldBackDropError.Describe prints through Render instead of the inline unCR(l.Text). Its output is unchanged, and its existing CRLF test passes without edits.
• internal/prep/unreleased_drop_test.go: adds TestCompose_refusalOnACRLFChangelog.
• internal/changelog/uncarried_test.go: adds TestUncarriedLine_Render.
• changelog.d/1507-prep-refusal-crlf.fixed.md.

Acceptance criteria (#1507)

• No raw \r on CRLF, same file lines as LF. TestCompose_refusalOnACRLFChangelog builds its input with the same two reads rt prep makes, UnreleasedContent and UnreleasedStartLine (cmd/rt/prep.go:195, :298). It runs the issue's CHANGELOG through them in both line endings. It asserts that the CRLF refusal contains no \r and is byte-equal to the LF refusal, so it names the same lines 5, 7 and 9.
• UncarriedLine.Text stays verbatim. In the same test, each CRLF Lines[i] has the LF line number and the LF text plus \r.
• LF output byte-identical. The same test pins the whole LF refusal, and that assertion passes against main's code. The header and the three refused lines are literals. The shared tail is built from changelog.AcceptedHeadingsNote() and changelog.UncarriedRemedy("rt prep") (since de49186, review 7168). The tail's bytes stay pinned as a literal by the existing TestCompose_refusesUnreleasedTextTheFoldWouldDrop, which is unchanged and passes.
• Red on main, mutation-verified. Both are below.

Verification

All runs used a fresh LF clone in golang:1.25.

Red on main (2060ed1 code, new tests). TestCompose_refusalOnACRLFChangelog fails. The CRLF refusal reads " CHANGELOG.md:5: Prose.\r <- before the first ### heading\n CHANGELOG.md:7: ### Notes\r ...", with all three lines carrying \r. The LF golden assertion passes on main, so the golden really is main's output. TestUncarriedLine_Render fails to build on main because Render does not exist yet, so its teeth come from the mutants below.

Green at head de4918684856ff3ef364de2381866e5e7576bd42:

• go test -count=1 ./... passes 29 of 29 packages, with no failures.
• go vet ./... is clean, and gofmt -l cmd internal is empty.
• golangci-lint run --max-issues-per-linter=0 --max-same-issues=0 (v2.14.0, whole repo) reports 0 issues.
• FRAGMENT_CHECK_COVERAGE=content rt fragment-check changelog.d passes at 5682563. de49186 does not touch the fragment.

Mutations (AGENTS.md section 8, Axis B). Each mutant ran on a copy, with the clone mounted read-only. M1-M6 were measured at 5682563. de49186 changes only how the test builds want, so I re-ran the red-on-main check, M1 and M5 at de49186, and each went red at the same assertions as before.

#	Mutant	Red
M1	rt prep prints l.Text raw again (main's rendering)	TestCompose_refusalOnACRLFChangelog
M2	Render keeps the \r	TestUncarriedLine_Render, TestCompose_refusalOnACRLFChangelog, TestFoldBackDescribeOnACRLFChangelog
M3	Render strips all trailing white space	TestUncarriedLine_Render
M4	Compose strips \r before detection, so Text is no longer verbatim	TestCompose_refusalOnACRLFChangelog, at the verbatim-Text assertion
M5	rt prep's line indent changes from two spaces to one	TestCompose_refusalOnACRLFChangelog, at the LF golden. The existing prep test stays green under this mutant, which is the gap the whole-string pin closes.
M6	the fold-back refusal prints l.Text raw	TestFoldBackDescribeOnACRLFChangelog

M2 is the one that shows both refusals print through Render: one change turns the CRLF tests of both verbs red.

Review fixes

de49186 takes the optional nit from review 7168: the CRLF test's LF golden builds its tail from the shared helpers instead of literals. Measured at de49186, with the clone read-only:

#	Probe	Result
M7	rt prep replaces the remedy line	Both tests go red: the literal tail pin (:55, :58) and the CRLF test's golden (:98).
K1	a ninth accepted kind is added ("Performance")	Only the literal tail pin goes red (:55), and TestCompose_refusalOnACRLFChangelog stays green. At 5682563 the same change also turned the CRLF test red, at :98 and :106.

Re-pin (AGENTS.md section 2)

By mechanism-of-touch this changes rt prep, a cut verb. Measured at this head, no re-pin is needed:

• gates.DefaultComposeScripts (internal/gates/selfboot_check.go:234) lists only the reusable workflows and scripts/workflow-parse-check.sh. It lists no Go files, so check-self-bootstrap has nothing to flag.
• Toolkit-self pins reusable-release.yml@main (.forgejo/workflows/release.yml:79).
• A toolkit-self cut compiles rt from the workspace commit (reusable-release.yml:405-407), so the next push:main prep runs this code.
• Adopters get the fix at the next cut tag.

Verify-at-source request (section 6)

Please verify both halves of the shared mechanism at source, not from this description:

1. rt prep: on a CRLF changelog, the refusal has no \r and matches LF. Run TestCompose_refusalOnACRLFChangelog, then reproduce M1 (revert the prep.go line to l.Text) and watch it go red.
2. rt recover-pending-cut: the fold-back output is unchanged. TestFoldBackDescribeOnACRLFChangelog is untouched and green, and M2 or M6 turns it red.

This changes only an error path, so a well-formed [Unreleased] takes the same path as before and the next cut is unaffected.

Review coordination

The PR follows section 6.2. I will announce HOLD here before pushing any review fix. Fixes land as new commits, never an amend or force-push. Before you submit, run rt review-submit-check --owner frankenbit --repo release-toolkit --pr 1508 --reviewed-head <full SHA>.

What this PR does NOT do

• It does not touch detection. UncarriedLines was already right on CRLF, and the new test confirms it finds the same lines.
• It does not change the reason text for a ### line inside a fenced block, or for an H2 after an accepted H3. That is #1492.
• It does not look for \r in rt prep output beyond this refusal. The other text in this refusal carries none: Reason() already trims Heading, and the header prints the configured path. Other messages were not measured.

🤖 Generated with Claude Code

Refs #1507 ## Why On a CRLF `CHANGELOG.md`, `rt prep`'s `[Unreleased]` refusal (`prep.UnreleasedDropError`, #1486) printed each `UncarriedLine.Text` verbatim, `\r` included. In a terminal the `\r` returns the cursor to column 0, so the reason is drawn over the file line and text the refusal exists to point at. Detection was already right. #1493 fixed the same thing in the fold-back refusal with a private `unCR`, so the two refusals rendered the same line two different ways. ## Decision tree **Where the `\r` is stripped** - **At render time** is right when the struct must keep each record verbatim (AC 2) and every refusal should print it the same way. **Taken.** - **At read time** (strip `\r` from the body before `UncarriedLines`) would be right only if nothing needed the raw record. `UncarriedLine.Text` is documented as verbatim, and #1493 kept it that way. Mutant M4 below shows the new test refuses this option. **Shape of the shared helper** - **`UncarriedLine.Render(where, reason)`, which renders the whole `<where>: <text> <- <reason>` line**, is right when the thing that drifted is the line format: both refusals carried their own copy of the same format literal. **Taken.** Each caller keeps its own indent, `where` and `reason`, because those differ. `rt prep` numbers lines within the file or within the body. The fold-back refusal overrides the reason for a pending line above its first heading. - **An exported `unCR`** would be right if callers needed to strip some field other than the line text. It leaves each caller to remember the call, which is how `rt prep` missed it. `unCR` stays unexported; the fold-back refusal still uses it for `PendingHeading`. **What is stripped** - **Only a trailing `\r`** (#1493's `strings.TrimRight(s, "\r")`) is right because LF output must stay byte-identical (AC 3). Trailing spaces or tabs the operator wrote print as before. **Taken.** - **All trailing awk white space** (the trim `Reason()` applies to `Heading`) would change LF output for a line ending in spaces. Mutant M3 shows the new unit test refuses it. ## What's in the PR - `internal/changelog/merge.go`: adds `UncarriedLine.Render`. The `Text` doc now says the field keeps a CRLF file's `\r` and that `Render` strips it. - `internal/prep/prep.go`: `UnreleasedDropError.Error` prints each line through `Render`. - `internal/changelog/foldback.go`: `FoldBackDropError.Describe` prints through `Render` instead of the inline `unCR(l.Text)`. Its output is unchanged, and its existing CRLF test passes without edits. - `internal/prep/unreleased_drop_test.go`: adds `TestCompose_refusalOnACRLFChangelog`. - `internal/changelog/uncarried_test.go`: adds `TestUncarriedLine_Render`. - `changelog.d/1507-prep-refusal-crlf.fixed.md`. ## Acceptance criteria (#1507) - [x] **No raw `\r` on CRLF, same file lines as LF.** `TestCompose_refusalOnACRLFChangelog` builds its input with the same two reads `rt prep` makes, `UnreleasedContent` and `UnreleasedStartLine` (`cmd/rt/prep.go:195`, `:298`). It runs the issue's CHANGELOG through them in both line endings. It asserts that the CRLF refusal contains no `\r` and is byte-equal to the LF refusal, so it names the same lines 5, 7 and 9. - [x] **`UncarriedLine.Text` stays verbatim.** In the same test, each CRLF `Lines[i]` has the LF line number and the LF text plus `\r`. - [x] **LF output byte-identical.** The same test pins the whole LF refusal, and that assertion passes against main's code. The header and the three refused lines are literals. The shared tail is built from `changelog.AcceptedHeadingsNote()` and `changelog.UncarriedRemedy("rt prep")` (since `de49186`, review 7168). The tail's bytes stay pinned as a literal by the existing `TestCompose_refusesUnreleasedTextTheFoldWouldDrop`, which is unchanged and passes. - [x] **Red on main, mutation-verified.** Both are below. ## Verification All runs used a fresh LF clone in `golang:1.25`. **Red on main (`2060ed1` code, new tests).** `TestCompose_refusalOnACRLFChangelog` fails. The CRLF refusal reads `" CHANGELOG.md:5: Prose.\r <- before the first ### heading\n CHANGELOG.md:7: ### Notes\r ..."`, with all three lines carrying `\r`. The LF golden assertion passes on main, so the golden really is main's output. `TestUncarriedLine_Render` fails to build on main because `Render` does not exist yet, so its teeth come from the mutants below. **Green at head `de4918684856ff3ef364de2381866e5e7576bd42`:** - `go test -count=1 ./...` passes 29 of 29 packages, with no failures. - `go vet ./...` is clean, and `gofmt -l cmd internal` is empty. - `golangci-lint run --max-issues-per-linter=0 --max-same-issues=0` (v2.14.0, whole repo) reports `0 issues.` - `FRAGMENT_CHECK_COVERAGE=content rt fragment-check changelog.d` passes at `5682563`. `de49186` does not touch the fragment. **Mutations (AGENTS.md section 8, Axis B).** Each mutant ran on a copy, with the clone mounted read-only. M1-M6 were measured at `5682563`. `de49186` changes only how the test builds `want`, so I re-ran the red-on-main check, M1 and M5 at `de49186`, and each went red at the same assertions as before. | # | Mutant | Red | |---|---|---| | M1 | `rt prep` prints `l.Text` raw again (main's rendering) | `TestCompose_refusalOnACRLFChangelog` | | M2 | `Render` keeps the `\r` | `TestUncarriedLine_Render`, `TestCompose_refusalOnACRLFChangelog`, `TestFoldBackDescribeOnACRLFChangelog` | | M3 | `Render` strips all trailing white space | `TestUncarriedLine_Render` | | M4 | `Compose` strips `\r` before detection, so `Text` is no longer verbatim | `TestCompose_refusalOnACRLFChangelog`, at the verbatim-`Text` assertion | | M5 | `rt prep`'s line indent changes from two spaces to one | `TestCompose_refusalOnACRLFChangelog`, at the LF golden. The existing prep test stays green under this mutant, which is the gap the whole-string pin closes. | | M6 | the fold-back refusal prints `l.Text` raw | `TestFoldBackDescribeOnACRLFChangelog` | M2 is the one that shows both refusals print through `Render`: one change turns the CRLF tests of both verbs red. ## Review fixes `de49186` takes the optional nit from review 7168: the CRLF test's LF golden builds its tail from the shared helpers instead of literals. Measured at `de49186`, with the clone read-only: | # | Probe | Result | |---|---|---| | M7 | `rt prep` replaces the remedy line | Both tests go red: the literal tail pin (`:55`, `:58`) and the CRLF test's golden (`:98`). | | K1 | a ninth accepted kind is added (`"Performance"`) | Only the literal tail pin goes red (`:55`), and `TestCompose_refusalOnACRLFChangelog` stays green. At `5682563` the same change also turned the CRLF test red, at `:98` and `:106`. | ## Re-pin (AGENTS.md section 2) By mechanism-of-touch this changes `rt prep`, a cut verb. Measured at this head, no re-pin is needed: - `gates.DefaultComposeScripts` (`internal/gates/selfboot_check.go:234`) lists only the reusable workflows and `scripts/workflow-parse-check.sh`. It lists no Go files, so `check-self-bootstrap` has nothing to flag. - Toolkit-self pins `reusable-release.yml@main` (`.forgejo/workflows/release.yml:79`). - A toolkit-self cut compiles `rt` from the workspace commit (`reusable-release.yml:405-407`), so the next push:main prep runs this code. - Adopters get the fix at the next cut tag. ## Verify-at-source request (section 6) Please verify both halves of the shared mechanism at source, not from this description: 1. **`rt prep`:** on a CRLF changelog, the refusal has no `\r` and matches LF. Run `TestCompose_refusalOnACRLFChangelog`, then reproduce M1 (revert the `prep.go` line to `l.Text`) and watch it go red. 2. **`rt recover-pending-cut`:** the fold-back output is unchanged. `TestFoldBackDescribeOnACRLFChangelog` is untouched and green, and M2 or M6 turns it red. This changes only an error path, so a well-formed `[Unreleased]` takes the same path as before and the next cut is unaffected. ## Review coordination The PR follows section 6.2. I will announce `HOLD` here before pushing any review fix. Fixes land as new commits, never an amend or force-push. Before you submit, run `rt review-submit-check --owner frankenbit --repo release-toolkit --pr 1508 --reviewed-head <full SHA>`. ## What this PR does NOT do - It does not touch detection. `UncarriedLines` was already right on CRLF, and the new test confirms it finds the same lines. - It does not change the reason text for a `### ` line inside a fenced block, or for an H2 after an accepted H3. That is #1492. - It does not look for `\r` in `rt prep` output beyond this refusal. The other text in this refusal carries none: `Reason()` already trims `Heading`, and the header prints the configured path. Other messages were not measured. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude added 1 commit 2026-10-02 12:43:40 +02:00

fix(prep): print the [Unreleased] refusal's lines without a CRLF file's \r ... 5682563f93

On a CRLF CHANGELOG.md each record keeps its trailing \r, and
UnreleasedDropError.Error printed UncarriedLine.Text verbatim. In a
terminal the \r returned the cursor to column 0, so the reason was drawn
over the file line and the text the refusal names. Detection was right.

UncarriedLine.Render now renders one refusal line,
`<where>: <text>    <- <reason>`, stripping only the text's trailing \r.
rt prep and the rt recover-pending-cut fold-back refusal both print their
lines through it, so the two cannot drift. Text stays verbatim. The LF
refusal is byte-identical, and its test now pins it whole, not just
line by line.

Refs #1507.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude requested review from alex 2026-10-02 12:44:16 +02:00

claude reviewed 2026-10-02 12:51:44 +02:00

claude left a comment

• Copy link
• Copy Markdown

Verdict: APPROVE at 5682563f93e3544838afa758f2fa83bf045b2729. I am posting this as a COMMENT because I can't post the approval itself. This session's forge identity is claude, the PR's author, and Forgejo refused the APPROVED review with 422 approve your own pull is not allowed. That block applies to every repo (AGENTS.md section 8, axis A, instance 7). @alex, the formal approval needs your click. Before submitting, rt review-submit-check --pr 1508 --reviewed-head 5682563f… returned rc=0 (READY), and I re-read the head right before posting.

Verify-at-source (section 6)

I checked everything below myself in a fresh LF clone at the head, using golang:1.25. Each mutant ran on a copy, with the clone mounted read-only.

• Suite: go test -count=1 ./... passes. go vet ./... is clean and gofmt -l cmd internal is empty. All 30 CI contexts on the head are success.
• Red on main: I restored prep.go, merge.go and foldback.go to 2060ed1 and kept the new tests. TestCompose_refusalOnACRLFChangelog fails on the raw-\r assertion and on the CRLF≠LF assertion, with Prose.\r, ### Notes\r and - n\r on lines 5, 7 and 9. The LF golden does not fail on main, so the golden really is main's LF output and AC 3 holds.
• Mutants: each one turns red the test the PR body says it does.

#	Mutant	Red, as I observed it
M1	rt prep prints l.Text raw	TestCompose_refusalOnACRLFChangelog
M2	Render keeps the \r	TestUncarriedLine_Render, TestCompose_refusalOnACRLFChangelog, TestFoldBackDescribeOnACRLFChangelog
M3	Render trims " \t\r"	TestUncarriedLine_Render only
M4	detection reads a \r-stripped body	TestCompose_refusalOnACRLFChangelog, at the verbatim-Text assertion (:114)
M5	prep indent goes from two spaces to one	TestCompose_refusalOnACRLFChangelog, at the LF golden (:98). The existing tail pin stays green, as claimed.
M6	fold-back prints l.Text raw	TestFoldBackDescribeOnACRLFChangelog only

M2 is the one that proves both halves: a single change to Render turns both refusals' CRLF tests red, so both print through it.

• Coverage of the class: UncarriedLine.Text is rendered in only two places, UnreleasedDropError.Error and FoldBackDropError.Describe, and both now go through Render. The other .Text readers in internal/gates are body-check hits, a different type.
• Re-pin (section 2): DefaultComposeScripts (internal/gates/selfboot_check.go:234) lists no Go files, and toolkit-self pins reusable-release.yml@main (release.yml:79). So no re-pin is needed, as stated.

AC (#1507)

All four hold as written: no \r and the same file lines on CRLF, Text kept verbatim, LF output byte-identical, and the tests red on main with mutants verified.

One optional nit, not a merge hold

The new LF golden spells out the accepted-headings list and the remedy sentence as literals. TestCompose_refusesUnreleasedTextTheFoldWouldDrop builds the same tail from changelog.AcceptedHeadingsNote() and changelog.UncarriedRemedy("rt prep"). With literals, adding an accepted kind now breaks this CRLF test as well, for a reason unrelated to \r. You could build want as the header and the three lines, then "\n" + AcceptedHeadingsNote() + "\n" + UncarriedRemedy("rt prep"). That still catches M5 and any change to the line format. Take it or leave it. It is fine as it stands. If you do take it, announce HOLD first (section 6.2), and I will re-read the new head.

**Verdict: APPROVE at `5682563f93e3544838afa758f2fa83bf045b2729`.** I am posting this as a COMMENT because I can't post the approval itself. This session's forge identity is `claude`, the PR's author, and Forgejo refused the APPROVED review with `422 approve your own pull is not allowed`. That block applies to every repo (AGENTS.md section 8, axis A, instance 7). **@alex, the formal approval needs your click.** Before submitting, `rt review-submit-check --pr 1508 --reviewed-head 5682563f…` returned rc=0 (READY), and I re-read the head right before posting. ## Verify-at-source (section 6) I checked everything below myself in a fresh LF clone at the head, using `golang:1.25`. Each mutant ran on a copy, with the clone mounted read-only. - **Suite:** `go test -count=1 ./...` passes. `go vet ./...` is clean and `gofmt -l cmd internal` is empty. All 30 CI contexts on the head are `success`. - **Red on main:** I restored `prep.go`, `merge.go` and `foldback.go` to `2060ed1` and kept the new tests. `TestCompose_refusalOnACRLFChangelog` fails on the raw-`\r` assertion and on the CRLF≠LF assertion, with `Prose.\r`, `### Notes\r` and `- n\r` on lines 5, 7 and 9. The LF golden does **not** fail on main, so the golden really is main's LF output and AC 3 holds. - **Mutants:** each one turns red the test the PR body says it does. | # | Mutant | Red, as I observed it | |---|---|---| | M1 | `rt prep` prints `l.Text` raw | `TestCompose_refusalOnACRLFChangelog` | | M2 | `Render` keeps the `\r` | `TestUncarriedLine_Render`, `TestCompose_refusalOnACRLFChangelog`, `TestFoldBackDescribeOnACRLFChangelog` | | M3 | `Render` trims `" \t\r"` | `TestUncarriedLine_Render` only | | M4 | detection reads a `\r`-stripped body | `TestCompose_refusalOnACRLFChangelog`, at the verbatim-`Text` assertion (`:114`) | | M5 | prep indent goes from two spaces to one | `TestCompose_refusalOnACRLFChangelog`, at the LF golden (`:98`). The existing tail pin stays green, as claimed. | | M6 | fold-back prints `l.Text` raw | `TestFoldBackDescribeOnACRLFChangelog` only | M2 is the one that proves both halves: a single change to `Render` turns both refusals' CRLF tests red, so both print through it. - **Coverage of the class:** `UncarriedLine.Text` is rendered in only two places, `UnreleasedDropError.Error` and `FoldBackDropError.Describe`, and both now go through `Render`. The other `.Text` readers in `internal/gates` are body-check hits, a different type. - **Re-pin (section 2):** `DefaultComposeScripts` (`internal/gates/selfboot_check.go:234`) lists no Go files, and toolkit-self pins `reusable-release.yml@main` (`release.yml:79`). So no re-pin is needed, as stated. ## AC (#1507) All four hold as written: no `\r` and the same file lines on CRLF, `Text` kept verbatim, LF output byte-identical, and the tests red on main with mutants verified. ## One optional nit, not a merge hold The new LF golden spells out the accepted-headings list and the remedy sentence as literals. `TestCompose_refusesUnreleasedTextTheFoldWouldDrop` builds the same tail from `changelog.AcceptedHeadingsNote()` and `changelog.UncarriedRemedy("rt prep")`. With literals, adding an accepted kind now breaks this CRLF test as well, for a reason unrelated to `\r`. You could build `want` as the header and the three lines, then `"\n" + AcceptedHeadingsNote() + "\n" + UncarriedRemedy("rt prep")`. That still catches M5 and any change to the line format. Take it or leave it. It is fine as it stands. If you do take it, announce `HOLD` first (section 6.2), and I will re-read the new head.

claude commented 2026-10-02 12:52:20 +02:00

Author

Member

• Copy link
• Copy Markdown

HOLD. Please do not approve until I release this hold with a new head SHA.

I am taking the optional nit from review 7168. The LF golden in TestCompose_refusalOnACRLFChangelog will build its tail from changelog.AcceptedHeadingsNote() and changelog.UncarriedRemedy("rt prep"), not from literals. The header and the three refused lines stay literal, so the indent mutant (M5) is still caught. The tail bytes stay pinned by the existing literal in TestCompose_refusesUnreleasedTextTheFoldWouldDrop.

This will be one test-only commit on top of 5682563f93e3544838afa758f2fa83bf045b2729, with no amend and no force-push. I will also correct the PR body's AC 3 line and its verification head to match.

🤖 Generated with Claude Code

**HOLD.** Please do not approve until I release this hold with a new head SHA. I am taking the optional nit from review 7168. The LF golden in `TestCompose_refusalOnACRLFChangelog` will build its tail from `changelog.AcceptedHeadingsNote()` and `changelog.UncarriedRemedy("rt prep")`, not from literals. The header and the three refused lines stay literal, so the indent mutant (M5) is still caught. The tail bytes stay pinned by the existing literal in `TestCompose_refusesUnreleasedTextTheFoldWouldDrop`. This will be one test-only commit on top of `5682563f93e3544838afa758f2fa83bf045b2729`, with no amend and no force-push. I will also correct the PR body's AC 3 line and its verification head to match. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude added 1 commit 2026-10-02 12:56:28 +02:00

test(prep): build the CRLF refusal golden's tail from the shared helpers ... de49186848

Review 7168, optional nit: the LF golden in
TestCompose_refusalOnACRLFChangelog spelled out the accepted-headings
note and the remedy as literals, so adding an accepted kind would break
this CRLF test for a reason unrelated to \r. The tail now comes from
changelog.AcceptedHeadingsNote and changelog.UncarriedRemedy, as the
existing tail check does. That test still pins the tail's bytes as a
literal, and the header and the three refused lines stay literal here,
so an indent change in the lines is still caught.

Refs #1507.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude commented 2026-10-02 12:57:37 +02:00

Author

Member

• Copy link
• Copy Markdown

HOLD released. The new head is de4918684856ff3ef364de2381866e5e7576bd42.

• The commit. de49186 is one new commit on top of 5682563f93e3544838afa758f2fa83bf045b2729, a fast-forward with no amend and no force-push. It changes only internal/prep/unreleased_drop_test.go: the LF golden builds its tail from changelog.AcceptedHeadingsNote() and changelog.UncarriedRemedy("rt prep"). That is the optional nit from review 7168.
• Measured at the new head (LF clone, golang:1.25): go test -count=1 ./... passes 29 of 29 packages, go vet is clean, gofmt -l is empty, and golangci-lint v2.14.0 reports 0 issues. The red-on-main check, M1 and M5 still go red at the same assertions as before. A ninth accepted kind now reddens only the literal tail pin, not the CRLF test.
• PR body corrected. It changed without a review event: the AC 3 line, the verification head, and a new "Review fixes" section with the probes above.

Review 7168 is bound to 5682563, so it needs a fresh read of de49186. Before submitting, run rt review-submit-check --owner frankenbit --repo release-toolkit --pr 1508 --reviewed-head de4918684856ff3ef364de2381866e5e7576bd42. @alex, the formal approval still needs your click on this head.

🤖 Generated with Claude Code

**HOLD released.** The new head is `de4918684856ff3ef364de2381866e5e7576bd42`. - **The commit.** `de49186` is one new commit on top of `5682563f93e3544838afa758f2fa83bf045b2729`, a fast-forward with no amend and no force-push. It changes only `internal/prep/unreleased_drop_test.go`: the LF golden builds its tail from `changelog.AcceptedHeadingsNote()` and `changelog.UncarriedRemedy("rt prep")`. That is the optional nit from review 7168. - **Measured at the new head** (LF clone, `golang:1.25`): `go test -count=1 ./...` passes 29 of 29 packages, `go vet` is clean, `gofmt -l` is empty, and `golangci-lint` v2.14.0 reports `0 issues.` The red-on-main check, M1 and M5 still go red at the same assertions as before. A ninth accepted kind now reddens only the literal tail pin, not the CRLF test. - **PR body corrected.** It changed without a review event: the AC 3 line, the verification head, and a new "Review fixes" section with the probes above. Review 7168 is bound to `5682563`, so it needs a fresh read of `de49186`. Before submitting, run `rt review-submit-check --owner frankenbit --repo release-toolkit --pr 1508 --reviewed-head de4918684856ff3ef364de2381866e5e7576bd42`. @alex, the formal approval still needs your click on this head. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude reviewed 2026-10-02 13:01:03 +02:00

claude left a comment

• Copy link
• Copy Markdown

Verdict: APPROVE at de4918684856ff3ef364de2381866e5e7576bd42. This review replaces review 7168, which was bound to 5682563. It is a COMMENT for the same reason as before: this session's forge identity is claude, the PR's author, so Forgejo refuses the approval with a 422. @alex, the formal approval needs your click. Before submitting, rt review-submit-check --pr 1508 --reviewed-head de491868… returned rc=0 (READY).

Fresh read of the new head

• Delta: 5682563..de49186 is one fast-forward commit, with no amend and no force-push. It touches only internal/prep/unreleased_drop_test.go, 3 lines added and 3 removed, and nothing outside the test changed. main is still 2060ed1.
• HOLD sequence (section 6.2): comment 113946 announced HOLD, the push followed, and comment 113949 released HOLD with this SHA. The PR body's new "Review fixes" section matches what I measured. The body and title carry no closing keyword.
• The comment "the test above pins its bytes" is true: TestCompose_refusesUnreleasedTextTheFoldWouldDrop still pins the tail as a literal const tail (:51-56), so the bytes stay pinned. The CRLF test now pins only what it is about.

Verify-at-source at de49186 (LF clone, golang:1.25, clone read-only)

• go test -count=1 ./... passes. go vet is clean and gofmt -l cmd internal is empty. All 30 CI contexts on de49186 are success.
• Red on main: with prep.go, merge.go, foldback.go and uncarried_test.go restored to 2060ed1, TestCompose_refusalOnACRLFChangelog fails at :103 (raw \r) and :106 (CRLF≠LF). The LF golden at :98 passes on main, so AC 3 holds.
• M5 (prep indent goes from two spaces to one) is red at :98 and :106. The literal tail is gone, and the LF golden still pins the line format.
• K1 (adding SectionName("Performance") to mergeFoldOrder) reddens only TestCompose_refusesUnreleasedTextTheFoldWouldDrop. TestCompose_refusalOnACRLFChangelog stays green. That is the decoupling the nit asked for.
• I did not re-run M1–M4 or M6 at this head. They exercise only code that is byte-identical to 5682563, where review 7168 measured them, and the test assertions they hit (:103, :106, :114) are unchanged.

AC (#1507)

All four still hold at de49186. I have no remaining findings.

**Verdict: APPROVE at `de4918684856ff3ef364de2381866e5e7576bd42`.** This review replaces review 7168, which was bound to `5682563`. It is a COMMENT for the same reason as before: this session's forge identity is `claude`, the PR's author, so Forgejo refuses the approval with a 422. **@alex, the formal approval needs your click.** Before submitting, `rt review-submit-check --pr 1508 --reviewed-head de491868…` returned rc=0 (READY). ## Fresh read of the new head - **Delta:** `5682563..de49186` is one fast-forward commit, with no amend and no force-push. It touches only `internal/prep/unreleased_drop_test.go`, 3 lines added and 3 removed, and nothing outside the test changed. `main` is still `2060ed1`. - **HOLD sequence (section 6.2):** comment 113946 announced HOLD, the push followed, and comment 113949 released HOLD with this SHA. The PR body's new "Review fixes" section matches what I measured. The body and title carry no closing keyword. - **The comment "the test above pins its bytes" is true:** `TestCompose_refusesUnreleasedTextTheFoldWouldDrop` still pins the tail as a literal `const tail` (`:51-56`), so the bytes stay pinned. The CRLF test now pins only what it is about. ## Verify-at-source at `de49186` (LF clone, `golang:1.25`, clone read-only) - `go test -count=1 ./...` passes. `go vet` is clean and `gofmt -l cmd internal` is empty. All 30 CI contexts on `de49186` are `success`. - **Red on main:** with `prep.go`, `merge.go`, `foldback.go` and `uncarried_test.go` restored to `2060ed1`, `TestCompose_refusalOnACRLFChangelog` fails at `:103` (raw `\r`) and `:106` (CRLF≠LF). The LF golden at `:98` passes on main, so AC 3 holds. - **M5** (prep indent goes from two spaces to one) is red at `:98` and `:106`. The literal tail is gone, and the LF golden still pins the line format. - **K1** (adding `SectionName("Performance")` to `mergeFoldOrder`) reddens only `TestCompose_refusesUnreleasedTextTheFoldWouldDrop`. `TestCompose_refusalOnACRLFChangelog` stays green. That is the decoupling the nit asked for. - I did not re-run M1–M4 or M6 at this head. They exercise only code that is byte-identical to `5682563`, where review 7168 measured them, and the test assertions they hit (`:103`, `:106`, `:114`) are unchanged. ## AC (#1507) All four still hold at `de49186`. I have no remaining findings.

alex approved these changes 2026-10-02 13:08:48 +02:00

claude merged commit de49186848 into main 2026-10-02 13:23:04 +02:00

claude referenced this pull request 2026-10-02 13:25:48 +02:00

bug(prep): the [Unreleased] drop refusal prints a CRLF file's \r, so a terminal overwrites the line it names #1507

claude referenced this pull request 2026-10-02 13:27:34 +02:00

bug(prep): the [Unreleased] drop refusal prints a CRLF file's \r, so a terminal overwrites the line it names #1507

claude referenced this pull request 2026-10-02 13:27:35 +02:00

GET /issues/{n}/comments ignores page and limit, so all three paginated readers of it fail on ordinary history #1495

claude referenced this pull request 2026-10-02 13:27:35 +02:00

[Unreleased] heading look-alikes: an H2 after an accepted H3 is misfiled ungraded, and a fenced ### line is refused with a misleading reason #1492

Sign in to join this conversation.

Reviewers

No reviewers

alex

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

epic

Tracker: its acceptance criteria are sub-issues; closed by hand when all are ticked

  

kind

feature

New feature or capability

  

loop/paused

rt-bugfix-loop kill switch: while this is on the loop log issue, no round claims or merges

  

mirror-drift

Automated: a release here has no asset-bearing counterpart on the mirror. One tracker per drift episode; the check never reopens a closed one (#1586)

  

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+

  

status

blocked

Cannot start until a named dependency resolves

No labels

bump

major

bump

minor

bump

patch

kind

bug

kind

chore

kind

docs

kind

epic

kind

feature

loop/paused

mirror-drift

priority

critical

priority

high

priority

low

priority

medium

size

L

size

M

size

S

size

XL

status

blocked

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!1508

No description provided.