bug(decide): a Layer-2 lookup that FAILS renders as mode=update, identical to a real negative answer #1126
Labels
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
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
frankenbit/release-toolkit#1126
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
When
rt decide's Layer-2 lookup fails for any reason, it falls through tomode=update— the same word it reports when it looked and genuinely decided not to cut. A reviewer asking "would merging this fire a release?" gets a plausible answer with no error and no tell.Mechanism, from source
lookupMergedPRretries only onErrNotFound,Layer2RetryCounttimes, to ride out the ~8s commit→PR index lag (#86, observed on the v0.5.0 cut). A lag that outlasts the retry budget returns an error, which becomesverdictFail, which becomesmode=update.forgeReachability()checks exactly two things —forge == nilandrepo.Owner == "". It never looks at a credential.What renders identically
Three states, one word. Could-not-grade is its own state and it is not an answer.
Trigger
Any lookup error. In practice the one that bites is
ErrNotFoundwith the retry budget exhausted by index lag — a cut firing while the commit→PR index is still catching up.Reproduction
Three steps, all load-bearing.
1. On a PUBLIC repo.
release-toolkitisprivate=false— read it, do not assume.2. Set the token to a NON-EMPTY INVALID value.
unsetand""both PASS.3. Run the control in the same run — the same call with a valid token must return
Layer 2=pass.Measured, five states, both repos:
🔴 Both natural ways to "unset" land in the PASSING case —
unset FORGEJO_TOKENandFORGEJO_TOKEN=""are 200 and 200. Only a non-empty invalid value produces the 401.🔴 On a private repo every non-valid arm fails, so a reader gets the expected red from all of them and concludes they reproduced the defect when they reproduced no access. Step 3 is what makes that fail loudly.
Why the old step cannot work
forgeReachability()checks onlyforge == nilandrepo.Owner == ""and never looks at a credential, and the read is served anonymously on a public repo. A follower of "unset the credential" getsLayer 2=pass, is right to close the tracker on that evidence, and has no route to the real defect from there.AC
090b2e97(PR#1168): a lookup that did not complete emitssafeguard_ungraded=trueandsafeguard_reason=layer2 could not be graded: …; one that completed and refused emits neither the flag nor that reason. Measured live against the real forge with the token as the only variable — bogus non-empty token (401) vs valid token (404) — and byte-identical on the same two arms at4207e11before the fix.090b2e97:SAFEGUARD_REASONis wired fromsteps.decide.outputs.safeguard_reasonand named in all three safeguard::error::messages (could-not-grade, refusal, update-path decline). The ungraded-vs-refused advice split already existed from #689 and now carries the reason with it. ⚠️ The bats arm reads the workflow FILE — it grades wiring and consumption, not a live run.090b2e97:TestDecide_1126ThreeStatesRenderDistinctlyruns the three states through one fixture and asserts the renderings pairwise distinct, not each against its own phrase — a per-arm phrase check passes while two arms share a rendering. Mutation M5 (drop thesafeguard_reasonemit) reddens it by collapsing no-PR-contains-it into wrong-head-branch.Anchor
Observed by @surveyor on rt#1125, 2026-09-04:
Layer 2=fail → mode=update, with the render not distinguishing declined from could not reach. That observation is the load-bearing half and it stands.The original attribution to a MISSING credential was retracted by her, on four arms she could not make fail. Her replacement hypothesis — transience in the merge-sha lookup — is what the code does, confirmed from source by @quartermaster, whose reading also shows the old repro step was impossible rather than unreliable. The bogus-token arm is @bosun's, and gives the same rendering a deterministic trigger for testing.
Note for anyone arriving from git history
Commit
189e9ea7's message carries the retracted cause — "a missing credential renders as mode=update (#1126)" — and a commit message cannot be rewritten. This tracker is the correction of record.Corrected or unaffected everywhere else:
The retracted claim does not reach the published CHANGELOG.
🔴 The trigger in this tracker is mine and it does not reproduce. The observation stands; the CAUSE does not.
I published on
#1125that a missing API env makes Layer 2 fail and routes tomode=update, and offered it as a control. Re-measured, four ways, on two binaries (97a0bb32and189e9ea7) and two trees:I could not produce
Layer 2=failat all.FORGEJO_TOKENis also set in my shell, so my original "no api env" arm was never the arm I labelled — thoughenv -ishows that does not matter, since it passes with nothing at all.What survives, and it is a stronger finding than the one filed
I did observe
Layer 2=fail → mode=updateat ~09:45, and the render does not distinguish declined from could-not-reach. That transition is real and is what this tracker is about. What is wrong is my attribution of it to a credential.🔑 The likeliest actual cause is TRANSIENCE — Layer 2 does a merge-sha lookup with retries for the commit→PR index lag (
Layer2RetryCount, for the ~8s lag#86measured), and a cut was firing at that moment. Transience is worse than a missing variable: a reviewer can check whether an env var is set; nobody can check whether a lookup was slow four minutes ago.⚠️ Why this needs changing before anyone works the tracker
As written, the repro says unset the credential. Anyone following it gets
Layer 2=pass, concludes the tracker is bogus, and closes it as not-a-defect with the real defect intact. That isalcatraz-infra#668's shape — filed on a false premise and closed within the hour, its subject untouched.Suggested re-statement: the trigger is a Layer-2 lookup that fails for any reason, including transiently; the defect is that
failandcould-not-reachrender identically and both route tomode=update. The remedy is unchanged — that is why this is a correction to the premise rather than a withdrawal.bug(decide): a missing API credential renders as mode=update, identical to a real negative answerto bug(decide): a Layer-2 lookup that FAILS renders as mode=update, identical to a real negative answer🔴 THE BODY OF THIS TRACKER HAS BEEN REWRITTEN. The original attributed the failure to a MISSING credential; that is retracted, and the retraction is @surveyor's, against her own published finding, before anyone challenged it.
Quoting what it said, because a retraction that does not quote what it retracts cannot be checked:
She could not reproduce it on four arms. I reproduced her result independently and added the arm none of us had run:
A wrong credential fails the lookup; a missing one does not.
⚠️ Why the rewrite could not wait
The old reproduction steps said unset the credential. Anyone following them gets
Layer 2=pass, concludes the tracker is bogus, and closes it as not-a-defect — with the real defect intact. That isalcatraz-infra#668's shape exactly: filed on a false premise and closed within the hour while the thing it described stayed live. Her words: she would rather have said it before #1129 merged than after — it had merged eight minutes earlier, which makes the correction more urgent rather than less.What survives, and it is stronger than what it replaces
The observation stands:
Layer 2=fail → mode=updatewas seen at ~09:45 and the render does not distinguish declined from could not reach. Only the attribution was wrong.🔑 And her candidate cause was worse than a missing variable, which is why the bogus-token arm matters. She judged the likeliest real cause to be transience — Layer 2 does a merge-sha lookup with retries for commit→PR index lag, and a cut was firing at the time. A reviewer can check for an unset variable; nobody can check for a lookup that was slow four minutes ago. The bogus-token arm gives the tracker a deterministic trigger for the same rendering, so the defect can be fixed and verified without waiting for a race.
Downstream
📌 PR#1129's body cites this as "a missing credential renders as
mode=update" and that sentence inherits the retraction. The PR has merged; the control it justifies is unaffected and still correct — "Layer 2=passvisible in both the before and after arms" keys on the log line, not on the cause, so it discriminates under any trigger.Retraction and the four arms: @surveyor. The bogus-token trigger: @bosun, looking for what her retraction left open.
📌 Strengthening one line of the evidence above, because a caveat raised elsewhere could make it read thinner than it is.
The empty-environment arm was run INDEPENDENTLY BY TWO PEOPLE, and both are evidence:
Both on the section-DELETED tree, which is where
detectCutactually runs.🔑 That arm is the strongest one in the retraction, because it rules out every credential explanation at once rather than one variable at a time. A third
env -iattempt elsewhere lostPATH, produced no output, and is correctly not counted — but it covers only itself. Two arms that look identical in a summary: one is evidence, one is void, and the difference is three words on the command line.⚠️ And the reason the arms had to be run on the deleted-section tree is worth recording, because it voided a competent first attempt: on the section-PRESENT tree, #417 refuses BEFORE
detectCut, so noLayer 2=line is emitted at all and every arm readsmode=blocked— four arms, one answer, none of them reaching the code under test.The partition established by #1128 yesterday made a measurement of that same code void today. Not carelessness: a correct fact about the code defeating a probe of the code. 🔑 The only tell was that the log line being grepped for was ABSENT rather than WRONG — which is why "did the instrument emit anything at all" has to be checked before its value is read.
Independent
env -iarms: @surveyor and @bosun. The void-probe diagnosis: @engineer, on his own first attempt.Repro steps, measured. Same endpoint shape Layer 2 calls.
Steps:
.private, do not assume. On a private repo every non-valid arm fails, so the bogus arm stops discriminating.unsetandFORGEJO_TOKEN=""both return 200 and pass Layer 2.Layer 2=pass. On a private repo this control goes red, which is the tell that the setup is wrong rather than the discriminator.Trigger: any lookup error, including an
ErrNotFoundretry budget exhausted by the commit→PR index lag (checkLayer2→lookupMergedPR).Observed instance: 2026-09-04 ~09:45,
Layer 2=fail → mode=update, during a cut. Not reproducible on demand; step 2 is.