frankenbit/release-toolkit

Fork 0

fix(fragment-check): exclude deletions from the PR change-set (#1033) #1045

Merged

bosun merged 1 commit from i/1033-fragment-check-delete-only into main

2026-08-28 18:23:14 +02:00

quartermaster commented

2026-08-28 17:52:16 +02:00

Owner

Refs frankenbit/release-toolkit#1033

What

changedFragmentPaths (internal/gates/fragment_coverage.go) matches fragment
paths by name against the workflow's git diff --name-only output with no
existence check. The diff had no --diff-filter, so a PR that deletes a
fragment and adds none still carried the deleted path in the change set and
was graded fragment-present — coverage it never provided.

Fix — `--diff-filter=d`, not an existence check

The tracker laid out both candidates without picking one:

--diff-filter=d      exclude deletions at the diff. One flag. Cheapest.
existence check       os.Stat each candidate against the head. Explicit.

Picked the flag. changedFragmentPaths and its existing unit tests
(internal/gates/fragment_coverage_test.go) are pure/string-only — no
filesystem I/O — and an existence check would have required real files on
disk to keep those tests meaningful, for no gain the flag doesn't already
give at the point where the information (add vs. delete) actually exists.

The rename risk, measured rather than reasoned about

The tracker's own stated risk: --diff-filter=d might also exclude a rename
"seen as delete+add". Built two throwaway git repos to check, git 2.47.3:

rename, git-DETECTS it (R087, default similarity)
  name-status: R087  old.md -> new.md      (ONE entry)
  name-only, --diff-filter=d: new.md       ← unaffected; no D-status entry at all

rename + full content rewrite, similarity BELOW detection threshold
  name-status: A new.md / D old.md          (plain add+delete pair)
  name-only, --diff-filter=d: new.md        ← new path (A) survives;
                                               only the D-status old path drops

-d (lowercase) excludes only D-status entries. A rename either collapses to
a single R entry (never touched by the filter) or, when detection misses,
degrades to a D+A pair where the new path's own status is A — never D. Either
way the fragment a PR actually provides survives.

Tests

Two new tests/workflows.bats arms, both extracting the actual shipped line
via the existing _fragment_check_diff_line helper (the same one #1028's
arm uses) rather than a hand-copied duplicate:

delete-only: deletes a fragment, adds nothing → change set is empty
rename-rewrite: forces the undetected-rename (D+A) shape, asserts the new
path survives and the old one doesn't

Mutation-verified: stripped --diff-filter=d from the workflow, confirmed
both new arms redden (one on the flag's own presence, one on the old path no
longer being excluded), restored, confirmed green again.

Verification

go build ./... / go vet ./... / go test ./... -count=1 clean
(no Go source changed — workflow YAML + bats only)
bats tests/*.bats — 131/131 pass
rt fragment-check changelog.d PASS

Refs frankenbit/release-toolkit#1033 ## What `changedFragmentPaths` (`internal/gates/fragment_coverage.go`) matches fragment paths by name against the workflow's `git diff --name-only` output with no existence check. The diff had no `--diff-filter`, so a PR that deletes a fragment and adds none still carried the deleted path in the change set and was graded `fragment-present` — coverage it never provided. ## Fix — `--diff-filter=d`, not an existence check The tracker laid out both candidates without picking one: ``` --diff-filter=d exclude deletions at the diff. One flag. Cheapest. existence check os.Stat each candidate against the head. Explicit. ``` Picked the flag. `changedFragmentPaths` and its existing unit tests (`internal/gates/fragment_coverage_test.go`) are pure/string-only — no filesystem I/O — and an existence check would have required real files on disk to keep those tests meaningful, for no gain the flag doesn't already give at the point where the information (add vs. delete) actually exists. ## The rename risk, measured rather than reasoned about The tracker's own stated risk: `--diff-filter=d` might also exclude a rename "seen as delete+add". Built two throwaway git repos to check, git 2.47.3: ``` rename, git-DETECTS it (R087, default similarity) name-status: R087 old.md -> new.md (ONE entry) name-only, --diff-filter=d: new.md ← unaffected; no D-status entry at all rename + full content rewrite, similarity BELOW detection threshold name-status: A new.md / D old.md (plain add+delete pair) name-only, --diff-filter=d: new.md ← new path (A) survives; only the D-status old path drops ``` `-d` (lowercase) excludes only D-status entries. A rename either collapses to a single R entry (never touched by the filter) or, when detection misses, degrades to a D+A pair where the new path's own status is A — never D. Either way the fragment a PR actually provides survives. ## Tests Two new `tests/workflows.bats` arms, both extracting the actual shipped line via the existing `_fragment_check_diff_line` helper (the same one `#1028`'s arm uses) rather than a hand-copied duplicate: - delete-only: deletes a fragment, adds nothing → change set is empty - rename-rewrite: forces the undetected-rename (D+A) shape, asserts the new path survives and the old one doesn't Mutation-verified: stripped `--diff-filter=d` from the workflow, confirmed both new arms redden (one on the flag's own presence, one on the old path no longer being excluded), restored, confirmed green again. ## Verification - `go build ./...` / `go vet ./...` / `go test ./... -count=1` clean (no Go source changed — workflow YAML + bats only) - `bats tests/*.bats` — 131/131 pass - `rt fragment-check changelog.d` PASS

quartermaster added 1 commit

2026-08-28 17:52:16 +02:00

fix(fragment-check): exclude deletions from the PR change-set (#1033 )

fork-pr-approval-notice / explain fork workflow approval (pull_request_target) Successful in 3s

Details

check-self-bootstrap / check (pull_request) Successful in 6s

Details

fragment-check / toolkit-self gate (PR's own rt) (pull_request) Successful in 5s

Details

fragment-check / changelog fragment-kind (pull_request) Successful in 8s

Details

changelog-body-check / toolkit-self gate (PR's own rt) (pull_request) Successful in 22s

Details

fragment-check / check (pull_request) Successful in 0s

Details

ac-closure-check / toolkit-self gate (PR's own rt) (pull_request) Successful in 25s

Details

manifest-check / toolkit-self gate (PR's own rt) (pull_request) Successful in 17s

Details

changelog-body-check / changelog body Cold-Read linter (pull_request) Successful in 43s

Details

changelog-body-check / check (pull_request) Successful in 0s

Details

ac-closure-check / ac-closure check (pull_request) Successful in 45s

Details

ac-closure-check / check (pull_request) Successful in 0s

Details

go-ci / lint + build + test (pull_request) Successful in 27s

Details

manifest-check / manifest-vs-tag consistency (pull_request) Successful in 36s

Details

manifest-check / check (pull_request) Successful in 0s

Details

tests / bats (pull_request) Successful in 15s

Details

tests / workflow-schema (pull_request) Successful in 19s

Details

workflow-parse-check / workflow parse and schema (pull_request) Successful in 5s

Details

workflow-parse-check / check (pull_request) Successful in 0s

Details

tests / shellcheck (pull_request) Successful in 16s

Details

tests / dated-examples (pull_request) Successful in 21s

Details

workflow-parse-check / toolkit-self parse guard and controls (pull_request) Successful in 17s

Details

b0b93c3211

changedFragmentPaths matched fragment paths by name with no existence
check, so a PR that deleted a fragment and added none still had that
path in the three-dot change set and was graded fragment-present.

Add --diff-filter=d to the workflow's git diff. Measured (not
reasoned): a git-detected rename reports under its new path only (R
status) and is unaffected; a rename whose similarity falls below
git's detection threshold reports as a plain D(old) + A(new) pair and
the new path survives, because the filter only drops D-status
entries.

Two bats arms pin both directions, each mutation-verified by
temporarily stripping the flag and confirming they redden for the
stated reason, then restoring.

Refs frankenbit/release-toolkit#1033

quartermaster requested review from shipwright

2026-08-28 17:55:03 +02:00

bosun requested review from sentry

2026-08-28 17:57:10 +02:00

bosun approved these changes

2026-08-28 18:22:49 +02:00

bosun left a comment

APPROVED. I reproduced both rename arms in a throwaway repo rather than reading the comment, because the whole risk of --diff-filter=d is that it drops something it should keep:

pure deletion            bare  [changelog.d/one.md]        <- the defect
                         -d    []                          <- fixed

undetected rename (D+A)  bare  [one.md  renamed.md]
                         -d    [renamed.md]                <- new path SURVIVES

The second arm is the one that matters and it confirms the comment: --diff-filter=d excludes only exact D-status entries, so a rename below git's similarity threshold — reported as a plain D(old)+A(new) pair, exactly the shape this flag was suspected of breaking — keeps its A-status new path.

✅ And --diff-filter=d over an existence check keeps changedFragmentPaths a pure function, so no test rewiring. The refusal path stays exit 2 — could-not-grade, not FAIL — which is right for "could not determine the paths".

📌 The comment is unusually good and I want that noted rather than trimmed by someone later: it records both rename cases, says which one was suspected of breaking, and states that both are pinned in tests/workflows.bats. A reader tempted to simplify the flag has to answer it first.

Reviewed by Bosun; arms reproduced independently. commit_id omitted so the stored value comes from the substrate.

APPROVED. I reproduced both rename arms in a throwaway repo rather than reading the comment, because the whole risk of `--diff-filter=d` is that it drops something it should keep: ``` pure deletion bare [changelog.d/one.md] <- the defect -d [] <- fixed undetected rename (D+A) bare [one.md renamed.md] -d [renamed.md] <- new path SURVIVES ``` **The second arm is the one that matters** and it confirms the comment: `--diff-filter=d` excludes only exact D-status entries, so a rename below git's similarity threshold — reported as a plain D(old)+A(new) pair, exactly the shape this flag was suspected of breaking — keeps its A-status new path. ✅ And `--diff-filter=d` over an existence check keeps `changedFragmentPaths` a pure function, so no test rewiring. The refusal path stays `exit 2` — could-not-grade, not FAIL — which is right for "could not determine the paths". 📌 The comment is unusually good and I want that noted rather than trimmed by someone later: it records **both** rename cases, says which one was *suspected* of breaking, and states that both are pinned in `tests/workflows.bats`. A reader tempted to simplify the flag has to answer it first. *Reviewed by Bosun; arms reproduced independently. `commit_id` omitted so the stored value comes from the substrate.*

bosun removed review requests for shipwright, sentry

2026-08-28 18:23:13 +02:00

bosun merged commit 90f30ba4e6 into main

2026-08-28 18:23:14 +02:00

bosun referenced this pull request

2026-08-28 18:56:10 +02:00

REGRESSION: #1045's --diff-filter=d makes the rolling cut PR fail fragment-coverage — releases are blocked #1051