so in ../entiredb "filtered_fetches" is enabled, now today one of my coworkers had this issue:

Here's my analysis. Short version: the shallow clone of entire/checkpoints/v1 is expected (the CLI makes it itself), but it's interacting badly with the reconcile logic, and the 1000-commit cap is what actually wedged him.

1. Is the shallow checkpoints clone expected? Yes.

The CLI creates shallow state on that branch deliberately. FetchMetadataTreeOnly (git_operations.go:420) fetches with --depth=1:

resume/explain use it for cheap tip resolution. And critically, FetchMetadataBranch (git_operations.go:407) refuses to undo it:

So once any tree-only probe runs, the checkpoints branch stays shallow, and nothing in the normal flow ever deepens it. With filtered_fetches on, fetches are also --filter=blob:none (checkpoint/remote/git.go:71). So his local checkpoints history is both shallow and blob-filtered — exactly the config in entiredb. That's not misuse; it's the designed steady state.

2. What actually failed

The commit chain exceeded 1000 commits without reaching root error comes from collectCommitChain (metadata_reconcile.go:241-267, added 2026-02-27 by Daniel Vydra). Reconcile cherry-picks the local first-parent chain onto the remote tip, and that walk aborts at 1000 if it hits neither a root nor a .git/shallow boundary.

The walk's early-stop only fires when the shallow boundary commit is actually on the local first-parent chain. In a disconnected scenario it isn't — local and remote were re-rooted relative to each other, so his local boundary commit isn't in the line being walked. The walk therefore runs toward the true local root, which is >1000 commits deep, and errors out. Both the push path (push_common.go:402) and doctor (doctor.go:385) call the same function, so both are blocked, and doctor offers a "Fix" that can never succeed — it loops.

Why is his local chain that deep? The "some checkpoints weren't pushed" symptom is the cause: pushes of checkpoints/v1 were silently failing for a while, so every git commit kept condensing new checkpoint commits onto the local branch with no sync. The pile grew past 1000.

3. The thing I'd flag hardest

isDisconnected (metadata_reconcile.go:219) decides disconnection purely from git merge-base local remote exit code 1. On a shallow repo, merge-base returns exit 1 when the real common ancestor lies below the shallow boundary — git simply doesn't have the objects to find it. So the "disconnected" verdict itself may be a false positive caused by shallowness, not a genuine re-root.

Note doctor.checkDisconnectedMetadata does no fetch and no unshallow before checking or reconciling (doctor.go:345-391). It runs merge-base and the cherry-pick walk against deliberately-incomplete history. The code comment asserts disconnection "only happens due to the empty-orphan bug" — but he's hitting it via filtered+shallow, which is a second, unaccounted-for path. Either:

• it's a genuine re-root and the 1000-cap blocks recovery, or
• it's a false disconnection from the shallow boundary hiding the merge base,

and the tooling can't currently tell the two apart because it never deepens the branch first.

4. Recovery for him right now

Deepen the metadata branch so merge-base and the walk see real ancestry, then re-check:

• If merge-base now finds an ancestor → it was a false disconnection; a plain entire push / fetch should reconcile normally.
• If it's genuinely disconnected → his local-only unpushed checkpoints are the at-risk data. Don't just reset to origin (that drops them). Either bump MaxCommitTraversalDepth in a local build to push the cherry-pick through, or manually graft. The shards are path-addressed (<id[:2]>/<id[2:]>/) so they don't collide — the data is recoverable, the tooling just won't do it under the cap.

5. Suggested fixes (for us)

• Deepen before deciding. IsMetadataDisconnected / checkDisconnectedMetadata must unshallow that ref before trusting merge-base, or merge-base on a shallow repo can't be trusted at all. A shallow-aware guard (git rev-parse --is-shallow-repository → refuse-or-deepen) before declaring DISCONNECTED.
• Don't present a fix that can't run. When the local chain exceeds the cap, doctor should say so and offer the deepen path, not re-offer the same failing cherry-pick.
• The 1000 cap on the recovery path is too low / too fatal. For reconcile specifically, either raise it substantially or fall back to git rev-list (no in-memory walk) the way collectCommitsSince already does at push_common.go:505.
• The silent push failures are the upstream cause worth chasing — if checkpoints/v1 pushes had been surfacing errors, the local branch never would have grown to 1000 disconnected commits.

Want me to dig into the push-failure side (why his checkpoints stopped pushing) or prototype the shallow-aware guard in IsMetadataDisconnected?

Good challenge — I was loose there. Let me be precise.

No, it doesn't establish that. The error only proves one narrow thing: walking the first-parent line from his local tip, collectCommitChain visited 1000 commit objects present in his local store and hit neither a root (len(ParentHashes)==0) nor a .git/shallow boundary. That count is the depth of his local first-parent chain — not a count of commits he authored or failed to push.

Those 1000 are some mix of three things, and the error can't tell them apart:

1. Shared team history. entire/checkpoints/v1 is team-wide — every contributor's condensed checkpoints land on it. Most of that depth is probably everyone's history, fetched at some earlier full-depth point, not his.

2. His own unpushed condensation commits — the ones the failed pushes left stranded locally. This is real, but it's likely a slice of the 1000, not the whole thing.

3. Stale pre-rebuild objects — and this is the one I'd actually worry about. The comment on collectCommitChain (metadata_reconcile.go:236-240) calls it out directly:

   without this check the walk would stroll past shallow boundaries into stale objects left in the pack (e.g., when the remote branch has been rebuilt since the last full fetch), producing a phantom chain of commits that no longer represent the actual checkpoint history.

   If the remote was re-rooted, his pack can still hold the old version of the branch. The shallow early-stop is supposed to cut the walk at the boundary — but only if the boundary commit is on this first-parent line. In a disconnected/re-rooted state it isn't, so the walk traverses straight into stale objects. A chunk of the 1000 could be history that no longer exists on the real branch at all.

So the honest read: he has a deep local first-parent chain (≥1000 objects) that doesn't pass through his shallow boundary. My earlier "the pile grew past 1000" was over-attributing that to his unpushed commits. The unpushed-checkpoints symptom explains the divergence; it does not establish the depth.

To actually find out what the 1000 are made of:

If --not origin/... returns a small number but the total is huge, the depth is inherited/stale, and the right move is to deepen+reset rather than cherry-pick 1000 commits. If it's genuinely large, then a lot of his work really is stranded locally and needs grafting, not discarding.

I dug in. The short answer: a gap in Alex's commits does not explain the 1000 — the 1000 is just the hardcoded cap, and this branch is structurally too deep for it. Here's the evidence.

The branch is ~5× deeper than the cap

collectCommitChain walks first-parent to root. On this branch that's 5,101 hops. The walk blows the 1000 cap at hop 1000 every time, ~4,100 commits short of root. So the "1000 commits without reaching root" is literally just MaxCommitTraversalDepth — it's the cap being hit, not a count of Alex's commits or a measure of how far behind he was. Any disconnected reconcile on this repo fails identically, Alex or not.

