Skip to content

Speed up checkpoints v2 pre-push recovery #1208

Open
computermode wants to merge 5 commits into
mainfrom
fix-slow-v2-pushes
Open

Speed up checkpoints v2 pre-push recovery #1208
computermode wants to merge 5 commits into
mainfrom
fix-slow-v2-pushes

Conversation

@computermode
Copy link
Copy Markdown
Contributor

@computermode computermode commented May 14, 2026

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

This PR improves the performance of the pre-push hook for v2-enabled repos with 100+ checkpoints down from ~60s (!) to 5s. Tested with the entiredb repo. Observed push times went down from close to a minute to about 5-6 seconds (still unacceptable but much better while the refs fixes are being addressed). Double-checked and ensured checkpoints were pushed up correctly.

  • Replace the v1 <> v2 missing checkpoints check for the migration hint message with a single "does v2 /main exist?" ref probe. The hint now only fires when migration was never run; partial migrations are no longer flagged (a little drift is acceptable post-migration since we fall back to v1 in the UI when we display transcripts for now anyway).

  • Probe rotation archive refs with --filter=blob:none and top up only the matched archive's blobs after pick. Falls back to a full fetch on any first-fetch error (e.g. server without uploadpack.allowFilter).

  • Bind the per-archive ancestry walk to 1s. On repos whose archives are fully disjoint from local /full/current we'd otherwise burn seconds concluding nothing matches. A future /full/root anchor will replace this with a constant-time lookup ( see v2 checkpoints: add deterministic /full/root anchor commit #1201).


Note

Medium Risk
Changes the v2 pre-push recovery fetch/rotation-conflict logic to use explicit filtered fetches and a new archive-selection path, which could affect correctness in edge cases (servers without filter support, unusual ref layouts) but is scoped to checkpoint sync/push flows.

Overview
Speeds up v2 pre-push recovery by switching recovery fetches to probe with --filter=blob:none (with fallback to unfiltered fetch) so merges can operate on commits/trees without downloading transcript blobs.

Adds remote.ResolveFilteredFetchTarget to always resolve remote names to URLs for explicit filtered fetches, preventing git from persisting promisor/partial-clone settings onto named remotes.

Refactors rotation-conflict handling for v2/full/current to fetch archived generations via a wildcard refspec, select the related archive using git merge-base with a bounded per-archive walk budget, and improves temp-ref cleanup; updates/adds tests to assert no blob downloads and no remote.origin.promisor/remote.origin.partialclonefilter side effects.

Reviewed by Cursor Bugbot for commit 1bc7c4e. Configure here.

Three independent fixes that drop entiredb's pre-push from ~60s to ~5s:

- Probe rotation archive refs with --filter=blob:none and top up only
  the matched archive's blobs after pick. Falls back to a full fetch on
  any first-fetch error (e.g. server without uploadpack.allowFilter).

- Bound the per-archive ancestry walk to 1s. On repos whose archives
  are fully disjoint from local /full/current we'd otherwise burn
  seconds concluding nothing matches. A future /full/root anchor will
  replace this with a constant-time lookup.

- Replace the v1↔v2 cross-check in the migration hint with a single
  "does v2 /main exist?" ref probe. The hint now only fires when
  migration was never run; partial migrations are no longer flagged
  (drift is acceptable post-migration).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: a15ddf881892
@computermode computermode requested a review from a team as a code owner May 14, 2026 01:04
Copilot AI review requested due to automatic review settings May 14, 2026 01:04
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 optimizes v2 checkpoint pre-push recovery by reducing expensive migration checks and limiting how much checkpoint archive data is fetched during rotation recovery.

Changes:

  • Replaces v1/v2 checkpoint diffing for the migration hint with a local v2 /main ref existence check.
  • Changes rotation archive probing to use blobless fetches, then fetch blobs only for the matched archive.
  • Adds a wall-clock budget for archive ancestry matching and tests for blobless archive probing behavior.

Reviewed changes

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

File Description
cmd/entire/cli/strategy/push_v2.go Updates remote rotation archive fetch/recovery to use blobless probing, blob top-up, and ancestry walk budgeting.
cmd/entire/cli/strategy/push_v2_test.go Adds coverage for avoiding blob fetches from unmatched rotation archives.
cmd/entire/cli/strategy/push_common.go Simplifies v2 migration hint detection to probe for the local v2 /main ref.
cmd/entire/cli/strategy/push_common_test.go Updates migration hint tests for the new v2 /main existence behavior.

Comment thread cmd/entire/cli/strategy/push_v2.go Outdated
Comment thread cmd/entire/cli/strategy/push_v2.go Outdated
Comment thread cmd/entire/cli/strategy/push_v2.go Outdated
Comment thread cmd/entire/cli/strategy/push_v2.go Outdated
Trim verbose doc comments and a redundant inline comment in the new
rotation-recovery test. No logic changes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: b466825dbff2
Fetch checkpoint recovery refs bloblessly through resolved URLs so pushes do not download transcript blobs or persist partial-clone settings on origin.

Replace rotation archive probing with a single wildcard archive fetch, check newest archives first, and use git merge-base under the existing ancestry timeout.

Add regression coverage for filtered fetch target resolution, blobless merge recovery, rotation recovery cleanup, and pushability after tree-only merges.

Entire-Checkpoint: 0845ffccc4f3
@computermode
Copy link
Copy Markdown
Contributor Author

Bugbot run

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 1bc7c4e. Configure here.

}
if _, ok := currentAncestors[c.Hash]; ok {
found = true
return errStop
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Merge-base exit codes not distinguished, masking errors

Medium Severity

commitsShareHistory uses cmd.Run() == nil which treats every non-zero exit code from git merge-base identically. Exit code 1 means "no common ancestor" (a valid negative result), while exit code 128 signals an actual failure (bad object, corrupt repo, etc.). By collapsing both into false, a real git error during the ancestry walk is silently swallowed, potentially causing the rotation-conflict recovery to skip the correct archive and produce a misleading "no remote archive shares history" error.

Fix in Cursor Fix in Web

Triggered by learned rule: Distinguish git command exit codes — exit 1 means "no results", not error

Reviewed by Cursor Bugbot for commit 1bc7c4e. Configure here.

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