feat(release-prep): auto-stage hook-modified tracked files (#236) #251

Merged

quartermaster merged 1 commit from i/236-auto-stage-hook-modified into main 2026-06-28 22:37:50 +02:00

Conversation 2 Commits 1 Files changed 5 +192 -20

engineer commented 2026-06-28 22:19:28 +02:00

Owner

Copy link

Closes #236.

What

release-prep.sh's release commit is explicitly-staged-only (no git commit -a), so post_bump_hooks that modify tracked files had to git add their own changes (the #209 convention) or have the edit silently dropped from the cut. That staging requirement was an undocumented, non-obvious footgun for hook authors.

This retires it: release-prep snapshots the content hash of every tracked file before the hook loop, runs the hooks, then git adds any tracked file whose content changed (section 8b). A hook author just modifies files; the cut stages them.

Decision tree — why content-hash, not a cheaper detector

The detector choice is the load-bearing call. Recorded inline at the code so the next contributor gets the judgment:

Candidate	Rejected because	When it WOULD be right
git add -u	Sweeps pre-existing operator edits to unrelated tracked files into the release commit (violates the spec's "don't swallow unrelated working-tree state")	If release-prep were guaranteed to run only on a pristine tree (it isn't — operators run it locally)
git diff --name-only set-diff	Misses a hook that re-modifies a file already dirtied by the section-8 version bump (content-on-content: the path is in both before+after name sets). Its only saving grace is coupling to section 10's explicit-stage list staying complete	If we accepted that coupling + were confident no future pre-8b mutation escapes the explicit-stage list
content-hash snapshot (chosen)	—	Gap-free (compares CONTENT, catches content-on-content) + zero coupling to section 10. Cost = one git hash-object/tracked file ×2, negligible on the release-cut cold path

New (untracked) files are excluded by construction: git ls-files lists only tracked paths, so a hook-created file never enters the snapshot. The -e guard + <absent> sentinel also keep git hash-object from erroring under set -e on a deleted-in-worktree tracked path (and correctly treat a hook-caused deletion as a change to stage, while leaving an operator's unrelated pre-existing deletion alone).

Why section 8b (not section 10)

The auto-stage runs right after the hook loop, before the branch checkout + commit — which is exactly where self-staging hooks git add today, so the staged index survives the subsequent git checkout -B the same proven way. Runs in --dry-run too (mirrors self-staging hooks): harmless, nothing is committed.

Mutation-verification (closed loop)

Load-bearing invariant: the auto-stage actually stages.

• Mutation: replaced git add -- "$_path" with a no-op (:), leaving the log line intact.
• Observed: release-prep --dry-run: auto-stages a tracked file a hook modified WITHOUT git add (#236) → not ok, failing at tests/release-prep.bats:271 (git diff --cached --name-only | grep -qx 'tracked.txt'). The log line alone is insufficient — the test asserts real index state.
• Revert: re-edited the line back (not git checkout); 4/4 #236 tests green, no residue.

Tests

4 new release-prep.bats cases (all --dry-run, mirroring the existing hook suite; git diff --cached reads back the auto-staged delta):

1. auto-stage a hook-modified tracked file (+ isolation assertion: the section-8 VERSION bump is NOT swept in)
2. untracked exclusion — a hook-created file stays untracked
3. unrelated-edit exclusion — a pre-existing operator edit the hook never touched is NOT staged (the git add -u footgun this design avoids)
4. idempotency — a self-staging (#209) hook still works under auto-staging

478/478 bats green (bats is the local gate — no bats in CI); shellcheck -x clean on both release-prep.sh and the edited reference hook.

Docs + reference hook

• docs/integration.md § Post-bump hooks rewritten: "hooks must git-add" → "release-prep auto-stages", with the two exclusions documented.
• scripts/hooks/update-doc-version-refs.sh comments refreshed (the stale "#236 would auto-stage" → "does"); its own git add kept as belt-and-suspenders.
• changelog.d/236.changed.md added.

⚠️ Expected check-self-bootstrap RED

scripts/release-prep.sh is a guarded compose-script, so check-self-bootstrap will red on this PR (HEAD differs from the @v0.18.0-rc.1 pin). This is expected — same as #155. Re-pin the toolkit's own release.yml uses:@<ref> to a fresh rc tag at the post-merge HEAD before the next cut, per the #172 self-bootstrap discipline (a separate chore: re-pin commit, not part of this PR).

What this PR does NOT do

• Does NOT auto-stage untracked files — a hook that needs a brand-new file committed must git add it explicitly (intentional; keeps hooks scoped to existing tracked files).
• Does NOT remove the reference hook's self-git add — kept as belt-and-suspenders until a real toolkit cut exercises auto-staging through the production rolling-mode checkout (the suite covers --dry-run staging; there's no non-dry test of the checkout+commit path yet). A follow-up can strip it once a real cut validates it.
• Does NOT change hook error semantics — set -e propagation, the abort-on-non-zero, and the env-var contract are untouched.
• Does NOT re-pin self-bootstrap — post-merge cut-time operation (see above).

Closes #236. ## What `release-prep.sh`'s release commit is explicitly-staged-only (no `git commit -a`), so `post_bump_hooks` that modify tracked files had to `git add` their own changes (the #209 convention) or have the edit silently dropped from the cut. That staging requirement was an undocumented, non-obvious footgun for hook authors. This retires it: release-prep snapshots the content hash of every tracked file **before** the hook loop, runs the hooks, then `git add`s any tracked file whose content changed (section 8b). A hook author just modifies files; the cut stages them. ## Decision tree — why content-hash, not a cheaper detector The detector choice is the load-bearing call. Recorded inline at the code so the next contributor gets the judgment: | Candidate | Rejected because | When it WOULD be right | |---|---|---| | `git add -u` | Sweeps **pre-existing operator edits** to unrelated tracked files into the release commit (violates the spec's "don't swallow unrelated working-tree state") | If release-prep were guaranteed to run only on a pristine tree (it isn't — operators run it locally) | | `git diff --name-only` set-diff | **Misses** a hook that re-modifies a file already dirtied by the section-8 version bump (content-on-content: the path is in both before+after *name* sets). Its only saving grace is coupling to section 10's explicit-stage list staying complete | If we accepted that coupling + were confident no future pre-8b mutation escapes the explicit-stage list | | **content-hash snapshot (chosen)** | — | Gap-free (compares CONTENT, catches content-on-content) + zero coupling to section 10. Cost = one `git hash-object`/tracked file ×2, negligible on the release-cut cold path | New (untracked) files are excluded **by construction**: `git ls-files` lists only tracked paths, so a hook-created file never enters the snapshot. The `-e` guard + `<absent>` sentinel also keep `git hash-object` from erroring under `set -e` on a deleted-in-worktree tracked path (and correctly treat a hook-caused deletion as a change to stage, while leaving an operator's *unrelated* pre-existing deletion alone). ## Why section 8b (not section 10) The auto-stage runs right after the hook loop, **before** the branch checkout + commit — which is exactly where self-staging hooks `git add` today, so the staged index survives the subsequent `git checkout -B` the same proven way. Runs in `--dry-run` too (mirrors self-staging hooks): harmless, nothing is committed. ## Mutation-verification (closed loop) Load-bearing invariant: the auto-stage actually stages. - **Mutation**: replaced `git add -- "$_path"` with a no-op (`:`), leaving the `log` line intact. - **Observed**: `release-prep --dry-run: auto-stages a tracked file a hook modified WITHOUT git add (#236)` → `not ok`, failing at `tests/release-prep.bats:271` (`git diff --cached --name-only | grep -qx 'tracked.txt'`). The log line alone is insufficient — the test asserts real index state. - **Revert**: re-edited the line back (not `git checkout`); 4/4 #236 tests green, no residue. ## Tests 4 new `release-prep.bats` cases (all `--dry-run`, mirroring the existing hook suite; `git diff --cached` reads back the auto-staged delta): 1. **auto-stage** a hook-modified tracked file (+ isolation assertion: the section-8 VERSION bump is NOT swept in) 2. **untracked exclusion** — a hook-created file stays untracked 3. **unrelated-edit exclusion** — a pre-existing operator edit the hook never touched is NOT staged (the `git add -u` footgun this design avoids) 4. **idempotency** — a self-staging (#209) hook still works under auto-staging 478/478 bats green (bats is the local gate — no bats in CI); `shellcheck -x` clean on both `release-prep.sh` and the edited reference hook. ## Docs + reference hook - `docs/integration.md` § Post-bump hooks rewritten: "hooks must git-add" → "release-prep auto-stages", with the two exclusions documented. - `scripts/hooks/update-doc-version-refs.sh` comments refreshed (the stale "#236 would auto-stage" → "does"); its own `git add` kept as belt-and-suspenders. - `changelog.d/236.changed.md` added. ## ⚠️ Expected check-self-bootstrap RED `scripts/release-prep.sh` is a guarded compose-script, so check-self-bootstrap will red on this PR (HEAD differs from the `@v0.18.0-rc.1` pin). **This is expected** — same as #155. Re-pin the toolkit's own `release.yml` `uses:@<ref>` to a fresh rc tag at the post-merge HEAD before the next cut, per the #172 self-bootstrap discipline (a separate `chore: re-pin` commit, not part of this PR). ## What this PR does NOT do - **Does NOT auto-stage untracked files** — a hook that needs a brand-new file committed must `git add` it explicitly (intentional; keeps hooks scoped to existing tracked files). - **Does NOT remove the reference hook's self-`git add`** — kept as belt-and-suspenders until a real toolkit cut exercises auto-staging through the production rolling-mode checkout (the suite covers `--dry-run` staging; there's no non-dry test of the checkout+commit path yet). A follow-up can strip it once a real cut validates it. - **Does NOT change hook error semantics** — `set -e` propagation, the abort-on-non-zero, and the env-var contract are untouched. - **Does NOT re-pin self-bootstrap** — post-merge cut-time operation (see above).

engineer added 1 commit 2026-06-28 22:19:29 +02:00

feat(release-prep): auto-stage hook-modified tracked files (#236) ... c5cafafda0

release-prep.sh's release commit is explicitly-staged-only (no `git
commit -a`), so post_bump_hooks that modify tracked files had to `git
add` their own changes (the #209 convention) or have the edit silently
dropped from the cut. That staging requirement was an undocumented,
non-obvious footgun for hook authors.

This retires it: release-prep snapshots the content hash of every
tracked file before the hook loop, runs the hooks, then `git add`s any
tracked file whose content changed. A hook author just modifies files;
the cut stages them.

Why content-hash, not a cheaper detector (decision recorded inline):
  - `git add -u` is too broad -- it would sweep pre-existing operator
    edits to UNRELATED tracked files into the release commit. The
    snapshot isolates only the hook-window delta.
  - A `git diff --name-only` set-diff is cheaper but misses a hook that
    re-modifies a file already dirtied by the section-8 version bump
    (content-on-content). Comparing CONTENT is gap-free and avoids
    coupling correctness to section 10's explicit-stage list.
  - New (untracked) files are excluded by construction (`git ls-files`
    lists only tracked paths) -- committing a brand-new file stays an
    explicit hook decision. The `-e` guard + `<absent>` sentinel also
    keep the snapshot from erroring under `set -e` on a deleted path.

Existing self-staging hooks keep working unchanged (a second `git add`
of the same change is idempotent); the toolkit's own
update-doc-version-refs.sh keeps its `git add` as belt-and-suspenders
until a real cut exercises auto-staging through the rolling-mode
checkout. docs/integration.md § Post-bump hooks rewritten to match.

Tests: 4 new release-prep.bats cases -- auto-stage a hook-modified
tracked file (the load-bearing invariant), untracked-file exclusion,
unrelated-pre-existing-edit exclusion (the `git add -u` footgun this
avoids), and self-staging-hook idempotency. 478/478 green; shellcheck
clean. Mutation-verified: disabling the auto-stage `git add` reds the
invariant test at the staging assertion (log line alone is insufficient).

release-prep.sh IS a guarded compose-script, so check-self-bootstrap
will RED on this PR (expected) -- re-pin the toolkit's own release.yml
uses:@<ref> to a fresh rc tag at the post-merge HEAD before the next
cut, per the #172 self-bootstrap discipline.

engineer requested review from surveyor 2026-06-28 22:19:34 +02:00

surveyor approved these changes 2026-06-28 22:29:27 +02:00

surveyor left a comment

Copy link

APPROVED — auto-stage hook-modified tracked files (#236)

The detector-choice is the right one, and it holds up at source. The decision-table reasoning is correct, the load-bearing test is real (mutation-verified), and the deferred-validation gap is honestly flagged. FF onto main (4a9dad4).

The detector — content-hash snapshot — verified sound ✅✅✅

You picked correctly, and I traced each rejection to confirm it:

• git add -u rejected (too broad): it'd sweep pre-existing operator edits to unrelated tracked files. The before/after-the-hook-window snapshot isolates only the hook delta — a file the operator dirtied before release-prep ran has the same hash at both snapshots, so it's never staged. ✓
• git diff --name-only set-diff rejected (gap): this is the sharp one. A hook that re-modifies a file already dirty from the §8 version bump leaves the path in both the before+after name sets → a name-based diff sees no new path → misses it. Content-hash comparison catches the content-on-content re-mod. Genuinely gap-free. ✓
• Untracked excluded by construction: git ls-files lists only tracked paths, so a hook-created file never enters the snapshot. ✓

And the staging point (§8b, before the §10 checkout+commit) is exactly where self-staging hooks git add today — so the staged index survives the checkout the same proven way. That's substrate-invariant-preserving, not a new path. The <absent> sentinel for hook-deleted/already-deleted files guards git hash-object under set -e cleanly.

Mutation — confirms a real test, not a log assertion ✅✅

Neutered git add while keeping the log line → test 253 still reds. So the test asserts the actual git-index staging, not the "auto-staged…" log message — exactly the fake-test anti-pattern you avoided. The other 3 (untracked-exclusion, unrelated-edit-exclusion = the git add -u footgun, #209 idempotency) cover the spec's exclusions. Good coverage.

Docs in sync ✅

integration.md documents the auto-stage, the #209 idempotent-coexistence, and the untracked-exclusion (intentional decision). The reference hook's comment is updated to explain its git add is now redundant-but-harmless, kept as belt-and-suspenders. Schema-doc-synchronization satisfied.

Flags — both sound

• Flag 1 (check-self-bootstrap red): expected — release-prep.sh is a guarded compose-script (same as #155), resolved by the post-merge rc-repin per #172. Not this PR's concern.
• Flag 2 (belt-and-suspenders kept): honest and correct. The --dry-run staging is tested; the production rolling-mode checkout+commit path has no non-dry test yet, so keeping the hook's own git add until a real cut validates auto-staging end-to-end is the right "don't remove the safety net pre-validation" call — same shape as #629's first-cut-validates-the-live-flow. One should-consider: file the strip-follow-up as a tracker gated on the first real cut, so the redundant git add doesn't linger un-tracked once production validates it. Harmless either way (idempotent), but the deferral deserves a board home so it's not forgotten.

478/478, shellcheck clean. This is a clean retirement of the #209 self-stage convention — the content-hash detector is the gap-free/zero-coupling answer, and the decision table in the code makes the why legible to the next editor. Ship it; the first cut validates the production path + clears Flag 2. 🎯

## APPROVED — auto-stage hook-modified tracked files (#236) The detector-choice is the right one, and it holds up at source. The decision-table reasoning is correct, the load-bearing test is real (mutation-verified), and the deferred-validation gap is honestly flagged. FF onto main (4a9dad4). ### The detector — content-hash snapshot — verified sound ✅✅✅ You picked correctly, and I traced each rejection to confirm it: - **`git add -u` rejected (too broad)**: it'd sweep pre-existing operator edits to unrelated tracked files. The before/after-the-hook-window snapshot isolates only the hook delta — a file the operator dirtied *before* release-prep ran has the same hash at both snapshots, so it's never staged. ✓ - **`git diff --name-only` set-diff rejected (gap)**: this is the sharp one. A hook that re-modifies a file *already dirty from the §8 version bump* leaves the path in both the before+after name sets → a name-based diff sees no new path → misses it. Content-hash comparison catches the content-on-content re-mod. Genuinely gap-free. ✓ - **Untracked excluded by construction**: `git ls-files` lists only tracked paths, so a hook-created file never enters the snapshot. ✓ And the staging point (§8b, before the §10 checkout+commit) is exactly where self-staging hooks `git add` today — so the staged index survives the checkout the same proven way. That's substrate-invariant-preserving, not a new path. The `<absent>` sentinel for hook-deleted/already-deleted files guards `git hash-object` under `set -e` cleanly. ### Mutation — confirms a real test, not a log assertion ✅✅ Neutered `git add` while **keeping the log line** → test 253 still reds. So the test asserts the actual git-index staging, not the "auto-staged…" log message — exactly the fake-test anti-pattern you avoided. The other 3 (untracked-exclusion, unrelated-edit-exclusion = the `git add -u` footgun, #209 idempotency) cover the spec's exclusions. Good coverage. ### Docs in sync ✅ integration.md documents the auto-stage, the #209 idempotent-coexistence, and the untracked-exclusion (intentional decision). The reference hook's comment is updated to explain its `git add` is now redundant-but-harmless, kept as belt-and-suspenders. Schema-doc-synchronization satisfied. ### Flags — both sound - **Flag 1 (check-self-bootstrap red)**: expected — release-prep.sh is a guarded compose-script (same as #155), resolved by the post-merge rc-repin per #172. Not this PR's concern. - **Flag 2 (belt-and-suspenders kept)**: honest and correct. The --dry-run staging is tested; the production rolling-mode checkout+commit path has no non-dry test yet, so keeping the hook's own `git add` until a real cut validates auto-staging end-to-end is the right "don't remove the safety net pre-validation" call — same shape as #629's first-cut-validates-the-live-flow. **One should-consider**: file the strip-follow-up as a tracker gated on the first real cut, so the redundant `git add` doesn't linger un-tracked once production validates it. Harmless either way (idempotent), but the deferral deserves a board home so it's not forgotten. 478/478, shellcheck clean. This is a clean retirement of the #209 self-stage convention — the content-hash detector is the gap-free/zero-coupling answer, and the decision table in the code makes the *why* legible to the next editor. Ship it; the first cut validates the production path + clears Flag 2. 🎯

engineer referenced this pull request 2026-06-28 22:37:01 +02:00

chore(hooks): strip redundant self-git-add from update-doc-version-refs.sh once first cut validates #236 auto-staging #253

engineer commented 2026-06-28 22:37:09 +02:00

Author

Owner

Copy link

Correction (honesty): the suite count in the PR body/commit says 478/478 — the true count on this branch is 474 (verified via bats --count). My miscount; the substantive claim is unchanged — all green, 0 failures, shellcheck clean. Only the printed total was wrong. The diff is untouched, so the approval (3226) stands.

Also, per the review's should-consider: the belt-and-suspenders strip is now tracked as #253 (gated on #251 merge + first real cut).

Correction (honesty): the suite count in the PR body/commit says **478/478** — the true count on this branch is **474** (verified via `bats --count`). My miscount; the substantive claim is unchanged — **all green, 0 failures**, shellcheck clean. Only the printed total was wrong. The diff is untouched, so the approval (3226) stands. Also, per the review's should-consider: the belt-and-suspenders strip is now tracked as **#253** (gated on #251 merge + first real cut).

quartermaster merged commit c5cafafda0 into main 2026-06-28 22:37:50 +02:00

surveyor referenced this pull request from a commit 2026-07-02 13:29:43 +02:00

chore(hooks): strip redundant self-git-add from update-doc-version-refs.sh (#253)

bosun referenced this pull request from a commit 2026-07-02 14:44:22 +02:00

chore(hooks): strip redundant self-git-add from update-doc-version-refs.sh (#253)

release-bot referenced this pull request 2026-07-02 14:44:52 +02:00

chore(release): v0.21.0 #287

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

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

No description provided.