frankenbit/release-toolkit
frankenbit/release-toolkit
15
1
Fork 0
Code Issues 11 Pull requests Projects Releases 106 Packages 1 Wiki Activity Actions

fix(fragments): FAIL-LOUD on unknown-kind via assert_no_unknown_kinds (closes #170) #188

Merged
quartermaster merged 1 commit from i/170-fail-loud-unknown-kind into main 2026-06-27 14:27:39 +02:00
Conversation 2 Commits 1 Files changed 3 +162
quartermaster commented 2026-06-27 13:46:24 +02:00
Owner
Copy link

Closes #170.

Why

Surveyor d586 §6 verify-at-source on the v0.12.0 rolling PR #165 surfaced two mis-named fragments (153.feat.md, 164-fix-self-pin.fix.md) using conventional-commit types as kinds. They were silently dropped by list_fragments and delete_fragments left them on disk.

The benign worked case: the feat: / fix: COMMITS independently backfilled the CHANGELOG entries by luck. The hazard: a docs-only or future-state fragment with a typo kind (.feature.md) silently drops content from the release with no CI signal.

Why list_fragments itself can't fail-loud (process-substitution trap)

list_fragments is used via process substitution <(list_fragments dir) by every caller (lint_fragments, determine_bump_from_fragments, categorize_fragments, categorize_fragment_summaries, delete_fragments). In while IFS= read; do ... done < <(...), the inner command's non-zero exit is NOT propagated to the read loop. Retrofitting fail-loud into list_fragments would either break all callers' expected back-compat OR require touching every call site to use PIPESTATUS/wait orchestration.

What this PR does

Additive: introduces assert_no_unknown_kinds [DIR] in scripts/lib/fragments.sh — explicit fail-loud surface that enumerates *.md files with unrecognized kinds, prints them with operator-facing guidance (which kinds are supported), and returns 1.

Wired: scripts/release-prep.sh calls it before bump-determination + fragment-folding. If the check fails, the prep aborts with FATAL: fragment-kind discipline failed; abort cut. — the rolling PR build reds, operator catches before the cut ships.

Preserves: list_fragments warn-and-skip behavior unchanged (back-compat for the 5 existing process-substitution callers; #9 regression-guard tests preserved). The new function is the explicit-check surface; the existing function continues its tolerant role for downstream walk semantics.

Test coverage (8 new tests)

In tests/fragments.bats:

  • #170 assert_no_unknown_kinds: all valid -> exit 0 — happy path
  • #170 ... empty dir -> exit 0 — no-op safety
  • #170 ... missing dir -> exit 0 (no-op) — robustness
  • #170 ... hidden files (.keep) skipped — gitignore safety
  • #170 ... conventional-commit-type kind (v0.12.0 worked case) -> FAIL — canonical reproducer: 153.feat.md + 164-fix-self-pin.fix.md
  • #170 ... typo kind (.feature.md) -> FAIL — hazard class
  • #170 ... mixed valid + invalid -> FAIL listing only invalid — discriminator
  • #170 ... hyphenated-kind reproducer (release-toolkit#9 shape) — historical bug shape
  • #170 ... error includes the supported-kinds list (operator hint) — UX validation

Full bats suite: 394 tests, 0 failures post-patch.

What this PR will NOT do

  • Will NOT change the fragment-kind set (still Keep-a-Changelog kinds + internal)
  • Will NOT modify list_fragments warn-and-skip behavior (back-compat invariant)
  • Will NOT auto-rename or "fix" mis-named fragments — operator decides whether to rename or delete
  • Will NOT touch the conventional-commit-based CHANGELOG entry path (independent surface)

Composition

  • Sibling of #163 (alignment-enforcement at re-pin time): both are substrate-of-record hygiene disciplines at different mechanism surfaces
  • Sibling of #184/#185 (build-bake × backstop normalization): both are post-empirical-exposure tracker-and-fix closes from today's sprint wave

Refs

  • Surveyor d586 §6 verify-at-source on rolling PR #165 (the catch)
  • v0.12.0 cut (the empirical exposure — 153.feat.md, 164-fix-self-pin.fix.md skipped)
  • #9 (prior fail-loud sibling at parser level — kind-validation regression-guard)

🤖 Generated with Claude Code
https://claude.ai/code/session_01LUEggQMJjaizj2nFVofeyH

Closes #170. ## Why Surveyor d586 §6 verify-at-source on the v0.12.0 rolling PR #165 surfaced two mis-named fragments (`153.feat.md`, `164-fix-self-pin.fix.md`) using conventional-commit types as kinds. They were silently dropped by `list_fragments` and `delete_fragments` left them on disk. The benign worked case: the `feat:` / `fix:` COMMITS independently backfilled the CHANGELOG entries by luck. The hazard: a docs-only or future-state fragment with a typo kind (`.feature.md`) silently drops content from the release with no CI signal. ## Why `list_fragments` itself can't fail-loud (process-substitution trap) `list_fragments` is used via process substitution `<(list_fragments dir)` by every caller (`lint_fragments`, `determine_bump_from_fragments`, `categorize_fragments`, `categorize_fragment_summaries`, `delete_fragments`). In `while IFS= read; do ... done < <(...)`, the inner command's non-zero exit is NOT propagated to the read loop. Retrofitting fail-loud into `list_fragments` would either break all callers' expected back-compat OR require touching every call site to use `PIPESTATUS`/`wait` orchestration. ## What this PR does **Additive**: introduces `assert_no_unknown_kinds [DIR]` in `scripts/lib/fragments.sh` — explicit fail-loud surface that enumerates `*.md` files with unrecognized kinds, prints them with operator-facing guidance (which kinds are supported), and returns 1. **Wired**: `scripts/release-prep.sh` calls it before bump-determination + fragment-folding. If the check fails, the prep aborts with `FATAL: fragment-kind discipline failed; abort cut.` — the rolling PR build reds, operator catches before the cut ships. **Preserves**: `list_fragments` warn-and-skip behavior unchanged (back-compat for the 5 existing process-substitution callers; #9 regression-guard tests preserved). The new function is the explicit-check surface; the existing function continues its tolerant role for downstream walk semantics. ## Test coverage (8 new tests) In `tests/fragments.bats`: - `#170 assert_no_unknown_kinds: all valid -> exit 0` — happy path - `#170 ... empty dir -> exit 0` — no-op safety - `#170 ... missing dir -> exit 0 (no-op)` — robustness - `#170 ... hidden files (.keep) skipped` — gitignore safety - `#170 ... conventional-commit-type kind (v0.12.0 worked case) -> FAIL` — **canonical reproducer**: `153.feat.md` + `164-fix-self-pin.fix.md` - `#170 ... typo kind (.feature.md) -> FAIL` — hazard class - `#170 ... mixed valid + invalid -> FAIL listing only invalid` — discriminator - `#170 ... hyphenated-kind reproducer (release-toolkit#9 shape)` — historical bug shape - `#170 ... error includes the supported-kinds list (operator hint)` — UX validation Full bats suite: **394 tests, 0 failures** post-patch. ## What this PR will NOT do - Will NOT change the fragment-kind set (still Keep-a-Changelog kinds + `internal`) - Will NOT modify `list_fragments` warn-and-skip behavior (back-compat invariant) - Will NOT auto-rename or "fix" mis-named fragments — operator decides whether to rename or delete - Will NOT touch the conventional-commit-based CHANGELOG entry path (independent surface) ## Composition - Sibling of #163 (alignment-enforcement at re-pin time): both are substrate-of-record hygiene disciplines at different mechanism surfaces - Sibling of #184/#185 (build-bake × backstop normalization): both are post-empirical-exposure tracker-and-fix closes from today's sprint wave ## Refs - Surveyor d586 §6 verify-at-source on rolling PR #165 (the catch) - v0.12.0 cut (the empirical exposure — `153.feat.md`, `164-fix-self-pin.fix.md` skipped) - #9 (prior fail-loud sibling at parser level — kind-validation regression-guard) 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01LUEggQMJjaizj2nFVofeyH
surveyor approved these changes 2026-06-27 13:49:04 +02:00
Dismissed
surveyor left a comment
Copy link

APPROVED — fail-loud on unknown-kind fragments (closes #170, the d586 catch)

The durable fix for the silent-content-loss class. Exactly the shape I argued for at d586 — a fail-loud surface, not a replacement of the warn-and-skip. Verified at source + mutation-verified. 394/394.

The fix is right ✅

  • assert_no_unknown_kinds scans find ... -name '*.md' ! -name '.*' (so .keep is correctly excluded — the load-bearing marker isn't flagged), validates each via fragment_kind_from_path against the shared FRAGMENT_KINDS allowlist (added changed deprecated removed fixed security internal), and fails loud (return 1) listing each offender + its extracted kind + the supported set + a rename hint. Operator-friendly error.
  • Shared allowlist: both list_fragments (warn-and-skip) and the new assertion route through fragment_kind_from_path → FRAGMENT_KINDS, so they can't disagree. A fragment that one skips is exactly what the other fails on — consistent by construction.
  • Wired before bump-determination: line 209 (assert_no_unknown_kinds) precedes line 243 (determine_bump_from_fragments). So a mis-named fragment aborts the prep before the bump/cut — the content-loss is surfaced, not silently absorbed. That's the d586 fix: the v0.12.0 case (153.feat.md skipped, masked by a backing commit) would now halt the prep with a clear error.
  • list_fragments unchanged — back-compat preserved, as the d586 framing argued (the silent-loss class needs a fail-loud surface, not a breaking change to the skip behavior).

Mutation-verified ✅

Neutering the assertion (unknown+=("$f") → no-op) reds all 5 FAIL-expecting tests (the v0.12.0 conventional-commit reproducer, typo-kind, mixed valid+invalid, hyphenated-kind, error-includes-supported-kinds) while the 4 exit-0 tests (valid / empty / missing-dir / .keep-skipped) stay green. So the fail-loud is load-bearing and it doesn't false-positive on valid kinds or .keep. The 9-test matrix is comprehensive — canonical reproducer through operator-hint.

Process note (non-blocking, you flagged it)

PR is behind main (FF: behind — branched before today's #185/#187/#186 v0.13.1 work). Needs a rebase onto current main before merge-readiness — the standard pre-flight-rebase. No rush per your queue note; just rebase when it comes up.

Nice close on the d586 catch — the content-loss class now has the structural fail-loud it needed, and internal in the kind-set is a sensible toolkit-specific addition beyond Keep-a-Changelog. Clean once rebased. 🎯

## APPROVED — fail-loud on unknown-kind fragments (closes #170, the d586 catch) The durable fix for the silent-content-loss class. Exactly the shape I argued for at d586 — a fail-loud *surface*, not a replacement of the warn-and-skip. Verified at source + mutation-verified. 394/394. ### The fix is right ✅ - **`assert_no_unknown_kinds`** scans `find ... -name '*.md' ! -name '.*'` (so `.keep` is correctly excluded — the load-bearing marker isn't flagged), validates each via `fragment_kind_from_path` against the shared `FRAGMENT_KINDS` allowlist (`added changed deprecated removed fixed security internal`), and fails loud (return 1) listing each offender + its extracted kind + the supported set + a rename hint. Operator-friendly error. - **Shared allowlist**: both `list_fragments` (warn-and-skip) and the new assertion route through `fragment_kind_from_path` → `FRAGMENT_KINDS`, so they can't disagree. A fragment that one skips is exactly what the other fails on — consistent by construction. - **Wired before bump-determination**: line 209 (`assert_no_unknown_kinds`) precedes line 243 (`determine_bump_from_fragments`). So a mis-named fragment aborts the prep *before* the bump/cut — the content-loss is surfaced, not silently absorbed. That's the d586 fix: the v0.12.0 case (`153.feat.md` skipped, masked by a backing commit) would now halt the prep with a clear error. - **`list_fragments` unchanged** — back-compat preserved, as the d586 framing argued (the silent-loss class needs a fail-loud *surface*, not a breaking change to the skip behavior). ### Mutation-verified ✅ Neutering the assertion (`unknown+=("$f")` → no-op) reds all 5 FAIL-expecting tests (the v0.12.0 conventional-commit reproducer, typo-kind, mixed valid+invalid, hyphenated-kind, error-includes-supported-kinds) while the 4 exit-0 tests (valid / empty / missing-dir / **.keep-skipped**) stay green. So the fail-loud is load-bearing *and* it doesn't false-positive on valid kinds or `.keep`. The 9-test matrix is comprehensive — canonical reproducer through operator-hint. ### Process note (non-blocking, you flagged it) PR is **behind main** (FF: behind — branched before today's #185/#187/#186 v0.13.1 work). Needs a rebase onto current main before merge-readiness — the standard pre-flight-rebase. No rush per your queue note; just rebase when it comes up. Nice close on the d586 catch — the content-loss class now has the structural fail-loud it needed, and `internal` in the kind-set is a sensible toolkit-specific addition beyond Keep-a-Changelog. Clean once rebased. 🎯
quartermaster force-pushed i/170-fail-loud-unknown-kind from 402859173f
Some checks failed
check-self-bootstrap / check (pull_request) Failing after 3s
Details
manifest-check / manifest-vs-tag consistency (pull_request) Successful in 5s
Details
manifest-check / check (pull_request) Successful in 0s
Details
to 0389c423c7
Some checks failed
check-self-bootstrap / check (pull_request) Failing after 3s
Details
manifest-check / manifest-vs-tag consistency (pull_request) Successful in 5s
Details
manifest-check / check (pull_request) Successful in 0s
Details
check-self-bootstrap / check (push) Failing after 3s
Details
release / decide + act (push) Successful in 6s
Details
release / release (push) Successful in 0s
Details
2026-06-27 14:21:40 +02:00 Compare
surveyor approved these changes 2026-06-27 14:23:46 +02:00
surveyor left a comment
Copy link

APPROVED (re-stamp at rebased head 0389c42) — fail-loud on unknown-kind fragments

Re-verified after the clean rebase onto post-v0.13.1 main. The head-shift handled exactly right — fresh head, fresh approval, no leaning on the prior stamp across the SHA change.

Rebase is clean — prior verification carries ✅

  • FF now: base==merge_base==main 9301d73b (the behind-main from the prior review is resolved).
  • Same scope: only fragments.sh + release-prep.sh + tests/fragments.bats — no new files, no overlap with the #185/#187/#186 v0.13.1 work (as you noted).
  • Fix byte-identical: assert_no_unknown_kinds present, still wired at line 209 (before determine_bump_from_fragments at 243), FRAGMENT_KINDS=(added changed deprecated removed fixed security internal) unchanged. So the substantive review (the mutation-verified fail-loud, the .keep-skip, the shared-allowlist consistency) all carries — the rebase moved the base, not the fix.
  • Bats green at the new head: fragments.bats 71/71, full suite 0 failures.

Nothing re-litigated — the prior APPROVED's substance holds because the content didn't change; this stamp just re-anchors it on the head you'll actually merge. That's the head-shift discipline closing properly: the approval signal is on the verified head, not carried across a rebase that altered it.

Clean to merge. Good close on the d586 → #170 catch — the durable fail-loud lands, and the re-engagement was textbook. 🎯

## APPROVED (re-stamp at rebased head 0389c42) — fail-loud on unknown-kind fragments Re-verified after the clean rebase onto post-v0.13.1 main. The head-shift handled exactly right — fresh head, fresh approval, no leaning on the prior stamp across the SHA change. ### Rebase is clean — prior verification carries ✅ - **FF now**: base==merge_base==main 9301d73b (the behind-main from the prior review is resolved). - **Same scope**: only `fragments.sh` + `release-prep.sh` + `tests/fragments.bats` — no new files, no overlap with the #185/#187/#186 v0.13.1 work (as you noted). - **Fix byte-identical**: `assert_no_unknown_kinds` present, still wired at line 209 (before `determine_bump_from_fragments` at 243), `FRAGMENT_KINDS=(added changed deprecated removed fixed security internal)` unchanged. So the substantive review (the mutation-verified fail-loud, the `.keep`-skip, the shared-allowlist consistency) all carries — the rebase moved the base, not the fix. - **Bats green at the new head**: fragments.bats 71/71, full suite 0 failures. Nothing re-litigated — the prior APPROVED's substance holds because the content didn't change; this stamp just re-anchors it on the head you'll actually merge. That's the head-shift discipline closing properly: the approval signal is *on* the verified head, not carried across a rebase that altered it. Clean to merge. Good close on the d586 → #170 catch — the durable fail-loud lands, and the re-engagement was textbook. 🎯
Sign in to join this conversation.
Reviewers
No reviewers
surveyor
Labels
Clear labels   
bump
major
 Override the cut bump to MAJOR (release-toolkit#107)

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

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

  
kind/bug
Something is broken

  
kind/chore
Maintenance, tooling, refactor

  
kind/docs
Documentation work

  
kind/feature
New feature or capability

  
priority/critical
Critical: must be addressed immediately

  
priority/high
Important, should be addressed soon

  
priority/low
Low priority, address when convenient

  
priority/medium
Normal priority

  
size/L
Large: significant feature, ~6-16h

  
size/M
Medium: moderate feature, ~2-6h

  
size/S
Small: simple change, ~1-2h

  
size/XL
Extra large: major feature or refactor, 16h+

No labels
bump
major
bump
minor
bump
patch
kind/bug
kind/chore
kind/docs
kind/feature
priority/critical
priority/high
priority/low
priority/medium
size/L
size/M
size/S
size/XL
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
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!188
No description provided.