Alex was indeed traveling, and there are gaps — but they're the wrong size

Timezone trail on his commits: +1000 (home/AU) → +0800 → +0900 → -0700. He's been hopping Asia and US west coast. His notable commit gaps:

But the gap during his most recent travel window only added ~200 commits to the branch:

So even the team's busiest stretch around his absence is nowhere near 1000, and his own personal gap is smaller still. The gap explains why his local diverged/disconnected from remote — it doesn't produce the 1000 number. Those are two different things:

• The gap (his being out) → his local checkpoints/v1 fell behind / became disconnected from a remote that kept moving (and note the 1,509-commit spike on 2026-05-29, plus an Entire Migration author with ~300 commits — exactly the kind of mass-rewrite/re-root event that produces a genuine "no common ancestor").
• The 1000 error → purely the cap colliding with a 5,101-deep first-parent chain. Structural, not gap-related.

So, restating the real cause

The reconcile design (cherry-pick the entire local chain onto remote tip, capped at 1000) was built for the empty-orphan bug, where the local chain is tiny. It's fundamentally incompatible with a real team checkpoints branch that's 5,000–10,000 commits deep. Once Alex's repo got into a disconnected state — for whatever reason his travel-induced divergence triggered — recovery was impossible because the branch is 5× the cap.

Side note on your own checkout

soph's entiredb is healthy, not the disconnected case:

You're just 544 behind; a normal fetch/fast-forward syncs you. No reconcile needed.

Fix implication

The earlier suggestion stands but is now sharper: the cherry-pick-the-whole-chain reconcile is the wrong shape for branches this deep. Options:

• Switch reconcile to replay only local-only commits (rev-list local --not remote) rather than the full chain to root — that set is small (~hundreds) even when the branch is 10k deep.
• The 1000 cap should be on the replay set, not the walk-to-root, which is what collectCommitsSince (push_common.go:505) already does correctly via rev-list exclude..tip. collectCommitChain's walk-to-root is the broken one for disconnected branches.

Want me to draft that change — have ReconcileDisconnectedMetadataRef compute the local-only set against the remote tip instead of walking to root?

Your timeline is plausible, but it hinges on one thing I should verify: what the May 29 1,509-commit spike actually was. Your hypothesis only produces a disconnection (no common ancestor) if May 29 was a re-root/rebuild of the branch — not just a busy day. Let me check.

Right — and my check backs you up: the May 29 spike was just a normal busy day (real author dates May 28–29, no backdating, and a pre-May-28 commit 8cee810 still has a merge-base with the current tip, exit 0). No re-root happened. So that changes the diagnosis in an important way.

If the branch was never rebuilt, the "disconnected" verdict was a false positive

A genuine "no common ancestor" requires one side to have been re-rooted. Neither was. So local and remote on Alex's machine did share an ancestor — git merge-base just couldn't see it because his clone was shallow. With --depth=1 on checkpoints/v1, git only has the tip and a grafted boundary; the real common ancestor sits below that boundary, git doesn't have the objects, so merge-base exits 1 → isDisconnected returns true → reconcile fires on a problem that doesn't exist.

That's exactly the false-positive path I flagged two messages ago (metadata_reconcile.go:219, and doctor.go:345 never deepens before checking). Your "no rebuild, ever" confirmation is what promotes it from hypothesis to most-likely cause.

The 1000-cap hit is itself evidence it wasn't the orphan bug

The code comments assert disconnection "only happens due to the empty-orphan bug." But an empty-orphan local branch is short — a fresh empty root plus a handful of commits — so the walk would reach root almost immediately and never approach 1000. Alex's walk blew past 1000, which means his local branch still held the full ~5,101-deep real history. A deep, intact local chain is the opposite of an orphan re-root. So the 1000 failure is positive evidence for "shallow false-positive on the real branch," not "empty-orphan."

Your timeline still holds — just for a different part

Your story explains the divergence, which is real and necessary for the bug to fire:

