Skip to content

strategy: don't treat shallow merge-base miss as disconnected metadata - #1434

Closed
Soph wants to merge 4 commits into
mainfrom
fix-shallow-merge-base-false-positive-in-metadata-disconnection-check
Closed

strategy: don't treat shallow merge-base miss as disconnected metadata#1434
Soph wants to merge 4 commits into
mainfrom
fix-shallow-merge-base-false-positive-in-metadata-disconnection-check

Conversation

@Soph

@Soph Soph commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

https://entire.io/gh/entireio/cli/trails/569

On a shallow checkpoint clone, git merge-base reports "no common ancestor" whenever the real ancestor lives below the shallow boundary — git has no objects there. The disconnection check trusted that exit code as proof of disconnection, so an ordinary diverged-but-behind metadata branch on a shallow clone was misread as disconnected. That triggered a full-history cherry-pick reconcile, which then blew MaxCommitTraversalDepth on a deep team branch and aborted both entire push and entire doctor ("commit chain exceeded 1000 commits without reaching root"), leaving doctor looping on a fix that could never succeed.

checkpoint/v1 clones are routinely shallow (resume/explain fetch the tip with --depth=1 via FetchMetadataTreeOnly and nothing deepens them again), so this hit a normal user who fell behind while out of office.

Fix:

  • New metadataDisconnected() only trusts a merge-base miss as a genuine disconnection when the repo is NOT shallow; on a shallow clone the verdict is suppressed and the refs are reported connected. Routed through IsMetadataDisconnected (warn hot path + doctor detection) and ReconcileDisconnectedMetadataRef (push + doctor fix), so the warn path stays network-free and the doomed reconcile no longer fires.
  • entire doctor now deepens the metadata branch (--unshallow, best-effort, without advancing the local ref) before the check when the repo is shallow, so it can still detect and repair genuine disconnections on an accurate, fully-materialized history.
  • Export remote.IsShallowRepository so both packages share one check.

With the false positive gone, push rebases the local-only commits onto the remote tip normally and doctor reports OK instead of erroring. Genuine empty-orphan disconnections are still caught (by doctor after deepening, and on push the rebase cap still refuses to combine unrelated histories).

Entire-Checkpoint: 175693b95a72


Note

Medium Risk
Changes metadata disconnection detection and doctor fetch behavior on shallow repos; incorrect logic could miss real disconnections or skip needed repair, but behavior is covered by new tests and doctor still deepens before authoritative checks.

Overview
Fixes false disconnected metadata detection on shallow checkpoint clones, where git merge-base can report no common ancestor even when local and remote still share history below the shallow boundary.

metadataDisconnected only trusts a merge-base miss when the repo is not shallow; on shallow clones it reports refs as connected so push/warn paths skip a doomed full-history cherry-pick reconcile (which could hit MaxCommitTraversalDepth). IsMetadataDisconnected and ReconcileDisconnectedMetadataRef route through this helper.

entire doctor best-effort calls new DeepenMetadataBranch (--unshallow fetch of the metadata branch without advancing the local ref) before the disconnection check when the clone is shallow, so genuine disconnections can still be found after history is materialized.

remote.IsShallowRepository is exported for shared shallow detection; tests cover shallow false-positive suppression and real disconnections on full repos.

Reviewed by Cursor Bugbot for commit 7742dd7. Configure here.

Copilot AI review requested due to automatic review settings June 15, 2026 19:40
@Soph
Soph requested a review from a team as a code owner June 15, 2026 19:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes false “metadata branch disconnected” detection on shallow checkpoint clones by making the ancestry check shallow-aware, and by teaching entire doctor to deepen metadata history before performing an authoritative disconnection check/repair.

Changes:

  • Route metadata disconnection detection through a new metadataDisconnected helper that suppresses merge-base “no common ancestor” results when the repo is shallow.
  • Add DeepenMetadataBranch and invoke it from entire doctor (best-effort) to unshallow metadata history before checking/reconciling.
  • Export remote.IsShallowRepository for shared shallow detection across packages and add tests for the new behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
cmd/entire/cli/strategy/metadata_reconcile.go Adds shallow-aware disconnection detection and uses it in warning + reconcile paths.
cmd/entire/cli/strategy/metadata_reconcile_test.go Adds coverage for shallow suppression vs genuine disconnection scenarios.
cmd/entire/cli/git_operations.go Introduces DeepenMetadataBranch to unshallow metadata history without advancing the local primary ref.
cmd/entire/cli/doctor.go Deepens metadata history on shallow repos before checking for disconnection (best-effort).
cmd/entire/cli/checkpoint/remote/git.go Exports IsShallowRepository and uses it to gate --unshallow.
cmd/entire/cli/checkpoint/remote/git_test.go Updates tests to use the exported IsShallowRepository.

