Conversation
Entire-Checkpoint: fc09dbaa3a88
Entire-Checkpoint: 317de6082c49
There was a problem hiding this comment.
Pull request overview
Adds a new entire commit subcommand that creates Git commits via go-git while embedding Entire checkpoint metadata directly in the commit object (extra header), reducing reliance on external Git hook configuration.
Changes:
- Register a new
entire commitCobra command on the root CLI. - Introduce a dedicated commit-object extra header key (
checkpoint) alongside the existing commit-message trailer. - Add unit tests covering header/trailer writing, empty-commit behavior, and root command registration.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| cmd/entire/cli/trailers/trailers.go | Adds CheckpointHeaderKey constant for commit extra headers. |
| cmd/entire/cli/root.go | Registers the new commit command with the root CLI. |
| cmd/entire/cli/commit.go | Implements commit creation (tree from index, commit encode/store, HEAD update) and checkpoint header/trailer handling. |
| cmd/entire/cli/commit_test.go | Adds tests validating trailer/header behavior and command wiring. |
| if len(idx.Entries) == 0 { | ||
| return plumbing.ZeroHash, git.ErrEmptyCommit | ||
| } | ||
|
|
There was a problem hiding this comment.
buildCommitTreeFromIndex treats an empty index (len(idx.Entries) == 0) as git.ErrEmptyCommit, which blocks valid commits that delete all tracked files (the resulting tree is empty, but differs from the previous tree). Instead, build an empty tree hash (via checkpoint.BuildTreeFromEntries with an empty map) and let the existing treeHash == previousTree check determine whether the commit is truly empty.
| if len(idx.Entries) == 0 { | |
| return plumbing.ZeroHash, git.ErrEmptyCommit | |
| } |
| hash, err := runCommit(cmd.Context(), message) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| fmt.Fprintf(cmd.OutOrStdout(), "[%s] %s\n", shortCommitHash(hash), firstLine(message)) | ||
| return nil |
There was a problem hiding this comment.
The command output uses firstLine(message) from the original -m value, but hooks (PrepareCommitMsg/CommitMsg) can modify the final commit message subject. Consider printing the first line of the final message (or reading back the created commit’s subject) so the displayed summary matches what was actually committed.
![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 prepared a fix for the issue found in the latest run.
- ✅ Fixed: Redundant duplicate call to force checkpoint logic
- Removed the dead code block in runCommit (lines 60-69) that redundantly called forceCheckpointIDForActiveSession, since prepareEntireCommitMessage already handles this case and no state changes between the two calls.
Or push these changes by commenting:
@cursor push 4a3db96ebe
Preview (4a3db96ebe)
diff --git a/cmd/entire/cli/commit.go b/cmd/entire/cli/commit.go
--- a/cmd/entire/cli/commit.go
+++ b/cmd/entire/cli/commit.go
@@ -57,16 +57,6 @@
return plumbing.ZeroHash, err
}
checkpointID = resolveCommitCheckpointID(finalMessage, checkpointID)
- if checkpointID == "" {
- forcedCheckpointID, forceErr := forceCheckpointIDForActiveSession(ctx)
- if forceErr != nil {
- return plumbing.ZeroHash, forceErr
- }
- if !forcedCheckpointID.IsEmpty() {
- checkpointID = forcedCheckpointID.String()
- finalMessage = trailers.FormatCheckpoint(finalMessage, forcedCheckpointID)
- }
- }
treeHash, err := buildCommitTreeFromIndex(ctx, repo)
if err != nil {This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit e401559. Configure here.
| checkpointID = forcedCheckpointID.String() | ||
| finalMessage = trailers.FormatCheckpoint(finalMessage, forcedCheckpointID) | ||
| } | ||
| } |
There was a problem hiding this comment.
Redundant duplicate call to force checkpoint logic
Low Severity
forceCheckpointIDForActiveSession is called in prepareEntireCommitMessage (which also calls trailers.FormatCheckpoint on success) and again in runCommit. The second call in runCommit is effectively dead code — it only executes when checkpointID is empty, which means prepareEntireCommitMessage already called forceCheckpointIDForActiveSession and got an empty result. Since no state changes between the two calls, the second invocation always returns the same empty result. This duplicated force-and-format logic across both functions makes the code harder to reason about and maintain.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit e401559. Configure here.
This command enables adding checkpoint information as a Git header directly into the Commit object:
When used, git hooks do not need to be configured.
Note
Medium Risk
Adds a new commit-writing code path that directly constructs git commit objects and updates
HEAD, so bugs could create incorrect commits or refs. Changes are isolated to the new command plus a small new trailer constant and tests.Overview
Adds a new
entire commit -mcommand that creates a git commit viago-git, running the configured strategy commit-msg hooks to inject/normalize theEntire-Checkpointtrailer and then persisting the checkpoint ID as an extra commit header (checkpoint) when present.The command builds the commit tree from the index, rejects empty commits (no staged changes or unchanged tree), updates
HEADto the new commit, and triggersstrategy.PostCommit; it can also force-link an eligible active session by generating and appending a checkpoint ID when none was produced by the message processing.Includes unit tests covering checkpoint trailer/header behavior, empty-commit errors, forced linking, header key constant, and registration of the new command on the root CLI.
Reviewed by Cursor Bugbot for commit e401559. Configure here.