-
Notifications
You must be signed in to change notification settings - Fork 85
fix: compute deleted files in task checkpoints #218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: compute deleted files in task checkpoints #218
Conversation
The handleClaudeCodePostTask handler was passing nil for DeletedFiles when creating task checkpoint metadata. This meant deleted files were silently dropped from task checkpoint records, making attribution tracking and session analysis incomplete. Compute deleted files from git status using the existing ComputeDeletedFiles() function, matching the pattern already used by the Stop handler and PostTodo handler. Also include deleted files in the no-changes guard so task checkpoints that only contain deletions are still recorded. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Big fan of the project & congrats on launch. Committing a small PR to contribute. |
|
Hey @fakepixels, good pickup and thanks for your contribution. I had a look, and made some improvements. I refactored the function a bit, to use Lines 211 to 217 in 07743be
This removes the reference to Lines 265 to 269 in 07743be
Lines 282 to 286 in 07743be
That let me remove some of the dead code in that module too, which is a win. Thanks! 🙌 |
968b269 to
954c25d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
Refactor the function a bit, and use ComputeFileChanges() which returns new files (created during session) and deleted files (tracked files that were deleted) using a single git status call. https://github.com/entireio/cli/blob/07743bedae1e5c206e11c3b1f0a3df48ce9fe131/cmd/entire/cli/state.go#L211-L217 This removes the reference to ComputeNewFilesFromTask and ComputeDeletedFiles, both of which are marked as deprecated: https://github.com/entireio/cli/blob/07743bedae1e5c206e11c3b1f0a3df48ce9fe131/cmd/entire/cli/state.go#L265-L269 https://github.com/entireio/cli/blob/07743bedae1e5c206e11c3b1f0a3df48ce9fe131/cmd/entire/cli/state.go#L282-L286 The obstacle was that ComputeFileChanges took a *PrePromptState, but handleClaudeCodePostTask has a *PreTaskState. Both structs carry the same UntrackedFiles field — the function never used anything else from the struct. Narrowing the parameter from *PrePromptState to []string makes ComputeFileChanges usable from both prompt and task handlers without introducing a shared interface or duplicating the function. Callers now pass preState.PreUntrackedFiles() instead of preState directly. The new PreUntrackedFiles() accessor (added to both PrePromptState and PreTaskState) preserves the three-way nil semantics that ComputeFileChanges relies on: - receiver nil → return nil → skip new-file detection - receiver non-nil, field nil → return []string{} → all untracked = new - receiver non-nil, field set → return the list → diff against it The nil-field guard (second case) matters for old state files where "untracked_files" deserializes as nil. Without it, ComputeFileChanges would see nil and skip detection entirely, instead of correctly treating all current untracked files as new. This matches the original behavior where any non-nil preState triggered detection. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Means exactly the same, but makes it crystal clear to me that we won't dereference a nil pointer.
getUntrackedFilesForState() used `var untrackedFiles []string`, which
is nil when no untracked files exist. This nil flowed through:
1. CapturePreTaskState saves nil to JSON as `"untracked_files": null`
2. LoadPreTaskState deserializes null back as nil
3. handleClaudeCodePostTask copies nil into previousUntracked
4. ComputeFileChanges(nil) treats nil as "caller doesn't want new file
detection", so it skips the Untracked case entirely and returns
newFiles==nil
Now, when a task creates new files in a repo with zero pre-existing
untracked files, post-task sees no changes and skips creating a
checkpoint.
This was not a problem before the ComputeFileChanges refactor in this
branch, because the old ComputeNewFilesFromTask checked `preState ==
nil` rather than `preState.UntrackedFiles == nil`. When preState existed
but had nil UntrackedFiles, it still called
findNewUntrackedFiles(currentUntracked, nil), which built an empty
pre-existing set and correctly detected all current untracked files as
new.
The fix initializes the slice as `[]string{}` instead of nil. This
serializes to `"untracked_files": []` in JSON, deserializes back as a
non-nil empty slice, and ComputeFileChanges correctly detects new files.
Both CapturePrePromptState and CapturePreTaskState call
getUntrackedFilesForState(), so this single fix covers both code paths.
The following functions in cmd/entire/cli/state.go were unreferenced except from the test suite. Remove: * ComputeNewFilesFromTask * computeNewFilesFromTaskState
a9b1b19 to
83cb9f2
Compare
Summary
TODO: compute deleted filesinhandleClaudeCodePostTask(line 668)Problem
handleClaudeCodePostTaskwas passingnilforDeletedFileswhen buildingTaskCheckpointContext. This meant that when a subagent deleted files during a Task, those deletions were silently dropped from checkpoint metadata — making attribution tracking and session analysis incomplete.Before: Task checkpoints recorded modified and new files, but deletions were always
nil.After: Deleted files are computed from git status using the existing
ComputeDeletedFiles()function and included in task checkpoint metadata.Changes
ComputeDeletedFiles()to detect tracked files that were deleted (same pattern used by the Stop handler at line 232 and PostTodo handler at line 423)nilwith the computedrelDeletedFilesinTaskCheckpointContextTest plan
ComputeFileChanges) and PostTodo handler (DetectChangedFiles)ComputeDeletedFilesis already covered by existing tests (TestComputeFileChanges_DeletedFilesWithNilPreState,TestComputeFileChanges_NewAndDeletedFiles)mise run fmt,mise run lint,mise run test:ci🤖 Generated with Claude Code
Note
Medium Risk
Touches core change-detection logic used by multiple hook paths; subtle nil-vs-empty semantics could affect whether checkpoints are created or which files are attributed.
Overview
Task (subagent) checkpoints now capture deleted tracked files and will create a checkpoint even when the only changes are deletions (instead of silently dropping them).
Refactors
ComputeFileChangesto accept a pre-captured untracked-file slice (nil meaning “skip new-file detection”), addsPreUntrackedFiles()helpers onPrePromptState/PreTaskStateto preserve nil-vs-empty semantics for backward compatibility, and updates all hook/debug call sites accordingly; removes older task new-file helpers and related tests.Written by Cursor Bugbot for commit 83cb9f2. This will update automatically on new commits. Configure here.