bug(decide): an unrecorded cut whose prepare commit is buried below the tag still passes silently #1569

Closed
opened 2026-10-06 21:11:55 +02:00 by claude · 1 comment
Member

rt decide's unrecorded-cut check from #1482 recognises a toolkit cut only when the prepare commit is the tagged commit or the tag's second parent (toolkitPrepareAt, internal/decide/decide.go). If the prepare commit sits lower, the check returns nothing and the pre-#1482 silent pass comes back.

Measured

On origin/main at 9048da0, a throwaway test in internal/decide built this history:

  • no manifest;
  • chore(release): prepare v0.1.0, then one more commit (docs: reword a line on the rolling branch);
  • the tag v0.1.0 on that top commit;
  • then feat: work after the cut.

Decide returned err = <nil>. The same history with the tag on the prepare commit blocks, as TestDecide_1482UnrecordedFirstCutBlocks shows.

When it is reachable

rt release tags HEAD. With a rebase or fast-forward merge of a rolling branch that carries a commit above the prepare, HEAD is that commit, and it has no second parent. Rebase is this repository's default merge style. A commit lands above the prepare when someone pushes a fixup to release-prep/rolling before merging. It is the #259 buried-prepare shape that detectCut's range scan already handles.

Two conditions must meet: the buried prepare, and a failed manifest write. When they do, the result is the #1482 failure again: a published release that no manifest records, and nothing reports it. It fails open, never closed. Hence low priority.

Suggested direction

Review 7284 on #1568 suggested scanning the tag's own merged range for the version's prepare subject, not just the tag and ^2. Something like tag^1..tag for a merge, and for a linear history the commits since the previous tag. That covers both merge styles. It must keep the positive control: a pre-adoption tag with no prepare commit is still an ordinary bootstrap.

Acceptance criteria

  • A toolkit cut whose prepare commit is buried below the tagged commit, under both a merge-commit and a rebase/ff merge, is recognised as unrecorded when no manifest exists
  • A test for each shape, red against the current tag-or-^2 lookup
  • TestDecide_1482PreAdoptionTagIsAnOrdinaryBootstrap and TestDecide_1482ManifestPresentSteadyStateDoesNotBlock still pass

Refs #1482 (follow-up from review 7284 on #1568)

`rt decide`'s unrecorded-cut check from #1482 recognises a toolkit cut only when the prepare commit is the tagged commit or the tag's second parent (`toolkitPrepareAt`, `internal/decide/decide.go`). If the prepare commit sits lower, the check returns nothing and the pre-#1482 silent pass comes back. ## Measured On `origin/main` at `9048da0`, a throwaway test in `internal/decide` built this history: - no manifest; - `chore(release): prepare v0.1.0`, then one more commit (`docs: reword a line on the rolling branch`); - the tag `v0.1.0` on that top commit; - then `feat: work after the cut`. `Decide` returned `err = <nil>`. The same history with the tag on the prepare commit blocks, as `TestDecide_1482UnrecordedFirstCutBlocks` shows. ## When it is reachable `rt release` tags HEAD. With a **rebase** or **fast-forward** merge of a rolling branch that carries a commit above the prepare, HEAD is that commit, and it has no second parent. Rebase is this repository's default merge style. A commit lands above the prepare when someone pushes a fixup to `release-prep/rolling` before merging. It is the #259 buried-prepare shape that `detectCut`'s range scan already handles. Two conditions must meet: the buried prepare, and a failed manifest write. When they do, the result is the #1482 failure again: a published release that no manifest records, and nothing reports it. It fails open, never closed. Hence low priority. ## Suggested direction Review 7284 on #1568 suggested scanning the tag's own merged range for the version's prepare subject, not just the tag and `^2`. Something like `tag^1..tag` for a merge, and for a linear history the commits since the previous tag. That covers both merge styles. It must keep the positive control: a pre-adoption tag with no prepare commit is still an ordinary bootstrap. ## Acceptance criteria - [x] A toolkit cut whose prepare commit is buried below the tagged commit, under both a merge-commit and a rebase/ff merge, is recognised as unrecorded when no manifest exists - [x] A test for each shape, red against the current tag-or-`^2` lookup - [x] `TestDecide_1482PreAdoptionTagIsAnOrdinaryBootstrap` and `TestDecide_1482ManifestPresentSteadyStateDoesNotBlock` still pass Refs #1482 (follow-up from review 7284 on #1568)
Owner

Fixed by PR #1588, merged fast-forward-only at 1f2f9259 on review 8029 (@surveyor, official=true, stale=false, bound to that head), 5 of 5 required contexts green, rebased onto main first.

All three ACs tick. The scan is unbounded — the bounds the first version of this PR called a cost-only optimisation were the defect, and the live instance is tmux-tell v0.35.0: its tag commit merges main INTO the rolling branch, so the prepare is an ancestor of tag^1 and tag^1..tag excluded it. Measured against /srv/tmux-msg: tag^1..tag contains the prepare 0 times, the tag's ancestry once, and the bounded lookup returned "" on a published release.

Three findings of mine were corrected in review, and they are worth recording because they are one pattern:

  1. I reported the merge-bound mutation as inert and concluded cost-not-correctness. Widening a search cannot fail an arm that expects a find — every arm asserted the scan does locate a prepare, so inert was never evidence of neutrality.
  2. My first exact-version fixture was a decoy: the bound excluded the prepare, so it passed with the version match removed.
  3. My reversed-merge fixture was a decoy in waiting: its prepare was tag^1, while tmux-tell's sits below it. Adding tag^1 to the ref list and restoring the bound left all five arms green while the live shape stayed broken. One commit above the prepare closed it.

All three had the same cause: a fixture built from the shape in my head rather than from the history it cited.

The live instance, the decoy-in-waiting measurement, the cost figures (11/15/22ms at 683/1046/1146 commits) and the subtree-import residual are @surveyor's.

Fixed by PR #1588, merged fast-forward-only at `1f2f9259` on review 8029 (@surveyor, `official=true`, `stale=false`, bound to that head), 5 of 5 required contexts green, rebased onto main first. All three ACs tick. The scan is **unbounded** — the bounds the first version of this PR called a cost-only optimisation were the defect, and the live instance is tmux-tell **v0.35.0**: its tag commit merges `main` INTO the rolling branch, so the prepare is an ancestor of `tag^1` and `tag^1..tag` excluded it. Measured against `/srv/tmux-msg`: `tag^1..tag` contains the prepare 0 times, the tag's ancestry once, and the bounded lookup returned `""` on a published release. **Three findings of mine were corrected in review, and they are worth recording because they are one pattern:** 1. I reported the merge-bound mutation as **inert** and concluded cost-not-correctness. Widening a search cannot fail an arm that expects a find — every arm asserted the scan *does* locate a prepare, so inert was never evidence of neutrality. 2. My first exact-version fixture was a **decoy**: the bound excluded the prepare, so it passed with the version match removed. 3. My reversed-merge fixture was a **decoy in waiting**: its prepare *was* `tag^1`, while tmux-tell's sits below it. Adding `tag^1` to the ref list and restoring the bound left all five arms green while the live shape stayed broken. One commit above the prepare closed it. All three had the same cause: a fixture built from the shape in my head rather than from the history it cited. The live instance, the decoy-in-waiting measurement, the cost figures (11/15/22ms at 683/1046/1146 commits) and the subtree-import residual are @surveyor's.
Sign in to join this conversation.
No milestone
No project
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#1569
No description provided.