Refuse to report a branch inventory from a shallow clone - #1392
Conversation
Closes ledger #109. Every signal `sweep-branch-ledger` reports — ahead/behind, `--cherry-pick` patch-uniqueness, and therefore `deletionCandidate` — is derived from a merge-base. A shallow clone has a grafted root, so those numbers are wrong WITHOUT erroring: nothing fails, the output is simply fiction. Measured in a real `--depth 1` clone of this repo: unguarded, the sweep exits 0 and names the live checked-out branch as a deletion candidate with "no unique patch content". A green run recommending deletion of an active branch is the hazard, so the guard fails closed — no inventory is printed at all, in either text or `--json` mode, and the exit code is 1. A partial inventory is what invites someone to act on it. `shallowCloneRefusal` takes the raw `git rev-parse --is-shallow-repository` stdout rather than a boolean so the coercion is covered too: that command prints the string "false" on a healthy clone, and reading truthiness instead of the exact value would refuse on every complete repo, which is the one way this guard could fail dangerously in the opposite direction. docs/branch-cleanup-guide.md gains the precondition ahead of its numbered Safety Rules, since the raw git commands it documents have no such protection. Remote sessions clone shallow by default, so this is the normal state rather than an edge case. Verified: npm run verify:cheap exit 0 (432 files, 4454 passed | 4 skipped); guard exercised end-to-end in a real shallow clone (exit 1, refusal) and on this full clone (exit 0, normal report). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012YRCXgX4AWZ579bKN6sk6b
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in:32 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe branch ledger sweep now fails closed on shallow clones before fetching or comparing branches. It supports plain-text and JSON refusal output, adds behavioral coverage, and documents the full-history precondition and remediation. ChangesShallow Clone Safety
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GitRepository
participant SweepBranchLedger
participant CLIOutput
GitRepository->>SweepBranchLedger: report shallow repository status
SweepBranchLedger->>CLIOutput: emit refusal message and exit code 1
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Comment |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/sweep-branch-ledger.mjs`:
- Around line 115-116: Update shallowCloneRefusal to allow the sweep only when
the normalized shallow-status value is exactly "false"; return distinct
non-empty refusal values for "true" and empty or unexpected output, preserving
the existing normalization. Adjust the related expectations in
tests/repo-hygiene.test.ts to cover complete, shallow, and indeterminate
statuses.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d0dcace4-5d4b-46e9-9ae6-9b2fac25a591
📒 Files selected for processing (4)
docs/branch-cleanup-guide.mddocs/outstanding-issues.mdscripts/sweep-branch-ledger.mjstests/repo-hygiene.test.ts
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:54e80a8a35
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. The branch was updated while autofix was in progress. Please try again. |
…when shallow CodeRabbit (Major) and Codex (P2) independently flagged the same hole in the guard added by the previous commit, and they are right. `tryGit` swallows every error into "", so the original `!== "true"` check treated an indeterminate result as proof of a complete clone. If `git rev-parse --is-shallow-repository` failed for any reason on a genuinely shallow clone, the sweep would emit exactly the merge-base-derived inventory this guard exists to prevent — the #109 defect wearing a different hat. The decision is now three-way, and only one branch proceeds: "false" -> complete history, sweep runs "true" -> shallow, refuse anything else, including "" -> INDETERMINATE, refuse The two refusals carry different remedies, since one needs `--unshallow` and the other needs a real git checkout. Whitespace is tolerated in both directions, and the exact value is compared both ways: "false" is a truthy string, so coercing on truthiness would refuse on every healthy clone instead. The `--json` error code becomes `history-not-verified`, which is what the condition actually establishes. One of my own tests asserted the wrong behaviour — that empty or unexpected output should permit the sweep. It is replaced, and the reason is recorded in the test so it is not reintroduced. Verified: 30 cases in tests/repo-hygiene.test.ts pass; typecheck, eslint and prettier clean; the sweep still runs normally on this full clone (exit 0). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012YRCXgX4AWZ579bKN6sk6b
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:1b0cbafd23
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
…tate
Codex (P2) found that the guide contradicted itself and that the guard had a
reachable bypass. Both are real.
The precondition block said "check this first, before anything else" but sat
BELOW the paragraph instructing operators to run reconciliation-preflight.mjs.
Worse, that preflight computes its own merge-base-derived ahead/behind at
scripts/reconciliation-preflight.mjs:153 via `git rev-list --left-right --count`
and had no shallow guard, so the documented sequence handed an operator exactly
the unreliable numbers this change exists to prevent, one step before they were
told to check.
Fixed both, not just the ordering: the precondition now precedes the preflight
instruction, and the preflight itself refuses unless history is verified
complete, reusing shallowCloneRefusal. Reordering alone would have left a script
that still reports fiction.
The shared refusal text no longer says "sweep", since two callers now use it.
Caught while verifying: this file's `tryGit` returns { ok, output }, not a bare
string like the sweep's. Passing the object stringified to "[object Object]",
which the guard correctly read as indeterminate and then refused on a HEALTHY
clone — the symmetric failure the guard's own test warns about. Fixed to pass
`.output`, with a note at the call site so the next caller does not repeat it.
Verified: npm run verify:cheap exit 0 (432 files, 4455 passed | 4 skipped);
tests/reconciliation-preflight.test.ts and tests/productivity-workflow.test.ts
pass; both scripts exit 0 on this full clone and exit 1 in a real --depth 1
clone.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012YRCXgX4AWZ579bKN6sk6bThere was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:7d57aefb90
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
… not the CLI The guard sat in the preflight CLI, but the CLI is not the only caller. buildReconciliationEvidencePack calls collectReconciliationState directly and stamps the result `status: "complete"`. Proven in a real --depth 1 clone: the guarded CLI exited 1 while `reconciliation-evidence-pack --output` exited 0 and wrote a completed pack around merge-base-derived ahead/behind values that a grafted root makes fiction. collectReconciliationState now throws UnverifiedHistoryError (code: "history-not-verified") before resolving the base ref, so every caller — CLI, evidence pack, and any future one — fails closed by default rather than by remembering to ask. The CLI catches it only to keep its --json envelope. The check runs against the passed root, so an injected fixture root is judged on its own history. Regression cases live with each entry point and build a real --depth 1 clone (--depth is ignored for local-path clones, hence the file:// origin), asserting is-shallow-repository is "true" first so a git behaviour change cannot make them pass vacuously. Same shallow clone, before -> after: exit 0 with a `status: "complete"` pack written -> exit 1, refusal with the --unshallow remedy, no pack written. Ledger #109. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012YRCXgX4AWZ579bKN6sk6b
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:6bdabde8cf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Summary
Closes ledger
#109. Makesnpm run sweep:branch-ledgerfail closed unless it can prove the clone has complete history, instead of printing a confident, wrong branch inventory.Every signal the sweep reports — ahead/behind,
--cherry-pickpatch-uniqueness, and thereforedeletionCandidate— is derived from a merge-base. A shallow clone has a grafted root, so those numbers are wrong without erroring: nothing fails, the output is simply fiction. Remote Claude Code sessions clone shallow by default, so this is the normal state, not an edge case.The decision is three-way, and only one branch proceeds:
The measured failure this prevents. In a real
--depth 1clone of this repo, the unguarded sweep exits 0 and names the live checked-out branch as a deletion candidate with "no unique patch content":A green run recommending deletion of an active branch is the hazard. Guarded, the same clone refuses and exits 1, printing no inventory at all in either text or
--jsonmode — a partial inventory is what invites someone to act on it.This is not hypothetical. On 2026-07-29 a session swept this repo with 74 of 2829 commits present and reported 90 of 91 branches as carrying unmerged work, while a stale local
mainread asahead 52withrefusing to merge unrelated histories. Acting on those numbers meant either deleting live branches or abandoning cleanup as impossible.The indeterminate case was a review finding, not my original design. The first revision checked
!== "true", which treated an unverifiable result as verified complete.tryGitswallows every error into"", so on a genuinely shallow clone wheregit rev-parse --is-shallow-repositoryfailed for any reason, the sweep would have emitted exactly the inventory this guard exists to prevent —#109in a different guise. CodeRabbit (Major) and Codex (P2) flagged it independently; both are resolved, and one of my own tests asserted the wrong behaviour and has been replaced, with the reason recorded in the test body.shallowCloneRefusaltakes the raw stdout rather than a boolean so the coercion is under test in both directions:"false"is a truthy string, so comparing on truthiness would refuse on every healthy clone — the symmetric failure, which has its own test.docs/branch-cleanup-guide.mdgains the precondition ahead of its numbered Safety Rules, because the rawgitcommands the guide documents have no such protection even once the script is guarded.Verification
npm run verify:cheap— exit 0.Test Files 432 passed (432),Tests 4455 passed | 4 skipped (4459).--depth 1clone: refusal on stdout,EXIT=1, no inventory printed;--jsonemits{"error":"history-not-verified", …, "branches":null}and also exits 1.npm run sweep:branch-ledger -- --no-fetchstill prints the normal inventory and exits 0.tests/repo-hygiene.test.ts: refusal content (remedy,#109,merge-base); only an explicit"false"permits the sweep; empty /undefined/ unexpected output must refuse; shallow and indeterminate refusals are distinct; whitespace tolerated on both values.npm run docs:check-linksandnpm run docs:check-scripts— PASS.npm run check:production-readinessnot run: no clinical workflow, privacy, environment, Supabase, source-governance or deployment change.Risk and rollout
Clinical Governance Preflight
Not applicable to this diff.
classifyPullRequestFilesreportsclinicalRisk: false— the changed paths arescripts/sweep-branch-ledger.mjs,tests/repo-hygiene.test.ts,docs/branch-cleanup-guide.mdanddocs/outstanding-issues.md.scripts/**is clinical-risk only when it matches ingest/reindex/migration/governance/production/drift/supabase, and a branch-hygiene reporter matches none of those. No ingestion, answer generation, search/ranking, source rendering, document access, privacy or clinical output behaviour changed.Notes
#108remains open and is unaffected: five branches are verified safe to delete, but ref deletion returns HTTP 403 in this environment. Its recorded order is append-the-branch-cleanup-rows-then-delete, becauseresolveHeadrejects a HEAD that is no longer a commit in the repository.CodeRabbit's Docstring Coverage pre-merge check reports 66.67% against an 80% threshold. That is measured across the whole touched file, not the diff: the two functions added or changed here both carry full JSDoc, and the shortfall is pre-existing helpers (
git,tryGit,main). Adding docstrings to untouched functions would widen this PR beyond its subject, so it is left alone deliberately.