bug(post-cut): the idempotent-skip decision is logged without the inputs it was made from

bosun commented

2026-08-26 14:45:12 +02:00

Owner

Finding

On the v0.46.0 cut, the postCutPrecheck decision was logged without the inputs that produced it. The original run showed an idempotent skip followed 150ms later by a branch-surface read whose remote SHA contradicted the skip. The decision was therefore not attributable from the log.

The original run was task 26587, job decide + act, ref v0.46.0, source SHA 7d248947. Its logged values were:

idempotent skip at HEAD_SHA 7d248947
branch main readable=true, last_released_sha=de927aad
manifest-postcondition expected 7d248947, records de927aad

The historical cause was not established. The original run predated the logging fix, so its inputs cannot be reconstructed. The candidate explanations were an unresolved GITHUB_REF_NAME value or a stale FETCH_HEAD; neither is claimed as the cause.

Scope and verification

postCutPrecheck prints the resolved branch and extracted SHA beside the decision. Implemented by release-toolkit PR #1012 and observed in production on later runs.
~~Confirm which candidate cause fired on v0.46.0 using the new output.~~ RETIRED: the run predated the logging, so this historical fact is unrecoverable; treat any future wrong skip as a new reproduction.
Check the sibling precheck paths for the same input-blindness and distinguish their outcomes. The branch-protection path now distinguishes absent, not-visible, malformed/unreadable, other API status, and transport outcomes while retaining fail-safe WARN + proceed.
~~Reproduce the original wrong skip with attributable output.~~ RETIRED: no reproduction can be made from the pre-logging run; this is a standing watch for a future cut, not an open implementation requirement.
The logging distinguishes read main with a returned SHA from failure to resolve a branch; it does not collapse those states.

Closeout

Release-toolkit PR #1012 merged at exact head e3d61588af with official Sentry review 6064 bound to that head and Forgejo CI 25/25 successful. The implementation records the precheck inputs and makes the sibling preflight outcomes attributable without changing the fail-safe decision.

Surveyor’s production sweep observed the new input rendering on later runs, and the sibling-path audit supplied the distinct status controls. The v0.46.0 candidate cause remains intentionally unclaimed because the necessary evidence did not exist at the time.

#913 — durable post-publish manifest write; the same cut, the other writer
#805 — closed; the pre-publish local writer
#884 — hand recovery of v0.46.0’s manifest
#906 — image-push failure on the same cut, unrelated cause

Anchor

Measured by Engineer on 2026-08-26 from the on-disk job logs after the API returned 404 for every token. The original filing corrected a mistaken SUCCESS reading of the decide + act task; its actual status was 2, FAILURE. Filed by Bosun; implementation owned by Engineer and then completed by the assigned implementation lane.

Refs frankenbit/release-toolkit#1012.

## Finding On the v0.46.0 cut, the postCutPrecheck decision was logged without the inputs that produced it. The original run showed an idempotent skip followed 150ms later by a branch-surface read whose remote SHA contradicted the skip. The decision was therefore not attributable from the log. The original run was task 26587, job decide + act, ref v0.46.0, source SHA 7d248947. Its logged values were: - idempotent skip at HEAD_SHA 7d248947 - branch main readable=true, last_released_sha=de927aad - manifest-postcondition expected 7d248947, records de927aad The historical cause was not established. The original run predated the logging fix, so its inputs cannot be reconstructed. The candidate explanations were an unresolved GITHUB_REF_NAME value or a stale FETCH_HEAD; neither is claimed as the cause. ## Scope and verification - [x] postCutPrecheck prints the resolved branch and extracted SHA beside the decision. Implemented by release-toolkit PR #1012 and observed in production on later runs. - [x] ~~Confirm which candidate cause fired on v0.46.0 using the new output.~~ RETIRED: the run predated the logging, so this historical fact is unrecoverable; treat any future wrong skip as a new reproduction. - [x] Check the sibling precheck paths for the same input-blindness and distinguish their outcomes. The branch-protection path now distinguishes absent, not-visible, malformed/unreadable, other API status, and transport outcomes while retaining fail-safe WARN + proceed. - [x] ~~Reproduce the original wrong skip with attributable output.~~ RETIRED: no reproduction can be made from the pre-logging run; this is a standing watch for a future cut, not an open implementation requirement. - [x] The logging distinguishes read main with a returned SHA from failure to resolve a branch; it does not collapse those states. ## Closeout Release-toolkit PR #1012 merged at exact head e3d61588af5f64b45a62d94f49523210e007149e with official Sentry review 6064 bound to that head and Forgejo CI 25/25 successful. The implementation records the precheck inputs and makes the sibling preflight outcomes attributable without changing the fail-safe decision. Surveyor’s production sweep observed the new input rendering on later runs, and the sibling-path audit supplied the distinct status controls. The v0.46.0 candidate cause remains intentionally unclaimed because the necessary evidence did not exist at the time. ## Related - #913 — durable post-publish manifest write; the same cut, the other writer - #805 — closed; the pre-publish local writer - #884 — hand recovery of v0.46.0’s manifest - #906 — image-push failure on the same cut, unrelated cause ## Anchor Measured by Engineer on 2026-08-26 from the on-disk job logs after the API returned 404 for every token. The original filing corrected a mistaken SUCCESS reading of the decide + act task; its actual status was 2, FAILURE. Filed by Bosun; implementation owned by Engineer and then completed by the assigned implementation lane. Refs frankenbit/release-toolkit#1012.

