Conversation
Entire-Checkpoint: 49280afe3e31
Entire-Checkpoint: 332f7c352062
Entire-Checkpoint: c2385b018377
Entire-Checkpoint: 7b9bb14c5953
![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 3 potential issues.
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
| // New content exists - will generate new checkpoint ID below | ||
| hasNewContent = true | ||
| } else { | ||
| // No new content - check if any session has a LastCheckpointID to reuse |
There was a problem hiding this comment.
Amend message drops checkpoint trailer
Medium Severity
PrepareCommitMsg now returns early when sessionsWithContent is empty, so source="message" amend flows never restore Entire-Checkpoint from LastCheckpointID. This drops linkage for git commit --amend -m ... commits that previously had a checkpoint, leaving amended commits unlinked from entire/checkpoints/v1.
Additional Locations (1)
| slog.String("session_id", state.SessionID), | ||
| slog.String("transcript_path", state.TranscriptPath), | ||
| ) | ||
| state.TurnCheckpointIDs = nil |
There was a problem hiding this comment.
Finalization failures silently discard pending checkpoints
Medium Severity
finalizeAllTurnCheckpoints clears TurnCheckpointIDs even when transcript loading or UpdateCommitted setup fails. This permanently drops pending checkpoint IDs, so provisional checkpoint data is never retried or finalized with the full turn transcript after transient failures.
Additional Locations (2)
| if err := s.replaceTranscript(opts.Transcript, opts.Agent, sessionPath, entries); err != nil { | ||
| return fmt.Errorf("failed to replace transcript: %w", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Empty finalization data never replaces provisional content
Medium Severity
UpdateCommitted only writes Transcript, Prompts, and Context when their lengths are non-zero. With replace semantics, empty finalized values should clear provisional files, but these guards keep old data instead, so checkpoints can retain stale provisional transcript or prompt/context content.
Additional Locations (2)
There was a problem hiding this comment.
Pull request overview
This PR updates Entire CLI’s checkpointing model to enforce a strict 1:1 mapping between user commits and checkpoint IDs, while still ensuring each checkpoint ultimately contains the full prompt-to-stop transcript via a provisional-at-commit then finalize-at-stop workflow. It also simplifies the session phase state machine by removing the ACTIVE_COMMITTED phase.
Changes:
- Switch to immediate condensation on PostCommit (even while ACTIVE) so every user commit gets a unique checkpoint ID, plus carry-forward of uncommitted work to a new shadow branch.
- Add TurnID and TurnCheckpointIDs to session state, and finalize mid-turn checkpoints at stop time by updating committed checkpoint content.
- Introduce
Store.UpdateCommittedto replace committed transcript/prompts/context for already-condensed checkpoints, and update docs/tests accordingly.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/architecture/sessions-and-checkpoints.md | Updates metadata lookup guidance to always read from the metadata branch tree at HEAD. |
| docs/KNOWN_LIMITATIONS.md | Updates amend-trailer restoration docs to reference LastCheckpointID only. |
| cmd/entire/cli/strategy/strategy.go | Simplifies TurnEndHandler interface to read work items from state (no action list). |
| cmd/entire/cli/strategy/phase_wiring_test.go | Adjusts phase wiring tests for new turn fields and removed phase. |
| cmd/entire/cli/strategy/phase_prepare_commit_msg_test.go | Updates tests for amend trailer restoration via LastCheckpointID; removes PendingCheckpointID-era tests. |
| cmd/entire/cli/strategy/phase_postcommit_test.go | Reworks PostCommit tests for immediate condensation + carry-forward + TurnCheckpointIDs. |
| cmd/entire/cli/strategy/mid_turn_commit_test.go | Removes tests tied to PendingCheckpointID preservation (no longer applicable). |
| cmd/entire/cli/strategy/manual_commit_test.go | Minor lint/comment cleanup in tests. |
| cmd/entire/cli/strategy/manual_commit_session.go | Initializes a per-turn TurnID when creating a session. |
| cmd/entire/cli/strategy/manual_commit_hooks.go | Implements immediate condensation, carry-forward, TurnCheckpointIDs recording, and stop-time finalization using UpdateCommitted. |
| cmd/entire/cli/strategy/manual_commit_git.go | Updates state comments to reflect LastCheckpointID usage. |
| cmd/entire/cli/strategy/manual_commit_condensation.go | Persists TurnID into committed checkpoint metadata; removes PendingCheckpointID clearing. |
| cmd/entire/cli/strategy/auto_commit.go | Adds TurnID propagation into committed metadata; generates new TurnID per turn. |
| cmd/entire/cli/session/state.go | Adds TurnID and TurnCheckpointIDs; clarifies LastCheckpointID semantics. |
| cmd/entire/cli/session/phase_test.go | Updates state machine expectations for removed ACTIVE_COMMITTED and migration normalization. |
| cmd/entire/cli/session/phase.go | Removes ACTIVE_COMMITTED + migrate action; normalizes legacy active_committed to ACTIVE. |
| cmd/entire/cli/reset.go | Updates active-session description to remove ACTIVE_COMMITTED references. |
| cmd/entire/cli/phase_wiring_test.go | Removes tests specifically exercising ACTIVE_COMMITTED behavior. |
| cmd/entire/cli/integration_test/phase_transitions_test.go | Updates integration flow expectations for mid-turn commits (condense immediately, remain ACTIVE). |
| cmd/entire/cli/integration_test/last_checkpoint_id_test.go | Updates integration tests to enforce one-checkpoint-per-commit behavior. |
| cmd/entire/cli/hooks_claudecode_handlers.go | Always dispatches TurnEndHandler (no action list); ensures state saved after turn end. |
| cmd/entire/cli/hooks.go | Updates TODO wording to remove ACTIVE_COMMITTED references. |
| cmd/entire/cli/doctor_test.go | Removes tests specific to ACTIVE_COMMITTED. |
| cmd/entire/cli/doctor.go | Updates “stuck session” definition to remove ACTIVE_COMMITTED. |
| cmd/entire/cli/checkpoint/committed_update_test.go | Adds unit tests covering UpdateCommitted replace behavior and content hash updates. |
| cmd/entire/cli/checkpoint/committed.go | Adds GitStore.UpdateCommitted and transcript replacement helper. |
| cmd/entire/cli/checkpoint/checkpoint.go | Extends Store interface + adds UpdateCommitted options + adds TurnID to committed metadata. |
| CLAUDE.md | Updates architecture docs to reflect 3-phase model and tree-at-HEAD metadata lookup. |
| // Replace prompts | ||
| if len(opts.Prompts) > 0 { | ||
| promptContent := strings.Join(opts.Prompts, "\n\n---\n\n") | ||
| blobHash, err := CreateBlobFromContent(s.repo, []byte(promptContent)) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to create prompt blob: %w", err) | ||
| } | ||
| entries[sessionPath+paths.PromptFileName] = object.TreeEntry{ | ||
| Name: sessionPath + paths.PromptFileName, | ||
| Mode: filemode.Regular, | ||
| Hash: blobHash, | ||
| } | ||
| } | ||
|
|
||
| // Replace context | ||
| if len(opts.Context) > 0 { | ||
| contextBlob, err := CreateBlobFromContent(s.repo, opts.Context) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to create context blob: %w", err) | ||
| } |
There was a problem hiding this comment.
UpdateCommitted writes prompt.txt and context.md without running redaction (WriteCommitted redacts via redact.String / redact.Bytes). Even if current callers redact, keeping redaction inside the store helps prevent future call sites from accidentally committing secrets.
|
|
||
| // Replace transcript (full replace, not append) | ||
| if len(opts.Transcript) > 0 { | ||
| if err := s.replaceTranscript(opts.Transcript, opts.Agent, sessionPath, entries); err != nil { |
There was a problem hiding this comment.
UpdateCommitted currently writes the replacement transcript chunks and updates content_hash.txt without performing secret redaction (unlike WriteCommitted/writeTranscript which runs redact.JSONLBytes first). To avoid accidentally persisting secrets when callers forget to redact, UpdateCommitted should apply the same redaction internally (and compute chunking + content hash from the redacted bytes).
| if err := s.replaceTranscript(opts.Transcript, opts.Agent, sessionPath, entries); err != nil { | |
| redactedTranscript, err := redact.JSONLBytes(opts.Transcript) | |
| if err != nil { | |
| return fmt.Errorf("failed to redact transcript: %w", err) | |
| } | |
| if err := s.replaceTranscript(redactedTranscript, opts.Agent, sessionPath, entries); err != nil { |
| // Replace prompts | ||
| if len(opts.Prompts) > 0 { | ||
| promptContent := strings.Join(opts.Prompts, "\n\n---\n\n") | ||
| blobHash, err := CreateBlobFromContent(s.repo, []byte(promptContent)) |
There was a problem hiding this comment.
The replace semantics in UpdateCommitted can’t currently clear prompt.txt / context.md: the updates are guarded by len(opts.Prompts) > 0 / len(opts.Context) > 0, so passing an empty set leaves whatever was previously written. This also means callers can’t intentionally replace prompts/context with empty content. Consider removing the guards (write empty files) or explicitly deleting these entries when the caller intends a replace (e.g., add ReplacePrompts/ReplaceContext flags or treat nil vs empty distinctly).
| if err != nil || len(fullTranscript) == 0 { | ||
| logging.Warn(logCtx, "finalize: failed to read transcript, skipping", | ||
| slog.String("session_id", state.SessionID), | ||
| slog.String("transcript_path", state.TranscriptPath), | ||
| ) | ||
| state.TurnCheckpointIDs = nil | ||
| return | ||
| } |
There was a problem hiding this comment.
When os.ReadFile(state.TranscriptPath) fails, the warning log omits the underlying error, which makes diagnosing stop-time finalization failures harder. Include err.Error() (and ideally distinguish the empty-file case) in the log attributes before clearing TurnCheckpointIDs.
| if err != nil || len(fullTranscript) == 0 { | |
| logging.Warn(logCtx, "finalize: failed to read transcript, skipping", | |
| slog.String("session_id", state.SessionID), | |
| slog.String("transcript_path", state.TranscriptPath), | |
| ) | |
| state.TurnCheckpointIDs = nil | |
| return | |
| } | |
| if err != nil { | |
| logging.Warn(logCtx, "finalize: failed to read transcript, skipping", | |
| slog.String("session_id", state.SessionID), | |
| slog.String("transcript_path", state.TranscriptPath), | |
| slog.String("error", err.Error()), | |
| ) | |
| state.TurnCheckpointIDs = nil | |
| return | |
| } | |
| if len(fullTranscript) == 0 { | |
| logging.Warn(logCtx, "finalize: transcript is empty, skipping", | |
| slog.String("session_id", state.SessionID), | |
| slog.String("transcript_path", state.TranscriptPath), | |
| slog.String("reason", "empty_transcript"), | |
| ) | |
| state.TurnCheckpointIDs = nil | |
| return | |
| } |
Entire-Checkpoint: 0bce1f68b21e
Entire-Checkpoint: ca5ddaabc88a
Entire-Checkpoint: 93fa19e317c5
Entire-Checkpoint: 7cf71da0278c
1 participant
1:1 Checkpoint-to-Commit Mapping with Deferred Transcript Finalization
Problem
Previously, checkpoints used a 1:N model — one checkpoint ID could span multiple commits. When an agent made several commits during a single turn, they all shared the same checkpoint ID. This made it impossible to tie a specific commit to a specific snapshot of session context, and the
ACTIVE_COMMITTEDphase addedcomplexity to the state machine.
Approach
Every user commit now gets its own unique checkpoint ID. The core challenge is that mid-turn commits happen before the agent is done — the transcript is incomplete. We solve this with a provisional-then-finalize pattern:
At commit time (PostCommit hook): Condense whatever transcript exists so far to
entire/checkpoints/v1with a fresh checkpoint ID. This is provisional — the transcript is partial. Then carry forward any remaining uncommitted files to a new shadow branch so the next commit gets its own checkpoint.At turn end (Stop hook):
HandleTurnEnditerates over all checkpoint IDs recorded during the turn (TurnCheckpointIDson session state) and replaces each provisional transcript with the complete one viaUpdateCommitted.This means every checkpoint on
entire/checkpoints/v1ends up with the full prompt-to-stop transcript, while each commit still gets its own unique ID for precise linking.Key changes
New:
UpdateCommittedon checkpoint storeWriteCommittedbehavior)WriteCommittedbehavior)New:
TurnCheckpointIDson session stateHandleTurnEndto finalize all checkpointsNew:
HandleTurnEndfinalizationTurnEndHandlerinterface simplified — no longer takes[]session.Actionparameter since work items come from state, not the action listif len(remaining) > 0guard)Phase simplification
ACTIVE_COMMITTEDphase — the state machine is now 3 phases: ACTIVE, IDLE, ENDEDactive_committedin persisted state normalizes to ACTIVE (not IDLE) so pending finalization isn't lost on upgradePostCommit carry-forward