Skip to content

Conversation

@roomote
Copy link

@roomote roomote bot commented Sep 7, 2025

Show notification when checkpoints fail to initialize due to nested repos

Copy link
Author

@roomote roomote bot left a 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 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")) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. Creating a specific error type for nested git repository detection
  2. Using error codes instead of message string matching
  3. 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) => {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.",
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Sep 7, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Sep 9, 2025
@hannesrudolph hannesrudolph added PR - Needs Preliminary Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Sep 9, 2025
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Sep 10, 2025
- 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 daniel-lxs force-pushed the fix/7765-checkpoint-init-nested-git branch from d3443c2 to 71df24b Compare September 10, 2025 20:17
@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Sep 10, 2025
"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.",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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".

Suggested change
"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 daniel-lxs changed the title fix: handle checkpoint initialization failure gracefully for nested git repos feat: show notification when the checkpoint initialization fails Sep 10, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Sep 10, 2025
@mrubens mrubens merged commit 3b1a6d1 into main Sep 10, 2025
16 of 17 checks passed
@mrubens mrubens deleted the fix/7765-checkpoint-init-nested-git branch September 10, 2025 21:08
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Sep 10, 2025
@github-project-automation github-project-automation bot moved this from PR [Needs Prelim Review] to Done in Roo Code Roadmap Sep 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer PR - Needs Preliminary Review size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants