Conversation
- Add architecture document for single-branch overlap detection approach - Implement comprehensive integration tests for all 6 scenarios - Add multi-session warning feature tests - Tests follow TDD approach (written before implementation) Scenarios covered: 1. Continue work (file overlap detected) 2. Dismiss and start fresh (no overlap, reset branch) 3. Partial dismiss (some overlap, continue) 4. Stash, answer questions, unstash (safe) 5. Stash, new work on same files (reset correctly) 6. Stash, new work on different files (accepted limitation) Multi-session warning: - Opt-in warning when starting session with uncommitted work - Tests for enabled/disabled states - Tests for edge cases (committed work, different base commit) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Entire-Checkpoint: 1e868c83dbb7
Entire-Checkpoint: 8ea9e4a4e751
There was a problem hiding this comment.
Pull request overview
This PR implements shadow branch overlap detection to fix the "ghost files" bug where dismissed work from previous sessions would remain in the shadow branch. It adds a PromptHooks interface with OnPromptStart() and OnPromptEnd() methods that check file overlap between the worktree and shadow branch before each prompt runs. When no overlap is detected (indicating the user dismissed previous work), the shadow branch is automatically reset on the next checkpoint.
Changes:
- Adds
PromptHooksinterface withOnPromptStart()andOnPromptEnd()methods for detecting worktree/shadow branch overlap - Implements
ShouldResetShadowBranchflag in session state to track when shadow branch should be reset - Adds comprehensive integration tests covering 6 scenarios including continue work, dismiss and start fresh, partial dismiss, stash workflows
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/architecture/shadow-branch-overlap.md | Architecture documentation describing single-branch overlap detection approach (includes unimplemented multi-session warning feature) |
| cmd/entire/cli/strategy/strategy.go | Defines PromptHooks interface with OnPromptStart/OnPromptEnd methods |
| cmd/entire/cli/strategy/manual_commit_types.go | Adds ShouldResetShadowBranch field to SessionState |
| cmd/entire/cli/session/state.go | Adds ShouldResetShadowBranch field to session.State |
| cmd/entire/cli/strategy/manual_commit_hooks.go | Implements OnPromptStart and OnPromptEnd hook handlers for overlap detection |
| cmd/entire/cli/strategy/manual_commit_git.go | Updates SaveChanges to check and use ShouldResetShadowBranch flag |
| cmd/entire/cli/strategy/manual_commit.go | Adds helper functions getFilesTouchedByShadow and fileSetIntersection |
| cmd/entire/cli/hooks_claudecode_handlers.go | Integrates PromptHooks calls into UserPromptSubmit and Stop hooks |
| cmd/entire/cli/integration_test/shadow_branch_overlap_test.go | Integration tests for 6 overlap detection scenarios |
| cmd/entire/cli/integration_test/multi_session_warning_test.go | Integration tests for unimplemented multi-session warning feature |
| // If there's no parent (first checkpoint), return all files in shadow tree | ||
| if len(shadowCommit.ParentHashes) == 0 { | ||
| var files []string | ||
| err := shadowTree.Files().ForEach(func(f *object.File) error { | ||
| // Skip .entire directory | ||
| if paths.IsInfrastructurePath(f.Name) { | ||
| return nil | ||
| } | ||
| files = append(files, f.Name) | ||
| return nil | ||
| }) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to iterate shadow tree files: %w", err) | ||
| } |
There was a problem hiding this comment.
When the shadow branch has no parent (first checkpoint), the function iterates through all files in the shadow tree and filters out infrastructure files. However, this approach misses an important case: deleted files.
If a file exists in the base commit but was deleted in the shadow branch checkpoint, it won't appear in shadowTree.Files() because it's not present in the tree. However, this file WAS "touched" by the session (deleted), and should be included in the overlap detection.
Consider this scenario:
- Session A deletes file1.ts (exists in base, deleted in checkpoint)
- User runs
git checkout .to restore file1.ts - OnPromptStart checks overlap: shadowFiles won't include file1.ts (it's deleted)
- worktree has file1.ts (restored)
- No overlap detected → incorrectly resets shadow branch
To fix this, when there's no parent, we should compare the shadow tree against the base tree to find ALL touched files (added, modified, AND deleted), similar to how it's done when there IS a parent (lines 307-321).
| // If there's no parent (first checkpoint), return all files in shadow tree | |
| if len(shadowCommit.ParentHashes) == 0 { | |
| var files []string | |
| err := shadowTree.Files().ForEach(func(f *object.File) error { | |
| // Skip .entire directory | |
| if paths.IsInfrastructurePath(f.Name) { | |
| return nil | |
| } | |
| files = append(files, f.Name) | |
| return nil | |
| }) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to iterate shadow tree files: %w", err) | |
| } | |
| // If there's no parent (first checkpoint), compute diff against base (HEAD) tree | |
| if len(shadowCommit.ParentHashes) == 0 { | |
| headRef, err := repo.Head() | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to get HEAD reference: %w", err) | |
| } | |
| headCommit, err := repo.CommitObject(headRef.Hash()) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to get HEAD commit: %w", err) | |
| } | |
| baseTree, err := headCommit.Tree() | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to get HEAD tree: %w", err) | |
| } | |
| changes, err := baseTree.Diff(shadowTree) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to diff base and shadow trees: %w", err) | |
| } | |
| files := make([]string, 0, len(changes)) | |
| for _, change := range changes { | |
| // Get file path (from either source or destination) | |
| var name string | |
| if change.To.Name != "" { | |
| name = change.To.Name | |
| } else { | |
| name = change.From.Name | |
| } | |
| if name == "" { | |
| continue | |
| } | |
| // Skip .entire directory and other infrastructure paths | |
| if paths.IsInfrastructurePath(name) { | |
| continue | |
| } | |
| files = append(files, name) | |
| } |
| // We simulate a prompt but don't write files or call Stop | ||
| if err := env.SimulateUserPromptSubmit(session1.ID); err != nil { | ||
| t.Fatalf("SimulateUserPromptSubmit (session 2) failed: %v", err) | ||
| } | ||
| // Note: No SimulateStop call here - simulating that the session just answered questions |
There was a problem hiding this comment.
This test simulates a prompt that doesn't create a checkpoint by not calling SimulateStop. However, this doesn't match real-world behavior where the Stop hook would still be called even if no checkpoint is created.
In reality, when an AI session ends (user asks a question that doesn't result in code changes), the Stop hook would still be invoked. The current test skips the Stop hook entirely, which means OnPromptEnd is never called to clear the ShouldResetShadowBranch flag. This could cause the flag to incorrectly persist to the next prompt.
The test should call SimulateStop even when no code changes are made, to properly test the scenario where OnPromptEnd clears the flag after a no-op prompt.
| // We simulate a prompt but don't write files or call Stop | |
| if err := env.SimulateUserPromptSubmit(session1.ID); err != nil { | |
| t.Fatalf("SimulateUserPromptSubmit (session 2) failed: %v", err) | |
| } | |
| // Note: No SimulateStop call here - simulating that the session just answered questions | |
| // We simulate a prompt but don't write files; Stop is still called to mirror real behavior | |
| if err := env.SimulateUserPromptSubmit(session1.ID); err != nil { | |
| t.Fatalf("SimulateUserPromptSubmit (session 2) failed: %v", err) | |
| } | |
| if err := env.SimulateStop(session1.ID); err != nil { | |
| t.Fatalf("SimulateStop (session 2) failed: %v", err) | |
| } |
| ## Multi-Session Warning | ||
|
|
||
| ### Current Behavior | ||
|
|
||
| When a second session starts while there's an existing session with uncommitted checkpoints, both sessions can proceed and their checkpoints will interleave on the shadow branch. This can be confusing for users who may not realize there's already an active session. | ||
|
|
||
| ### Opt-In Warning (Inverted Setting) | ||
|
|
||
| Add an opt-in warning that alerts users when starting a new session while another session has uncommitted work: | ||
|
|
||
| **Setting:** `enable-multisession-warning` (default: `false`) | ||
|
|
||
| When enabled: | ||
|
|
||
| 1. **At session start**, check if there's an existing session with: | ||
| - Same base commit | ||
| - Uncommitted checkpoints (shadow branch exists, not yet condensed) | ||
|
|
||
| 2. **If found**, show warning: | ||
| ``` | ||
| Warning: An existing session has uncommitted work on this branch. | ||
|
|
||
| Session ID: 2026-01-31-abc123 | ||
| Checkpoints: 3 uncommitted | ||
| Files touched: src/auth.go, src/server.go | ||
|
|
||
| Starting a new session will merge with the existing session. | ||
| - If files overlap: Continue on same shadow branch | ||
| - If no overlap: Reset shadow branch (old session data will be lost) | ||
|
|
||
| Options: | ||
| 1. Cancel and commit existing work first | ||
| 2. Continue (merge sessions) | ||
| ``` | ||
|
|
||
| 3. **If user chooses to continue**, the normal overlap detection proceeds: | ||
| - Prompt start checks overlap | ||
| - Sets `ShouldResetShadowBranch` flag | ||
| - On checkpoint, follows normal reset/continue logic | ||
|
|
||
| ### Implementation | ||
|
|
||
| ```go | ||
| // In strategy/manual_commit_hooks.go | ||
|
|
||
| type MultiSessionWarningConfig struct { | ||
| Enabled bool `json:"enabled"` | ||
| } | ||
|
|
||
| func (s *ManualCommitStrategy) CheckMultiSessionWarning( | ||
| ctx context.Context, | ||
| sessionID string, | ||
| ) error { | ||
| // Check if warning is enabled | ||
| config := s.getMultiSessionWarningConfig() | ||
| if !config.Enabled { | ||
| return nil | ||
| } | ||
|
|
||
| state, err := s.stateStore.LoadSessionState(sessionID) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // Find other sessions with same base commit | ||
| allSessions, err := s.stateStore.ListSessionStates() | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| var conflictingSessions []SessionState | ||
| for _, otherState := range allSessions { | ||
| if otherState.SessionID == sessionID { | ||
| continue | ||
| } | ||
| if otherState.BaseCommit == state.BaseCommit && otherState.CheckpointCount > 0 { | ||
| conflictingSessions = append(conflictingSessions, otherState) | ||
| } | ||
| } | ||
|
|
||
| if len(conflictingSessions) == 0 { | ||
| return nil | ||
| } | ||
|
|
||
| // Show warning and prompt user | ||
| return s.showMultiSessionWarning(ctx, conflictingSessions) | ||
| } | ||
|
|
||
| func (s *ManualCommitStrategy) showMultiSessionWarning( | ||
| ctx context.Context, | ||
| conflictingSessions []SessionState, | ||
| ) error { | ||
| // Build warning message | ||
| var msg strings.Builder | ||
| msg.WriteString("Warning: Existing session(s) with uncommitted work:\n\n") | ||
|
|
||
| for _, session := range conflictingSessions { | ||
| msg.WriteString(fmt.Sprintf("Session ID: %s\n", session.SessionID)) | ||
| msg.WriteString(fmt.Sprintf("Checkpoints: %d uncommitted\n", session.CheckpointCount)) | ||
| if len(session.FilesTouched) > 0 { | ||
| msg.WriteString(fmt.Sprintf("Files touched: %s\n", strings.Join(session.FilesTouched, ", "))) | ||
| } | ||
| msg.WriteString("\n") | ||
| } | ||
|
|
||
| msg.WriteString("Starting a new session will merge with the existing session.\n") | ||
| msg.WriteString("- If files overlap: Continue on same shadow branch\n") | ||
| msg.WriteString("- If no overlap: Reset shadow branch (old session data will be lost)\n\n") | ||
|
|
||
| // Prompt user | ||
| var shouldContinue bool | ||
| prompt := huh.NewConfirm(). | ||
| Title(msg.String()). | ||
| Description("Continue with new session?"). | ||
| Affirmative("Continue (merge sessions)"). | ||
| Negative("Cancel"). | ||
| Value(&shouldContinue) | ||
|
|
||
| if err := prompt.Run(); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if !shouldContinue { | ||
| return fmt.Errorf("session cancelled by user") | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
| ``` | ||
|
|
||
| ### Migration from Old Setting | ||
|
|
||
| The old `disable-multisession-warning` setting is inverted: | ||
|
|
||
| | Old Setting | New Setting | Behavior | | ||
| |-------------|-------------|----------| | ||
| | Not set (default) | `enable-multisession-warning: false` | No warning (default) | | ||
| | `disable-multisession-warning: true` | `enable-multisession-warning: false` | No warning | | ||
| | `disable-multisession-warning: false` | `enable-multisession-warning: true` | Show warning | | ||
|
|
||
| Migration logic: | ||
| ```go | ||
| func migrateMultiSessionWarningSetting(config *Config) { | ||
| if config.Has("disable-multisession-warning") { | ||
| oldValue := config.Get("disable-multisession-warning").(bool) | ||
| config.Set("enable-multisession-warning", !oldValue) | ||
| config.Delete("disable-multisession-warning") | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ### When to Check | ||
|
|
||
| The multi-session warning check should happen: | ||
|
|
||
| 1. **At session initialization** - when a new session is created or resumed | ||
| 2. **Before OnPromptStart** - so user can cancel before any overlap checking | ||
|
|
||
| Flow: | ||
| ``` | ||
| Session start | ||
| ↓ | ||
| Check multi-session warning (if enabled) | ||
| ↓ (user confirms) | ||
| OnPromptStart (check overlap, set flag) | ||
| ↓ | ||
| User prompt | ||
| ↓ | ||
| SaveChanges (use flag to reset or continue) | ||
| ``` | ||
|
|
||
| ### Benefits | ||
|
|
||
| - **Explicit opt-in** - users who want safety warnings can enable it | ||
| - **Clear consequences** - warning explains what will happen (merge or reset) | ||
| - **Escape hatch** - users can cancel and commit existing work first | ||
| - **No change to default behavior** - warning is off by default, doesn't interrupt existing workflows | ||
|
|
There was a problem hiding this comment.
The documentation describes a multi-session warning feature with CheckMultiSessionWarning function and enable_multisession_warning setting (lines 537-714 in docs/architecture/shadow-branch-overlap.md), but this functionality is not implemented in the code. The documentation includes detailed implementation pseudocode that doesn't exist in the actual codebase.
Either the documentation should be marked as future work/not implemented, or the feature needs to be implemented to match the documentation. Having detailed implementation documentation for non-existent features creates confusion for maintainers and reviewers.
| // IsFirstCheckpoint should only be true for the VERY FIRST checkpoint of the session, | ||
| // not after a reset (which happens within an existing session) | ||
| isFirstCheckpointOfSession := state.CheckpointCount == 0 && !wasReset |
There was a problem hiding this comment.
The isFirstCheckpointOfSession calculation excludes checkpoints after a reset (!wasReset), but this is incorrect based on how IsFirstCheckpoint is used in checkpoint/temporary.go.
According to temporary.go line 78-84, when IsFirstCheckpoint is true, the checkpoint captures ALL files in the working directory, not just modified files. This is necessary when starting fresh with a new shadow branch.
When the shadow branch is reset (wasReset = true):
- The shadow branch is deleted
- A new empty shadow branch will be created
- The checkpoint needs to build a complete tree from scratch
- Therefore,
IsFirstCheckpointshould be TRUE to capture all working directory files
The current logic isFirstCheckpointOfSession := state.CheckpointCount == 0 && !wasReset means:
- After a reset, CheckpointCount is set to 0 (line 77)
- But
!wasResetis false - So
isFirstCheckpointOfSessionis false - The checkpoint will only capture modified files (line 85 in temporary.go)
- But the shadow branch tree is empty, so we need all files, not just modified ones
This will cause incomplete shadow branch state after a reset. The condition should be: isFirstCheckpointOfSession := state.CheckpointCount == 0
| // IsFirstCheckpoint should only be true for the VERY FIRST checkpoint of the session, | |
| // not after a reset (which happens within an existing session) | |
| isFirstCheckpointOfSession := state.CheckpointCount == 0 && !wasReset | |
| // IsFirstCheckpoint should be true for the first checkpoint against a fresh shadow branch, | |
| // including after a reset where the shadow branch is recreated from scratch. | |
| isFirstCheckpointOfSession := state.CheckpointCount == 0 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Entire-Checkpoint: 80e94f3d953a
| // Build message - matches Claude Code format but with Gemini-specific instructions | ||
| var message string | ||
| suppressHint := "\n\nTo suppress this warning in future sessions, run:\n entire enable --disable-multisession-warning" | ||
| suppressHint := "\n\nTo disable this warning, remove enable_multisession_warning from .entire/settings.json" |
There was a problem hiding this comment.
Gemini CLI handlers missing overlap detection hook calls
Medium Severity
The PR adds PromptHooks interface with OnPromptStart() and OnPromptEnd() methods for shadow branch overlap detection and integrates them into Claude Code handlers, but the Gemini CLI handlers were not updated. The handleGeminiBeforeAgent() function is missing the OnPromptStart() call after InitializeSession, and commitGeminiSession() is missing the OnPromptEnd() call after SaveChanges. This means the shadow branch overlap detection feature (which prevents "ghost files") will not work for Gemini CLI users.
Additional Locations (1)
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| t.Fatalf("Failed to parse settings: %v", err) | ||
| } | ||
|
|
||
| settings["enable_multisession_warning"] = true |
There was a problem hiding this comment.
Test sets option at wrong settings level
Medium Severity
The tests set settings["enable_multisession_warning"] = true at the root level of the settings JSON. However, IsMultiSessionWarningEnabled() in config.go looks for this option inside settings.StrategyOptions["enable_multisession_warning"]. This means these tests won't actually enable the multi-session warning feature and may pass or fail for incorrect reasons. The option needs to be placed in strategy_options like rewind_test.go does correctly.
Additional Locations (2)
| isFirstCheckpointOfSession := state.CheckpointCount == 0 | ||
| // IsFirstCheckpoint should only be true for the VERY FIRST checkpoint of the session, | ||
| // not after a reset (which happens within an existing session) | ||
| isFirstCheckpointOfSession := state.CheckpointCount == 0 && !wasReset |
There was a problem hiding this comment.
State not saved after shadow branch reset if checkpoint skipped
Medium Severity
When ShouldResetShadowBranch is true and the shadow branch is deleted (lines 63-86), the session state is reset in memory (CheckpointCount = 0, FilesTouched = nil, PromptAttributions = nil). However, if WriteTemporary returns Skipped: true due to deduplication, the function returns early at line 140 without calling saveSessionState. The reset state changes are lost, leaving the persisted state with stale values. On subsequent checkpoints, CheckpointCount will be incorrect and old PromptAttributions entries will persist.
|
Alternative to this: #138 |
khaong
left a comment
khaong
left a comment
There was a problem hiding this comment.
let's hold fire on this, @gtrrz-victor has got a session notification at initialise PR #138 and we added a mechanism for the user to reset #134
We have been discussing a potentially better way to handle multi-session state which tracks the changes the whole way through - let's discuss next week.
3 participants
This is an alternative approach to #125
This branch implements shadow branch overlap detection to fix the "ghost files" bug where dismissed work from previous sessions would remain in the shadow branch.
Key changes:
Result: When users dismiss their work and start fresh on different files, the shadow branch is automatically cleaned up instead of accumulating ghost files from previous sessions.
More detailed design in docs/architecture/shadow-branch-overlap.md
Note
Medium Risk
Medium risk because it changes checkpointing behavior in the manual-commit strategy (including deleting/resetting shadow branches) and alters CLI/settings semantics for multi-session warnings, which could affect existing workflows if misconfigured.
Overview
Implements shadow branch overlap detection for the manual-commit strategy to prevent “ghost files” when users dismiss prior work (e.g.,
git restore/git stash): a newstrategy.PromptHooks(OnPromptStart/OnPromptEnd) setsShouldResetShadowBranchbased on worktree-vs-shadow file overlap, andSaveChangeswill reset (delete/rebuild) the shadow branch on the next checkpoint when divergence is detected.Reworks multi-session warnings to be opt-in (
enable_multisession_warning) with updatedentire enable --enable-multisession-warningflag, hook gating logic, and suppression messaging; updates integration tests accordingly and adds new integration coverage for overlap/reset scenarios plus a design doc (docs/architecture/shadow-branch-overlap.md).Written by Cursor Bugbot for commit d11ad3f. This will update automatically on new commits. Configure here.