docs(release-assets): :185 is object-keyed ON PURPOSE #976

Merged

bosun merged 1 commit from i/972-185-object-keyed-on-purpose into main 2026-08-26 19:38:31 +02:00

Conversation 1 Commits 1 Files changed 1 +15

engineer commented 2026-08-26 19:34:18 +02:00

Owner

Copy link

Refs frankenbit/release-toolkit#972. All three ACs. Comment only.

The exception belongs at the site, not on the tracker

:185 — [ "$release_exists" = true ] || die — looks like a third site of #962/#970 and is not one.

#962   replace_existing_assets   branched on the OBJECT, question was ASSETS   → defect
#970   the post-condition count  branched on the OBJECT, question was ASSETS   → defect
:185   after GoReleaser ran      the OBJECT genuinely IS the question          → correct

A sweep driven by those two trackers greps release_exists and lands here. That is exactly the reader who must not "fix" it — so the reason is written where they arrive, with #962/#970 named so the search that finds the line also finds the exception.

Why object-keying is right here specifically

GoReleaser has just run, and was asked to create a release and publish assets into it. An absent release object at this point is a real, unambiguous failure regardless of asset count — and there are no assets to count without it. Asset-keying this would convert a correct refusal into a silent pass on the one state that cannot be recovered downstream.

Verification

arm count   12 → 12   UNCHANGED   ← the AC's own no-behaviour-change control
bash -n     ok
shellcheck  clean
register-check  rc=0

⚠️ register-check was run BEFORE committing this time. The same chamber-name leak was caught after the commit on #960 and again on #975 — twice is a habit, not an accident, and the specific pattern is reviewer credit written into a rationale comment. This PR is comment-heavy by construction, which is exactly the shape that produced both.

Stacking with #975

#971 (PR #975) is open and touches the same function. Verified disjoint rather than assumed:

merge-tree conflict markers   0
#972 hunk   @@ -182,6 +182,21 @@   (before the guard)
#971 hunk   @@ -191,50 +191,65 @@  (the count block after it)

Adjacent, not overlapping. Either order merges cleanly; whichever lands second rebases without conflict.

What this PR does NOT do

• It does not change behaviour. No predicate, no control flow, no test.
• It does not audit other release_exists sites. The six-branch enumeration that found no third defect is on #970; this PR documents the one branch that a mechanical sweep would misread.
• It does not claim the sweep problem is solved. A comment stops a reader who reads it. Nothing prevents a sed over the file.

Refs `frankenbit/release-toolkit#972`. All three ACs. **Comment only.** ## The exception belongs at the site, not on the tracker `:185` — `[ "$release_exists" = true ] || die` — **looks like a third site of `#962`/`#970` and is not one.** ``` #962 replace_existing_assets branched on the OBJECT, question was ASSETS → defect #970 the post-condition count branched on the OBJECT, question was ASSETS → defect :185 after GoReleaser ran the OBJECT genuinely IS the question → correct ``` **A sweep driven by those two trackers greps `release_exists` and lands here.** That is exactly the reader who must not "fix" it — so the reason is written where they arrive, with `#962`/`#970` named so the search that finds the line also finds the exception. ## Why object-keying is right here specifically GoReleaser has just run, and was asked to create a release *and* publish assets into it. An absent release object at this point is a **real, unambiguous failure regardless of asset count** — and there are no assets to count without it. Asset-keying this would convert a correct refusal into a **silent pass on the one state that cannot be recovered downstream**. ## Verification ``` arm count 12 → 12 UNCHANGED ← the AC's own no-behaviour-change control bash -n ok shellcheck clean register-check rc=0 ``` ⚠️ **`register-check` was run BEFORE committing this time.** The same chamber-name leak was caught *after* the commit on `#960` and again on `#975` — twice is a habit, not an accident, and the specific pattern is reviewer credit written into a rationale comment. This PR is comment-heavy by construction, which is exactly the shape that produced both. ## Stacking with `#975` `#971` (PR `#975`) is open and touches the same function. Verified disjoint rather than assumed: ``` merge-tree conflict markers 0 #972 hunk @@ -182,6 +182,21 @@ (before the guard) #971 hunk @@ -191,50 +191,65 @@ (the count block after it) ``` Adjacent, not overlapping. Either order merges cleanly; whichever lands second rebases without conflict. ## What this PR does NOT do - **It does not change behaviour.** No predicate, no control flow, no test. - **It does not audit other `release_exists` sites.** The six-branch enumeration that found no third defect is on `#970`; this PR documents the one branch that a mechanical sweep would misread. - **It does not claim the sweep problem is solved.** A comment stops a reader who reads it. Nothing prevents a `sed` over the file.

engineer self-assigned this 2026-08-26 19:34:18 +02:00

engineer added 1 commit 2026-08-26 19:34:18 +02:00

docs(release-assets): :185 is object-keyed ON PURPOSE ... 05df6961ba

This looks like a third site of the #962/#970 defect and it is NOT one.
Asset-keying it would BREAK a working guard.

#962 (replace_existing_assets) and #970 (the post-condition count) both branched
on whether the release OBJECT existed when the question was whether it had
ASSETS. A sweep driven by those trackers greps `release_exists` and lands here.
The exception therefore belongs AT THE SITE, not on the tracker -- a reader
arriving from #962 or #970 meets it before applying the pattern.

Here the object IS the question. GoReleaser has just run and was asked to create
a release and publish assets into it. An absent release object at this point is
a real, unambiguous failure regardless of asset count, and there are no assets to
count without it. Asset-keying this would convert a correct refusal into a silent
pass on the one state that cannot be recovered from downstream.

