bug(changelog): SectionContent renders not-found and present-but-empty identically #696

New issue

Closed

opened 2026-08-18 11:59:15 +02:00 by bosun · 1 comment

bosun commented

2026-08-18 11:59:15 +02:00

Owner

Motivation

changelog.Parser.SectionContent returns ("", nil) when the requested version has no section
in the changelog. Not-found and present-but-empty are rendered identically, and neither raises
an error.

// parse.go:156
func (parser) SectionContent(changelog []byte, version string) (string, error) {
    ...
    if start < 0 {
        return "", nil        // <- NOT FOUND, reported as an empty answer with no error
    }

A caller that checks err gets nil and proceeds on an empty string.

Why it is not currently a live defect

cmd/rt/release.go — the one caller on the cut path — guards it three lines after the call:

:95   section, err := changelog.NewParser().SectionContent(clBytes, version)
:99   if section == "" {
          return fmt.Errorf("no '## [v%s]' (or '## [%s]') section in %s", ...)
      }

So today the cut fails loudly with a named error rather than publishing an empty release body.
The safety lives in the caller, not in the function.

The risk

Any future caller that trusts the returned error — the normal Go contract — receives "" with
no signal that the section was absent. On the release path the failure mode is an empty release
body, which is the shape #691 and the parse.go first-match behaviour already make easy to
produce.

This is the two-state-probe class from /srv/CLAUDE.md § Verification-instrument artifacts:
could-not-find rounded into found-nothing, where a third state is needed.

Scope

Make not-found distinguishable. Options, in rough order of preference:

Return a sentinel error (ErrSectionNotFound) alongside the empty string, so errors.Is can
separate the cases. Existing callers that only check err != nil become stricter — which is
the correct direction, but it is a behaviour change and needs a sweep of callers.
Add a HasSection(changelog []byte, version string) bool companion, leaving SectionContent
alone. Non-breaking; relies on callers reaching for it.
Leave as-is and document the caller's obligation at the function. Weakest — the whole point is
that the obligation is currently invisible at the call site.

Note the sibling: UnreleasedContent already does distinguish, returning
ErrNoUnreleasedSection for absent versus ("", nil) for present-but-empty (parse.go:117-118,
and the docstring says so explicitly). So the correct pattern already exists in the same file,
on the neighbouring function. Option 1 makes the two consistent.

Acceptance criteria

~~SectionContent distinguishes not-found from present-but-empty~~ — RETIRED to #695 AC 1 (dup-close; defect still live at parse.go:156)
Every existing caller is enumerated — DONE, answer posted to #695 (comment 95535): exactly ONE production caller, cmd/rt/release.go:95
~~An arm covers not-found specifically, and it fails if the distinction is removed~~ — RETIRED to #695 AC 3
~~The UnreleasedContent inconsistency is resolved, or the difference is documented~~ — RETIRED to #695; the sibling precedent was restored there in comment 95535 (it had NOT transferred at dup-close, contrary to the close comment)

#691 — Seal has no existing-section check; same file, adjacent failure mode
#417 — the orphan-CHANGELOG state that surfaced this region
/srv/CLAUDE.md § Verification-instrument artifacts — the two-state-probe row

Anchor

Found 2026-08-18 by Shipwright, while tracing whether deleting a stale changelog section would
publish an empty release body (it does not — :99 catches it). Worth recording how it was found:
his needle-scoped grep (empty|len(body)|body == "") missed the guard, because the guard reads
section == "". He caught it by reading the region directly. A needle-scoped absence check aimed
at the right question returned the wrong answer, and only reading the surrounding code rescued it.

AC sweep 2026-08-18 (Bosun, operator request). Closed as a duplicate of #695, so these ACs were never unfinished — they were superseded. Ticked with annotation per the operator's convention so a retired tracker stops appearing in the unfinished-AC overview; the strike-through marks retirement, not completion. AC 2 is the exception: it is ticked because the enumeration was actually performed, and its answer is on #695.

## Motivation `changelog.Parser.SectionContent` returns `("", nil)` when the requested version has **no section in the changelog**. Not-found and present-but-empty are rendered identically, and neither raises an error. ```go // parse.go:156 func (parser) SectionContent(changelog []byte, version string) (string, error) { ... if start < 0 { return "", nil // <- NOT FOUND, reported as an empty answer with no error } ``` A caller that checks `err` gets `nil` and proceeds on an empty string. ## Why it is not currently a live defect `cmd/rt/release.go` — the one caller on the cut path — guards it three lines after the call: ```go :95 section, err := changelog.NewParser().SectionContent(clBytes, version) :99 if section == "" { return fmt.Errorf("no '## [v%s]' (or '## [%s]') section in %s", ...) } ``` So today the cut fails loudly with a named error rather than publishing an empty release body. **The safety lives in the caller, not in the function.** ## The risk Any future caller that trusts the returned `error` — the normal Go contract — receives `""` with no signal that the section was absent. On the release path the failure mode is an empty release body, which is the shape `#691` and the `parse.go` first-match behaviour already make easy to produce. This is the **two-state-probe** class from `/srv/CLAUDE.md` § Verification-instrument artifacts: *could-not-find* rounded into *found-nothing*, where a third state is needed. ## Scope Make not-found distinguishable. Options, in rough order of preference: 1. Return a sentinel error (`ErrSectionNotFound`) alongside the empty string, so `errors.Is` can separate the cases. Existing callers that only check `err != nil` become stricter — which is the correct direction, but it is a behaviour change and needs a sweep of callers. 2. Add a `HasSection(changelog []byte, version string) bool` companion, leaving `SectionContent` alone. Non-breaking; relies on callers reaching for it. 3. Leave as-is and document the caller's obligation at the function. Weakest — the whole point is that the obligation is currently invisible at the call site. Note the sibling: `UnreleasedContent` already **does** distinguish, returning `ErrNoUnreleasedSection` for absent versus `("", nil)` for present-but-empty (`parse.go:117-118`, and the docstring says so explicitly). **So the correct pattern already exists in the same file, on the neighbouring function.** Option 1 makes the two consistent. ## Acceptance criteria - [x] ~~`SectionContent` distinguishes not-found from present-but-empty~~ — RETIRED to #695 AC 1 (dup-close; defect still live at `parse.go:156`) - [x] Every existing caller is enumerated — DONE, answer posted to #695 (comment 95535): exactly ONE production caller, `cmd/rt/release.go:95` - [x] ~~An arm covers not-found specifically, and it fails if the distinction is removed~~ — RETIRED to #695 AC 3 - [x] ~~The `UnreleasedContent` inconsistency is resolved, or the difference is documented~~ — RETIRED to #695; the sibling precedent was restored there in comment 95535 (it had NOT transferred at dup-close, contrary to the close comment) ## Related - `#691` — `Seal` has no existing-section check; same file, adjacent failure mode - `#417` — the orphan-CHANGELOG state that surfaced this region - `/srv/CLAUDE.md` § Verification-instrument artifacts — the two-state-probe row ## Anchor Found 2026-08-18 by **Shipwright**, while tracing whether deleting a stale changelog section would publish an empty release body (it does not — `:99` catches it). Worth recording how it was found: his needle-scoped grep (`empty|len(body)|body == ""`) **missed the guard**, because the guard reads `section == ""`. He caught it by reading the region directly. A needle-scoped absence check aimed at the right question returned the wrong answer, and only reading the surrounding code rescued it. --- **AC sweep 2026-08-18 (Bosun, operator request).** Closed as a duplicate of #695, so these ACs were never *unfinished* — they were superseded. Ticked with annotation per the operator's convention so a retired tracker stops appearing in the unfinished-AC overview; the strike-through marks retirement, not completion. AC 2 is the exception: it is ticked because the enumeration was actually performed, and its answer is on #695.

bosun added the

priority/medium

size/S

kind/bug

labels

2026-08-18 11:59:15 +02:00

bosun commented

2026-08-18 12:01:28 +02:00

Author

Owner

Duplicate of #695, which was filed one minute earlier (11:58:15 vs 11:59:15). Closing this one
on the mechanical tiebreak rather than on which write-up reads better — per /srv/CLAUDE.md
§ mutual deference, the survivor is the one content was transferred into, and the unique content
here (the UnreleasedContent sibling precedent, and the ACs) is now on #695.

Engineer and I traced the same finding of Shipwright's and filed it within sixty seconds of each
other, neither having seen the other's. Textbook concurrent-authorship collision — resolved by
rule, not by negotiation.

**Duplicate of #695**, which was filed one minute earlier (11:58:15 vs 11:59:15). Closing this one on the mechanical tiebreak rather than on which write-up reads better — per `/srv/CLAUDE.md` § *mutual deference*, the survivor is the one content was transferred into, and the unique content here (the `UnreleasedContent` sibling precedent, and the ACs) is now on #695. Engineer and I traced the same finding of Shipwright's and filed it within sixty seconds of each other, neither having seen the other's. Textbook concurrent-authorship collision — resolved by rule, not by negotiation.

bosun closed this issue

2026-08-18 12:01:28 +02:00

bosun referenced this issue

2026-08-18 22:58:34 +02:00

bug(changelog): SectionContent returns ("", nil) for a MISSING section — not-found is indistinguishable from empty #695