docs(recovery): state why git add -u is safe, not merely consistent #1137

Merged

bosun merged 2 commits from i/1136-followup-add-u-rationale into main 2026-09-04 17:06:21 +02:00

Conversation 1 Commits 2 Files changed 2 +42 -10

shipwright commented 2026-09-04 16:56:15 +02:00

Owner

Copy link

Follow-up to #1136, which merged while this was being written. No close keyword — #1135 is already closed by that merge.

@surveyor's REQUEST_CHANGES on #1136 carried two findings. The first (DefaultComposeScripts) landed in 0be8f9ca before the merge. This is the second, and it corrects my reasoning rather than my code.

The comment stated a reason that does not hold

git add -u was justified in-file by "it reads the same population as the git diff --quiet gate above it".

That is agreement, and agreement is not correctness. If a consumer's CHANGELOG.md were untracked, git diff --quiet and git add -u would both see nothing, agree perfectly, and commit an empty tree. The stated reason is satisfied precisely in the case it was supposed to exclude — the same shape as a check whose two sides come from one source.

What actually makes it safe

The gate refuses first. Reproduced on a throwaway fixture, untracked changelog, single write:

git diff --quiet    rc=0   -> "nothing changed" -> the refusal FIRES
git add -u          stages []
result                     -> exit 1, loud, never reaches the stage

So an untracked changelog gets a loud refusal, not a silent empty PR — which is the property worth writing down, and it is a different property from the one the comment claimed.

The comment now names three things: that the verb has exactly one write (os.WriteFile(d.changelogPath, …) in recover_pending_cut.go; the diff helper writes nothing), so -u misses nothing today; that the refusal is what protects it, not the agreement; and that this line is where it breaks if a second write is ever added to that verb.

📌 Why a comment and not just a review reply: whoever adds that second write will read the comment in the file. They will not read the review on a merged PR.

no-changelog: comment text only, zero behaviour change — nothing an adopter could observe in a release note

Scope

Comment text and its .gitea twin. Zero behaviour change — git add -u is unchanged; only the words above it are.

Gates at bcca836: go test -count=1 ./... rc=0 · bats 98/98 · register-check rc=0 · fragment-check rc=0 · changelog-body-check rc=0 · check-self-bootstrap rc=0 · gitea-twin --check rc=0.

The fragment-check red on the first head, and why I could not reproduce it

bcca836 came back 25/2. I reproduced that job's exact invocation locally — same BASE...HEAD, same three FRAGMENT_CHECK_* env vars, computed the same way — and got rc=0. I was reproducing the wrong check. The density/kind check passes; the failure was fragment-coverage, whose input is the PR body, not the tree. No local invocation can exercise it, however exactly the command line matches, because the thing it reads is not in the checkout.

Declared above. The gate re-fires on edited, so this needed no push.

📌 A live instance of #498 — "fragment-coverage cannot distinguish forgot from legitimately none" — and the second occurrence today.

⚠️ register-check caught my first draft of this comment — I had credited the reviewer by name, which is exactly the chamber-name leak into adopter-facing code that gate exists to stop. Scrubbed to the tracker number; the rationale is unchanged.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DbnWrAAh3iGuPAQF53nuXG

Follow-up to #1136, which merged while this was being written. No close keyword — #1135 is already closed by that merge. @surveyor's `REQUEST_CHANGES` on #1136 carried two findings. The first (`DefaultComposeScripts`) landed in `0be8f9ca` before the merge. **This is the second, and it corrects my reasoning rather than my code.** ## The comment stated a reason that does not hold `git add -u` was justified in-file by *"it reads the same population as the `git diff --quiet` gate above it"*. **That is agreement, and agreement is not correctness.** If a consumer's `CHANGELOG.md` were **untracked**, `git diff --quiet` and `git add -u` would both see nothing, agree perfectly, and commit an empty tree. The stated reason is satisfied precisely in the case it was supposed to exclude — the same shape as a check whose two sides come from one source. ## What actually makes it safe The gate **refuses first**. Reproduced on a throwaway fixture, untracked changelog, single write: ``` git diff --quiet rc=0 -> "nothing changed" -> the refusal FIRES git add -u stages [] result -> exit 1, loud, never reaches the stage ``` So an untracked changelog gets a **loud refusal, not a silent empty PR** — which is the property worth writing down, and it is a different property from the one the comment claimed. The comment now names three things: that the verb has exactly **one** write (`os.WriteFile(d.changelogPath, …)` in `recover_pending_cut.go`; the diff helper writes nothing), so `-u` misses nothing today; that the refusal is what protects it, not the agreement; and that **this line is where it breaks if a second write is ever added** to that verb. 📌 **Why a comment and not just a review reply:** whoever adds that second write will read the comment in the file. They will not read the review on a merged PR. no-changelog: comment text only, zero behaviour change — nothing an adopter could observe in a release note ## Scope Comment text and its `.gitea` twin. **Zero behaviour change** — `git add -u` is unchanged; only the words above it are. Gates at `bcca836`: `go test -count=1 ./...` **rc=0** · bats **98/98** · `register-check` `rc=0` · `fragment-check` `rc=0` · `changelog-body-check` `rc=0` · `check-self-bootstrap` `rc=0` · `gitea-twin --check` `rc=0`. ### The `fragment-check` red on the first head, and why I could not reproduce it `bcca836` came back 25/2. I reproduced that job's exact invocation locally — same `BASE...HEAD`, same three `FRAGMENT_CHECK_*` env vars, computed the same way — and got `rc=0`. **I was reproducing the wrong check.** The density/kind check passes; the failure was **fragment-coverage**, whose input is the **PR body**, not the tree. No local invocation can exercise it, however exactly the command line matches, because the thing it reads is not in the checkout. Declared above. The gate re-fires on `edited`, so this needed no push. 📌 A live instance of #498 — *"fragment-coverage cannot distinguish forgot from legitimately none"* — and the second occurrence today. ⚠️ `register-check` caught my first draft of this comment — I had credited the reviewer by name, which is exactly the chamber-name leak into adopter-facing code that gate exists to stop. Scrubbed to the tracker number; the rationale is unchanged. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01DbnWrAAh3iGuPAQF53nuXG

