-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix: resolve checkpoint initialization timeout in large repositories #7846
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 timeout handling for initial git staging operation (5 seconds) - Add --ignore-errors flag to git add command for better error handling - Add isInitialCall parameter to allow unlimited time for first initialization - Prevent blocking on large repositories during checkpoint setup This fix addresses issue #7843 where checkpoints would fail to initialize in large repositories due to the git add operation taking longer than the 15-second timeout limit.
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.
Reviewing my own code is like debugging in production - technically possible but morally questionable.
| { interval, timeout }, | ||
| ) | ||
| // For initial calls, don't apply timeout to allow large repositories to initialize | ||
| 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 the right approach? The issue discussion indicates that the problem is subsequent calls timing out while waiting for initialization, not the initial call itself. This implementation removes the timeout for initial calls, but the actual bottleneck (git add during initialization) still has a hard-coded 5-second timeout in ShadowCheckpointService. Could we consider a more comprehensive solution?
| }, 5000) // 5 second timeout for initial staging | ||
| }) | ||
|
|
||
| await Promise.race([stagePromise, timeoutPromise]) |
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.
This Promise.race() implementation could lead to incomplete staging. If the timeout wins, the staging operation continues in the background but we proceed with a potentially empty commit. This could miss important files. Consider:
- Cancelling the staging operation if timeout occurs
- Or waiting for staging to complete but with a longer, configurable timeout
- Or deferring staging until the first actual checkpoint (lazy staging)
| `[${this.constructor.name}#initShadowGit] Initial staging timed out after 5 seconds, proceeding with empty commit`, | ||
| ) | ||
| resolve() | ||
| }, 5000) // 5 second timeout for initial staging |
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, the 5-second timeout is a magic number. Consider defining it as a constant at the top of the file:
This would make it easier to adjust if needed.
| { | ||
| interval = 250, | ||
| timeout = 15_000, | ||
| isInitialCall = false, |
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 new isInitialCall parameter lacks JSDoc documentation. Could we add documentation explaining its purpose?
|
@roomote-agent close this pr. |
|
Acknowledged @NaccOll. I will close this PR as requested. |
|
Pull request #7846 has been closed as requested. The fix for checkpoint initialization timeout in large repositories will not be merged at this time. |
Summary
This PR addresses issue #7843 where checkpoint initialization would timeout in large repositories.
Problem
As reported by @NaccOll and confirmed through analysis of Cline's implementation, RooCode's checkpoint service was calling
git add .during initialization, which could take longer than the 15-second timeout in large repositories, causing checkpoints to be disabled.Solution
Based on the comparison with Cline's implementation, this PR implements the following fixes:
git addoperation during shadow git initialization--ignore-errorsflag to git add commands to handle permission issuesisInitialCallparameter to allow unlimited time for the first checkpoint service initializationChanges
ShadowCheckpointService.tsto use Promise.race() with a 5-second timeout for initial stagingstageAll()method to use--ignore-errorsflaggetCheckpointService()to support unlimited timeout on initial callTask.tsto passisInitialCall: trueflagTesting
Future Improvements
The analysis also identified that Cline handles nested git repositories by temporarily disabling them, while RooCode currently blocks initialization. This could be addressed in a future PR for even better compatibility with monorepos and projects with submodules.
Fixes #7843
Important
Fixes checkpoint initialization timeout in large repositories by adding a 5-second timeout for
git add, handling permission issues, and allowing unlimited time for first initialization.git addinShadowCheckpointService.tsusingPromise.race().--ignore-errorsflag togit addinShadowCheckpointService.tsto handle permission issues.isInitialCallparameter ingetCheckpointService()inindex.tsto allow unlimited time for first initialization.initiateTaskLoop()inTask.tsto passisInitialCall: truefor unlimited initialization time.This description was created by
for a7f2acf. You can customize this summary. It will automatically update as commits are pushed.