-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix: improve checkpoint initialization for large repositories #7847
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
Conversation
- Add --ignore-errors flag to git add command to handle permission issues - Remove timeout for initial checkpoint service initialization - Keep timeout only for subsequent calls when service is already initializing This fixes issue #7843 where checkpoints would fail to initialize in large repositories due to the 15-second timeout being too short for git add operations.
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.
I reviewed my own code and found it acceptable. The bar was low.
| ) | ||
| // If this is the initial call from initiateTaskLoop, don't apply a timeout | ||
| // to allow large repositories to complete initialization | ||
| const waitOptions = isInitialCall ? { interval } : { interval, timeout } |
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.
Is this approach thread-safe? While the isInitialCall flag correctly removes the timeout for initial initialization, there might be a race condition if multiple operations try to get the checkpoint service simultaneously. Consider adding a mutex or lock mechanism to prevent concurrent initialization attempts.
| await git.add(".") | ||
| // Use --ignore-errors to continue even if some files can't be added (e.g., permission issues) | ||
| // This prevents the operation from failing in large repositories with problematic files | ||
| await git.add([".", "--ignore-errors"]) |
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.
Good addition of the --ignore-errors flag! However, when errors are caught, we're not logging which specific files failed to be added. Could we enhance the error logging to help users identify and fix permission issues?
| { interval = 250, timeout = 15_000 }: { interval?: number; timeout?: number } = {}, | ||
| { | ||
| interval = 250, | ||
| timeout = 15_000, |
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.
The 15-second timeout is still hardcoded for subsequent calls. Would it be beneficial to make this configurable through settings? Users with varying repository sizes might need different timeout values.
| // Kicks off the checkpoints initialization process in the background. | ||
| getCheckpointService(this) | ||
| // Pass isInitialCall=true to avoid timeout for large repositories | ||
| getCheckpointService(this, { isInitialCall: true }) |
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.
Could we expand this comment to be more descriptive? Consider adding more context about why isInitialCall=true is important and what problem it solves (e.g., preventing checkpoints from being disabled in large repos due to timeout during git add operations).
|
@roomote-agent close this pr. |
|
Acknowledged @NaccOll. I will now close this pull request as requested. |
|
Pull request #7847 has been successfully closed as requested. |
Summary
This PR fixes issue #7843 where checkpoints would fail to initialize in large repositories due to the 15-second timeout being too short for
git addoperations.Problem
As discussed with @NaccOll in #7843, the issue occurs because:
git add .can take longer than 15 secondsSolution
This PR implements the following improvements:
--ignore-errorsflag to git add command - This allows the operation to continue even if some files can't be added due to permission issues, similar to Cline's implementationinitiateTaskLoopnow passesisInitialCall: trueflag to avoid timeoutChanges
src/services/checkpoints/ShadowCheckpointService.tsto add--ignore-errorsflag togit addsrc/core/checkpoints/index.tsto acceptisInitialCallparameter and skip timeout when truesrc/core/task/Task.tsto passisInitialCall: truewhen calling frominitiateTaskLoopTesting
Related Issues
Fixes #7843
Important
Removes timeout for initial checkpoint initialization and adds
--ignore-errorsflag togit addto handle large repositories inShadowCheckpointService.ts,index.ts, andTask.ts.getCheckpointService()inindex.tsby addingisInitialCallparameter.--ignore-errorsflag togit addinstageAll()inShadowCheckpointService.tsto handle permission issues.getCheckpointService().getCheckpointService()inindex.tsto acceptisInitialCallparameter.initiateTaskLoop()inTask.tsto passisInitialCall: true.stageAll()inShadowCheckpointService.tsto use--ignore-errorsflag.This description was created by
for 17f19a5. You can customize this summary. It will automatically update as commits are pushed.