From 28ecf4e462795723f3169452cf97bd823bdc3d07 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Wed, 11 Feb 2026 17:12:05 +0100 Subject: [PATCH 1/2] fixed wrong reusage of checkpoint id for multiple commits Entire-Checkpoint: 32f95fca3df2 --- .../cli/strategy/manual_commit_hooks.go | 51 +++++++------------ .../strategy/phase_prepare_commit_msg_test.go | 37 ++++++++------ 2 files changed, 39 insertions(+), 49 deletions(-) diff --git a/cmd/entire/cli/strategy/manual_commit_hooks.go b/cmd/entire/cli/strategy/manual_commit_hooks.go index b5f927957..13f20c749 100644 --- a/cmd/entire/cli/strategy/manual_commit_hooks.go +++ b/cmd/entire/cli/strategy/manual_commit_hooks.go @@ -380,23 +380,15 @@ func (s *ManualCommitStrategy) PrepareCommitMsg(commitMsgFile string, source str } if hasNewContent { - // New content: check PendingCheckpointID first (set during previous condensation), - // otherwise generate a new one. This ensures idempotent IDs across hook invocations. - for _, state := range sessionsWithContent { - if state.PendingCheckpointID != "" { - if cpID, err := id.NewCheckpointID(state.PendingCheckpointID); err == nil { - checkpointID = cpID - break - } - } - } - if checkpointID.IsEmpty() { - cpID, err := id.Generate() - if err != nil { - return fmt.Errorf("failed to generate checkpoint ID: %w", err) - } - checkpointID = cpID + // New content: generate a fresh checkpoint ID. + // Idempotency for hook re-runs is handled by the trailer existence check above. + // NOTE: Do NOT reuse PendingCheckpointID here - it's from a previous commit's + // PostCommit and would cause duplicate IDs across different commits. + cpID, err := id.Generate() + if err != nil { + return fmt.Errorf("failed to generate checkpoint ID: %w", err) } + checkpointID = cpID } // Otherwise checkpointID is already set to LastCheckpointID from above @@ -1068,23 +1060,6 @@ func (s *ManualCommitStrategy) sessionHasNewContentFromLiveTranscript(repo *git. // (ACTIVE session + no TTY). Generates a checkpoint ID and adds the trailer // directly, bypassing content detection and interactive prompts. func (s *ManualCommitStrategy) addTrailerForAgentCommit(logCtx context.Context, commitMsgFile string, state *SessionState, source string) error { - // Use PendingCheckpointID if set, otherwise generate a new one - var cpID id.CheckpointID - if state.PendingCheckpointID != "" { - var err error - cpID, err = id.NewCheckpointID(state.PendingCheckpointID) - if err != nil { - cpID = "" // fall through to generate - } - } - if cpID.IsEmpty() { - var err error - cpID, err = id.Generate() - if err != nil { - return nil //nolint:nilerr // Hook must be silent on failure - } - } - content, err := os.ReadFile(commitMsgFile) //nolint:gosec // commitMsgFile is provided by git hook if err != nil { return nil //nolint:nilerr // Hook must be silent on failure @@ -1092,11 +1067,19 @@ func (s *ManualCommitStrategy) addTrailerForAgentCommit(logCtx context.Context, message := string(content) - // Don't add if trailer already exists + // Don't add if trailer already exists (idempotency for hook re-runs) if _, found := trailers.ParseCheckpoint(message); found { return nil } + // Generate a fresh checkpoint ID. + // NOTE: Do NOT reuse PendingCheckpointID here - it's from a previous commit's + // PostCommit and would cause duplicate IDs across different commits. + cpID, err := id.Generate() + if err != nil { + return nil //nolint:nilerr // Hook must be silent on failure + } + message = addCheckpointTrailer(message, cpID) logging.Info(logCtx, "prepare-commit-msg: agent commit trailer added", diff --git a/cmd/entire/cli/strategy/phase_prepare_commit_msg_test.go b/cmd/entire/cli/strategy/phase_prepare_commit_msg_test.go index 377708122..27ce8ad3e 100644 --- a/cmd/entire/cli/strategy/phase_prepare_commit_msg_test.go +++ b/cmd/entire/cli/strategy/phase_prepare_commit_msg_test.go @@ -134,17 +134,22 @@ func TestPrepareCommitMsg_AmendNoTrailerNoPendingID(t *testing.T) { "commit message should be unchanged when no trailer to restore") } -// TestPrepareCommitMsg_NormalCommitUsesPendingCheckpointID verifies that during -// a normal commit (source=""), if the session is in ACTIVE_COMMITTED phase with -// a PendingCheckpointID, the pending ID is reused instead of generating a new one. -// This ensures idempotent checkpoint IDs across prepare-commit-msg invocations. -func TestPrepareCommitMsg_NormalCommitUsesPendingCheckpointID(t *testing.T) { +// TestPrepareCommitMsg_ConsecutiveCommits_UniqueCheckpointIDs verifies that +// consecutive commits during an active session each get their own unique +// checkpoint ID. This is a regression test for a bug where PendingCheckpointID +// (set by PostCommit for deferred condensation) was incorrectly reused by +// PrepareCommitMsg for subsequent commits, causing duplicate checkpoint IDs. +// +// The fix: PrepareCommitMsg should always generate a fresh ID for new commits. +// Idempotency for hook re-runs on the same commit is handled by checking if +// the commit message already has a trailer (not by reusing PendingCheckpointID). +func TestPrepareCommitMsg_ConsecutiveCommits_UniqueCheckpointIDs(t *testing.T) { dir := setupGitRepo(t) t.Chdir(dir) s := &ManualCommitStrategy{} - sessionID := "test-session-normal-pending" + sessionID := "test-session-consecutive-commits" err := s.InitializeSession(sessionID, agent.AgentTypeClaudeCode, "", "") require.NoError(t, err) @@ -152,34 +157,36 @@ func TestPrepareCommitMsg_NormalCommitUsesPendingCheckpointID(t *testing.T) { createShadowBranchWithTranscript(t, dir, sessionID) // Set the session to ACTIVE_COMMITTED with a PendingCheckpointID + // This simulates the state after PostCommit ran for a previous commit state, err := s.loadSessionState(sessionID) require.NoError(t, err) require.NotNil(t, state) state.Phase = session.PhaseActiveCommitted - state.PendingCheckpointID = "fedcba987654" - // Ensure StepCount reflects that a checkpoint exists on the shadow branch + state.PendingCheckpointID = "fedcba987654" // ID from previous commit state.StepCount = 1 err = s.saveSessionState(state) require.NoError(t, err) - // Write a commit message file with no trailer (normal editor flow) + // Write a NEW commit message file (simulating a second commit) commitMsgFile := filepath.Join(t.TempDir(), "COMMIT_EDITMSG") - normalMsg := "Feature: add new functionality\n" + normalMsg := "Feature: second commit\n" require.NoError(t, os.WriteFile(commitMsgFile, []byte(normalMsg), 0o644)) - // Call PrepareCommitMsg with source="" (normal commit, editor flow) + // Call PrepareCommitMsg for the new commit err = s.PrepareCommitMsg(commitMsgFile, "") require.NoError(t, err) - // Read the file back - trailer should use PendingCheckpointID + // Read the file back - trailer should have a NEW checkpoint ID content, err := os.ReadFile(commitMsgFile) require.NoError(t, err) cpID, found := trailers.ParseCheckpoint(string(content)) assert.True(t, found, - "trailer should be present for normal commit with active session content") - assert.Equal(t, "fedcba987654", cpID.String(), - "normal commit should reuse PendingCheckpointID instead of generating a new one") + "trailer should be present for commit with active session content") + assert.NotEqual(t, "fedcba987654", cpID.String(), + "consecutive commits should get unique checkpoint IDs, not reuse PendingCheckpointID from previous commit") + assert.Len(t, cpID.String(), 12, + "checkpoint ID should be 12 hex characters") } // createShadowBranchWithTranscript creates a shadow branch commit with a minimal From ddc57af23709f2f2671dca30af061ce51d22ec20 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Wed, 11 Feb 2026 17:46:22 +0100 Subject: [PATCH 2/2] added docs section for PendingCheckpointID Entire-Checkpoint: 50e6e7e6ec02 --- docs/architecture/sessions-and-checkpoints.md | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/docs/architecture/sessions-and-checkpoints.md b/docs/architecture/sessions-and-checkpoints.md index 7b1f30277..9862e3c1a 100644 --- a/docs/architecture/sessions-and-checkpoints.md +++ b/docs/architecture/sessions-and-checkpoints.md @@ -166,6 +166,38 @@ Location: `.git/entire-sessions/.json` Stored in git common dir (shared across worktrees). Tracks active session info. +#### Checkpoint ID Fields + +Two checkpoint ID fields in session state track different lifecycle stages: + +| Field | Set By | Cleared By | Purpose | +|-------|--------|------------|---------| +| `PendingCheckpointID` | PostCommit (ACTIVE → ACTIVE_COMMITTED) | `condenseAndUpdateState` | Deferred condensation coordination | +| `LastCheckpointID` | `condenseAndUpdateState` | `InitializeSession` (new prompt) | Amend trailer restoration | + +**Key invariant:** `PendingCheckpointID` is for **deferred condensation only** — it must NEVER be reused when generating checkpoint IDs for new commits. Each new commit gets a fresh ID; idempotency for hook re-runs is handled by checking if the commit message already has a trailer. + +**Deferred condensation flow:** + +``` +1. Agent is working [ACTIVE phase] +2. User commits mid-turn + → PostCommit sets PendingCheckpointID + → Phase transitions to ACTIVE_COMMITTED + → Condensation deferred (agent still working) +3. Agent continues working [ACTIVE_COMMITTED phase] +4. Agent stops (TurnEnd event) + → handleTurnEndCondense runs + → Uses PendingCheckpointID for condensation + → Clears PendingCheckpointID + → Sets LastCheckpointID +5. Session idle [IDLE phase] +``` + +**Why not reuse `PendingCheckpointID` for new commits:** + +If a user makes multiple commits during a session, each commit needs a unique checkpoint ID. `PendingCheckpointID` persists from step 2 until step 4 completes. If `PrepareCommitMsg` reused it for a second commit before condensation finished, both commits would share the same checkpoint ID — corrupting the 1:1 link between commits and metadata. + ### Temporary Checkpoints Branch: `entire/-`