fix(recovery): fold back only what the restored fragments do not render (#1487) #1490

Merged
claude merged 3 commits from claude/1487-recover-single-source into main 2026-10-02 14:23:08 +02:00
Member

Refs #1487

Head 7bfd0ba on base de49186 (live main after #1493 merged). Rebased from the first-reviewed 9973c89; the range-diff and what changed are in the comment on this PR naming that head.

Problem

rt recover-pending-cut does two things to a stranded section. It restores the fragment files the prepare consumed, from prepare^ (restoreFragmentsFromPrepareParent, added by #1166). It also folds the whole section back under ## [Unreleased] (FoldBackIntoUnreleased). So every fragment entry comes back with two sources.

The next rt prep merges the [Unreleased] body with the fragment sections (mergeUnreleased -> changelog.MergeSections), and that merge concatenates per kind without dedup. Every fragment entry then ships twice. changelog-body-check exits 0 on the doubled section (re-measured for #1487: all nine checks PASS), so no gate catches it.

Measured with the new test on main (02fae6a): re-composed = 2, sealed = 2 for each fragment entry. The hand-written [Unreleased] entry correctly stays at 1.

Where to fix it (decision tree)

  • Recovery subtracts what the restored fragments render, then folds back the rest. This is the right layer when recovery is what creates the second source, and here it is: the fold was the only source until #1166 added the restore without making the two disjoint.
    • It keeps both halves that were each correct on their own. The coverage gate still sees cut-consumed (#1166), and [Unreleased] prose that the prepare composed into the section comes back.
    • Cost: matching rendered fragment text against the section. The match is exact because both sides use one renderer: the new prep.FragmentSections shares composeCombined with Compose.
    • When it cannot be exact (the section was edited after the prepare, or an older rt rendered it differently), it refuses with rc=2 and writes nothing.
    • Chosen.
  • Restore OR fold, not both.
    • Restore-only is right only when the section is entirely fragment-derived. Otherwise it silently drops the [Unreleased] prose the prepare composed in (mutation M2 below: the #1001 entry renders 0 times).
    • Fold-only reopens #1166: a changelog-only re-prepare, which the coverage gate grades forgotten.
    • The chosen option reduces to restore-only exactly when the section is fully fragment-derived, so it covers that case anyway.
  • rt prep dedups [Unreleased] lines that equal fragment-rendered lines. This is right when several producers can create the overlap and prep is the only common point. Recovery is the only producer I found.
    • A line-equality dedup in the compose path changes every cut for every adopter, a mechanism-of-touch change to rt prep output.
    • It would collapse lines that legitimately repeat (continuation lines, an author's deliberate duplicate).
    • It would leave the recovered state still double-sourced in the commit: the entry stays in [Unreleased] and in a fragment, and is hidden only at render time.
    • Reach for it if a second producer of the overlap appears.
  • Also considered: rebuild [Unreleased] from prepare^:CHANGELOG.md (the exact inverse of the prepare). This is right when the stranded section is known to be untouched since the prepare. If it was edited (for example, fixups on the rolling PR before merge), those edits are silently discarded. The chosen subtraction refuses on edits to fragment-derived lines and keeps edits to prose.

What changed

  • internal/changelog/foldback.go: new FoldBackIntoUnreleasedExcept(changelog, version, covered).
    • It subtracts covered content from the pending section, then folds back what is left.
    • Content runs: the last contiguous match is removed, because prep merges [Unreleased] first. An author's deliberate duplicate therefore keeps its [Unreleased] copy.
    • None. sentinels remove a subsection only when it is sentinel-only. A fully fragment-derived section therefore folds back nothing, and [Unreleased] returns to its pre-prepare state. Before this change, the scaffolded None. subsections came back too.
    • Missing content returns the typed ErrCoveredContentMissing.
    • Since #1493, it runs on #1493's fileRecords, between bodyRecords and the drop grading. It only deletes whole records, so FoldBackDropError still names true changelog lines. The drop check grades what is left, and a fully covered section skips the merge.
    • A CRLF file's trailing \r is ignored when matching, because the covered side is rendered with LF.
    • FoldBackIntoUnreleased is now Except(nil), which is byte-identical for existing callers (all existing fold-back tests unchanged and green).
  • internal/prep/prep.go: new FragmentSections(frags). Compose's merge/scaffold/normalize chain is extracted to composeCombined, so both use one chain. Compose output is unchanged: the existing prep tests are green, and TestFragmentSectionsIsComposeWithoutUnreleased pins the equivalence.
  • cmd/rt/recover_pending_cut.go: the restore is read before the fold, because the fold needs it. New rc=2 arm for ErrCoveredContentMissing. Long help text updated (TestHelpSnapshot unaffected).
    • Refusal precedence: fragment writes (checkRecoveryTargets) still come after the fold, so a second run still refuses with rc=1 "already been performed". The only change: if the restore fails and the section is missing, rc=2 (restore) now wins over rc=1 (not found).
  • docs/integration.md: the recovery (B) paragraph, the exit-code line, and the by-hand instructions.
  • changelog.d/1487-recover-single-source.fixed.md.

Verification

  • Test-first: TestRecoverPendingCutReprepareRendersEachEntryOnce strands a real git prepare ([Unreleased] prose plus two fragments, one with a body below its summary line). It then recovers through the live restoreFragmentsFromPrepareParent, re-prepares, and asserts each entry appears exactly once in both the re-composed section and the sealed section, and that the re-prepared section equals the stranded one. TestRecoverPendingCutFoldThenReprepare gained the same count assertion. Both were red on main.

  • Mutation run (AGENTS.md section 8, Axis B): every mutation goes red.

    # Mutation Red tests
    M1 production code reverted to main 4 cmd/rt (incl. the two re-prepare tests)
    M2 restore-only: drop the whole section 3 cmd/rt + 4 changelog
    M3 remove the FIRST matching run KeepsTheUnreleasedCopyOfADuplicate
    M4 a covered None. drops real content 2 cmd/rt + 2 changelog
    M5 covered None. never removes FoldsOnlyUncoveredContent, FullyCovered...
    M6 no blank-line collapse after the cut FoldsOnlyUncoveredContent
    M7 verb calls the old full fold 4 cmd/rt
    M8 ErrCoveredContentMissing arm deleted RefusesASectionEditedAfterPrepare
    M9 missing covered content silently ignored cmd/rt refusal + both changelog refusal subtests
    M10 covered rendered with full bodies (pre-#854) e2e + FragmentSectionsIsComposeWithoutUnreleased
    M11 (after #1493) the drop check grades the unsubtracted section FullyCoveredSkipsTheDropCheck
    M12 (after #1493) an exact \r-sensitive match MatchesACRLFChangelog

    M3, M5 and M6 are caught only by the internal/changelog unit tests. The end-to-end test stays green under them because prep's own merge masks those shapes, which is why the unit tests exist.

  • On the first head 9973c89: the full go test ./... passed in golang:1.25 on an LF clone, with gofmt -l reporting nothing and go vet ./... passing.

  • On 7bfd0ba: see the rebase comment for exactly what ran where. Everything except the git-backed TestRecoverPendingCutReprepareRendersEachEntryOnce ran locally (docker was unavailable), so go-ci is that test's first Linux run on this head.

  • rt fragment-check in content mode: kind, summary, ref and density pass.

  • Not verified: a live forge run of recover-pending-cut, and PR fragment coverage in required mode (that needs PR metadata, so CI grades it).

Self-bootstrap re-pin (AGENTS.md section 2)

  • Mechanism-of-touch: yes. This changes rt prep's compose code (an output-equivalent refactor) and the recovery verb's behavior.
  • check-self-bootstrap should stay green. The PR touches none of DefaultComposeScripts (internal/gates/selfboot_check.go:234), which is reusable workflow YAML plus scripts/workflow-parse-check.sh only.
  • Toolkit-self's wrappers currently pin @main (.forgejo/workflows/release.yml:79, .forgejo/workflows/recover-pending-cut.yml:33), so they resolve to post-merge main without a re-pin. If they are on an rc pin when this merges, run rt repin vX.Y.Z-rc.N before the next cut. Tag-pinned adopters get the fix with the next cut tag.

Notes

  • The stale docs/integration.md bake-set note found while checking adopter reach has since been corrected on main.
  • Also out of scope, recorded in #1487: changelog-body-check has no duplicate-entry check. This PR fixes the producer, not that gate.

Review request: verify at source (AGENTS.md section 6)

@bosun: you measured #1166 and approved #1182, which added the restore this PR builds on. Please verify both halves at source, not by reading:

  1. Duplicate side: run TestRecoverPendingCutReprepareRendersEachEntryOnce against 02fae6a's production files. It should go red with 2 and 2 for both fragment entries.
  2. Overcorrection side: a restore-only mutant (drop the whole section) should redden the #1001 count, because the hand-written [Unreleased] prose must survive exactly once.
  3. Refusal: TestRecoverPendingCutRefusesASectionEditedAfterPrepare should exit rc=2 and leave both surfaces untouched.

Handling review fixes (AGENTS.md section 6.2)

  • Any fix you ask for lands as a new commit on claude/1487-recover-single-source. No force-push and no amend, so the head you reviewed stays reachable and the delta is reviewable on its own.
    • One exception, already made: the rebase onto main after #1493, which the conflicts forced. It was a force-push with lease, announced by a HOLD beforehand and a range-diff afterwards.
  • Before pushing a fix, I post HOLD in this thread. Once it's pushed, I post the new full head SHA and what changed. Please read the new head, not the one before the HOLD.
  • A body-only correction fires no review event, so I'll announce it in a comment as well.
  • Before submitting, please pin your review to the head you read: rt review-submit-check --owner frankenbit --repo release-toolkit --pr 1490 --reviewed-head <FULL_SHA>.
  • #1487's acceptance criteria get ticked DONE @ #1490 (<sha>) once the PR is approved, so the AC-closure gate sees them finished. This body uses Refs, not a closing keyword.

🤖 Generated with Claude Code

Refs #1487 Head `7bfd0ba` on base `de49186` (live `main` after #1493 merged). Rebased from the first-reviewed `9973c89`; the range-diff and what changed are in the comment on this PR naming that head. ## Problem `rt recover-pending-cut` does two things to a stranded section. It restores the fragment files the prepare consumed, from `prepare^` (`restoreFragmentsFromPrepareParent`, added by #1166). It also folds the **whole** section back under `## [Unreleased]` (`FoldBackIntoUnreleased`). So every fragment entry comes back with two sources. The next `rt prep` merges the `[Unreleased]` body with the fragment sections (`mergeUnreleased` -> `changelog.MergeSections`), and that merge concatenates per kind without dedup. Every fragment entry then ships twice. `changelog-body-check` exits 0 on the doubled section (re-measured for #1487: all nine checks PASS), so no gate catches it. Measured with the new test on `main` (`02fae6a`): re-composed = 2, sealed = 2 for each fragment entry. The hand-written `[Unreleased]` entry correctly stays at 1. ## Where to fix it (decision tree) - **Recovery subtracts what the restored fragments render, then folds back the rest.** This is the right layer when recovery is what creates the second source, and here it is: the fold was the only source until #1166 added the restore without making the two disjoint. - It keeps both halves that were each correct on their own. The coverage gate still sees `cut-consumed` (#1166), and `[Unreleased]` prose that the prepare composed into the section comes back. - Cost: matching rendered fragment text against the section. The match is exact because both sides use one renderer: the new `prep.FragmentSections` shares `composeCombined` with `Compose`. - When it cannot be exact (the section was edited after the prepare, or an older `rt` rendered it differently), it refuses with rc=2 and writes nothing. - **Chosen.** - **Restore OR fold, not both.** - Restore-only is right only when the section is entirely fragment-derived. Otherwise it silently drops the `[Unreleased]` prose the prepare composed in (mutation M2 below: the #1001 entry renders 0 times). - Fold-only reopens #1166: a changelog-only re-prepare, which the coverage gate grades `forgotten`. - The chosen option reduces to restore-only exactly when the section is fully fragment-derived, so it covers that case anyway. - **`rt prep` dedups `[Unreleased]` lines that equal fragment-rendered lines.** This is right when several producers can create the overlap and prep is the only common point. Recovery is the only producer I found. - A line-equality dedup in the compose path changes every cut for every adopter, a mechanism-of-touch change to `rt prep` output. - It would collapse lines that legitimately repeat (continuation lines, an author's deliberate duplicate). - It would leave the recovered state still double-sourced in the commit: the entry stays in `[Unreleased]` *and* in a fragment, and is hidden only at render time. - Reach for it if a second producer of the overlap appears. - **Also considered: rebuild `[Unreleased]` from `prepare^:CHANGELOG.md`** (the exact inverse of the prepare). This is right when the stranded section is known to be untouched since the prepare. If it was edited (for example, fixups on the rolling PR before merge), those edits are silently discarded. The chosen subtraction refuses on edits to fragment-derived lines and keeps edits to prose. ## What changed - `internal/changelog/foldback.go`: new `FoldBackIntoUnreleasedExcept(changelog, version, covered)`. - It subtracts covered content from the pending section, then folds back what is left. - Content runs: the **last** contiguous match is removed, because prep merges `[Unreleased]` first. An author's deliberate duplicate therefore keeps its `[Unreleased]` copy. - `None.` sentinels remove a subsection only when it is sentinel-only. A fully fragment-derived section therefore folds back nothing, and `[Unreleased]` returns to its pre-prepare state. Before this change, the scaffolded `None.` subsections came back too. - Missing content returns the typed `ErrCoveredContentMissing`. - Since #1493, it runs on #1493's `fileRecord`s, between `bodyRecords` and the drop grading. It only deletes whole records, so `FoldBackDropError` still names true changelog lines. The drop check grades what is left, and a fully covered section skips the merge. - A CRLF file's trailing `\r` is ignored when matching, because the covered side is rendered with LF. - `FoldBackIntoUnreleased` is now `Except(nil)`, which is byte-identical for existing callers (all existing fold-back tests unchanged and green). - `internal/prep/prep.go`: new `FragmentSections(frags)`. `Compose`'s merge/scaffold/normalize chain is extracted to `composeCombined`, so both use one chain. `Compose` output is unchanged: the existing prep tests are green, and `TestFragmentSectionsIsComposeWithoutUnreleased` pins the equivalence. - `cmd/rt/recover_pending_cut.go`: the restore is read before the fold, because the fold needs it. New rc=2 arm for `ErrCoveredContentMissing`. `Long` help text updated (`TestHelpSnapshot` unaffected). - Refusal precedence: fragment **writes** (`checkRecoveryTargets`) still come after the fold, so a second run still refuses with rc=1 "already been performed". The only change: if the restore fails **and** the section is missing, rc=2 (restore) now wins over rc=1 (not found). - `docs/integration.md`: the recovery (B) paragraph, the exit-code line, and the by-hand instructions. - `changelog.d/1487-recover-single-source.fixed.md`. ## Verification - **Test-first:** `TestRecoverPendingCutReprepareRendersEachEntryOnce` strands a real git prepare (`[Unreleased]` prose plus two fragments, one with a body below its summary line). It then recovers through the live `restoreFragmentsFromPrepareParent`, re-prepares, and asserts each entry appears exactly once in both the re-composed section and the sealed section, and that the re-prepared section equals the stranded one. `TestRecoverPendingCutFoldThenReprepare` gained the same count assertion. Both were red on `main`. - **Mutation run (AGENTS.md section 8, Axis B):** every mutation goes red. | # | Mutation | Red tests | |---|---|---| | M1 | production code reverted to `main` | 4 cmd/rt (incl. the two re-prepare tests) | | M2 | restore-only: drop the whole section | 3 cmd/rt + 4 changelog | | M3 | remove the FIRST matching run | `KeepsTheUnreleasedCopyOfADuplicate` | | M4 | a covered `None.` drops real content | 2 cmd/rt + 2 changelog | | M5 | covered `None.` never removes | `FoldsOnlyUncoveredContent`, `FullyCovered...` | | M6 | no blank-line collapse after the cut | `FoldsOnlyUncoveredContent` | | M7 | verb calls the old full fold | 4 cmd/rt | | M8 | `ErrCoveredContentMissing` arm deleted | `RefusesASectionEditedAfterPrepare` | | M9 | missing covered content silently ignored | cmd/rt refusal + both changelog refusal subtests | | M10 | covered rendered with full bodies (pre-#854) | e2e + `FragmentSectionsIsComposeWithoutUnreleased` | | M11 | (after #1493) the drop check grades the unsubtracted section | `FullyCoveredSkipsTheDropCheck` | | M12 | (after #1493) an exact `\r`-sensitive match | `MatchesACRLFChangelog` | M3, M5 and M6 are caught only by the `internal/changelog` unit tests. The end-to-end test stays green under them because prep's own merge masks those shapes, which is why the unit tests exist. - **On the first head `9973c89`:** the full `go test ./...` passed in `golang:1.25` on an LF clone, with `gofmt -l` reporting nothing and `go vet ./...` passing. - **On `7bfd0ba`:** see the rebase comment for exactly what ran where. Everything except the git-backed `TestRecoverPendingCutReprepareRendersEachEntryOnce` ran locally (docker was unavailable), so `go-ci` is that test's first Linux run on this head. - `rt fragment-check` in content mode: kind, summary, ref and density pass. - **Not verified:** a live forge run of `recover-pending-cut`, and PR fragment coverage in `required` mode (that needs PR metadata, so CI grades it). ## Self-bootstrap re-pin (AGENTS.md section 2) - **Mechanism-of-touch: yes.** This changes `rt prep`'s compose code (an output-equivalent refactor) and the recovery verb's behavior. - **`check-self-bootstrap` should stay green.** The PR touches none of `DefaultComposeScripts` (`internal/gates/selfboot_check.go:234`), which is reusable workflow YAML plus `scripts/workflow-parse-check.sh` only. - **Toolkit-self's wrappers currently pin `@main`** (`.forgejo/workflows/release.yml:79`, `.forgejo/workflows/recover-pending-cut.yml:33`), so they resolve to post-merge main without a re-pin. **If they are on an rc pin when this merges, run `rt repin vX.Y.Z-rc.N` before the next cut.** Tag-pinned adopters get the fix with the next cut tag. ## Notes - The stale `docs/integration.md` bake-set note found while checking adopter reach has since been corrected on `main`. - Also out of scope, recorded in #1487: `changelog-body-check` has no duplicate-entry check. This PR fixes the producer, not that gate. ## Review request: verify at source (AGENTS.md section 6) @bosun: you measured #1166 and approved #1182, which added the restore this PR builds on. Please verify both halves at source, not by reading: 1. **Duplicate side:** run `TestRecoverPendingCutReprepareRendersEachEntryOnce` against `02fae6a`'s production files. It should go red with 2 and 2 for both fragment entries. 2. **Overcorrection side:** a restore-only mutant (drop the whole section) should redden the `#1001` count, because the hand-written `[Unreleased]` prose must survive exactly once. 3. **Refusal:** `TestRecoverPendingCutRefusesASectionEditedAfterPrepare` should exit rc=2 and leave both surfaces untouched. ## Handling review fixes (AGENTS.md section 6.2) - Any fix you ask for lands as a **new commit** on `claude/1487-recover-single-source`. No force-push and no amend, so the head you reviewed stays reachable and the delta is reviewable on its own. - **One exception, already made:** the rebase onto `main` after #1493, which the conflicts forced. It was a force-push with lease, announced by a `HOLD` beforehand and a range-diff afterwards. - Before pushing a fix, I post `HOLD` in this thread. Once it's pushed, I post the new full head SHA and what changed. Please read the new head, not the one before the HOLD. - A body-only correction fires no review event, so I'll announce it in a comment as well. - Before submitting, please pin your review to the head you read: `rt review-submit-check --owner frankenbit --repo release-toolkit --pr 1490 --reviewed-head <FULL_SHA>`. - #1487's acceptance criteria get ticked `DONE @ #1490 (<sha>)` once the PR is approved, so the AC-closure gate sees them finished. This body uses `Refs`, not a closing keyword. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
fix(recovery): fold back only what the restored fragments do not render (#1487)
All checks were successful
fork-pr-approval-notice / explain fork workflow approval (pull_request_target) Successful in 5s
ac-closure-check / check (pull_request) Successful in 0s
base-divergence-check / check (pull_request) Successful in 6s
changelog-body-check / changelog body Cold-Read linter (pull_request) Successful in 6s
changelog-body-check / check (pull_request) Successful in 0s
check-self-bootstrap / check (pull_request) Successful in 5s
gitea-twin-check / check (pull_request) Successful in 6s
go-ci / lint + build + test (pull_request) Successful in 29s
go-ci / record reviewed vs landed commit (pull_request) Has been skipped
manifest-check / manifest-vs-tag consistency (pull_request) Successful in 7s
manifest-check / check (pull_request) Successful in 0s
prep-order-check / check (pull_request) Successful in 6s
readme-pin-check / mirror (pull_request) Successful in 7s
readme-pin-check / digest (pull_request) Successful in 6s
register-check / register-drift check (pull_request) Successful in 7s
register-check / check (pull_request) Successful in 0s
tests / workflow-schema (pull_request) Successful in 4s
tests / bats (pull_request) Successful in 32s
tests / dated-examples (pull_request) Successful in 5s
tests / contract-paths (pull_request) Successful in 4s
tests / shellcheck (pull_request) Successful in 4s
workflow-parse-check / toolkit-self parse guard and controls (pull_request) Successful in 58s
workflow-parse-check / workflow parse and schema (pull_request) Successful in 5s
workflow-parse-check / check (pull_request) Successful in 0s
ac-closure-check / ac-closure check (pull_request) Successful in 7s
fragment-check / coverage (pull_request) Successful in 7s
fragment-check / density (pull_request) Successful in 7s
fragment-check / check (pull_request) Successful in 0s
toolkit-self-gates / toolkit-self gates (PR's own rt) (pull_request) Successful in 7s
go-ci / page landing-tree failure (pull_request) Has been skipped
9973c89994
rt recover-pending-cut restored the fragments a stranded prepare consumed
AND folded the whole stranded section back under [Unreleased], so every
fragment entry came back with two sources. The next rt prep merges the
[Unreleased] body and the fragment sections without dedup (mergeUnreleased
-> MergeSections), and the re-prepared section carried each fragment entry
twice. changelog-body-check passes the doubled section.

Recovery now renders the restored fragments with prep's own compose chain
(new prep.FragmentSections, sharing composeCombined with Compose) and
subtracts that from the stranded section before folding the rest back
(changelog.FoldBackIntoUnreleasedExcept). Content the fragments render but
the section no longer carries as written is could-not-grade (rc=2,
ErrCoveredContentMissing) and nothing is written.

TestRecoverPendingCutReprepareRendersEachEntryOnce strands a real git
prepare, recovers, re-prepares, and asserts each entry once in both the
re-composed and the sealed section; it was red on main (2 and 2).

Refs #1487

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude requested review from bosun 2026-10-02 09:58:00 +02:00
claude left a comment

Verdict: the code is correct (LGTM). Merge is blocked on a mechanical rebase onto main.

Reviewed head: 9973c8999431ba7c9679b0cad0621c1402512ad3 (rt review-submit-check returned rc=0 immediately before this was submitted).

⚠️ This is a COMMENT, not an approval. It is posted from the claude account, which is this PR's author, and Forgejo hard-blocks self-approval (AGENTS.md section 8, self-approval-globally-hardcoded). @bosun's approval is still the one that counts.

Verified at source (golang:1.25, LF clone)

All three checks the body asks for were run, not read:

  1. Duplicate side. I restored the four production files from 02fae6a and kept the new tests. TestRecoverPendingCutReprepareRendersEachEntryOnce goes red:
    • #980: re-composed 2, sealed 2.
    • #981: re-composed 2, sealed 2.
    • TestRecoverPendingCutFoldThenReprepare is red on #980 (2 and 2). AppliesTheFold and RefusesASectionEditedAfterPrepare are red as well.
  2. Overcorrection side. I used my own restore-only mutant: subtractCovered still runs, so the refusals stay intact, and then pendingLines = nil. The #1001 prose renders 0 times in both the re-composed and the sealed section. AppliesTheFold goes red, as do three internal/changelog unit tests.
  3. Refusal. TestRecoverPendingCutRefusesASectionEditedAfterPrepare is green at head. It asserts rc=2, a byte-identical CHANGELOG.md, and that no fragment was written.
  4. At head, go test ./... passes all 29 packages. go vet and gofmt -l are clean.

The design is right. FragmentSections shares composeCombined with Compose, which makes the exact match a construction property rather than a hope, and TestFragmentSectionsIsComposeWithoutUnreleased pins that. Removing the last run, plus sentinel-only removal for None., matches how prep merges ([Unreleased] first). The second-run rc=1 test is still in place.

Findings

1. Blocks the merge, not the code: mergeable: false. main has moved 6 commits past the base (02fae6a → 55a12b9).

  • The only conflict is internal/prep/prep.go. #1486 (31677cc) added UnreleasedDropError right after Compose, at the same spot where this PR adds composeCombined and FragmentSections.
  • The resolution is additive: keep both sides.
  • ⚠️ Make sure UnreleasedDropError.Error()'s closing } survives. A naive keep-both resolution drops it, because it sits in the shared context, and the build then fails with syntax error: unexpected name composeCombined.
  • I did that rebase in a scratch clone. go test ./... passes all 29 packages there.
  • #1486's new Compose refusal does not interact with this change. FragmentSections never reaches it, and the fold-back output is only ### Kind subsections.
  • A rebase means a new head. Per section 6.2, post HOLD, then the new full SHA. I expect the delta to be only that prep.go resolution.

2. Coordinate with sibling branch claude/1491-foldback-refuse-drop (8d7c1cd).

  • It edits the same FoldBackIntoUnreleased body, the same switch in recover_pending_cut.go, and docs/integration.md.
  • git merge-tree of the two heads conflicts in foldback.go, integration.md and prep.go.
  • Whichever lands second must rebuild #1491's drop-check (uncarriedAt(recs, nextAt+1, endAt)) next to this PR's subtraction. That covers two things:
    • the unreleasedBody != "" && pendingBody != "" arm must merge the subtracted pendingLines;
    • a fully covered section must still skip the merge.
  • Both PRs' fold-back tests then need to run together.

3. Non-blocking, forward risk: FragmentSections passes nil for the CC half. That is correct today, because #717 makes CC bump-only and ccMd is always empty. The planned release_notes: {source: commits} opt-in will put commit-rendered entries into the sealed section. Recovery would not subtract them, and the re-prepare would render them from commits again, so they would be duplicated once more. The refusal would not fire, since nothing covered is missing; the extra lines simply fold back. Please name this at FragmentSections, or as a note on the opt-in's tracker, so that work picks up the recovery half.

4. Nit, no change needed. Both the ErrCoveredContentMissing arm and the default arm are rc=2, so M8 is caught only by the message assertion. That is fine, and the test pins the message.

Out-of-scope items (the integration.md:983 marker.go claim, and the missing duplicate-entry check in changelog-body-check) are noted in the body and in #1487. I agree they belong elsewhere.

## Verdict: the code is correct (LGTM). Merge is blocked on a mechanical rebase onto `main`. Reviewed head: `9973c8999431ba7c9679b0cad0621c1402512ad3` (`rt review-submit-check` returned rc=0 immediately before this was submitted). ⚠️ **This is a COMMENT, not an approval.** It is posted from the `claude` account, which is this PR's author, and Forgejo hard-blocks self-approval (AGENTS.md section 8, `self-approval-globally-hardcoded`). @bosun's approval is still the one that counts. ### Verified at source (golang:1.25, LF clone) All three checks the body asks for were run, not read: 1. **Duplicate side.** I restored the four production files from `02fae6a` and kept the new tests. `TestRecoverPendingCutReprepareRendersEachEntryOnce` goes red: - `#980`: re-composed 2, sealed 2. - `#981`: re-composed 2, sealed 2. - `TestRecoverPendingCutFoldThenReprepare` is red on `#980` (2 and 2). `AppliesTheFold` and `RefusesASectionEditedAfterPrepare` are red as well. 2. **Overcorrection side.** I used my own restore-only mutant: `subtractCovered` still runs, so the refusals stay intact, and then `pendingLines = nil`. The `#1001` prose renders **0** times in both the re-composed and the sealed section. `AppliesTheFold` goes red, as do three `internal/changelog` unit tests. 3. **Refusal.** `TestRecoverPendingCutRefusesASectionEditedAfterPrepare` is green at head. It asserts rc=2, a byte-identical `CHANGELOG.md`, and that no fragment was written. 4. At head, `go test ./...` passes all 29 packages. `go vet` and `gofmt -l` are clean. The design is right. `FragmentSections` shares `composeCombined` with `Compose`, which makes the exact match a construction property rather than a hope, and `TestFragmentSectionsIsComposeWithoutUnreleased` pins that. Removing the last run, plus sentinel-only removal for `None.`, matches how prep merges (`[Unreleased]` first). The second-run rc=1 test is still in place. ### Findings **1. Blocks the merge, not the code: `mergeable: false`.** `main` has moved 6 commits past the base (`02fae6a` → `55a12b9`). - The only conflict is `internal/prep/prep.go`. #1486 (`31677cc`) added `UnreleasedDropError` right after `Compose`, at the same spot where this PR adds `composeCombined` and `FragmentSections`. - The resolution is additive: keep both sides. - ⚠️ Make sure `UnreleasedDropError.Error()`'s closing `}` survives. A naive keep-both resolution drops it, because it sits in the shared context, and the build then fails with `syntax error: unexpected name composeCombined`. - I did that rebase in a scratch clone. `go test ./...` passes all 29 packages there. - #1486's new `Compose` refusal does not interact with this change. `FragmentSections` never reaches it, and the fold-back output is only `### Kind` subsections. - A rebase means a new head. Per section 6.2, post `HOLD`, then the new full SHA. I expect the delta to be only that `prep.go` resolution. **2. Coordinate with sibling branch `claude/1491-foldback-refuse-drop` (`8d7c1cd`).** - It edits the same `FoldBackIntoUnreleased` body, the same `switch` in `recover_pending_cut.go`, and `docs/integration.md`. - `git merge-tree` of the two heads conflicts in `foldback.go`, `integration.md` and `prep.go`. - Whichever lands second must rebuild #1491's drop-check (`uncarriedAt(recs, nextAt+1, endAt)`) next to this PR's subtraction. That covers two things: - the `unreleasedBody != "" && pendingBody != ""` arm must merge the **subtracted** `pendingLines`; - a fully covered section must still skip the merge. - Both PRs' fold-back tests then need to run together. **3. Non-blocking, forward risk: `FragmentSections` passes `nil` for the CC half.** That is correct today, because #717 makes CC bump-only and `ccMd` is always empty. The planned `release_notes: {source: commits}` opt-in will put commit-rendered entries into the sealed section. Recovery would not subtract them, and the re-prepare would render them from commits again, so they would be duplicated once more. The refusal would **not** fire, since nothing covered is missing; the extra lines simply fold back. Please name this at `FragmentSections`, or as a note on the opt-in's tracker, so that work picks up the recovery half. **4. Nit, no change needed.** Both the `ErrCoveredContentMissing` arm and the `default` arm are rc=2, so M8 is caught only by the message assertion. That is fine, and the test pins the message. Out-of-scope items (the `integration.md:983` marker.go claim, and the missing duplicate-entry check in `changelog-body-check`) are noted in the body and in #1487. I agree they belong elsewhere.
Author
Member

HOLD. Please don't approve 9973c8999431ba7c9679b0cad0621c1402512ad3.

  • It cannot merge: internal/prep/prep.go conflicts with main after #1486's UnreleasedDropError.
  • #1493 edits the same five files.
  • The operator's call is to let #1493 land first, then rebase #1490 onto it once.

That rebase replaces the head. It is the one exception to this PR's no-force-push line, and the conflict forces it. When it is pushed, I'll post the new full head SHA and a git range-diff against 9973c89.

Thanks for review 7155. Status of its three items:

  1. Rebase: done locally onto 55a12b9, with UnreleasedDropError.Error() keeping its closing brace. The suite passes 29/29, and the new test still goes red with production reverted to 55a12b9. I'll redo it onto #1493 once that lands.
  2. Landing second: #1490 will be the second to land. So it will merge the subtracted pendingLines in the both-non-empty arm, with #1491's drop-check in place, and keep the fully-covered case skipping the merge.
  3. The commit-notes note: added at FragmentSections and at the renderCommits seam, as its own new commit.
**HOLD.** Please don't approve `9973c8999431ba7c9679b0cad0621c1402512ad3`. - It cannot merge: `internal/prep/prep.go` conflicts with `main` after #1486's `UnreleasedDropError`. - #1493 edits the same five files. - The operator's call is to let #1493 land first, then rebase #1490 onto it **once**. That rebase replaces the head. It is the one exception to this PR's no-force-push line, and the conflict forces it. When it is pushed, I'll post the new full head SHA and a `git range-diff` against `9973c89`. Thanks for review 7155. Status of its three items: 1. **Rebase:** done locally onto `55a12b9`, with `UnreleasedDropError.Error()` keeping its closing brace. The suite passes 29/29, and the new test still goes red with production reverted to `55a12b9`. I'll redo it onto #1493 once that lands. 2. **Landing second:** #1490 will be the second to land. So it will merge the subtracted `pendingLines` in the both-non-empty arm, with #1491's drop-check in place, and keep the fully-covered case skipping the merge. 3. **The commit-notes note:** added at `FragmentSections` and at the `renderCommits` seam, as its own new commit.
claude force-pushed claude/1487-recover-single-source from 9973c89994
All checks were successful
fork-pr-approval-notice / explain fork workflow approval (pull_request_target) Successful in 5s
ac-closure-check / check (pull_request) Successful in 0s
base-divergence-check / check (pull_request) Successful in 6s
changelog-body-check / changelog body Cold-Read linter (pull_request) Successful in 6s
changelog-body-check / check (pull_request) Successful in 0s
check-self-bootstrap / check (pull_request) Successful in 5s
gitea-twin-check / check (pull_request) Successful in 6s
go-ci / lint + build + test (pull_request) Successful in 29s
go-ci / record reviewed vs landed commit (pull_request) Has been skipped
manifest-check / manifest-vs-tag consistency (pull_request) Successful in 7s
manifest-check / check (pull_request) Successful in 0s
prep-order-check / check (pull_request) Successful in 6s
readme-pin-check / mirror (pull_request) Successful in 7s
readme-pin-check / digest (pull_request) Successful in 6s
register-check / register-drift check (pull_request) Successful in 7s
register-check / check (pull_request) Successful in 0s
tests / workflow-schema (pull_request) Successful in 4s
tests / bats (pull_request) Successful in 32s
tests / dated-examples (pull_request) Successful in 5s
tests / contract-paths (pull_request) Successful in 4s
tests / shellcheck (pull_request) Successful in 4s
workflow-parse-check / toolkit-self parse guard and controls (pull_request) Successful in 58s
workflow-parse-check / workflow parse and schema (pull_request) Successful in 5s
workflow-parse-check / check (pull_request) Successful in 0s
ac-closure-check / ac-closure check (pull_request) Successful in 7s
fragment-check / coverage (pull_request) Successful in 7s
fragment-check / density (pull_request) Successful in 7s
fragment-check / check (pull_request) Successful in 0s
toolkit-self-gates / toolkit-self gates (PR's own rt) (pull_request) Successful in 7s
go-ci / page landing-tree failure (pull_request) Has been skipped
to 7bfd0bafe9
Some checks failed
tests / bats (pull_request) Successful in 33s
tests / dated-examples (pull_request) Successful in 5s
tests / contract-paths (pull_request) Successful in 4s
tests / shellcheck (pull_request) Successful in 4s
workflow-parse-check / toolkit-self parse guard and controls (pull_request) Successful in 4s
workflow-parse-check / workflow parse and schema (pull_request) Successful in 4s
workflow-parse-check / check (pull_request) Successful in 0s
go-ci / page landing-tree failure (pull_request) Has been skipped
ac-closure-check / ac-closure check (pull_request) Successful in 6s
ac-closure-check / check (pull_request) Successful in 0s
fragment-check / coverage (pull_request) Successful in 7s
fragment-check / density (pull_request) Successful in 7s
fragment-check / check (pull_request) Successful in 0s
toolkit-self-gates / toolkit-self gates (PR's own rt) (pull_request) Successful in 7s
check-self-bootstrap / check (push) Successful in 5s
gitea-twin-check / check (push) Successful in 5s
go-ci / lint + build + test (push) Successful in 30s
go-ci / record reviewed vs landed commit (push) Failing after 3s
prepared-uncut-check / toolkit-self prepared-uncut controls (push) Successful in 5s
prepared-uncut-check / prepared-but-uncut release (push) Successful in 7s
prepared-uncut-check / check (push) Successful in 0s
release / decide + act (push) Successful in 9s
release / release (push) Successful in 0s
tests / workflow-schema (push) Successful in 3s
tests / bats (push) Successful in 31s
tests / dated-examples (push) Successful in 5s
tests / contract-paths (push) Successful in 4s
tests / shellcheck (push) Successful in 3s
go-ci / page landing-tree failure (push) Has been skipped
release / fire-cut (push) Has been skipped
2026-10-02 14:06:21 +02:00
Compare
Author
Member

Rebased onto main after #1493. New head: 7bfd0bafe9b165c624bd0c0abb97c7fea16b5b57.

This was a force-push with lease against the reviewed 9973c8999431ba7c9679b0cad0621c1402512ad3. That commit is still in this PR's history, and review 7155 stays bound to it. git range-diff 02fae6a..9973c89 de49186..7bfd0ba:

1:  9973c89 ! 1:  01976b5 fix(recovery): fold back only what the restored fragments do not render (#1487)
-:  ------- > 2:  daf62c8 docs(prep): FragmentSections must follow any commit-note opt-in
-:  ------- > 3:  7bfd0ba test(changelog): pin the subtraction's CRLF match and the fully covered skip

What changed in 01976b5: the rebase, adapted to #1493

  • foldback.go. #1493 reads both bodies as fileRecords, each carrying its changelog line. subtractCovered now runs on those records, between bodyRecords and #1493's drop grading.
    • It only deletes whole records, so the kept ones still carry their own lines. uncarriedIn's mapping holds for "any subsequence", so FoldBackDropError still names true changelog lines.
    • The fold, and #1493's code in it, is otherwise taken verbatim from main.
  • Review 7155, item 2 (the second to land).
    • The both-non-empty arm merges the subtracted pending records, with #1491's drop check grading what is left.
    • A fully covered section leaves pendingBody empty, so the merge, and its check, are skipped. [Unreleased] stays as it was.
  • CRLF. #1493 keeps a CRLF file's trailing \r on each record, and the covered side is rendered with LF. So the run match ignores that \r. Without it, every CRLF changelog would be refused as edited after the prepare.
  • cmd/rt/recover_pending_cut.go. Both refusal arms are kept: #1493's FoldBackDropError (exit 1) and this PR's ErrCoveredContentMissing (exit 2).
  • #1491's TestRecoverPendingCutRefusesTextTheFoldWouldDrop. Its stranded section held only #980, which the fixture's restored fragment covers. With the subtraction, nothing is left to merge, so the refusal it asserts can no longer fire.
    • droppingFixture now mirrors pendingFixture's uncovered ### Changed #1001 entry, so the merge and the refusal still run. The line numbers it asserts are unchanged.
    • The original fully covered shape is now its own test, TestRecoverPendingCutFullyCoveredSkipsTheMerge: exit 0, [Unreleased] byte-identical with the uncarried text included, fragment restored.
  • docs/integration.md. #1493's paragraph now says the merge sees what is left after the fragments' entries are taken out, and adds one sentence on the fully covered case.

Verification

  • Mutation runs:

    Mutation Red
    M1: production reverted to de49186 4 cmd/rt recovery tests: AppliesTheFold, RefusesASectionEditedAfterPrepare, FullyCoveredSkipsTheMerge, FoldThenReprepare
    the drop check grades the unsubtracted section TestFoldBackExceptFullyCoveredSkipsTheDropCheck
    exact \r-sensitive match TestFoldBackExceptMatchesACRLFChangelog
  • Native Go 1.27.1: internal/changelog passes, and internal/prep passes except TestRunPostBumpHooks_theHookDoesNotReceiveTheGitCredential. That test fails the same way on untouched de49186 under native Windows, so it is not from this branch.

  • Linux, via GOOS=linux: go vet ./cmd/rt ./internal/... is clean, and the cmd/rt test binary builds.

  • Linux, run in WSL: every TestRecoverPendingCut* passes, as do TestHelpSnapshot and TestSubcommandsMatchC5Contract.

  • ⚠️ Not run locally on this head: TestRecoverPendingCutReprepareRendersEachEntryOnce.

    • It drives real git. The WSL distro has none, and this machine's docker daemon stopped answering mid-run (docker version timed out at 30s).
    • It passed on the pre-#1493 rebase (55a12b9: 29/29), and none of #1493's changes are in the re-prepare path it measures.
    • go-ci on 7bfd0ba is the first full Linux run of it here. Please read that job's result rather than this note.

HOLD is lifted

Ready for review at 7bfd0bafe9b165c624bd0c0abb97c7fea16b5b57, once go-ci is green on it. Before submitting, pin your review to that head with rt review-submit-check --owner frankenbit --repo release-toolkit --pr 1490 --reviewed-head 7bfd0bafe9b165c624bd0c0abb97c7fea16b5b57.

**Rebased onto `main` after #1493. New head: `7bfd0bafe9b165c624bd0c0abb97c7fea16b5b57`.** This was a force-push with lease against the reviewed `9973c8999431ba7c9679b0cad0621c1402512ad3`. That commit is still in this PR's history, and review 7155 stays bound to it. `git range-diff 02fae6a..9973c89 de49186..7bfd0ba`: ``` 1: 9973c89 ! 1: 01976b5 fix(recovery): fold back only what the restored fragments do not render (#1487) -: ------- > 2: daf62c8 docs(prep): FragmentSections must follow any commit-note opt-in -: ------- > 3: 7bfd0ba test(changelog): pin the subtraction's CRLF match and the fully covered skip ``` ## What changed in `01976b5`: the rebase, adapted to #1493 - **`foldback.go`.** #1493 reads both bodies as `fileRecord`s, each carrying its changelog line. `subtractCovered` now runs on those records, between `bodyRecords` and #1493's drop grading. - It only deletes whole records, so the kept ones still carry their own lines. `uncarriedIn`'s mapping holds for "any subsequence", so `FoldBackDropError` still names true changelog lines. - The fold, and #1493's code in it, is otherwise taken verbatim from `main`. - **Review 7155, item 2 (the second to land).** - The both-non-empty arm merges the **subtracted** pending records, with #1491's drop check grading what is left. - A fully covered section leaves `pendingBody` empty, so the merge, and its check, are skipped. `[Unreleased]` stays as it was. - **CRLF.** #1493 keeps a CRLF file's trailing `\r` on each record, and the covered side is rendered with LF. So the run match ignores that `\r`. Without it, every CRLF changelog would be refused as edited after the prepare. - **`cmd/rt/recover_pending_cut.go`.** Both refusal arms are kept: #1493's `FoldBackDropError` (exit 1) and this PR's `ErrCoveredContentMissing` (exit 2). - **#1491's `TestRecoverPendingCutRefusesTextTheFoldWouldDrop`.** Its stranded section held only #980, which the fixture's restored fragment covers. With the subtraction, nothing is left to merge, so the refusal it asserts can no longer fire. - `droppingFixture` now mirrors `pendingFixture`'s uncovered `### Changed` #1001 entry, so the merge and the refusal still run. The line numbers it asserts are unchanged. - The original fully covered shape is now its own test, `TestRecoverPendingCutFullyCoveredSkipsTheMerge`: exit 0, `[Unreleased]` byte-identical with the uncarried text included, fragment restored. - **`docs/integration.md`.** #1493's paragraph now says the merge sees what is left after the fragments' entries are taken out, and adds one sentence on the fully covered case. ## Verification - **Mutation runs:** | Mutation | Red | |---|---| | M1: production reverted to `de49186` | 4 `cmd/rt` recovery tests: `AppliesTheFold`, `RefusesASectionEditedAfterPrepare`, `FullyCoveredSkipsTheMerge`, `FoldThenReprepare` | | the drop check grades the unsubtracted section | `TestFoldBackExceptFullyCoveredSkipsTheDropCheck` | | exact `\r`-sensitive match | `TestFoldBackExceptMatchesACRLFChangelog` | - **Native Go 1.27.1:** `internal/changelog` passes, and `internal/prep` passes except `TestRunPostBumpHooks_theHookDoesNotReceiveTheGitCredential`. That test fails the same way on untouched `de49186` under native Windows, so it is not from this branch. - **Linux, via `GOOS=linux`:** `go vet ./cmd/rt ./internal/...` is clean, and the `cmd/rt` test binary builds. - **Linux, run in WSL:** every `TestRecoverPendingCut*` passes, as do `TestHelpSnapshot` and `TestSubcommandsMatchC5Contract`. - ⚠️ **Not run locally on this head: `TestRecoverPendingCutReprepareRendersEachEntryOnce`.** - It drives real `git`. The WSL distro has none, and this machine's docker daemon stopped answering mid-run (`docker version` timed out at 30s). - It passed on the pre-#1493 rebase (`55a12b9`: 29/29), and none of #1493's changes are in the re-prepare path it measures. - **`go-ci` on `7bfd0ba` is the first full Linux run of it here.** Please read that job's result rather than this note. ## HOLD is lifted Ready for review at `7bfd0bafe9b165c624bd0c0abb97c7fea16b5b57`, once `go-ci` is green on it. Before submitting, pin your review to that head with `rt review-submit-check --owner frankenbit --repo release-toolkit --pr 1490 --reviewed-head 7bfd0bafe9b165c624bd0c0abb97c7fea16b5b57`.
Author
Member

go-ci / lint + build + test reports success on 7bfd0bafe9b165c624bd0c0abb97c7fea16b5b57 (run 18871), and check-self-bootstrap is green, as the body predicted.

I could not read that job's log from here: the log route answers 404, with or without a token. So I have the status but not the log line. Per AGENTS.md section 8 Axis C, please confirm in the log that TestRecoverPendingCutReprepareRendersEachEntryOnce actually ran and passed, before treating that test as verified on this head.

`go-ci / lint + build + test` reports **success** on `7bfd0bafe9b165c624bd0c0abb97c7fea16b5b57` (run 18871), and `check-self-bootstrap` is green, as the body predicted. I could not read that job's log from here: the log route answers 404, with or without a token. So I have the status but not the log line. Per AGENTS.md section 8 Axis C, please confirm in the log that `TestRecoverPendingCutReprepareRendersEachEntryOnce` actually ran and passed, before treating that test as verified on this head.
claude left a comment

Verdict: APPROVE at 7bfd0bafe9

rt review-submit-check returned rc=0 immediately before this was submitted. This supersedes review 7155 at 9973c89.

⚠️ This is a COMMENT, not an approval. It is posted from the claude account, which authored this PR, and Forgejo blocks self-approval. @alex has to click Approve.

Verified at source (golang:1.25, LF clone of the live head)

  1. The full suite passes at 7bfd0ba, all 29 packages, with -count=1.
  2. The open gap is closed: TestRecoverPendingCutReprepareRendersEachEntryOnce ran. It was confirmed two ways:
    • A verbose run of my own: --- PASS: TestRecoverPendingCutReprepareRendersEachEntryOnce (0.05s). RefusesTextTheFoldWouldDrop (both dry-run arms), RefusesASectionEditedAfterPrepare, FullyCoveredSkipsTheMerge and FoldThenReprepare also pass.
    • The go-ci log, read in the web UI rather than from the status row: run 18871, job 0, step go test, on this commit, shows ok cmd/rt 7.405s. The test has no skip path. testGitRepo calls t.Fatalf when git fails, and the step runs go test -count=1 ./... with no -short, so a green cmd/rt means it ran.
  3. The tests still discriminate against today's main. I took the four production files from origin/main (de49186) and kept this head's tests:
    • ReprepareRendersEachEntryOnce: #980 comes out 2 re-composed and 2 sealed, and #981 the same.
    • FoldThenReprepare is red on #980 (2 and 2).
    • FullyCoveredSkipsTheMerge is red, because main refuses that shape with "would drop 3 line(s)".
    • AppliesTheFold and RefusesASectionEditedAfterPrepare are red as well.
  4. Line numbers stay true after subtraction. I ran a scratch probe, not committed. The pending section is ### Fixed/- frag (#2), then ### Notes/- a note, under a non-empty [Unreleased], and #2 is covered.
    • FoldBackDropError.Pending reports exactly 15: ### Notes and 17: - a note, which are the true file lines.
    • It never names the covered entry.
    • This confirms the claim that subtraction deletes whole fileRecords only.

Item 2 (#1491 composition): resolved as asked

subtractCovered runs between bodyRecords and uncarriedIn(pending). So the drop check grades what is left, and the both-non-empty arm merges the subtracted records. A fully covered pending section leaves pendingBody == "", which takes the unreleasedBody != "" arm, and [Unreleased] is written back unchanged.

The adapted #1491 test, checked especially as you asked: it keeps its intent.

  • main's droppingFixture had no pending-side drop. Every line it asserts is [Unreleased]-side (CHANGELOG.md:5, :7, :9).
  • Adding the uncovered ### Changed #1001 entry below those lines leaves their numbers unchanged. It keeps the pending section non-empty after subtraction, so the merge and its refusal still run.
  • Nothing the test originally graded was weakened.

The fully covered move is a deliberate behaviour change, and I agree with it.

  • On main, that shape refused with rc=1. Now it exits 0 and leaves [Unreleased] byte for byte, including New prose after the prepare. and ### Notes.
  • Nothing is lost. That text was already in [Unreleased] before the recovery, and the next rt prep refuses it on its own (UnreleasedDropError, #1486).
  • FullyCoveredSkipsTheMerge pins this, and it is red on main as expected.

Item 3: resolved

The notes at FragmentSections and renderCommits name the coupling and the failure mode. That is enough for whoever builds the commit-note opt-in.

CRLF (\r ignored in lastRun): correct

Heading matching (trimTrailingAwkSpace), blank detection (isBlankRecord) and the None. check (TrimSpace) already tolerate a trailing \r. lastRun was the only exact comparison, and it is now the only place the \r is stripped. Your mutation note covers it.

No further findings.

## Verdict: APPROVE at 7bfd0bafe9b165c624bd0c0abb97c7fea16b5b57 `rt review-submit-check` returned rc=0 immediately before this was submitted. This supersedes review 7155 at `9973c89`. ⚠️ **This is a COMMENT, not an approval.** It is posted from the `claude` account, which authored this PR, and Forgejo blocks self-approval. @alex has to click Approve. ### Verified at source (golang:1.25, LF clone of the live head) 1. **The full suite passes** at `7bfd0ba`, all 29 packages, with `-count=1`. 2. **The open gap is closed: `TestRecoverPendingCutReprepareRendersEachEntryOnce` ran.** It was confirmed two ways: - **A verbose run of my own:** `--- PASS: TestRecoverPendingCutReprepareRendersEachEntryOnce (0.05s)`. `RefusesTextTheFoldWouldDrop` (both dry-run arms), `RefusesASectionEditedAfterPrepare`, `FullyCoveredSkipsTheMerge` and `FoldThenReprepare` also pass. - **The go-ci log, read in the web UI rather than from the status row:** run 18871, job 0, step `go test`, on this commit, shows `ok cmd/rt 7.405s`. The test has no skip path. `testGitRepo` calls `t.Fatalf` when git fails, and the step runs `go test -count=1 ./...` with no `-short`, so a green `cmd/rt` means it ran. 3. **The tests still discriminate against today's `main`.** I took the four production files from `origin/main` (`de49186`) and kept this head's tests: - `ReprepareRendersEachEntryOnce`: `#980` comes out 2 re-composed and 2 sealed, and `#981` the same. - `FoldThenReprepare` is red on `#980` (2 and 2). - `FullyCoveredSkipsTheMerge` is red, because main refuses that shape with "would drop 3 line(s)". - `AppliesTheFold` and `RefusesASectionEditedAfterPrepare` are red as well. 4. **Line numbers stay true after subtraction.** I ran a scratch probe, not committed. The pending section is `### Fixed`/`- frag (#2)`, then `### Notes`/`- a note`, under a non-empty `[Unreleased]`, and `#2` is covered. - `FoldBackDropError.Pending` reports exactly `15: ### Notes` and `17: - a note`, which are the true file lines. - It never names the covered entry. - This confirms the claim that subtraction deletes whole `fileRecord`s only. ### Item 2 (#1491 composition): resolved as asked `subtractCovered` runs between `bodyRecords` and `uncarriedIn(pending)`. So the drop check grades what is left, and the both-non-empty arm merges the subtracted records. A fully covered pending section leaves `pendingBody == ""`, which takes the `unreleasedBody != ""` arm, and `[Unreleased]` is written back unchanged. **The adapted #1491 test, checked especially as you asked: it keeps its intent.** - `main`'s `droppingFixture` had no pending-side drop. Every line it asserts is `[Unreleased]`-side (`CHANGELOG.md:5`, `:7`, `:9`). - Adding the uncovered `### Changed` `#1001` entry below those lines leaves their numbers unchanged. It keeps the pending section non-empty after subtraction, so the merge and its refusal still run. - Nothing the test originally graded was weakened. **The fully covered move is a deliberate behaviour change, and I agree with it.** - On `main`, that shape refused with rc=1. Now it exits 0 and leaves `[Unreleased]` byte for byte, including `New prose after the prepare.` and `### Notes`. - Nothing is lost. That text was already in `[Unreleased]` before the recovery, and the next `rt prep` refuses it on its own (`UnreleasedDropError`, #1486). - `FullyCoveredSkipsTheMerge` pins this, and it is red on `main` as expected. ### Item 3: resolved The notes at `FragmentSections` and `renderCommits` name the coupling and the failure mode. That is enough for whoever builds the commit-note opt-in. ### CRLF (`\r` ignored in `lastRun`): correct Heading matching (`trimTrailingAwkSpace`), blank detection (`isBlankRecord`) and the `None.` check (`TrimSpace`) already tolerate a trailing `\r`. `lastRun` was the only exact comparison, and it is now the only place the `\r` is stripped. Your mutation note covers it. No further findings.
Author
Member

The gap is closed: docker is back, and 7bfd0bafe9b165c624bd0c0abb97c7fea16b5b57 was verified locally (golang:1.25, read-only mount of an LF clone, copied inside the container).

  • go test ./... -count=1: all 29 packages ok. go vet ./... is clean, and gofmt -l cmd internal reports nothing.
  • TestRecoverPendingCutReprepareRendersEachEntryOnce: PASS, as do FullyCoveredSkipsTheMerge, RefusesTextTheFoldWouldDrop, FoldBackExceptFullyCoveredSkipsTheDropCheck and FoldBackExceptMatchesACRLFChangelog.
  • Mutation, git-backed: with production reverted to the new base de49186, TestRecoverPendingCutReprepareRendersEachEntryOnce goes red. Both #980 and #981 come out at re-composed 2, sealed 2, so the bug still reproduces on today's main, and the test catches it.

This replaces the caveat in my previous two comments. The request to read go-ci run 18871's log still stands only as an independent check.

**The gap is closed: docker is back, and `7bfd0bafe9b165c624bd0c0abb97c7fea16b5b57` was verified locally** (`golang:1.25`, read-only mount of an LF clone, copied inside the container). - `go test ./... -count=1`: all 29 packages ok. `go vet ./...` is clean, and `gofmt -l cmd internal` reports nothing. - `TestRecoverPendingCutReprepareRendersEachEntryOnce`: **PASS**, as do `FullyCoveredSkipsTheMerge`, `RefusesTextTheFoldWouldDrop`, `FoldBackExceptFullyCoveredSkipsTheDropCheck` and `FoldBackExceptMatchesACRLFChangelog`. - **Mutation, git-backed:** with production reverted to the new base `de49186`, `TestRecoverPendingCutReprepareRendersEachEntryOnce` goes red. Both #980 and #981 come out at re-composed 2, sealed 2, so the bug still reproduces on today's `main`, and the test catches it. This replaces the caveat in my previous two comments. The request to read go-ci run 18871's log still stands only as an independent check.
alex approved these changes 2026-10-02 14:22:00 +02:00
claude merged commit 7bfd0bafe9 into main 2026-10-02 14:23:08 +02:00
claude deleted branch claude/1487-recover-single-source 2026-10-02 14:23:09 +02:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
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!1490
No description provided.