fixed wrong reusage of checkpoint id for multiple commits#268
fixed wrong reusage of checkpoint id for multiple commits#268
Conversation
Entire-Checkpoint: 32f95fca3df2
Entire-Checkpoint: 50e6e7e6ec02
|
bugbot run |
![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 2 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
| if err != nil { | ||
| return fmt.Errorf("failed to generate checkpoint ID: %w", err) | ||
| } | ||
| checkpointID = cpID |
There was a problem hiding this comment.
Earlier commit IDs become unreachable
High Severity
PrepareCommitMsg now always generates a new checkpoint ID for new content, but session state still keeps only one PendingCheckpointID. In multi-commit active turns, later PostCommit calls overwrite earlier IDs, and turn-end condensation uses only the last one. Earlier commits keep trailers pointing to checkpoint IDs that never get condensed.
Additional Locations (2)
| if err != nil { | ||
| return fmt.Errorf("failed to generate checkpoint ID: %w", err) | ||
| } | ||
| checkpointID = cpID |
There was a problem hiding this comment.
Message commits skip confirmation unexpectedly
Medium Severity
After switching to fresh IDs, hasNewContent commits are still marked as restoring when any session has PendingCheckpointID. For source == "message", this routes to the non-confirmation path even though a brand-new checkpoint is being created, so trailer insertion can bypass the intended interactive confirmation.
Additional Locations (1)
| | Field | Set By | Cleared By | Purpose | | ||
| |-------|--------|------------|---------| | ||
| | `PendingCheckpointID` | PostCommit (ACTIVE → ACTIVE_COMMITTED) | `condenseAndUpdateState` | Deferred condensation coordination | | ||
| | `LastCheckpointID` | `condenseAndUpdateState` | `InitializeSession` (new prompt) | Amend trailer restoration | |
There was a problem hiding this comment.
LastCheckpointID is documented as only for “Amend trailer restoration”, but in the manual-commit strategy it’s also used to reuse/link an existing condensed checkpoint when there’s no new content (e.g., split Claude’s work across multiple commits). Consider expanding the Purpose (and/or the surrounding text) to reflect both behaviors so readers don’t assume it’s amend-only.
| | `LastCheckpointID` | `condenseAndUpdateState` | `InitializeSession` (new prompt) | Amend trailer restoration | | |
| | `LastCheckpointID` | `condenseAndUpdateState` | `InitializeSession` (new prompt) | Amend trailer restoration and reusing/linking an existing condensed checkpoint when new commits have no additional content (e.g., manual-commit strategy) | |
| // 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 |
There was a problem hiding this comment.
This hunk correctly stops reusing PendingCheckpointID when generating a checkpoint ID for new content, but later in PrepareCommitMsg the isRestoringExisting logic is still influenced by PendingCheckpointID when hasNewContent is true. With fresh IDs now always generated here, PendingCheckpointID should no longer cause the commit to be treated as “restoring existing” (it can incorrectly bypass the interactive confirmation path for source == "message"). Consider removing/reworking that PendingCheckpointID check so only actual reuse/restoration paths skip confirmation.
2 participants
This fixes a bug that could lead to creating multiple commits with the same checkpoint id. A checkpoint id should only be used in one commit.