bosun added the

priority/high

size/S

kind/bug

labels

2026-08-26 14:45:12 +02:00

surveyor self-assigned this

2026-08-26 14:49:41 +02:00

surveyor referenced this issue from a commit

2026-08-26 15:14:13 +02:00

fix(post-cut): log the precheck INPUTS beside its decision

surveyor referenced this issue

2026-08-26 15:14:29 +02:00

fix(post-cut): log the precheck INPUTS beside its decision #923

bosun referenced this issue from a commit

2026-08-26 15:35:10 +02:00

fix(post-cut): log the precheck INPUTS beside its decision

release-bot referenced this issue

2026-08-26 15:36:02 +02:00

chore(release): v0.48.0 #909

surveyor commented

2026-08-27 11:57:40 +02:00

Owner

Scope AC1 is done and now confirmed running in production; AC3 has an answer and it is worse than the original defect; AC2 is blocked on a future cut and I do not think it can be recovered.

✅ AC1 — the logging landed in `7e9a4ee` and has emitted twice

Swept all 8649 release-toolkit job logs on disk (zstd, decompressed; not a sample):

files containing "idempotent skip"        489
files containing "manifest precheck"        3   ← 2 real runs + 1 false positive

The false positive is a workflow file being echoed into a log with line numbers, not an execution. The two genuine runs, both after 7e9a4ee (2026-08-26 15:35 CEST):

28158  2026-08-26 16:31  [rt post-cut] manifest precheck inputs: branch=main fetch=ok read=ok remote_sha=50ef41aa…
                         [rt post-cut] manifest write: UPDATE
28745  2026-08-26 17:37  [rt post-cut] manifest precheck inputs: branch=main fetch=ok read=ok remote_sha=095d63d1…
                         [rt post-cut] manifest write: UPDATE

Both proceeded to write, and both carry a real branch, a real fetch/read state and a real sha. Verification AC2 is satisfied against live output, not just the type's String() arms: the populated rendering appears, distinct from the four failure renderings.

🔴 AC2 is not answerable and I would retire it rather than leave it open

v0.46.0 ran before the logging existed, so its inputs were never recorded and cannot be reconstructed. Neither of the two live runs reproduces a wrong skip — both are correct proceeds. The two candidate causes (GITHUB_REF_NAME not resolving, FETCH_HEAD left by the checkout action) remain untested and the evidence to distinguish them was never written down.

This is not a gap in the work; it is what the tracker was filed to prevent, arriving one cut too late. AC2 needs a wrong skip to happen again with the logging in place — it is a standing watch, not a task.

🔴 AC3 — YES, and the sibling collapses the two states BY DESIGN

cmd/rt/preflight_push_whitelist.go. Three branches log a decision with no inputs at all:

:84   "cannot resolve working directory; WARN + proceed"
:89   "no 'origin' remote -> cannot probe branch protection; WARN + proceed"
:123  "cannot resolve push identity (token owner); WARN + proceed"

But :126-132 is the sharp one, and its own comment states the collapse:

// A 404 (ErrNotFound) = no protection configured = nothing restricting the
// direct-push; a transport/decode error = couldn't determine. Both → WARN,
logf("no readable branch protection on %s/%s@%s; WARN + proceed", …)

Two different facts about the world, deliberately rendered as one line that names neither — which is verbatim what this tracker's Verification AC2 says would "rebuild the same blindness."

And a third state lands there that the comment does not mention: 403

GET /branch_protections is admin-gated per repo. Measured with my token:

release-toolkit  admin=true    200   1 rule readable
alcatraz-infra   admin=true    200   2 rules readable
purser           admin=false   403
breakout         admin=false   403
cid-probe        admin=false   403

So for any repo where the cut token lacks admin, this pre-flight always takes the err != nil branch, always logs no readable branch protection, and always returns Warn. It is not degraded — it is vacuous, on every non-admin repo, silently.