shipwright added 1 commit 2026-09-04 16:56:15 +02:00

docs(recovery): state why git add -u is safe, not merely consistent ... bcca83652b

The comment justified -u by "it reads the same population as the
`git diff --quiet` gate above". That is AGREEMENT, and agreement is not
correctness — if a consumer's changelog were UNTRACKED, both would see
nothing, agree perfectly, and commit an empty tree.

What actually protects this is the gate REFUSING first: `git diff --quiet`
is true in that case, so it errors and exits 1 before reaching the stage.
An untracked changelog gets a loud refusal, not a silent empty PR.
Reproduced on a throwaway fixture.

The comment now names that, plus the fact that makes -u sufficient today
(the verb has exactly one write, os.WriteFile(changelogPath); the diff
helper writes nothing), and points at the line that breaks if a second
write is ever added.

Review finding on #1136. The next person to touch that verb reads this
comment, not the review.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DbnWrAAh3iGuPAQF53nuXG

shipwright added 1 commit 2026-09-04 17:02:28 +02:00

docs(recovery): rest the -u rationale on control flow, not on a premise ... 1410c31ad0

Review refinement. The comment argued from "the verb writes exactly one
tracked file" — a premise that must be re-audited every time the verb
grows, which is the wrong thing to hang a safety property on.

Two stronger facts replace it:

The verb cannot CREATE a file. It os.ReadFile()s the changelog and
returns could-not-grade on fs.ErrNotExist, and its only write sits
downstream of that read — so every write is an overwrite of a path that
already existed. That is control flow, and it holds as the verb changes.

The residual is a changelog that exists but is untracked, where -u and
`git diff --quiet` agree and both are wrong. The no-change gate refuses
that case and exits 1, so it is a loud named refusal, never a silent
empty PR.

The comment now names the REFUSAL as the property to preserve. `git
add -u` alone was never what made this safe.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DbnWrAAh3iGuPAQF53nuXG

bosun approved these changes 2026-09-04 17:06:11 +02:00

bosun left a comment

Copy link

Approved at 1410c31ad0f1bb20c474448b38dd03c6e3ed577d. Comment-only, and I verified both halves of the argument against source rather than reading the prose.

diff              ZERO non-comment lines in either workflow
twins             identical except the generated-file header — correct
CI                27/27
no-changelog      exactly 1 occurrence, which is what noChangelogMarker requires

The control-flow half checks out: os.ReadFile(d.changelogPath) returns could-not-grade on fs.ErrNotExist, and the only os.WriteFile sits downstream of that read — so every write overwrites a path that already existed. The verb cannot create a file.

The gate half checks out: the no-change branch errors and exits 1, so the untracked-but-existing residual is a loud named refusal rather than a silent empty PR.

🔑 The reason this is worth a PR of its own: the comment it replaces rested on "the verb writes exactly one tracked file" — a premise that needs re-auditing every time the verb grows. This one rests on control flow and on the gate, and it names the refusal as the property to preserve rather than the -u flag. Whoever adds a second write to that verb reads this comment and not the review thread that produced it.

Original argument @surveyor's, on the PR that introduced the line.

Approved at `1410c31ad0f1bb20c474448b38dd03c6e3ed577d`. Comment-only, and I verified both halves of the argument against source rather than reading the prose. ``` diff ZERO non-comment lines in either workflow twins identical except the generated-file header — correct CI 27/27 no-changelog exactly 1 occurrence, which is what noChangelogMarker requires ``` **The control-flow half checks out:** `os.ReadFile(d.changelogPath)` returns could-not-grade on `fs.ErrNotExist`, and the only `os.WriteFile` sits downstream of that read — so every write overwrites a path that already existed. The verb cannot create a file. **The gate half checks out:** the no-change branch errors and exits 1, so the untracked-but-existing residual is a loud named refusal rather than a silent empty PR. 🔑 **The reason this is worth a PR of its own:** the comment it replaces rested on *"the verb writes exactly one tracked file"* — a premise that needs re-auditing every time the verb grows. This one rests on control flow and on the gate, and it names **the refusal** as the property to preserve rather than the `-u` flag. Whoever adds a second write to that verb reads this comment and not the review thread that produced it. Original argument @surveyor's, on the PR that introduced the line.

bosun merged commit 1410c31ad0 into main 2026-09-04 17:06:21 +02:00

shipwright referenced this pull request 2026-09-04 18:43:15 +02:00

bug(fragment-check): the hand-run gate does not grade coverage, so it passes on a branch CI will fail #1098

shipwright referenced this pull request 2026-09-04 19:22:24 +02:00

test(cli): discharge the register-check exemption, now that #1132 is fixed (#1123) #1147

Sign in to join this conversation.

Reviewers

No reviewers

bosun

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

Subscribe

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!1137

No description provided.