Support resuming from squash merge commits with multiple checkpoints#534
Support resuming from squash merge commits with multiple checkpoints#534
Conversation
Change checkpointID field to checkpointIDs slice, update findBranchCheckpoint and findCheckpointInHistory to use ParseAllCheckpoints, and add test for squash merge commits with multiple Entire-Checkpoint trailers. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: e49c84c2d596
Add ParseAllCheckpoints() to trailers package for extracting all Entire-Checkpoint trailers from squash merge commits. Add resumeMultipleCheckpoints() to resume.go that iterates over all checkpoint IDs, restores sessions for each, and displays aggregated resume commands. Refactor test helper to support custom checkpoint IDs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: 43abb5b45ab5
Entire-Checkpoint: c585b5e466a1
PR SummaryMedium Risk Overview This introduces Written by Cursor Bugbot for commit c9405b9. Configure here. |
There was a problem hiding this comment.
Pull request overview
Adds support for resuming sessions from squash-merge commits that contain multiple Entire-Checkpoint trailers, enabling entire resume <branch> to restore more than one session when multiple checkpoints are embedded in a single commit message.
Changes:
- Add
trailers.ParseAllCheckpoints()to extract and deduplicate multiple checkpoint trailers from a commit message. - Update resume branch-checkpoint discovery to return multiple checkpoint IDs and add a multi-checkpoint resume flow.
- Add unit + integration tests (including a squash-merge simulation) and a new integration test helper to create commits with multiple checkpoint trailers.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| cmd/entire/cli/trailers/trailers.go | Adds ParseAllCheckpoints() to return all checkpoint trailers (deduped, ordered). |
| cmd/entire/cli/trailers/trailers_test.go | Adds unit tests covering single/multiple/dedup/invalid/mixed checkpoint trailers. |
| cmd/entire/cli/resume.go | Switches checkpoint discovery to multiple IDs and introduces resumeMultipleCheckpoints(). |
| cmd/entire/cli/resume_test.go | Adds unit tests for multiple-checkpoint history/branch discovery and a helper to create distinct checkpoint metadata. |
| cmd/entire/cli/integration_test/testenv.go | Adds GitCommitWithMultipleCheckpoints() helper for squash-merge style commit messages. |
| cmd/entire/cli/integration_test/resume_test.go | Adds integration test validating restore from squash-merge commit containing two checkpoints. |
cmd/entire/cli/resume.go
Outdated
| sessionDir, dirErr := ag.GetSessionDir(repoRoot) | ||
| if dirErr != nil { | ||
| logging.Debug(logCtx, "skipping checkpoint: cannot determine session dir", | ||
| slog.String("checkpoint_id", cpID.String()), | ||
| slog.String("error", dirErr.Error()), | ||
| ) | ||
| continue | ||
| } | ||
| if mkErr := os.MkdirAll(sessionDir, 0o700); mkErr != nil { | ||
| logging.Debug(logCtx, "skipping checkpoint: cannot create session dir", | ||
| slog.String("checkpoint_id", cpID.String()), | ||
| slog.String("error", mkErr.Error()), | ||
| ) | ||
| continue | ||
| } | ||
|
|
There was a problem hiding this comment.
The ag.GetSessionDir + os.MkdirAll(sessionDir, ...) work here is redundant and can be misleading: RestoreLogsOnly already computes/creates the correct per-session target directories (and sessions in a checkpoint may not all share the checkpoint-level agent). Consider deleting this block and relying on RestoreLogsOnly for directory creation.
| sessionDir, dirErr := ag.GetSessionDir(repoRoot) | |
| if dirErr != nil { | |
| logging.Debug(logCtx, "skipping checkpoint: cannot determine session dir", | |
| slog.String("checkpoint_id", cpID.String()), | |
| slog.String("error", dirErr.Error()), | |
| ) | |
| continue | |
| } | |
| if mkErr := os.MkdirAll(sessionDir, 0o700); mkErr != nil { | |
| logging.Debug(logCtx, "skipping checkpoint: cannot create session dir", | |
| slog.String("checkpoint_id", cpID.String()), | |
| slog.String("error", mkErr.Error()), | |
| ) | |
| continue | |
| } |
cmd/entire/cli/resume.go
Outdated
| if restoreErr != nil || len(sessions) == 0 { | ||
| logging.Debug(logCtx, "skipping checkpoint: restore failed", | ||
| slog.String("checkpoint_id", cpID.String()), | ||
| ) | ||
| continue | ||
| } |
There was a problem hiding this comment.
When RestoreLogsOnly fails, the code logs "restore failed" but drops restoreErr, making diagnosis difficult. Include the error details in the debug log attributes (or log a warn) so users/devs can understand why a checkpoint was skipped.
| if restoreErr != nil || len(sessions) == 0 { | |
| logging.Debug(logCtx, "skipping checkpoint: restore failed", | |
| slog.String("checkpoint_id", cpID.String()), | |
| ) | |
| continue | |
| } | |
| if restoreErr != nil { | |
| logging.Debug(logCtx, "skipping checkpoint: restore failed", | |
| slog.String("checkpoint_id", cpID.String()), | |
| slog.String("error", restoreErr.Error()), | |
| ) | |
| continue | |
| } | |
| if len(sessions) == 0 { | |
| logging.Debug(logCtx, "skipping checkpoint: restore failed", | |
| slog.String("checkpoint_id", cpID.String()), | |
| slog.String("reason", "no sessions restored"), | |
| ) | |
| continue | |
| } |
cmd/entire/cli/resume.go
Outdated
| logging.Debug(logCtx, "skipping checkpoint with unknown agent", | ||
| slog.String("checkpoint_id", cpID.String()), | ||
| slog.String("error", agErr.Error()), | ||
| ) | ||
| continue | ||
| } | ||
|
|
||
| sessionDir, dirErr := ag.GetSessionDir(repoRoot) | ||
| if dirErr != nil { | ||
| logging.Debug(logCtx, "skipping checkpoint: cannot determine session dir", | ||
| slog.String("checkpoint_id", cpID.String()), | ||
| slog.String("error", dirErr.Error()), | ||
| ) | ||
| continue | ||
| } | ||
| if mkErr := os.MkdirAll(sessionDir, 0o700); mkErr != nil { | ||
| logging.Debug(logCtx, "skipping checkpoint: cannot create session dir", | ||
| slog.String("checkpoint_id", cpID.String()), | ||
| slog.String("error", mkErr.Error()), | ||
| ) | ||
| continue | ||
| } | ||
|
|
There was a problem hiding this comment.
In resumeMultipleCheckpoints, skipping a checkpoint when strategy.ResolveAgentForRewind(metadata.Agent) fails is overly restrictive. Strategy.RestoreLogsOnly restores per-session logs using session-level agent metadata, so an unknown/empty checkpoint-level agent shouldn’t prevent attempting the restore; consider removing this gate or falling back to calling RestoreLogsOnly even when the checkpoint agent cannot be resolved.
| logging.Debug(logCtx, "skipping checkpoint with unknown agent", | |
| slog.String("checkpoint_id", cpID.String()), | |
| slog.String("error", agErr.Error()), | |
| ) | |
| continue | |
| } | |
| sessionDir, dirErr := ag.GetSessionDir(repoRoot) | |
| if dirErr != nil { | |
| logging.Debug(logCtx, "skipping checkpoint: cannot determine session dir", | |
| slog.String("checkpoint_id", cpID.String()), | |
| slog.String("error", dirErr.Error()), | |
| ) | |
| continue | |
| } | |
| if mkErr := os.MkdirAll(sessionDir, 0o700); mkErr != nil { | |
| logging.Debug(logCtx, "skipping checkpoint: cannot create session dir", | |
| slog.String("checkpoint_id", cpID.String()), | |
| slog.String("error", mkErr.Error()), | |
| ) | |
| continue | |
| } | |
| // Unknown or unresolved checkpoint-level agent should not prevent attempting restore. | |
| // RestoreLogsOnly will rely on session-level agent metadata instead. | |
| logging.Debug(logCtx, "checkpoint has unknown agent; attempting restore anyway", | |
| slog.String("checkpoint_id", cpID.String()), | |
| slog.String("error", agErr.Error()), | |
| ) | |
| } else { | |
| sessionDir, dirErr := ag.GetSessionDir(repoRoot) | |
| if dirErr != nil { | |
| logging.Debug(logCtx, "skipping checkpoint: cannot determine session dir", | |
| slog.String("checkpoint_id", cpID.String()), | |
| slog.String("error", dirErr.Error()), | |
| ) | |
| continue | |
| } | |
| if mkErr := os.MkdirAll(sessionDir, 0o700); mkErr != nil { | |
| logging.Debug(logCtx, "skipping checkpoint: cannot create session dir", | |
| slog.String("checkpoint_id", cpID.String()), | |
| slog.String("error", mkErr.Error()), | |
| ) | |
| continue | |
| } | |
| } |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
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
The sorting, header formatting, and resume command printing was duplicated between resumeSession and resumeMultipleCheckpoints. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: 0ff68845c557
RestoreLogsOnly already resolves agents per-session from session-level metadata, so an unknown checkpoint-level agent shouldn't block the restore attempt. The session dir creation was also redundant since RestoreLogsOnly handles it internally. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: 831e4afa5e83
Split the error and empty-sessions cases into separate log entries so the actual failure reason is visible in debug logs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: 932c2642d8ac
When a single session spans multiple commits, different checkpoint IDs can contain the same session. In a squash merge this would produce duplicate resume commands. Keep the entry with the latest CreatedAt. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: e92b8f8525e9
|
As someone who prefers¹ squash-merge to messy merge-tennis with careless commit messages, this effort really pleases me.
|
The inline dedup logic in resumeMultipleCheckpoints did not update the seen map's CreatedAt after replacing a session, so a third occurrence could incorrectly overwrite the newest entry. Extract to a standalone function with a correct update and add targeted unit tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: 7132c7b6328a
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Entire-Checkpoint: 27bbbac46736
…uash-merge-resume
| // TestResumeMultipleCheckpoints_SortsByCreatedAt verifies that resumeMultipleCheckpoints | ||
| // sorts checkpoints by CreatedAt ascending before restoring, so that the newest checkpoint | ||
| // writes last and wins on disk. This fixes the git CLI squash merge bug where trailers | ||
| // appear in reverse chronological order (newest first). | ||
| func TestResumeMultipleCheckpoints_SortsByCreatedAt(t *testing.T) { | ||
| tmpDir := t.TempDir() | ||
| t.Chdir(tmpDir) |
There was a problem hiding this comment.
TestResumeMultipleCheckpoints_SortsByCreatedAt doesn’t exercise resumeMultipleCheckpoints (it re-implements the sort inline), so it will keep passing even if the production code stops sorting before restoring. To make this test meaningful, either (a) extract the “sort checkpoints by CreatedAt” logic into a helper used by resumeMultipleCheckpoints and unit-test that helper, or (b) refactor resumeMultipleCheckpoints to accept an injected restorer so the test can assert the restore call order end-to-end.
Summary
entire resumeon squash merge commits that contain multipleEntire-CheckpointtrailersSessionIDacross checkpoints, keeping the most recent transcriptProblem
When a feature branch with multiple commits (each with its own
Entire-Checkpointtrailer) is squash-merged, all trailers end up in a single commit message.entire resumepreviously only parsed the first trailer, losing all other sessions.Additionally, GitHub squash merges list trailers chronologically (oldest first), but git CLI squash merges list them in reverse order (newest first). Since
RestoreLogsOnlywrites session files eagerly, the last checkpoint processed wins on disk — meaning reverse-ordered trailers caused the oldest transcript to overwrite the newest.Changes
cmd/entire/cli/trailers/trailers.go— AddParseAllCheckpoints()to extract allEntire-Checkpointtrailers from a commit message (not just the first).cmd/entire/cli/resume.go—ParseCheckpoint(single) toParseAllCheckpoints(multi) throughoutfindBranchCheckpointandfindCheckpointInHistorybranchCheckpointResult.checkpointID→checkpointIDs []CheckpointIDresumeMultipleCheckpoints: reads metadata for all checkpoint IDs, sorts byCreatedAtascending, then restores in order so the newest checkpoint always writes lastdeduplicateSessions: merges sessions across checkpoints, keeping the one with the latestCreatedAtwhen aSessionIDappears in multiple checkpointsdisplayRestoredSessionsto share session display logic between single and multi-checkpoint pathscmd/entire/cli/resume_test.go— Add unit tests fordeduplicateSessions(including three-occurrence staleness edge case),findCheckpointInHistorywith multiple trailers,findBranchCheckpointwith squash merge, and checkpoint timestamp sorting.cmd/entire/cli/integration_test/resume_test.go— Add integration test simulating a full squash merge workflow: two sessions on a feature branch, squash merge to main, thenentire resumerestoring both sessions.cmd/entire/cli/trailers/trailers_test.go— Add tests forParseAllCheckpoints.Test plan
mise run test:cipasses (unit + integration)TestResume_SquashMergeMultipleCheckpointscovers the end-to-end squash merge flowTestResumeMultipleCheckpoints_SortsByCreatedAtverifies reverse-ordered checkpoints are sorted correctlydeduplicateSessionscover no-duplicates, newer-wins, older-loses, three-occurrence, and mixed scenarios