• Left May 29 → back Jun 5, didn't fetch: his local checkpoints/v1 fell hundreds–thousands of commits behind, and stayed shallow.
• Pushes silently failing last week: he kept condensing checkpoints onto his stale local base, so local diverged further (the local-only commits that didn't push).

But that divergence by itself is normal — diverged-with-shared-ancestor is the everyday case the push path handles fine. What tipped it into the failure was the shallow clone fooling merge-base into calling a normal divergence a disconnection, after which reconcile tried to cherry-pick his whole 5,101-deep chain and hit the cap.

So the corrected causal chain:

1. checkpoints/v1 is shallow + blob-filtered (CLI's normal steady state via tree-only fetch).
2. Out for a week + failing pushes → local diverged and fell far behind, never re-deepened.
3. git merge-base on the shallow repo can't reach the real (still-existing) common ancestor → false "disconnected" verdict.
4. Reconcile fires needlessly → walks the full local chain → branch is 5× the 1000 cap → hard error; doctor re-offers the same doomed fix → loop.

Fix implications (now sharper)

• The real bug is step 3, not the cap. IsMetadataDisconnected must refuse to trust merge-base on a shallow repo — either git rev-parse --is-shallow-repository guard, or deepen the ref first (--deepen/targeted unshallow) before deciding. Right now it silently converts "I don't have the history" into "there is no shared history."
• The cap raise / local-not-remote replay set is still worth doing as defence-in-depth, but it's secondary — with the false positive fixed, reconcile wouldn't even run for Alex.

One caveat: I'm inferring Alex's local state from this repo + the error text; I can't see his actual .git. If he still has the repo, the confirming check is, on his machine: git rev-parse --is-shallow-repository (expect true) and git merge-base refs/entire/checkpoints/v1 origin/entire/checkpoints/v1 (expect exit 1 while shallow, then exit 0 after git fetch origin --deepen=200 entire/checkpoints/v1). If deepening makes the merge-base appear, that nails it as the shallow false positive.

Want me to write up these findings (with the reproduction steps) as an issue, or prototype the shallow guard in IsMetadataDisconnected?

The fix is complete and everything passes (go build, gofmt, golangci-lint 0 issues, affected-package tests green). Here's what I changed.

The fix for step 3

The root cause: isDisconnected trusted a git merge-base exit-1 as proof of "no common ancestor," but on a shallow clone that miss is a false positive — the real ancestor lives below the shallow boundary where git has no objects.

1. Shallow-aware detection — strategy/metadata_reconcile.go New metadataDisconnected() wraps the raw isDisconnected():

• merge-base finds an ancestor → connected.
• merge-base misses and repo is not shallow → genuinely disconnected (true).
• merge-base misses and repo is shallow → suppress the verdict (treat as connected) + debug-log pointing at entire doctor.

Both call sites (IsMetadataDisconnected, used by the per-commit warn hot path and doctor detection; and the internal check in ReconcileDisconnectedMetadataRef, used by push + doctor fix) now route through it. This keeps the hot path network-free and stops the doomed full-history reconcile from ever firing on Alex's situation.

2. Doctor deepens before deciding — doctor.go + git_operations.go So doctor can still catch genuine disconnections on a shallow clone, checkDisconnectedMetadata now deepens the metadata branch first (best-effort) when the repo is shallow, via a new DeepenMetadataBranch() that --unshallows just that branch and updates the remote-tracking ref without moving the local primary ref. After deepening, merge-base sees real ancestry and the verdict is accurate.

3. Exported remote.IsShallowRepository (was unexported) so both packages reuse one implementation instead of duplicating the rev-parse --is-shallow-repository check.

4. Tests — TestMetadataDisconnected_ShallowSuppressesFalsePositive (reproduces the incident: diverged-but-connected branch, shallow boundary hides the ancestor, merge-base misses, suppression kicks in) and TestMetadataDisconnected_GenuineDisconnectionOnFullRepo (unrelated roots on a full repo still report disconnected). Plus updated two now-inaccurate comments.

Net effect on Alex's case

• entire push: reconcile no-ops (shallow + miss suppressed) → push proceeds to its normal rebase, which replays his local-only commits onto the remote tip and syncs. No more metadata reconciliation failed.
• entire doctor: deepens checkpoints/v1, merge-base finds the shared ancestor, reports OK instead of looping on a fix that always errored.
• Genuine disconnections (real empty-orphan bug) are still detected — by doctor after deepening, and on push the rebase cap still refuses to silently combine unrelated histories.

What I deliberately left out of scope (you asked for step 3 only): the 1000-cap in collectCommitChain is still there. With the false positive fixed it no longer fires for this class of bug, but if you want defence-in-depth I'd still switch reconcile's walk to the local-not-remote set (rev-list remote..local) the way collectCommitsSince already does. Say the word and I'll do that as a follow-up.

Done. Committed on a new branch:

• Branch: soph/shallow-metadata-disconnect-false-positive
• Commit: 7742dd751 — strategy: don't treat shallow merge-base miss as disconnected metadata

The message captures the root cause (shallow merge-base false positive at the boundary), why it bit a normal user (checkpoint clones are routinely shallow via --depth=1), the cascade into the 1000-cap abort, the three-part fix, and the net effect on push/doctor plus how genuine disconnections are still caught.

Nothing pushed — it's a local branch on a clean tree (build, gofmt, lint 0 issues, affected tests green). Want me to open a PR, or run the broader mise run test:ci before you push?

Leaving everything as-is. Current state:

• Trail: "Fix shallow merge-base false-positive in metadata disconnection check" — ID 019eccca-4c7b-7d9d-ae8f-d527bd833e6c, status draft, on branch fix-shallow-merge-base-false-positive-in-metadata-disconnection-check (pushed to origin).
• Local branches: soph/shallow-metadata-disconnect-false-positive (currently checked out) and fix-shallow-merge-base-... — both at commit 7742dd751.

Nothing further changed. The fix is committed, the branch is pushed, and the trail exists.

Pushed — PR #1434's branch is now at 0ec849ce8 and CI will re-run with the fixes.

Summary of what landed on top of the original fix:

Also reverted the unneeded IsShallowRepository export and added TestFetch_Deepen. gofmt/golangci-lint clean; strategy, checkpoint/remote, and cli tests green (including CI-like identity conditions).

Both local branches (soph/... and the PR branch) and origin now point at 0ec849ce8. Want me to watch the PR #1434 CI run and report back when it finishes?

Here's the full picture of the PR comments:

Comments on PR #1434

Reviewer: only Copilot (the AI reviewer) so far — no human reviews posted on the PR itself. (The "Findings" you relayed earlier came to me directly, not as PR comments.)

2 inline comments — both the same P1 issue, both already fixed:

Both are pinned to the old commit 7742dd751 and duplicate the P1 author-identity finding you already passed me. My follow-up commit switched both tests to testutil.InitRepo (repo-local identity), so they're resolved.

Copilot's overview also still describes the first commit's approach (global IsShallowRepository export + --unshallow) — that predates 0ec849ce8, which reverted the export and switched doctor to ref-scoped --deepen. Copilot hasn't re-reviewed the new commit.

Net

Nothing on the PR is unaddressed — both inline comments are fixed by the commit I pushed. No human review yet (reviewDecision: REVIEW_REQUIRED). CI is currently pending on the new commit (lint, test-core, test-canary, test-integration ×3, license-check all running; binary-size already passed).

Want me to wait for CI to finish and report results, or post a reply on the PR noting that the two Copilot comments are resolved by 0ec849ce8?

Done. Here's the full picture of entire trail finding for trail #569:

Findings on the trail

1 finding was low, then a re-review surfaced 1 high — both now addressed:

Current dashboard: Open: 0 · Resolved: 1 · Dismissed: 0 · Stale: 0 — clean.

A note on the high finding

Its stated mechanism (repo.Storer.Shallow() returns a cached boundary) is actually not true for go-git v6 — I checked the source: ShallowStorage.Shallow() reads .git/shallow fresh every call, and the disconnection checks themselves shell out to git CLI (merge-base/rev-list), so the deepen's effect is observed correctly today. But the recommendation is sound: the codebase already reopens after git operations (resume.go:freshRepo), and depending on go-git's internal read semantics is fragile. So I applied the reopen — cheap, idiomatic, and robust against object/pack-index caching or future go-git changes. I noted this nuance honestly in the commit and the resolve message rather than implying a live bug.

All three follow-up commits are pushed to the PR branch (428c3274b is the tip), build/lint/tests are green. Want me to check CI status on the latest push, or watch for any further findings the re-review might add (entire trail finding watch)?

Here's what PR #1434 does, end to end.

The problem it fixes

Entire stores checkpoint metadata on a shared git branch, entire/checkpoints/v1. To decide whether your local copy of that branch is out of sync with the remote, the CLI ran git merge-base local remote — if git reports "no common ancestor," the code concluded the branches were disconnected and tried to repair them.

That conclusion is wrong on a shallow clone. The CLI itself routinely makes that branch shallow (resume/explain fetch just the tip with --depth=1, and nothing deepens it again). On a shallow clone the real common ancestor often lives below the shallow boundary — git doesn't have those objects, so merge-base reports "no common ancestor" even though one exists.

That false "disconnected" verdict then triggered a recovery routine that tries to cherry-pick the entire local history onto the remote tip. On a real team branch (thousands of commits deep) that blew a 1000-commit safety cap and aborted both git push and entire doctor — and doctor kept re-offering a "fix" that could never succeed. That's the exact incident a coworker hit after falling behind while traveling.

What the PR changes

1. Don't trust a merge-base miss when the metadata history is shallow. A new metadataDisconnected helper only treats "no common ancestor" as a genuine disconnection when the metadata commits' history does not reach a shallow boundary. If it's shallow-bounded, the verdict is suppressed (branches reported connected) and you're pointed at entire doctor. This is keyed on the metadata refs specifically — so a genuine disconnection on a repo that's shallow only for its source tree is still caught.

2. entire doctor deepens before deciding. When the metadata refs are shallow-bounded, doctor first fetches more history so merge-base can see the real ancestor and give an authoritative answer. Crucially it uses a ref-scoped --deepen, not --unshallow — --unshallow is repository-global and would convert a deliberately-shallow source checkout into a full clone; --deepen only extends the metadata branch and leaves the repo shallow.

3. Supporting correctness fixes (from review + the trail's agent findings):

• Tests use testutil.InitRepo so commits have a git identity under isolated config (was failing CI).
• FetchOptions.Deepen honors the documented precedence (ignored whenever Unshallow is set).
• doctor reopens the repo after deepening so the follow-up check sees the freshly-fetched objects.

Net effect

• git push: a behind-but-shallow branch is no longer misread as disconnected, so push just rebases your local-only checkpoints onto the remote tip and syncs normally.
• entire doctor: deepens, gets the true answer, and reports OK instead of looping on an impossible fix.
• Genuine disconnections (the real empty-orphan bug) are still detected and repaired.

In one sentence: it stops the CLI from mistaking "I have a shallow clone" for "my checkpoint history is broken," and makes entire doctor deepen just the metadata branch to confirm the truth before acting.

Want me to walk through any specific file's diff, or is the behavioral summary enough?

Great question — you're right that a shallow branch still has a ref pointing at a real commit. The key is the difference between "the ref exists" and "git can walk the history behind it." Shallow doesn't touch the ref; it truncates what's reachable.

What "shallow" actually means

When the CLI shallow-fetches checkpoints/v1 with --depth=1, three things happen:

1. The ref is written: refs/.../checkpoints/v1 → commit T. ✅ exists
2. T's commit object is downloaded — and internally it still names its parent P.
3. .git/shallow is written containing T, and P and everything below it are never downloaded.

That .git/shallow file is a graft: it tells git "pretend T has no parents — treat it as a root." So even though T records "parent = P", git refuses to walk past T, and P's object isn't in your store anyway.

So your object store looks like:

The ref is fine. The ancestry is a dead end one commit down.

Why merge-base then reports "no common ancestor"

git merge-base L R finds the newest commit reachable from both L and R by walking parent links downward. If the only shared ancestor lies below a shallow boundary, git has no objects to walk into and no permission to (the graft), so it returns exit 1 = "none" — even though, on the server, they obviously share history.

I verified this earlier: same two commits, full history → merge-base exit 0; write their tips into .git/shallow → exit 1. Nothing about the refs changed; only what git could traverse.

How you end up with two tips that can't be bridged

There are two refs in play, both pointing at real commits:

• L is real and fully present — those are checkpoint commits you made locally (they were never shallow; you authored them).
• R is real — a fetch updated the remote-tracking ref to the server's tip.
• But the commits that connect them — the shared trunk further down — were pruned by the --depth=1 fetch and never re-downloaded.

So both tips exist, both are valid, and they genuinely share ancestry on the server — but locally git can't see the join, so merge-base says "disconnected."

The coworker's path into it concretely

1. At some point entire resume/explain did a --depth=1 fetch of checkpoints/v1 → .git/shallow got set, ancestry truncated.
2. He was behind (never fetched the 1,509 Tokyo-week commits), and his pushes were silently failing, so his local branch kept accumulating its own checkpoint commits on top.
3. When push/doctor ran merge-base local remote, the shared trunk that links his local tip to the remote tip was below the shallow boundary → git couldn't reach it → false "disconnected."
4. The old code believed it and launched the full-history cherry-pick reconcile, which blew the 1000-commit cap on that deep branch → the aborts you saw.

So: shallow never broke the refs — it just hid the connecting commits, and merge-base mistook "I didn't download the link" for "there is no link." That's exactly the misread the PR removes (suppress the verdict when the metadata history is shallow-bounded; have doctor deepen first to fetch the missing link before deciding).

You're already touching the heart of it — but there are two things bundled in the question, so let me separate them.

First, a clarification: we are using the remote ref

The check already runs merge-base between the local branch and the remote-tracking ref:

So R is refs/remotes/origin/entire/checkpoints/v1. There's no "other" remote ref to switch to — that's the one we compare against.

Why picking the remote ref doesn't help

Here's the crux: the remote-tracking ref is not a complete view of the server. It only goes as deep as your last fetch pulled. After a --depth=1 fetch, refs/remotes/origin/... points at the right commit, but its ancestry is truncated at the same .git/shallow boundary. So R is just as blind below the boundary as L is.

And critically — any check that walks parent links has the identical blind spot. It doesn't matter whether you phrase it as:

• merge-base L R (find shared ancestor), or
• merge-base --is-ancestor L R ("is local contained in remote?"), or
• "is L somewhere in rev-list R?"

All three traverse parents and all three stop dead at .git/shallow. None of them can see a commit that wasn't downloaded. So reframing the check doesn't escape it — the missing information is the objects, not the query.

And we can't assume "local ⊆ remote"

The other half of your question — "assume the local ref has to be in the remote ref" — is the part that actually doesn't hold, and it's why this is genuinely hard.

Local is not always contained in remote. The whole manual-commit strategy is built so you can have unpushed local checkpoints — commits on L that aren't on the server yet. That's exactly the coworker's case: his pushes were failing, so his local branch was ahead of the remote with commits the remote didn't have.

So the legitimate states are: equal, behind, ahead, or diverged-but-sharing-history — all normal. The only truly bad state is "no shared history at all" (the orphan bug). If we could assume containment, we'd just reset local to remote and be done — but that would silently throw away his unpushed work.

What actually resolves it

To tell "genuinely disconnected" apart from "connected but the link wasn't downloaded," you need the missing objects. Two ways to get them:

1. Deepen / fetch the link — pull enough history that the connecting commit is local, then the same merge-base answers correctly. (This is what the PR's doctor now does, ref-scoped.)
2. Ask the server — git's fetch negotiation already computes connectivity from your "haves," but wiring that into a standalone check is a lot more machinery.

So your instinct — "can't we just compare differently against the remote?" — is the right question, and the answer is: the comparison was never the problem; the truncated object store is. Changing which ref or which traversal you run can't conjure commits that aren't on disk. The PR's two moves are therefore (a) don't trust the answer when the history is shallow-bounded, and (b) in doctor, fetch the link and then trust it.

Almost — but not from those three facts alone. There's one more ingredient, and it's the decisive one: a second shallow (--depth=1) fetch that brings the new remote tip without the commits linking it back to your base. "Remote advances" only matters once it's been fetched, and how it's fetched is what determines whether you get stranded.

Let me walk it concretely.

Start: shallow, with a local checkpoint

Right now merge-base(L1, A) = A. A is present, reachable from both. Connected. Fine.

Remote advances on the server

Server is now A → R1 → R2 → … → Z. But your repo doesn't know that yet — your remote-tracking ref still says A. Nothing has changed locally, so still fine.

The fork in the road: how does the remote-tracking ref get to Z?

If a normal (full) fetch updates it: git downloads Z and R1…R2, i.e. the whole chain back down to where it meets A (which you already have). Now merge-base(L1, Z) walks Z → … → A, finds A. Connected — no problem.

If a --depth=1 / tree-only fetch updates it (which is what entire resume/explain do via FetchMetadataTreeOnly): git downloads only Z, marks Z as a new shallow root, and does not download R1…R2:

Now merge-base(L1, Z):

• from L1: reach A, stop (shallow root)
• from Z: stop immediately (shallow root)
• A ≠ Z, neither reachable from the other → exit 1, "no common ancestor" → false "disconnected."

On the server they're obviously connected (Z descends from A), but the linking commits R1…R2 are simply not on your disk.

So, precisely

The trigger is: a depth-1 fetch pulls a newer remote tip whose ancestry back to your local base wasn't downloaded. Shallow + local commit + remote-advancing-on-the-server is the setup; the truncating fetch is the event.

Two side notes that make it sticky:

• Your local checkpoint is what keeps you trapped. Without L1, the resume/bootstrap logic would just fast-forward your local ref to Z and you'd never notice. With an unpushed L1, you're genuinely diverged-in-the-local-view, so it can't fast-forward past it — and that's the state where the old code launched the doomed reconcile.
• That's also why it was rare but real: it needs the tree-only fetch path to run while you're behind and holding local-only checkpoints — exactly the coworker's situation (behind from travel, pushes silently failing).

Exactly — and there's only one truncating fetch in the whole codebase, so this is tractable. Here's what I found.

The single source of truncation

FetchMetadataTreeOnly (--depth=1) is called from one place: resume.go:367. And look at what it's for — the comment says it all:

It's a read optimization: resume wants to cheaply read the tree of the latest checkpoint, so it grabs just the tip. The problem is that this read-only operation has durable, ancestry-destroying side effects that outlive the read. Inside fetchMetadataFromOrigin (Shallow path) it:

1. advances refs/remotes/origin/entire/checkpoints/v1 to the new tip Z,
2. SafelyAdvanceLocalRef advances the local primary ref too (when it can fast-forward),
3. and git fetch --depth=1 writes .git/shallow, truncating Z's ancestry.

So a cheap read permanently repoints the durable remote-tracking ref at a commit whose history isn't on disk. That's the poison the later merge-base trips over.

Why this reframes the fix

You're right that the cleaner root cause is here: a read path shouldn't durably advance shared refs to a tip it didn't download the ancestry for. The surgical source-side fix would be to make the tree-only read non-durable:

• Fetch the tip into a throwaway ref (the codebase already has this pattern — FetchTmpRefPrefix in checkpoint_remote.go), read the tree from there, and don't advance refs/remotes/origin/... (or the local primary) with a shallow tip.
• Let the durable remote-tracking ref only be moved by the full fetches (push's pre-fetch, resume's FetchMetadataBranch fallback), which bring ancestry.

Then merge-base(local, origin/...) always compares against a fully-connected origin/..., and the false-disconnect can't arise from resume at all.

My recommendation: do both, in two changes

I'd keep the PR's detection fix and add the source fix as a separate follow-up, because they protect different things:

A wrong "disconnected" verdict is catastrophic (it aborts push and loops doctor), so I wouldn't remove the detection robustness even after fixing the source — defense in depth is warranted there.

The source fix touches the resume hot path and deserves its own commit + tests (and arguably its own PR), rather than being bolted onto this one. I held off changing it because it's a bigger, riskier edit and you're still scoping it.

Want me to:

1. Implement the source fix (make FetchMetadataTreeOnly non-durable via a temp ref) as a separate commit/PR, or
2. just write it up as a follow-up issue/finding on the trail and leave this PR as the detection fix?

A pre-existing test-package build error appeared (unrelated to my change — trail_cmd_test.go references undefined auth test helpers). Let me check whether this is broken on main itself or a checkout artifact:

Confirmed — it's a genuine main breakage, not my change:

• go build ./... (production code) is clean — my fix compiles fine.
• The failure is only the test build of the cli package: trail_cmd_test.go references newAuthMemStore, newResolveTestManager, auth.SetManagerForTest, auth.DiscoveryUnavailableForTest — none of which are defined anywhere (grep found zero definitions). That came in with the merged PR #1410 ("demolish auth fallbacks"), which removed the helpers but left the test referencing them.

So I can't run the cli package tests on this base until that compiles. Let me validate my fix by temporarily moving the broken file aside (local only, restored immediately after):

I hit a fundamental wall, and it changes the conclusion. Let me report what I found rather than push a fix that doesn't work.

The temp-ref approach doesn't actually work

My test still fails, and digging into git 2.54's behavior shows why — there are two mechanisms poisoning origin/..., and the temp-ref trick defeats neither:

1. Opportunistic tracking updates are unavoidable. I tested every escape hatch — explicit refspec to a throwaway ref, --refmap=, fetching by URL instead of name, no-destination (FETCH_HEAD-only), -c remote.origin.fetch=. In git 2.54 every one still wrote refs/remotes/origin/entire/checkpoints/v1, because git maps the fetch back to the origin remote (even by file:// URL) and updates its tracking ref.

2. The deeper problem: --depth=1 shallows the commit, not just a ref. Even when the tip hash doesn't change, the depth-1 fetch adds that commit to .git/shallow. Any ref pointing at it — origin/..., local, or my temp ref — is now shallow-bounded. So even a perfect "don't advance origin/..." would still leave it pointing at a now-shallow commit. You can't read a tip via --depth=1 without creating a shallow boundary on the shared object store.

So "fetch the tip cheaply into a throwaway ref" can't avoid the poison — the poison is intrinsic to --depth=1.

The real options (all are tradeoffs)

My read: Option A is the cleanest real source fix, and it's probably affordable because git fetches incrementally (you pay the commit+tree history once, then deltas). It's especially natural for repos with filtered_fetches on (like entiredb), where the fetch is already --filter=blob:none. The downside is the first resume on a freshly-cloned deep metadata branch pulls the whole commit+tree graph instead of one commit.

The temp-ref commit I made is a dead end — I'd replace it with Option A (drop the Shallow flag from FetchMetadataTreeOnly, keep blob filtering), or abandon this branch and let the PR's detection fix stand as the protection.

How do you want to proceed?

Now I have the definitive answer.

What triggers the shallow state: the CLI does it to itself

There is exactly one thing in the whole codebase that creates shallow state, and it's self-inflicted — the CLI never runs git clone, so it's not your clone being shallow:

FetchMetadataTreeOnly → git fetch --depth=1 of entire/checkpoints/v1. That's the only --depth=1 / shallow fetch in production code. It runs inside getMetadataTree, whose fallback order is:

1. FetchMetadataFromCheckpointRemote — returns immediately with an error if no checkpoint_remote is configured (remote.Configured(ctx) false).
2. → falls through to FetchMetadataTreeOnly (--depth=1) ← this shallows the branch.
3. local
4. full fetch

So the trigger is precisely:

Running entire resume, entire explain --export, or entire attach on a repo where checkpoints live on origin (i.e. no separate checkpoint_remote configured). Each of those calls getMetadataTree, the checkpoint_remote step errors out, and the --depth=1 origin fetch runs — adding the checkpoints/v1 tip to .git/shallow.

Two consequences worth noting:

• It's effectively every resume. The --depth=1 fetch "always runs so the local lookup doesn't return stale data" (resume.go:365). So a heavy entire user on an origin-stored-checkpoints repo shallows checkpoints/v1 routinely — they don't have to do anything unusual. That matches the coworker: he never "made a shallow clone," his own entire resume calls did it.

• A configured checkpoint_remote avoids it. If checkpoints are fetched from a dedicated checkpoint remote, step 1 succeeds (and that fetch is full, NoFilter:true), so the --depth=1 path never runs and the branch never gets shallowed this way. So this whole class of bug is specific to origin-stored checkpoints — which is presumably how entiredb is set up.

Why this matters for the fix

This confirms the shallow state is self-inflicted by the CLI's own read path, which is exactly why "fix the source" was the right instinct. But as we found, the source operation is --depth=1, and --depth=1 intrinsically shallows the fetched commit (and git 2.54 opportunistically points origin/... at it regardless of refspec tricks). So you can't keep --depth=1 and avoid the poison — the only true source fixes are:

• Stop shallowing: make the tip-read full-depth + --filter=blob:none (Option A — correctness at the cost of pulling commit+tree history, incremental after the first fetch), or
• Make checkpoint_remote the norm so the shallow path is never taken, or
• Keep the detection-side PR fix as the safety net.

So the honest framing: it's not "when do we get a shallow clone" by accident — the CLI deliberately shallow-fetches checkpoints/v1 on every resume/explain/attach when checkpoints are on origin, and that's the root trigger. Want me to go ahead with Option A (drop --depth=1, keep blob filtering) on this branch now that we know it's the only real source fix?

Done. PR opened: https://github.com/entireio/cli/pull/1443

Summary

Fresh branch soph/treeless-fetch-full-depth off the latest main, implementing Option A — the root-cause source fix.

The one-line change in behavior: FetchMetadataTreeOnly no longer fetches with --depth=1. It fetches the metadata commit+tree graph at full depth and relies on --filter=blob:none (when filtered fetches are on) for cheapness.

Why this is the fix: we established the shallow boundary is self-inflicted — the CLI shallow-fetches entire/checkpoints/v1 on essentially every resume/explain/attach when checkpoints are on origin. And we proved the boundary is intrinsic to --depth=1 (can't be dodged with throwaway refs / --refmap= / URL fetch — git 2.54 shallows the commit and points origin/... at it regardless). So the only real source fix is to stop shallowing.

Diff: just git_operations.go (drop the Shallow flag from the tip-read + the now-dead Shallow field) plus a regression test TestFetchMetadataTreeOnly_DoesNotShallowRepo asserting the repo stays non-shallow after the tip-read.

Verification: go build, gofmt, golangci-lint (0 issues), the cli unit tests (resume/attach/explain/fetch/metadata), and the relevant integration tests (with filtered_fetches enabled) all pass.

Relationship to the other PR: this is independent of and complementary to #1434 (the detection-side hardening — IsMetadataDisconnected shallow-awareness + doctor deepen). This PR removes the source so the boundary never gets created; #1434 stays as defense-in-depth for any other shallow source (e.g. a user's manual git clone --depth).

Want me to watch CI on #1443, or note on #1434 that #1443 addresses the root cause?

The four cleanup agents (reuse, simplification, efficiency, altitude) are running in parallel on the PR #1443 diff. I'll wait for them to complete, then dedup their findings and apply the fixes.

Two of four agents are in. Key findings so far — reuse: gitOut duplicates an existing gitOutput helper in git_operations_test.go; simplification: Unshallow is now a dead field and fetchMetadataOpts collapses to a single noFilter bool. Waiting on the efficiency and altitude agents before applying.

Three of four in. Noting the efficiency finding: its recommended "fix" (conditionally use --depth=1 when filtered fetches are off) would reintroduce the exact shallow bug this PR removes — that's a behavior change reverting the intended fix, and the user explicitly chose Option A knowing the no-filter tradeoff (also largely mitigated by git's incremental fetch + normal clones already having the branch). I'll skip that one. Waiting on the altitude agent before applying the real cleanups (reuse gitOutput, drop dead Unshallow).

Cleanups applied and pushed to PR #1443. Here's the /simplify summary.

Fixed

1. Collapsed fetchMetadataOpts → noFilter bool (simplification + altitude). With Shallow gone, the struct held only NoFilter plus a dead Unshallow field (never set true by any caller). Replaced the options bag with a single bool; the two named wrappers (FetchMetadataBranch / FetchMetadataTreeOnly) document intent at the only two call sites.

2. Reused the existing gitOutput test helper (reuse + efficiency overlap). The new test defined its own gitOut — a byte-for-byte duplicate of gitOutput already in git_operations_test.go (same package). Switched all call sites to gitOutput and dropped the thin gitRevParse wrapper too (inlined gitOutput(t, dir, "rev-parse", rev)), removing ~16 lines of duplicated helper code and three now-unused imports.

Skipped (with reasons)

• Efficiency: "conditionally fall back to --depth=1 when filtered fetches are off." Rejected — its fix reintroduces the exact shallow boundary this PR removes for all no-filter repos, i.e. a behavior change that reverts the intended fix. The no-filter cost is the tradeoff you explicitly chose for Option A, and it's largely mitigated anyway: a normal clone already has the full metadata branch, and git fetches incrementally, so only new commits/trees travel after the first fetch.

• Altitude: rename FetchMetadataTreeOnly (agent found "TreeOnly" misleading now that it's full-depth). Skipped — "tree-only" reads accurately as trees, not blobs (which is exactly what --filter=blob:none does), the docstring spells out the behavior, and a rename ripples to resume.go + test files outside the diff for marginal benefit. Judgment call to leave it.

• Altitude: remote.FetchOptions.Shallow now unused in production. Confirmed acceptable by the agent itself — it's general-purpose infra with its own test (TestFetch_Shallow); not dead code.

Verification after cleanup: go build, gofmt, golangci-lint (0 issues), and the regression + neighbor tests all pass.

The PR is already open — I created it before the /simplify pass, and the cleanup commit (46c209c11) was pushed to the same branch, so it's already included.

• PR #1443 (OPEN): git_operations: stop shallow-fetching the metadata tip — https://github.com/entireio/cli/pull/1443
• Branch soph/treeless-fetch-full-depth, both commits present (the fix + the /simplify cleanups).

No new PR needed. Want me to check its CI status, or did you mean a separate PR for something else (e.g. the detection-side branch #1434)?

Pushed. Here's the rundown of the PR #1443 comments and what I did.

Comments (all from Copilot — no human reviews yet)

The fix

I verified Copilot's concern empirically: a plain local clone already has origin/checkpoints/v1 present before the fetch, whereas a single-branch file:// clone leaves it absent and forces a real fetch-pack. Crucially, under the old --depth=1 code that single-branch path flips the repo to shallow — so the --is-shallow-repository == false assertion now genuinely catches the regression instead of passing trivially.

Changed the clone to git clone --single-branch --branch main file://<bare> <clone>. Test still passes under the new code; lint clean (0 issues).

The second comment needed no action — it was about helper code I'd already removed during /simplify.

Want me to reply on the PR noting both are addressed (one fixed, one obsoleted by the cleanup), so Copilot/reviewers see the resolution?

Both replies posted and threaded correctly under Copilot's comments on PR #1443:

• Comment 1 (test sensitivity) → reply 3428831224: noted it's fixed in 83f3f0936 (single-branch file:// clone), with the empirical confirmation that the assertion now actually trips under the old --depth=1 behavior.
• Comment 2 (gitOut stderr/context) → reply 3428831437: noted it's obsolete — gitOut was removed in the /simplify cleanup (46c209c11); the test uses the existing gitOutput helper now, and changing that shared helper is out of scope.

Both threads now show the resolution in context for reviewers.

All checks are green now. It was a flake, not a real failure.

What happened

The failing job was test-core, and the one failing test was REDACTED — but it failed in t.TempDir cleanup (unlinkat .../.git/objects: directory not empty), not in any assertion. That's a known race where a transient git subprocess is still writing as the temp dir is torn down.

I confirmed it's unrelated to this PR:

• setupAttachTestRepo configures no origin remote, so FetchMetadataTreeOnly fails fast (git fetch origin → no such remote) in both the old --depth=1 code and my full-depth version — my one-line change can't alter this path.
• The test passes locally (ran it 3×), and it's in the actively-churning attach+metadata area (last touched by 8efab2905 fix review attach metadata guard).

I re-ran the failed jobs and they passed: test-core ✅, test ✅, plus canary and all integration shards green.

PR #1443 is now fully green. Nothing to fix on our side — want me to flag that flaky cleanup race separately (it's a pre-existing test-infra issue, not part of this PR), or leave it?

That was just my earlier background poll finishing — it reports the same result I already confirmed directly: test-core and the rest are green. Nothing new.

PR #1443 is fully passing. Still open from my side: do you want me to flag that flaky TempDir cleanup race in REDACTED separately (pre-existing test-infra issue, not part of this PR), or leave it?

I've created the clean branch soph/fix-attach-review-tempdir-flake off the latest main and launched a diagnostic agent to trace what attach --review spawns asynchronously (background goroutine, detached analytics process, or a lingering git subprocess) that could write into .git/objects during t.TempDir cleanup. I'll dig into the fix once it reports back the exact culprit.

Done. PR opened: https://github.com/entireio/cli/pull/1448 on a clean branch soph/fix-attach-review-tempdir-flake off the latest main.

What it fixes

The flaky TempDir cleanup failure (unlinkat .../.git/objects: directory not empty) in the attach/review tests.

Root cause (diagnosed, = COR-394): attach runs git commit --amend through the git CLI, which inherits the dev's real git config. With default gc.auto/gc.autoDetach, that commit forks a detached git gc that keeps writing into the temp repo's .git/objects after the command returns — racing Go's t.TempDir cleanup. setupAttachTestRepo set repo-local identity but never isolated global/system git config, so the gc-disabling pins weren't applied.

Fix: one line — add testutil.IsolateGitConfigEnv(t) to setupAttachTestRepo, which points GIT_CONFIG_GLOBAL/SYSTEM at the isolation config (gc.auto=0, gc.autoDetach=false, …). This is the exact remedy testutil already documents for this race.

I ruled out the red herrings (detached telemetry — gated off in tests and never touches the repo; version-check — early-returns on test builds), and confirmed testutil.GitAdd/GitCommit use go-git so they never forked gc. Build, lint (0 issues), and the attach/review tests all pass.

One thing worth noting (didn't expand scope): other setup helpers that run gc-triggering git CLI commands (e.g. some resume tests) could have the same latent gap. This PR fixes the one that was failing; happy to sweep the others into a follow-up if you want.

COR-394 is a tracking-ticket reference, not something defined in the code. It shows up in exactly two places:

• testutil.go:220 — the pre-existing comment on the git-isolation config, documenting the git gc / t.TempDir "directory not empty" race and why the isolation config pins gc.auto=0/autoDetach=false.
• attach_test.go:1507 — the comment I just added, pointing back to the same issue.

The COR- prefix matches this org's Linear tickets (your branch list has cor-393, cor-371, cor-96, etc. — "COR" is presumably the Core team's key). So COR-394 is the Linear issue that originally diagnosed/tracked this flaky-test-from-detached-gc class, which is why testutil centralized the fix.

To be clear about what I actually know: I know its subject (the gc/TempDir race) purely from that code comment — I haven't read the ticket itself, so I can't vouch for its full contents, status, or history.

Want me to pull the actual ticket from Linear so you get the real description and current status?

The diff is a single change: adding one call to an existing shared helper (testutil.IsolateGitConfigEnv(t)) plus an explanatory comment in setupAttachTestRepo. There's no new logic, no new code — it is a reuse of purpose-built infrastructure. Fanning out four parallel agents to scrutinize one call to an existing helper would be wasteful theater, so I reviewed it directly across the four angles:

• Reuse ✓ — The change calls the existing testutil.IsolateGitConfigEnv, the central remedy already built for this race. It's the embodiment of reuse, not a violation; nothing is re-implemented.
• Simplification ✓ — One call + comment. No redundant/derivable state, no nesting, no dead code. The 4-line comment is justified (it explains a non-obvious detached-gc race) and matches the documentation density of the helper it mirrors in testutil.go.
• Efficiency ✓ — IsolateGitConfigEnv is t.Setenv-based, runs once in test setup, no hot path / startup cost / scope-capturing closure.
• Altitude ✓ — Right depth: it fixes the shared setupAttachTestRepo (covering all attach/review tests) using the documented central mechanism, rather than a local bandaid like hand-setting gc config or retrying cleanup.

No fixes to apply — the diff is already clean.

One altitude observation I deliberately did not act on (out of scope): you could push the isolation even higher by folding IsolateGitConfigEnv into testutil.InitRepo so every CLI test gets gc isolation by default. I skipped it because it's well outside this diff, would affect every InitRepo caller's assumptions, and InitRepo itself uses go-git (never forks gc) — so it's a separate design decision, not a cleanup for this PR. Worth a follow-up if you want the broader hardening.

Pushed to PR #1443 (commit 301e0da34).

What I added

The PR now also heals repos already poisoned by the old code, not just prevents new poisoning:

• fetchMetadataFromOrigin sets Unshallow: true on the metadata fetch. remote.Fetch only emits --unshallow when the repo is actually shallow, so:
  • normally-cloned repo → no-op (flag not added),
  • repo with a lingering --depth=1 boundary from the old CLI → unshallowed, removing the graft so merge-base works again.
• --unshallow fetches the missing ancestry (the repo becomes genuinely complete, not a "claims-complete-but-missing-objects" state) and composes cleanly with --filter=blob:none — both verified empirically.
• Updated the now-stale FetchMetadataBranch comment that said it never unshallows.

Test

Added TestFetchMetadataTreeOnly_HealsPriorShallow: reproduces the poisoned state (a --depth=1 fetch grafts the metadata tip → repo shallow), runs the tip-read, and asserts the repo is unshallowed with full history restored. Also extracted the shared seed/clone setup into a helper (so the two tests don't duplicate ~30 lines) and replaced the repeated "true"/"false" string checks with a repoIsShallow helper (cleared a goconst lint hit).

Verification: gofmt, golangci-lint (0 issues), both unit tests, and the relevant integration suite (filtered_fetches enabled) all pass.

One note for your call: --unshallow is repo-global, so on the rare repo where the source tree itself was deliberately shallow-cloned, the first metadata fetch would also deepen it to full history (correct, but a larger one-time download). The realistic case is metadata-only shallow (self-inflicted by the old CLI), where it's cheap. Want me to reflect this healing behavior in the PR description, or leave the description as-is?

I tested it directly, and unfortunately --depth=2147483647 does not stay scoped to our branch in git 2.54 — it behaves exactly like --unshallow:

So the huge-depth fetch deepened main (the unrelated source-tree branch) from 1→5 and fully unshallowed the repo — same global effect as --unshallow. I'd earlier found --deepen=<huge> does the same.

Why it's unavoidable: .git/shallow is repo-global. To remove a commit from it (full heal), git must guarantee no ref is left dangling-shallow — so it deepens everything before deleting the file. The only way to stay ref-scoped is a bounded --deepen=N that doesn't fully resolve — which keeps .git/shallow (and the source-tree boundary) intact, but only partially heals the metadata branch (and leaves the repo still "shallow").

So the realistic options are:

My honest read: the source-tree-shallow case is rare for entire users (the tooling assumes a real dev clone you can commit/rebase/push on), the self-inflicted case is metadata-only shallow where --unshallow only deepens the metadata branch (cheap), and in the rare both-shallow case --unshallow leaves a complete, correct repo — just a bigger one-time fetch. So I'd lean toward keeping --unshallow rather than a heuristic bounded deepen.

Your call:

1. Keep --unshallow (what's pushed) — simplest, fully correct, accept the rare extra deepening.
2. Bounded --deepen=N — strictly ref-scoped but only a partial/heuristic heal.
3. Drop healing from this PR — let doctor handle it.

Which way do you want to go?

Done — switched from --unshallow to your ref-scoped --depth approach. Pushed to PR #1443 (d775042ad).

The empirical finding that made it work

Your instinct was right, just not at the exact value you proposed:

And critically, on a full repo --depth=1000000000 is a no-op (stays non-shallow, fetches the branch fully) — so it never re-introduces the poison.

What changed

• remote.FetchOptions.Depth int → emits --depth=<N> (ref-scoped). Documented the INT_MAX caveat.
• fetchMetadataFromOrigin now passes Depth: metadataFetchDepth (1_000_000_000) instead of Unshallow: true. So the metadata fetch:
  • full repo → no-op (no shallow introduced),
  • metadata-only shallow (self-inflicted) → heals it,
  • source-tree also shallow → heals metadata, leaves the source tree shallow (your concern, fixed).
• Const metadataFetchDepth = 1_000_000_000 with a comment explaining it's above any realistic checkpoint-branch length yet below math.MaxInt32.
• TestFetch_Depth (remote package) locks in the ref-scoping: heals the named branch while a second shallow branch stays shallow. The existing cli heal / no-shallow tests still pass.

Verification: build, gofmt, golangci-lint (0 issues), the remote TestFetch_* suite, the cli tree-only tests, and the integration suite (filtered fetches) all pass.

The only residual edge: if a metadata branch ever exceeds ~1e9 commits it wouldn't fully heal — not a real concern (entiredb's was ~10k). Want me to update the PR description to reflect the --depth approach (it currently describes --unshallow)?

The four cleanup agents (reuse, simplification, efficiency, altitude) are running in parallel on the PR #1443 diff. I'll wait for all four, dedup their findings, and apply the worthwhile cleanups.

Simplification agent is in (keep Shallow+Depth, keep noFilter; minor optional test-helper consistency nit). Waiting on reuse, efficiency, and altitude before applying.

Efficiency (unconditional --depth=1e9 is fine, gating would add a syscall — no change) and altitude (two "pragmatic but below-ideal" findings: knob sprawl and the magic constant) are in. Waiting on the reuse agent before deduping and applying.

All four agents are in and I've confirmed the key fact: after this PR, Shallow and Unshallow have no production callers — only Depth: metadataFetchDepth is used in prod; Shallow/Unshallow appear only in their own remote-package tests.

Deduping the findings, my conclusion is that the diff is clean — no fixes worth applying. Here's the disposition of each finding:

Verdict: clean (no changes)

Reuse ✓ — No same-package duplication. revListCount (remote pkg) and the inline gitOutput(…, "rev-list", "--count", …) (cli pkg) are in different packages; helpers aren't cross-package callable, so it's not a violation.

Efficiency ✓ — The unconditional --depth=1000000000 is correct: it's a no-op on full repos (no extra negotiation), and gating it on "already shallow" would add a rev-parse --is-shallow-repository syscall on every resume. Keep as-is.

Simplification ✓ — Shallow+Depth and the noFilter param both earn their keep. The one nit (cli test asserts "not shallow" via a repoIsShallow bool helper but checks commit count inline) is internally consistent (both shallow checks use the helper, both count checks inline) and the reviewer rated it "not urgent." Skipped as marginal — extracting a count helper would add a strconv import for two readable two-line checks.

Altitude — two "pragmatic but below-ideal" findings, both skipped:

• Knob sprawl (Shallow/Depth/Unshallow): collapsing them touches pre-existing fields and their dedicated tests — well outside this diff's scope, and git genuinely has no single "depth mode." The simplification agent independently recommended keeping them.
• Magic constant 1_000_000_000: the dependency on git's INT_MAX special-case is real, but the const is named and documented, with a deliberate 2× safety margin below MaxInt32 — expressing it as MaxInt32-1 (the "intent-revealing" alternative) would be riskier, not safer. An intent-named wrapper would add API surface for a single caller.

On the now-unused Shallow/Unshallow

This PR removed their last production callers. I'm leaving them: they're coherent general-purpose remote.FetchOptions capabilities with their own regression tests (TestFetch_Shallow, TestFetch_Unshallow), and deleting pre-existing tested API is outside this PR's intent. Worth a separate "prune unused fetch options" cleanup if you want, but not here.

No commit needed — the code was already clean. Want me to update the PR #1443 description (it still describes --unshallow) to reflect the final --depth approach?