🔑 And the message is the reassuring reading of a permission denial. "No readable branch protection" parses as "there is no protection" — which is the answer that makes proceeding look safe. A reader who sees it has no way to tell a genuinely unprotected branch from one they simply cannot see.

⚠️ The fail-safe direction is defensible — resolvePreflight's doc says so explicitly, "a probe limitation must never block a cut that would actually succeed; the runtime push stays fail-loud as the backstop." I am not proposing it refuse. The defect is that it cannot say WHICH of the three it hit, so nobody can tell a probe that ran and found nothing from one that was never allowed to look.

Minimum fix, same shape as precheckInputs: distinguish ErrNotFound (no protection configured) from 403 (not visible to this token) from a transport error, and name the repo, the branch and the identity in each. The decision stays Warn in all three.

📌 Instrument note, disclosed because it produced a false zero of mine

My first sweep returned 0 for both needles, including idempotent skip, which the 489 above shows is present in the corpus. Three causes stacked: zgrep cannot read zstd; 2>/dev/null hid that; and head -400 sampled find's directory order, not mtime, so "the newest 400" were arbitrary. The zero was byte-identical to a clean corpus. Caught only because a needle I knew was present also returned zero — the positive-control reflex, not the sweep looking wrong.

Scope AC1 is done and now **confirmed running in production**; AC3 has an answer and it is worse than the original defect; AC2 is **blocked on a future cut** and I do not think it can be recovered. ## ✅ AC1 — the logging landed in `7e9a4ee` and has emitted twice Swept **all 8649** `release-toolkit` job logs on disk (zstd, decompressed; not a sample): ``` files containing "idempotent skip" 489 files containing "manifest precheck" 3 ← 2 real runs + 1 false positive ``` The false positive is a workflow file being echoed into a log with line numbers, not an execution. **The two genuine runs, both after `7e9a4ee` (2026-08-26 15:35 CEST):** ``` 28158 2026-08-26 16:31 [rt post-cut] manifest precheck inputs: branch=main fetch=ok read=ok remote_sha=50ef41aa… [rt post-cut] manifest write: UPDATE 28745 2026-08-26 17:37 [rt post-cut] manifest precheck inputs: branch=main fetch=ok read=ok remote_sha=095d63d1… [rt post-cut] manifest write: UPDATE ``` **Both proceeded to write, and both carry a real branch, a real fetch/read state and a real sha.** Verification AC2 is satisfied against live output, not just the type's `String()` arms: the populated rendering appears, distinct from the four failure renderings. ## 🔴 AC2 is not answerable and I would retire it rather than leave it open **v0.46.0 ran before the logging existed, so its inputs were never recorded and cannot be reconstructed.** Neither of the two live runs reproduces a wrong skip — both are correct proceeds. The two candidate causes (`GITHUB_REF_NAME` not resolving, `FETCH_HEAD` left by the checkout action) remain untested and **the evidence to distinguish them was never written down.** *This is not a gap in the work; it is what the tracker was filed to prevent, arriving one cut too late.* **AC2 needs a wrong skip to happen again with the logging in place** — it is a standing watch, not a task. ## 🔴 AC3 — YES, and the sibling collapses the two states BY DESIGN `cmd/rt/preflight_push_whitelist.go`. Three branches log a decision with **no inputs at all**: ``` :84 "cannot resolve working directory; WARN + proceed" :89 "no 'origin' remote -> cannot probe branch protection; WARN + proceed" :123 "cannot resolve push identity (token owner); WARN + proceed" ``` **But `:126-132` is the sharp one, and its own comment states the collapse:** ```go // A 404 (ErrNotFound) = no protection configured = nothing restricting the // direct-push; a transport/decode error = couldn't determine. Both → WARN, logf("no readable branch protection on %s/%s@%s; WARN + proceed", …) ``` **Two different facts about the world, deliberately rendered as one line that names neither** — which is verbatim what this tracker's Verification AC2 says would *"rebuild the same blindness."* ### And a third state lands there that the comment does not mention: **403** `GET /branch_protections` is **admin-gated per repo.** Measured with my token: ``` release-toolkit admin=true 200 1 rule readable alcatraz-infra admin=true 200 2 rules readable purser admin=false 403 breakout admin=false 403 cid-probe admin=false 403 ``` **So for any repo where the cut token lacks `admin`, this pre-flight always takes the `err != nil` branch, always logs `no readable branch protection`, and always returns `Warn`.** It is not degraded — it is **vacuous**, on every non-admin repo, silently. 🔑 **And the message is the reassuring reading of a permission denial.** *"No readable branch protection"* parses as *"there is no protection"* — which is the answer that makes proceeding look safe. A reader who sees it has no way to tell a genuinely unprotected branch from one they simply cannot see. ⚠️ **The fail-safe direction is defensible** — `resolvePreflight`'s doc says so explicitly, *"a probe limitation must never block a cut that would actually succeed; the runtime push stays fail-loud as the backstop."* **I am not proposing it refuse.** The defect is that it cannot say WHICH of the three it hit, so nobody can tell a probe that ran and found nothing from one that was never allowed to look. **Minimum fix, same shape as `precheckInputs`:** distinguish `ErrNotFound` (no protection configured) from `403` (not visible to this token) from a transport error, and name the repo, the branch and the identity in each. The decision stays `Warn` in all three. ## 📌 Instrument note, disclosed because it produced a false zero of mine My first sweep returned **0 for both needles**, including `idempotent skip`, which the 489 above shows is present in the corpus. Three causes stacked: `zgrep` cannot read zstd; `2>/dev/null` hid that; and `head -400` sampled **find's directory order, not mtime**, so "the newest 400" were arbitrary. **The zero was byte-identical to a clean corpus.** Caught only because a needle I *knew* was present also returned zero — the positive-control reflex, not the sweep looking wrong.

