Stop inventing "no PR" for the canonical checkout's own branch - #1695
Merged
Conversation
…wn branch
The PR lookup was skipped for the canonical checkout and substituted with a
literal:
const pr = w.branch && !isCanonical ? prFor(w.branch)
: { lookup: 'OK', state: 'NONE' };
`{ lookup: 'OK', state: 'NONE' }` does not mean "not checked". It means
"checked, and there is no PR" — and classifyBranch read it exactly as written,
returning UNKNOWN_PR ("no PR found — cannot prove the work landed") for
canonical's branch even with an OPEN PR against it. Reproduced against #1694
on 2026-08-30: `gh` answered `1694 OPEN` from the shell while the tool's own
report printed PR `NONE`.
Nothing was lost — UNKNOWN_PR fails safe to KEEP — but the tool was reaching a
right answer from an invented fact, in the evidence layer, which is the exact
failure this repo keeps removing everywhere else.
The shortcut also bought nothing. classifyWorktree returns ACTIVE for canonical
on its FIRST line, before any PR fact is read. Only the BRANCH verdict ever
consumed these — and that is the separation AGENTS.md insists on: may this
CHECKOUT go, versus may this BRANCH be deleted. The short-circuit conflated the
two inside the tool that defines the distinction.
The test took two attempts, and the first one is worth recording because it
looked fine. It passed with the bug reverted — because canonicalRoot() resolves
.claude/hooks/lib/workspace-identity.mjs against the repo under test, fixtures
do not have one, so isCanonical is ALWAYS false in the harness and the bug is
unreachable there. That null is deliberate and correct (a fixture must never be
able to claim it is the real canonical checkout), but it means every
canonical-only branch of this tool was untestable. The fixture now writes its
own identity stub, and the test asserts ACTIVE first — proving it really is
being treated as canonical — before asserting the branch verdict.
Verified by injection: reverting the one-line fix turns the new test red and
everything else green; restoring it returns the file byte-identical by sha256.
typecheck 0, lint 0, npm test 1275 files / 12,210 passed, guards 37/37.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NbDxyygyXRUEERuGocpZZH
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
…#1695 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NbDxyygyXRUEERuGocpZZH
njrini99-code
added a commit
that referenced
this pull request
Aug 31, 2026
…#1696 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NbDxyygyXRUEERuGocpZZH
njrini99-code
added a commit
that referenced
this pull request
Aug 31, 2026
… reads as a git bug (#1696) * docs(audit): record the branch-durability blind spot and the zsh :r trap Two findings from finishing the wiring audit. **Nine branches held commits that existed nowhere but this laptop** — up to 95 on one — and the lifecycle tool reported none of them. It was not wrong to: it checks the protected prefixes (backup/, preserve/, recovered/, stage/) BEFORE it checks for unique upstream-less work, so those branches return KEEP_PROTECTED and never reach the durability verdict. Both verdicts are correct in isolation. The ordering means the branches named for preservation are exactly the ones whose non-durability is invisible. That also reconciles an earlier claim rather than contradicting it. The 2026-08-30 closeout recorded `NO_UPSTREAM_UNIQUE_WORK remaining: NONE` after pushing ten branches, and that was true as stated — protected branches were never in that verdict's population. All nine were scanned and pushed. gitleaks: no leaks found across 127 commits on the four largest, run BEFORE any push; the only secret-shaped literal was .env.example's placeholder `your-service-role-key`. Branches whose work exists in one place: 9 -> 0. Left as a design question, not repaired: whether KEEP_PROTECTED should also report unique-work status. A protected branch cannot be deleted by the tool, so the durability fact is advisory rather than load-bearing — that is a decision about what the report should say, not a defect to fix. **A zsh trap that reads as a git bug.** Pushing those branches failed seven times with `src refspec ... does not match any` against refs that resolved fine. In zsh, "refs/heads/$b:refs/heads/$b" parses $b:r as the `:r` history modifier and eats it, producing `refs/heads/recovered/stash-0efs/heads/recovered/stash-0`. The same command typed literally works, which is what makes it convincing — it only appears when the branch name is a variable. Recorded in shipping.md §3 alongside the other traps of its kind (`timeout` on macOS, `ls` aliased to eza). Verified: docs:schema-drift 0, docs:path-drift 0, knowledge:check 0, markdown ratchet at baseline, and the durability re-measure returns 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NbDxyygyXRUEERuGocpZZH * chore(control-plane): clear #1695's expired transitional row, register #1696 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NbDxyygyXRUEERuGocpZZH --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found by the repo wiring audit (#1694), in the audit's own tooling.
The bug
{ lookup: 'OK', state: 'NONE' }does not say "not checked." It says"checked, and there is no PR."
classifyBranchread it exactly as writtenand returned
UNKNOWN_PR— "no PR found — cannot prove the work landed" — forthe canonical checkout's branch, even with an OPEN PR against it.
Reproduced against #1694:
ghanswered1694 OPENfrom the shell while thetool's own report printed PR
NONE.Nothing was lost —
UNKNOWN_PRfails safe toKEEP. But the tool was reachinga right answer from an invented fact, in the evidence layer, which is the
precise failure this repo keeps removing everywhere else.
The shortcut bought nothing
classifyWorktreereturnsACTIVEfor canonical on its first line, beforeany PR fact is read. Only the BRANCH verdict ever consumed these — and that is
the separation
AGENTS.mdinsists on:The short-circuit conflated the two inside the tool that defines the
distinction.
The test took two attempts, and the first one looked fine
It passed with the bug reverted.
canonicalRoot()resolves.claude/hooks/lib/workspace-identity.mjsagainst the repo under test; fixturesdo not have one; so
isCanonicalis always false in the harness and the bugis unreachable there.
That
nullis deliberate and correct — a fixture must never be able to claim itis the real canonical checkout — but it also means every canonical-only branch
of this tool was untestable. The fixture now writes its own identity stub, and
the test asserts
ACTIVEfirst, proving it really is being treated ascanonical, before asserting the branch verdict.
Verified
Injection: reverting the one-line fix turns the new test red and everything
else green; restoring it returns the file byte-identical by sha256.
typecheck 0 · lint 0 ·
npm test1275 files / 12,210 passed · guards 37/37🤖 Generated with Claude Code
https://claude.ai/code/session_01NbDxyygyXRUEERuGocpZZH