Add fetch-replay regression tests for diverged/disconnected checkpoint metadata#1260
Open
Soph wants to merge 3 commits into
Open
Add fetch-replay regression tests for diverged/disconnected checkpoint metadata#1260Soph wants to merge 3 commits into
Soph wants to merge 3 commits into
Conversation
…branches Adds FetchMetadataBranch test cases for scenarios beyond the existing fast-forward / no-rewind contract: - diverged local (sibling commits sharing a base) - disconnected local with empty-tree orphan root (review C1) - disconnected local with a multi-commit chain (review M1) - shallow-clone divergence that must not silently merge - shallow-boundary error wording (review M3) Three of these (DivergedLocal_ReplaysOntoRemote, DisconnectedEmptyRoot_FiltersOrphan, DisconnectedMultiCommit_PreservesAllLocalData) fail against the current main contract (PR #1252 preserves but does not replay diverged refs) and are intended as an executable specification of the in-flight replay work in the config-perm branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 9ff7f60c868d
Two CLI-level integration tests covering the cross-machine scenarios the strategy-package tests don't reach: TestResume_StaleLocalMetadata_RemoteHasNewerCheckpoint reproduces the PR #1252 user-visible bug end-to-end via the real CLI binary: when the local entire/checkpoints/v1 ref is behind refs/remotes/origin/... after another machine pushed, `entire session resume <branch>` printed "session log not available" for the newer checkpoint. The unit-level TestResumeFromCurrentBranch_FastForwardsStaleLocalMetadata covers the same path with a direct function call; this one catches wiring regressions the unit test would miss. TestPushAfterDiverged_NoDoubleReplayCommits guards against the architectural concern raised in the config-perm review (finding H1): if both fetchMetadataFromOrigin -> SafelyAdvanceLocalRef AND pushCheckpointBranch -> ReconcileDisconnectedMetadataBranch run during a single push retry, the merged history could contain a replayed commit twice. Asserts exactly 3 commits on the metadata branch after the divergence-then-push flow (root + A + B rebased); a double-replay would produce 4. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 68e22cb66b76
Contributor
There was a problem hiding this comment.
Pull request overview
Adds regression coverage around fetching and reconciling the entire/checkpoints/v1 metadata branch when local and remote histories diverge or are disconnected, including shallow-history safety cases and end-to-end CLI validation for stale local metadata scenarios.
Changes:
- Adds a new strategy-level test suite specifying replay/merge behavior for diverged/disconnected metadata refs (including empty-tree orphan filtering and shallow-boundary safety).
- Adds new integration tests covering
entire session resumewith stale local metadata vs. advanced origin, and guarding against “double replay” commit duplication after diverged push retries.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| cmd/entire/cli/strategy/replay_disconnected_test.go | New unit tests specifying fetch/replay behavior for diverged, disconnected, and shallow metadata branch scenarios. |
| cmd/entire/cli/integration_test/diverged_replay_test.go | New integration tests for stale-local-metadata resume and commit-count regression after diverged push retries. |
Comment on lines
+19
to
+23
| // Several of these tests document behavior introduced by the in-flight | ||
| // "replay local checkpoints when fetch finds a diverged remote" work and will | ||
| // fail against the current main contract (which preserves but does not replay | ||
| // diverged local refs). That is intentional -- they serve as an executable | ||
| // specification of the target behavior. |
The "K" shorthand wasn't a standard convention — just disambiguation from A/B/C used elsewhere. Spell out "first checkpoint" / "second checkpoint" in both variable names and comments so the scenario reads without needing a key. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 91ff5ce70013
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds 7 new tests covering the FetchMetadataBranch behavior introduced by PR #1252 and the in-flight replay work in PR #1251 (
config-perm). Pairs with the existing fast-forward / no-rewind contract tests incheckpoint_remote_test.go.Tests
cmd/entire/cli/strategy/replay_disconnected_test.go(Groups A + E)mainTestFetchMetadataBranch_DivergedLocal_ReplaysOntoRemoteTestFetchMetadataBranch_DisconnectedEmptyRoot_FiltersOrphanTestFetchMetadataBranch_DisconnectedMultiCommit_PreservesAllLocalDataTestFetchMetadataBranch_ShallowClone_RefusesReplayAcrossBoundaryTestFetchMetadataBranch_ShallowBoundary_ErrorIsActionabledoctor/--unshallow(review M3). Skips onmainbecause the preserve-on-divergence path returns no error.cmd/entire/cli/integration_test/diverged_replay_test.go(Groups C + D)mainTestResume_StaleLocalMetadata_RemoteHasNewerCheckpointentire session resume <branch>must not say "session log not available". Catches wiring regressions the unit test would miss.TestPushAfterDiverged_NoDoubleReplayCommitsSafelyAdvanceLocalRefandReconcileDisconnectedMetadataBranchcherry-pick the same commits).Expected outcome once #1251 merges
DivergedLocal_ReplaysOntoRemoteDisconnectedEmptyRoot_FiltersOrphanreplayDisconnectedLocalRef. This test holds review C1 open until fixed.DisconnectedMultiCommit_PreservesAllLocalDataShallowClone_RefusesReplayAcrossBoundaryhistoryReachesShallowBoundaryguard refuses replay, which the test accepts as a safe outcome)ShallowBoundary_ErrorIsActionableerrNoMergeBaseerror message is the exact wording review M3 flagged as not actionable ("reachable shallow history prevents proving refs are disconnected"); the test asserts the user-facing string mentionsdoctor/--unshallow/fetch --depth.In other words: landing #1251 as-is turns 2 RED → GREEN and exposes 2 NEW RED that correspond to the C1 + M3 review findings. The two new reds are the forcing function for the follow-up fixes.
Why three tests fail on
maintodayThis branch is off
origin/main. PR #1252 is merged but PR #1251 is not — those three RED tests document behavior introduced by #1251 (replay local commits when fetch finds a diverged remote). They serve as an executable specification: GREEN when #1251 lands, RED until then. The SKIP test will start running after #1251 and gate on review M3 wording.What this PR does NOT cover
TestFetchMetadataBranch_{FetchesAndCreatesLocalBranch,UpdatesExistingLocalBranch,DoesNotRewindLocalAhead}incheckpoint_remote_test.go.e0834e9766onconfig-perm): V2 write paths are being deprecated (Remove checkpoints v2 write paths #1249 removed the v2 settings, recent commits removed write/dual-write paths). Adding a test that exercises soon-to-be-removed code wasn't worth the cost.TestConcurrentPush_SecondPusherRebasesAndRetriesalready covers the basic concurrent-push case;TestPushAfterDiverged_NoDoubleReplayCommitsadds a commit-count assertion on top.Test plan
mise run lint— 0 issuesmise run test:integrationfor adjacent tests (TestPrePush*,TestCloneAndResume*,TestConcurrentPush*,TestGracefulDegradation*, new tests) — all passgo test ./cmd/entire/cli/strategy/...— only the 3 intentional Group A failuresDisconnectedEmptyRoot_FiltersOrphanandShallowBoundary_ErrorIsActionableare still red, that's the C1 + M3 follow-up gate🤖 Generated with Claude Code