Comment thread cmd/entire/cli/strategy/metadata_reconcile_test.go Outdated
Comment thread cmd/entire/cli/strategy/metadata_reconcile_test.go Outdated
Soph and others added 2 commits June 15, 2026 22:10
On a shallow checkpoint clone, `git merge-base` reports "no common
ancestor" whenever the real ancestor lives below the shallow boundary —
git has no objects there. The disconnection check trusted that exit code
as proof of disconnection, so an ordinary diverged-but-behind metadata
branch on a shallow clone was misread as disconnected. That triggered a
full-history cherry-pick reconcile, which then blew MaxCommitTraversalDepth
on a deep team branch and aborted both `entire push` and `entire doctor`
("commit chain exceeded 1000 commits without reaching root"), leaving
doctor looping on a fix that could never succeed.

checkpoint/v1 clones are routinely shallow (resume/explain fetch the tip
with --depth=1 via FetchMetadataTreeOnly and nothing deepens them again),
so this hit a normal user who fell behind while out of office.

Fix:
- New metadataDisconnected() only trusts a merge-base miss as a genuine
  disconnection when the repo is NOT shallow; on a shallow clone the
  verdict is suppressed and the refs are reported connected. Routed through
  IsMetadataDisconnected (warn hot path + doctor detection) and
  ReconcileDisconnectedMetadataRef (push + doctor fix), so the warn path
  stays network-free and the doomed reconcile no longer fires.
- `entire doctor` now deepens the metadata branch (--unshallow, best-effort,
  without advancing the local ref) before the check when the repo is
  shallow, so it can still detect and repair genuine disconnections on an
  accurate, fully-materialized history.
- Export remote.IsShallowRepository so both packages share one check.

With the false positive gone, push rebases the local-only commits onto the
remote tip normally and doctor reports OK instead of erroring. Genuine
empty-orphan disconnections are still caught (by doctor after deepening,
and on push the rebase cap still refuses to combine unrelated histories).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 175693b95a72
P1: the two new metadata-disconnection tests initialized repos with raw
`git init` then committed under testutil.GitIsolatedEnv() (which clears
global/system config), so CI failed with "Author identity unknown". Use
testutil.InitRepo, which writes repo-local user.name/user.email and
disables signing, via a shared gitRunnerInDir helper.

P2: `entire doctor` deepened the metadata branch with --unshallow whenever
the repo was shallow. --unshallow is repository-global — it removes
.git/shallow and deepens unrelated branches — so doctor could convert a
deliberately shallow source-tree checkout into a full clone just to check
metadata. Two changes:

- Gate the deepen on the metadata refs actually reaching a shallow boundary
  (new strategy.MetadataHistoryShallowBounded, reusing the existing
  hasReachableShallowBoundary helper), so an unrelated shallow boundary
  never triggers a deepen. The suppression in metadataDisconnected is now
  keyed on the metadata commits' shallow-boundedness too, so a genuine
  disconnection on a repo that is shallow only for the source tree is still
  reported.
- Add a ref-scoped FetchOptions.Deepen (--deepen=N) and use it instead of
  --unshallow in DeepenMetadataBranch. --deepen keeps the repo shallow and
  touches only the metadata ref's boundary.

Revert the now-unused IsShallowRepository export back to unexported.
Add TestFetch_Deepen covering the ref-scoped behavior.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: f4c6e69aa804
@Soph
Soph force-pushed the fix-shallow-merge-base-false-positive-in-metadata-disconnection-check branch from 0ec849c to 63c7435 Compare June 15, 2026 20:11
Soph and others added 2 commits June 16, 2026 07:22
Trail finding (low): when opts.Unshallow was true but the repo was not
shallow, the --unshallow switch case fell through to the Deepen case, so
--deepen fired even though Deepen is documented as ignored whenever
Unshallow is set. (Harmless in practice — git treats --deepen on a
non-shallow repo as a no-op — but it contradicts the contract.) Guard the
Deepen case with !opts.Unshallow so the precedence holds regardless of
shallow state.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 09f7d86d9966
Trail finding (high): checkDisconnectedMetadata kept using the repo handle
opened before DeepenMetadataBranch shelled out to git, then ran the
connectivity check (and any reconcile) against that stale handle.

go-git v6's ShallowStorage.Shallow() actually reads .git/shallow fresh on
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 depending on that is fragile, and the codebase already reopens after git
operations (resume.go's freshRepo) to avoid storer/pack-index staleness.

Swap to a freshly opened repo after a successful deepen so detection and
reconcile operate on a handle that knows about the fetched objects. The
deferred close is now a closure so the post-swap handle is the one closed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 80fc470879d1
@Soph

Soph commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator Author

close in favor of #1443

@Soph Soph closed this Jun 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants