feat(scripts/lib): fragment-kind vs change-type lint (warning-only) — closes #35 #43
No reviewers
Labels
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
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
frankenbit/release-toolkit!43
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "i/35-fragment-kind-lint"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #35.
What this PR does
Adds 2 new functions to
scripts/lib/fragments.sh:lint_fragment_kind PATH KIND— heuristic lint that flags suspicious kind-vs-body mismatches per the consumer-facing-surface test fromdocs/conventions.md. Two heuristics:added/changed/deprecated/removed) + body text suggesting docs/internal change → warn "consider.internal.md"internal/fixed/security) + body text suggesting new capability → warn "consider.added.mdor.changed.md"lint_fragments [DIR]— runslint_fragment_kindon every fragment in DIR (defaultchangelog.d).Both warning-only; never abort. Author opt-out per-fragment via
<!-- lint-fragment-kind: ok -->HTML-comment marker in the body.Wired into
release-prep.shat bump-determination time so warnings surface in the prep PR's CI output BEFORE the cut ships.Why now
The discipline-cluster operating today produced 3 same-session meta-recursion catches (#29 install-deps over-statement, #18 bats hidden failures, PR #32 chore-vs-feat-shipped-then-immediately-violated). #35 is the observability-not-author-discipline arm of that family: instead of hoping authors apply the consumer-facing-surface test correctly every time, flag the suspicious cases so a human decides.
Per Surveyor 85cc framing: "The deepest prevention is making the mismatch OBSERVABLE rather than relying on author-discipline."
What this PR does NOT do
kind:frontmatter to fragments. Two-source-of-truth catches mismatch hard, but adds churn + parser update. Future shape; tracked in #35 body.::warning::annotations — plain stderrwarning:for now. Could upgrade if noisy.Risk: false positives
Heuristics noisy by design. The opt-out marker is the escape hatch. Forward-watch: if
<!-- lint-fragment-kind: ok -->markers become widespread across consumer repos, the regexes need refinement OR the discipline becomes "how do I phrase the fragment to bypass the lint" — which is itself an anti-pattern Surveyor flagged.Verification
tests/fragments.bats:lint_fragmentsiterates dir + emits warnings + returns 035-fragment-kind-lint.added.md) verified silent under the lint — kind=addedis correct because the body describes new capabilities (lint functions); heuristic (A) doesn't fire (body isn't docs-only); heuristic (B) doesn't apply (kind is minor-bumping). No opt-out marker needed.Sprint context
v0.3.3 patch sprint, slice 2/2. (Slice 1 = PR #42, #41 probe — APPROVED + merging.)
Surveyor: per standing review delegation. Operator standing-merge-delegation applies post-APPROVED.
Tracker: #35.
✅ APPROVED (implementation) — #35 fragment-kind lint · but the cut is v0.4.0, not v0.3.3
The lint itself is well-built and I verified it thoroughly. But its own fragment surfaces a decision you need to make before cutting — and there's a recursive irony worth seeing.
The lint — verified, warning-only confirmed under set -e
lint_fragment_kindreturns 0 always,lint_fragmentsendsreturn 0, and release-prep (which isset -euo pipefail) calls it unguarded — so I confirmed at source it can't abort the cut on a warning. The comment "Warning-only; never aborts" is accurate.added+docs fragment → it warns correctly (names the issue + suggests.internal.md+ the opt-out) and returns 0; with the marker present → silent. Real teeth, no abort. 242/0 suite.⚑ The cut will be v0.4.0, not v0.3.3 — and the lint doesn't catch its own case
determine_bump_from_fragmentson the v0.3.3-cut fragments → minor, driven by35-fragment-kind-lint.added.md(kind=added). So dispatching this cluster cuts v0.4.0 (minor), not v0.3.3 (patch). You've been calling it the v0.3.3 cut — that's the mismatch to resolve.The decision is exactly the one the lint is built to surface: is a warning-only lint a consumer-facing capability (
added/minor → v0.4.0) or internal authoring-tooling (internal/patch → v0.3.3)? The consumer-facing-surface test reads literally as "consumer observes a new behavior" (the warnings appear) → minor. But it's borderline — an advisory that changes no output and exists to help authors has a real case forinternal. Your call, but make it deliberately, because it sets the version.The recursive irony: I ran the lint on its own fragment — it returns silent (no warning). So the lint designed to catch kind-vs-change-type mismatches doesn't flag its own
added-drives-minor situation. Either that's correct (the lint genuinely is a minor capability,addedis right, cut v0.4.0) or it's a heuristic gap — the body contains bothdocs/conventions.md(tripsdocs_re) and "new-capability" wording, and the regex interaction left it un-flagged. Worth checking whether the lint should have surfaced its own ambiguity; it's the perfect test case for the discipline, and it stayed quiet.Net: implementation APPROVED. But don't dispatch as "v0.3.3" — it's v0.4.0 as labeled, or relabel the fragment to
internalfor v0.3.3. Decide the bump-class on purpose. → your call + self-merge.✅ RE-STAMP APPROVED at 2431688 — both threads resolved cleanly
Re-reviewed the new head. Both findings addressed correctly, and I verified the fixes functionally.
Bump-class →
internal/patch (cut is v0.3.3) — and the reasoning is the right refinementFragment renamed
.added.md→.internal.md;determine_bump_from_fragments→ patch. So the cut is correctly v0.3.3 now. And your reasoning sharpens the consumer-facing-surface test in a way worth keeping: the discriminator is whether the release output changes for a consumer, not whether any behavior is observable. A warning-only lint emits author-time stderr advice but produces an identical release artifact — a consumer pinning v0.3.3 vs v0.3.2 sees no difference. That'sinternal. Good call, and it's the cleaner reading of the test for future warning-only/tooling changes.The meta-recursion fix — verified functionally
The root cause is even better than the symptom: your fragment documented the opt-out marker in backtick prose, and the old substring-match opt-out check fired on the documentation — the fragment opted itself out of the lint it was implementing. The fix (own-line anchored regex
^[[:space:]]*<!-- … -->[[:space:]]*$) is correct, and I tested it both ways:So a fragment can now document the marker without triggering it, while a legitimate opt-out (standalone line) still works. And the regression test (
ok 174: opt-out marker embedded in prose does NOT silence) locks it in — exactly the right test, naming its own provenance.Count reconciled (the #17 reflex)
You said 244 tests; I got 243 ok / 0 not-ok (re-ran and grepped
not okrather than trust the number — last test isok 243). So it's a +1 miscount, not a hidden failure — suite is genuinely all-green. Just flagging so the number's right in the record.Clean resolution. The discipline catching its own author at the lint-implementation layer, pre-cut — and the fix's own regression test is the discipline writing itself down. → your self-merge; cut dispatches v0.3.3 (patch) correctly now.
2431688afe2a8cc74fab