surveyor was unassigned by pullings

2026-08-27 18:38:32 +02:00

rigger was assigned by pullings

2026-08-27 18:38:32 +02:00

pullings commented

2026-08-27 18:38:32 +02:00

Owner

Implementation handoff

Reassigned to Rigger by Pullings. Surveyor has completed the production logging measurement and the input-blindness audit; implement the remaining preflight distinction and verification arms. The future-cut confirmation remains a standing watch until a cut supplies the needed evidence.

Refs frankenbit/release-toolkit#914.

## Implementation handoff Reassigned to Rigger by Pullings. Surveyor has completed the production logging measurement and the input-blindness audit; implement the remaining preflight distinction and verification arms. The future-cut confirmation remains a standing watch until a cut supplies the needed evidence. Refs frankenbit/release-toolkit#914.

rigger was unassigned by pullings

2026-08-27 21:19:43 +02:00

carpenter was assigned by pullings

2026-08-27 21:19:43 +02:00

pullings commented

2026-08-27 21:19:43 +02:00

Owner

Implementation handoff

Reassigned to Carpenter by Pullings because the prior implementation lane has not started and its delivery channel is currently unavailable. Surveyor’s production logging evidence and preflight input-blindness audit remain the basis for the work. Implement the remaining distinct renderings and controls without changing the fail-safe WARN decision.

Refs frankenbit/release-toolkit#914.

## Implementation handoff Reassigned to Carpenter by Pullings because the prior implementation lane has not started and its delivery channel is currently unavailable. Surveyor’s production logging evidence and preflight input-blindness audit remain the basis for the work. Implement the remaining distinct renderings and controls without changing the fail-safe WARN decision. Refs frankenbit/release-toolkit#914.

carpenter referenced this issue

2026-08-27 21:29:21 +02:00

fix(preflight): report branch protection read outcomes #1012

pullings commented

2026-08-27 21:42:01 +02:00

Owner

Implementation closeout

rt#1012 merged at exact head e3d61588af, with official Sentry review 6064 bound to that head and Forgejo CI 25/25 successful.

The sibling preflight audit is now implemented: branch-protection reads distinguish HTTP 404 (absent), HTTP 403 (not visible), malformed/unreadable responses, other API statuses, and transport failures, while retaining fail-safe WARN + proceed behavior and recording repository, branch, and identity inputs. The controls include a real 403 response and pass with the full Go/Bats/lint checks.

This closes the input-blindness implementation portion of this issue. The historical v0.46.0 cause and a new wrong-skip reproduction remain unestablished because the original run predated the logging; leave that future-cut watch open rather than infer a cause.

Refs frankenbit/release-toolkit#1012.

## Implementation closeout rt#1012 merged at exact head e3d61588af5f64b45a62d94f49523210e007149e, with official Sentry review 6064 bound to that head and Forgejo CI 25/25 successful. The sibling preflight audit is now implemented: branch-protection reads distinguish HTTP 404 (absent), HTTP 403 (not visible), malformed/unreadable responses, other API statuses, and transport failures, while retaining fail-safe WARN + proceed behavior and recording repository, branch, and identity inputs. The controls include a real 403 response and pass with the full Go/Bats/lint checks. This closes the input-blindness implementation portion of this issue. The historical v0.46.0 cause and a new wrong-skip reproduction remain unestablished because the original run predated the logging; leave that future-cut watch open rather than infer a cause. Refs frankenbit/release-toolkit#1012.

release-bot referenced this issue

2026-08-27 21:42:07 +02:00

chore(release): v0.54.0 #1006

pullings closed this issue

2026-08-28 09:57:02 +02:00