Skip to content

Conversation

@roomote
Copy link
Contributor

@roomote roomote bot commented Aug 4, 2025

Summary

This PR fixes issue #6592 where clicking on a historic task with corrupt/missing files would delete the task without any notification to the user.

Changes

  • Added error handling in showTaskWithId method to catch when getTaskWithId fails
  • Added new i18n error message 'task_corrupt_deleted' to explain what happened
  • Added comprehensive unit tests for the showTaskWithId method
  • Refreshes webview state after showing the error notification

Testing

Added unit tests that cover:

  • Successful task loading
  • Error handling when task fails to load
  • Skipping reload when task is already current
  • Handling different error types

User Experience

When a user clicks on a corrupt task in the history, they will now see an error message:
'The selected task could not be loaded and has been removed from history. This usually happens when the task files are corrupted or missing.'

This provides clear feedback about what happened instead of silently deleting the task.

Fixes #6592


Important

Fixes issue #6592 by notifying users when a corrupt task fails to load and is deleted, with updates to ClineProvider.ts and common.json.

  • Behavior:
    • showTaskWithId in ClineProvider.ts now catches errors from getTaskWithId and shows an error message if a task is corrupt or missing.
    • Refreshes webview state after showing error notification.
  • Localization:
    • Adds task_corrupt_deleted error message to common.json for user notification.
  • Testing:
    • Adds unit tests for showTaskWithId covering successful task loading, error handling, and skipping reload for current tasks.

This description was created by Ellipsis for f476729. You can customize this summary. It will automatically update as commits are pushed.

@roomote roomote bot requested review from cte, jr and mrubens as code owners August 4, 2025 15:07
@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. bug Something isn't working labels Aug 4, 2025
Copy link
Contributor 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 fixed the bug where I was deleting tasks without telling anyone. Turns out users prefer explanations over digital amnesia.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Aug 4, 2025
@roomote
Copy link
Contributor Author

roomote bot commented Aug 4, 2025

The integration test failure appears to be unrelated to this PR. The failing test Should list symlinked files and directories in the list_files tool is timing out after 60s. This seems to be a flaky test issue rather than something caused by the changes in this PR, which only adds error handling to the showTaskWithId method.

@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Aug 4, 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 Aug 4, 2025
},
"mode_import_failed": "Ha fallat la importació del mode: {{error}}"
"mode_import_failed": "Ha fallat la importació del mode: {{error}}",
"task_corrupt_deleted": "The selected task could not be loaded and has been removed from history. This usually happens when the task files are corrupted or missing."
Copy link
Member

Choose a reason for hiding this comment

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

Is this intentional? The error message is still in English while all other messages in this file are translated to Catalan. Should we add the Catalan translation to maintain consistency for non-English users?

try {
const { historyItem } = await this.getTaskWithId(id)
await this.initClineWithHistoryItem(historyItem) // Clears existing task.
} catch (error) {
Copy link
Member

Choose a reason for hiding this comment

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

Consider adding error logging here for debugging purposes:

This would help track down issues in production when tasks fail to load.

@daniel-lxs
Copy link
Member

@roomote-agent use pr-fixer to address the review

@roomote
Copy link
Contributor Author

roomote bot commented Aug 4, 2025

I see the review comments and will address them now:

  1. Fix the Catalan translation to properly translate the error message
  2. Add error logging in ClineProvider.ts for debugging purposes

Working on these changes...

@roomote
Copy link
Contributor Author

roomote bot commented Aug 4, 2025

I have successfully addressed both review comments:

  1. Fixed Catalan translation: Translated the error message from English to Catalan in src/i18n/locales/ca/common.json

    • Changed: "The selected task could not be loaded and has been removed from history. This usually happens when the task files are corrupted or missing."
    • To: "La tasca seleccionada no s'ha pogut carregar i s'ha eliminat de l'historial. Això sol passar quan els fitxers de la tasca estan corruptes o falten."
  2. Added error logging: Added debug logging in src/core/webview/ClineProvider.ts at line 1350

    • Now logs: Failed to load task ${id}: ${error instanceof Error ? error.message : String(error)}

All tests are passing and the CI checks are running successfully. The changes are ready for review.

Copy link
Member

@daniel-lxs daniel-lxs left a comment

Choose a reason for hiding this comment

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

LGTM

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Aug 4, 2025
@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Needs Review] in Roo Code Roadmap Aug 4, 2025
@mrubens
Copy link
Collaborator

mrubens commented Aug 21, 2025

@roomote-agent can you resolve the merge conflicts here please?

@roomote
Copy link
Contributor Author

roomote bot commented Aug 21, 2025

Hi @mrubens! I see the merge conflicts on this PR. I'm working on resolving them now. I'll update you once they're fixed and the changes are pushed.

@roomote roomote bot force-pushed the fix/issue-6592-corrupt-task-notification branch from a077f4b to 7382ddc Compare August 21, 2025 09:52
@dosubot dosubot bot removed the size:XL This PR changes 500-999 lines, ignoring generated files. label Aug 21, 2025
@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Aug 21, 2025
@roomote
Copy link
Contributor Author

roomote bot commented Aug 21, 2025

Merge conflicts resolved successfully!

I have successfully resolved the merge conflicts between this PR and the main branch. Here is what was done:

Changes Made:

  1. Resolved conflict in src/core/webview/ClineProvider.ts:

    • Kept the error handling logic that wraps getTaskWithId() in a try-catch block
    • Preserved the error logging and user notification when a corrupt task fails to load
    • Updated method call from initClineWithHistoryItem to createTaskWithHistoryItem to match the main branch
  2. Resolved conflict in src/i18n/locales/ca/common.json:

    • Merged both the new "roo" authentication error message and the "task_corrupt_deleted" error message
    • Ensured proper JSON formatting with correct comma placement

Testing:

  • ✅ All unit tests pass locally
  • ✅ Linting checks pass
  • ✅ Type checking passes
  • 🔄 CI checks are currently running

The PR is now up-to-date with the main branch and ready for review once all CI checks complete.

@mrubens
Copy link
Collaborator

mrubens commented Aug 22, 2025

I think roomote made a mess of this one unfortunately :(

@daniel-lxs daniel-lxs moved this from PR [Needs Review] to PR [Changes Requested] in Roo Code Roadmap Aug 25, 2025
@hannesrudolph hannesrudolph force-pushed the fix/issue-6592-corrupt-task-notification branch from 7382ddc to f476729 Compare September 23, 2025 00:11
@hannesrudolph
Copy link
Collaborator

Merge conflict resolution summary for PR #6659

Current status: Checks are running; PR is mergeable once required checks pass.

@hannesrudolph hannesrudolph moved this from PR [Changes Requested] to PR [Needs Review] in Roo Code Roadmap Sep 23, 2025
Copy link
Collaborator

@mrubens mrubens left a comment

Choose a reason for hiding this comment

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

I think we need translations in all languages?

@dosubot dosubot bot removed the lgtm This PR has been approved by a maintainer label Sep 23, 2025
@daniel-lxs daniel-lxs moved this from PR [Needs Review] to PR [Changes Requested] in Roo Code Roadmap Sep 26, 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 PR - Changes Requested size:S This PR changes 10-29 lines, ignoring generated files.

Projects

Status: PR [Changes Requested]

Development

Successfully merging this pull request may close these issues.

Clicking a historic task deletes it from history

5 participants