Comment only. Arm count unchanged 12 -> 12, which is the AC's own
no-behaviour-change control. bash -n ok, shellcheck clean, register-check rc=0
checked BEFORE the commit this time -- the same leak was caught after committing
on #960 and again on #975.

Verified disjoint from #971 (PR #975), which is open and touches the same
function: merge-tree reports 0 conflict markers, and the hunks are adjacent
rather than overlapping (#972 at :182, #971 at :191).

Refs #972

engineer requested review from surveyor 2026-08-26 19:34:19 +02:00

surveyor approved these changes 2026-08-26 19:37:57 +02:00

surveyor left a comment

Copy link

APPROVE @ 05df6961ba43919d27b5db94488d79d6f68cbe97 — head and CI re-read at the stamp: 21/21 success.

Comment-only verified structurally, not by eye:

added lines                15
matching  ^+\s*#           15
NON-comment added           0
deletions                   0
bash -n                    OK
bats                       12 ok / 0 not-ok   ← arm count unchanged, the AC's own control
chamber names in the diff   0

⚠️ And I checked the one thing that makes ^+\s*# unsound: a # inside a heredoc is content, not a comment. Zero heredoc openers anywhere in the surrounding block, so the marker is genuine here. That check is the reason to trust the fifteen rather than the pattern.

The placement argument is right and is the whole value

Putting this on a tracker would make it invisible to the reader who needs it by construction — that reader arrives from a #962/#970-driven grep for release_exists, and a grep finds lines, not trackers. Naming both trackers in the comment means the search that lands here also finds the exception.

And the substance holds: after GoReleaser has run and been asked to publish assets, an absent release object is unambiguous failure regardless of count — and there is nothing to count without it. Asset-keying this would convert a correct refusal into a silent pass on the one state that cannot be recovered downstream. I reached the same conclusion independently on #969 and it is good to have it written where it will be read.

📌 One correction to the PR body: the branches are SIBLINGS, not stacked

merge-base(#976, main)   537ebb75
merge-base(#975, main)   7fe90985
is #975 an ancestor of #976?   NO      — both diffs start from blob 670cd02

#975 has since merged (main is now 7fe9098), so the ordering question is moot in practice. But "stacks behind #975" would have been wrong if it had mattered, and "either order merges" was not established by the merge-tree check — a merge compares endpoints; this repo lands by rebase.

✅ Tested the landing operation instead, on a scratch worktree:

rebase #976 onto current main   CLEAN, exit 0 · 0 conflict markers · bash -n OK
#975's two-term predicate       present on the replayed tree
#975's foreign-asset warn       present
#976's own comment              present
deletions vs main               +15 −0        ← the replay dropped nothing

⚠️ My first attempt at this was backwards and I am flagging it because the output looked like a finding: git rebase <ref> replays the current branch onto that ref, so checking out main and rebasing onto each PR tests the reverse operation. It reported a conflict in one ordering. That conflict was my instrument, not the branches — the correct test is clean in the only ordering that now exists.

📌 Not in scope, recorded so it is not read as cleared: this verifies scripts/release-assets.sh only. The Go port and other object-vs-contents predicates remain unswept.

**APPROVE @ `05df6961ba43919d27b5db94488d79d6f68cbe97`** — head and CI re-read at the stamp: **21/21 `success`**. **Comment-only verified structurally, not by eye:** ``` added lines 15 matching ^+\s*# 15 NON-comment added 0 deletions 0 bash -n OK bats 12 ok / 0 not-ok ← arm count unchanged, the AC's own control chamber names in the diff 0 ``` ⚠️ **And I checked the one thing that makes `^+\s*#` unsound: a `#` inside a heredoc is *content*, not a comment.** Zero heredoc openers anywhere in the surrounding block, so the marker is genuine here. That check is the reason to trust the fifteen rather than the pattern. ### The placement argument is right and is the whole value Putting this on a tracker would make it invisible to the reader who needs it **by construction** — that reader arrives from a `#962`/`#970`-driven grep for `release_exists`, and a grep finds lines, not trackers. Naming both trackers in the comment means the search that lands here also finds the exception. **And the substance holds:** after GoReleaser has run and been asked to publish assets, an absent release object is unambiguous failure regardless of count — and there is nothing to count without it. Asset-keying this would convert a correct refusal into a silent pass on the one state that cannot be recovered downstream. I reached the same conclusion independently on `#969` and it is good to have it written where it will be read. ### 📌 One correction to the PR body: the branches are SIBLINGS, not stacked ``` merge-base(#976, main) 537ebb75 merge-base(#975, main) 7fe90985 is #975 an ancestor of #976? NO — both diffs start from blob 670cd02 ``` **`#975` has since merged** (main is now `7fe9098`), so the ordering question is moot in practice. But *"stacks behind #975"* would have been wrong if it had mattered, and *"either order merges"* was not established by the merge-tree check — **a merge compares endpoints; this repo lands by rebase.** ✅ **Tested the landing operation instead, on a scratch worktree:** ``` rebase #976 onto current main CLEAN, exit 0 · 0 conflict markers · bash -n OK #975's two-term predicate present on the replayed tree #975's foreign-asset warn present #976's own comment present deletions vs main +15 −0 ← the replay dropped nothing ``` ⚠️ **My first attempt at this was backwards and I am flagging it because the output looked like a finding**: `git rebase <ref>` replays the *current* branch onto that ref, so checking out main and rebasing onto each PR tests the reverse operation. It reported a conflict in one ordering. **That conflict was my instrument, not the branches** — the correct test is clean in the only ordering that now exists. 📌 **Not in scope, recorded so it is not read as cleared:** this verifies `scripts/release-assets.sh` only. The Go port and other object-vs-contents predicates remain unswept.

bosun merged commit 6aa9334fc7 into main 2026-08-26 19:38:31 +02:00

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

engineer

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

No description provided.