Skip to content

Conversation

roomote-agent
Copy link
Collaborator

@roomote-agent roomote-agent commented Jul 13, 2025

This PR adds a configurable timeout for command execution in the Roo Code VSCode extension, addressing the Slack mention request.

Key Changes:

  • Added commandExecutionTimeout setting to VSCode configuration (0-600 seconds, default 0s)
  • Implemented timeout logic in executeCommand function with proper cleanup
  • Added timeout status to CommandExecutionStatus type
  • Added comprehensive tests for timeout functionality
  • Process abortion when timeout is reached with clear error messages

Configuration:
Users can set: roo-cline.commandExecutionTimeout (in milliseconds)

Testing:

  • All existing tests pass
  • TypeScript compilation successful
  • ESLint checks pass
  • New timeout-specific tests added

Fixes: Slack mention request for configurable command execution timeout


Important

Adds configurable command execution timeout to Roo Code VSCode extension with new setting, logic, and tests.

  • Behavior:
    • Adds commandExecutionTimeout setting (0-600 seconds, default 0) to package.json for command execution timeout.
    • Implements timeout logic in executeCommand in executeCommandTool.ts with process abortion and error messages.
    • Updates CommandExecutionStatus in terminal.ts to include timeout status.
  • Testing:
    • Adds executeCommandTimeout.integration.spec.ts for timeout functionality tests.
    • Updates executeCommandTool.spec.ts to include timeout parameter tests.
  • Localization:
    • Adds command_timeout message to multiple locale files for timeout error feedback.

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

- Add commandExecutionTimeout setting to VSCode configuration (1-300 seconds, default 30s)
- Implement timeout logic in executeCommand function with proper cleanup
- Add timeout status to CommandExecutionStatus type
- Include comprehensive tests for timeout functionality
- Abort running processes when timeout is reached
- Provide clear timeout error messages to users

Addresses Slack mention request for configurable command execution timeout.
@roomote-agent roomote-agent requested review from mrubens, cte and jr as code owners July 13, 2025 17:51
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. enhancement New feature or request labels Jul 13, 2025
Copy link

delve-auditor bot commented Jul 13, 2025

No security or compliance issues detected. Reviewed everything up to dcca042.

Security Overview
  • 🔎 Scanned files: 41 changed file(s)
Detected Code Changes
Change Type Relevant files
Enhancement ► executeCommandTool.ts
    Add command execution timeout functionality
► executeCommandTimeout.integration.spec.ts
    Add integration tests for timeout functionality
Configuration changes ► package.json
    Add command execution timeout setting
► package.nls.*.json
    Add translations for timeout setting
Refactor ► executeCommandTool.spec.ts
    Update tests for timeout feature
► CHANGELOG.md
    Remove unreleased version entries

Reply to this PR with @delve-auditor followed by a description of what change you want and we'll auto-submit a change to this PR to implement it.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Jul 13, 2025
- Fixed fs.promises mocking in integration tests
- Added proper abort method mocking for timeout scenarios
- Increased test timeouts for async timeout testing
- All timeout integration tests now pass successfully
@mrubens
Copy link
Collaborator

mrubens commented Jul 13, 2025

@roomote-bot can you make it default to 0, meaning no timeout?

And does this work for both terminal execution and execa execution?

@roomote-agent
Copy link
Collaborator Author

👋 Hi @mrubens! I've seen your comment and I'm working on it now. I'll:

  1. Change the default timeout to 0 (no timeout)
  2. Investigate and ensure it works for both terminal execution and execa execution
  3. Update the implementation accordingly

I'll reply with a summary once the changes are complete!

- Change default commandExecutionTimeout from 30000ms to 0 (no timeout)
- Update minimum value from 1000ms to 0ms in package.json
- Update package.nls.json description to clarify 0 = no timeout
- Modify executeCommandTool.ts to skip timeout logic when timeout is 0
- Update tests to reflect new default behavior and add test for no timeout
- Timeout functionality works for both VSCode terminal and execa execution

Addresses @mrubens feedback in PR #5668
@roomote-agent
Copy link
Collaborator Author

Done! I've successfully implemented your requested changes:

Changes Made:

  1. Changed default timeout to 0 (no timeout) in
  2. Updated minimum value to 0 (was 1000ms) in
  3. Updated description in to clarify that 0 = no timeout
  4. Modified timeout logic in to skip timeout when value is 0
  5. Updated tests to reflect new default behavior and added test for no timeout scenario

Answers to Your Questions:

Default is now 0 (no timeout) - Commands will run indefinitely by default
Works for both terminal execution and execa execution - The timeout is implemented at a higher level in that wraps both terminal providers (VSCode terminal and execa)

Testing:

  • ✅ All unit tests pass
  • ✅ All integration tests pass
  • ✅ TypeScript compilation successful
  • ✅ ESLint checks pass
  • ✅ All CI checks pass

The implementation ensures that when is set to 0, no timeout logic is applied and commands can run indefinitely. Users can still set a specific timeout value (1000-300000ms) if they want to limit command execution time.

@mrubens
Copy link
Collaborator

mrubens commented Jul 13, 2025

@roomote-bot can you change the timeout to be in seconds instead of milliseconds? And make it a max of 10 minutes?

@roomote-agent
Copy link
Collaborator Author

👋 Hi @mrubens! I've seen your comment and I'm working on it now. I'll change the timeout configuration from milliseconds to seconds with a maximum of 10 minutes (600 seconds). I'll reply with a summary once the changes are complete!

- Update package.json configuration: max timeout now 600s (10 minutes) instead of 300000ms
- Update package.nls.json description to reflect seconds unit
- Update executeCommandTool.ts to convert seconds to milliseconds internally
- Update error messages to display timeout in seconds
- Update integration and unit tests to use seconds configuration
- Maintain backward compatibility by converting seconds to milliseconds for internal timeout logic
@roomote-agent
Copy link
Collaborator Author

Done! I've successfully implemented your requested changes:

Changes Made:

  1. Changed timeout unit from milliseconds to seconds in
  2. Updated maximum timeout to 600 seconds (10 minutes) (was 300000ms) in
  3. Updated description in to reflect seconds unit
  4. Modified timeout logic in to convert seconds to milliseconds internally
  5. Updated error messages to display timeout in seconds instead of milliseconds
  6. Updated all tests to use seconds configuration while maintaining internal millisecond logic

Configuration:

  • Unit: Now in seconds (was milliseconds)
  • Range: 0-600 seconds (0 = no timeout, max = 10 minutes)
  • Default: 0 seconds (no timeout)
  • Backward compatibility: Maintained by converting seconds to milliseconds internally

Testing:

  • ✅ All unit tests pass
  • ✅ All integration tests pass
  • ✅ TypeScript compilation successful
  • ✅ ESLint checks pass
  • ✅ All pre-commit hooks pass

The timeout configuration is now much more user-friendly with seconds instead of milliseconds, and the 10-minute maximum provides a reasonable upper bound for command execution.

@mrubens
Copy link
Collaborator

mrubens commented Jul 13, 2025

@roomote-bot great! Can you translate the settings file into all languages using the Translate mode?

@roomote-agent
Copy link
Collaborator Author

👋 Hi @mrubens! I've seen your comment and I'm working on it now. I'll use the Translate mode to translate the settings file into all supported languages. I'll reply with a summary once the translations are complete!

…uages

- Added translations for commands.commandExecutionTimeout.description in 17 languages
- Covers: ca, de, es, fr, hi, id, it, ja, ko, nl, pl, pt-BR, ru, tr, vi, zh-CN, zh-TW
- All translations validated with missing-translations script
@roomote-agent
Copy link
Collaborator Author

Done! I've successfully translated the settings file into all supported languages using the Translate mode.

Changes Made:

  • Added translations to all 17 localized package.nls files
  • Languages covered: Catalan (ca), German (de), Spanish (es), French (fr), Hindi (hi), Indonesian (id), Italian (it), Japanese (ja), Korean (ko), Dutch (nl), Polish (pl), Portuguese Brazil (pt-BR), Russian (ru), Turkish (tr), Vietnamese (vi), Simplified Chinese (zh-CN), Traditional Chinese (zh-TW)

Translation Details:

Each translation accurately conveys: "Maximum time in seconds to wait for command execution to complete before timing out (0 = no timeout, 1-600s, default: 0s)"

Validation:

  • ✅ All translations validated with missing-translations script
  • ✅ All linting checks pass
  • ✅ All TypeScript type checks pass
  • ✅ Changes committed and pushed to PR branch
  • ✅ Security checks pending (normal for new commits)

The new setting is now fully localized and ready for international users!

@@ -113,6 +124,7 @@ export type ExecuteCommandOptions = {
customCwd?: string
terminalShellIntegrationDisabled?: boolean
terminalOutputLineLimit?: number
commandExecutionTimeout?: number
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this ever actually undefined? (Somewhat relatedly: is executeCommand just exported to make the tests easier, or am I missing a way it's being called?)

mrubens added 2 commits July 13, 2025 16:00
- Read commandExecutionTimeout from VSCode config in seconds
- Convert to milliseconds internally for timeout logic
- Display timeout in seconds in error messages
- Update integration tests to expect seconds format
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Jul 14, 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 Jul 14, 2025
Copy link
Collaborator

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

Not completely necessary but I think it would be a good idea to show some visual feedback if this setting is ever added to the settings menu.

LGTM

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jul 14, 2025
@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Needs Review] in Roo Code Roadmap Jul 14, 2025
clineProvider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })

// Add visual feedback for timeout
await cline.say("text", `Command execution timed out after ${commandExecutionTimeoutSeconds} seconds`)
Copy link

Choose a reason for hiding this comment

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

Consider replacing the hardcoded timeout feedback message with a translatable key. Instead of using an inline fallback string in cline.say (i.e., Command execution timed out after ${commandExecutionTimeoutSeconds} seconds), use a proper translation key (e.g. t('commandExecutionTimeout')) to ensure UI consistency and proper localization.

This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.

Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
@mrubens mrubens merged commit e019632 into main Jul 14, 2025
11 checks passed
@mrubens mrubens deleted the feature/configurable-command-execution-timeout branch July 14, 2025 18:12
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Jul 14, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jul 14, 2025
fxcl added a commit to tameslabs/Roo-Cline that referenced this pull request Jul 16, 2025
* main:
  fix: Resolve confusing auto-approve checkbox states (RooCodeInc#5602)
  fix: prevent empty mode names from being saved (RooCodeInc#5766) (RooCodeInc#5794)
  Format time in ISO 8601 (RooCodeInc#5793)
  fix: resolve DirectoryScanner memory leak and improve file limit handling (RooCodeInc#5785)
  Fix settings dirty check (RooCodeInc#5779)
  feat: increase Ollama API timeout values and extract as constants (RooCodeInc#5778)
  fix: Exclude Terraform and Terragrunt cache directories from checkpoints (RooCodeInc#4601) (RooCodeInc#5750)
  Move less commonly used provider settings into an advanced dropdown (RooCodeInc#5762)
  feat: Add configurable error & repetition limit with unified control (RooCodeInc#5654) (RooCodeInc#5752)
  list-files must include at least the first-level directory contents (RooCodeInc#5303)
  Update evals repo link (RooCodeInc#5758)
  Feature/vertex ai model name conversion (RooCodeInc#5728)
  fix(litellm): handle baseurl with paths correctly (RooCodeInc#5697)
  Add telemetry for todos (RooCodeInc#5746)
  feat: add undo functionality for enhance prompt feature (fixes RooCodeInc#5741) (RooCodeInc#5742)
  Fix max_tokens limit for moonshotai/kimi-k2-instruct on Groq (RooCodeInc#5740)
  Changeset version bump (RooCodeInc#5735)
  Add changeset for v3.23.12 patch release (RooCodeInc#5734)
  Update the max-token calculation in model-params to use the shared logic (RooCodeInc#5720)
  Changeset version bump (RooCodeInc#5719)
  chore: add changeset for v3.23.11 patch release (RooCodeInc#5718)
  Add Kimi K2 model and better support (RooCodeInc#5717)
  Fix: Remove invalid skip-checkout parameter from GitHub Actions workflows (RooCodeInc#5676)
  feat: add Cmd+Shift+. keyboard shortcut for previous mode switching (RooCodeInc#5695)
  Changeset version bump (RooCodeInc#5708)
  chore: add changeset for v3.23.10 patch release (RooCodeInc#5707)
  Add padding to the index model options (RooCodeInc#5706)
  fix: prioritize built-in model dimensions over custom dimensions (RooCodeInc#5705)
  Update CHANGELOG.md
  Changeset version bump (RooCodeInc#5702)
  chore: add changeset for v3.23.9 patch release (RooCodeInc#5701)
  Tweaks to command timeout error (RooCodeInc#5700)
  Update contributors list (RooCodeInc#5639)
  feat: enable Claude Code provider to run natively on Windows (RooCodeInc#5615)
  feat: Add configurable timeout for command execution (RooCodeInc#5668)
  feat: add gemini-embedding-001 model to code-index service (RooCodeInc#5698)
  fix: resolve vector dimension mismatch error when switching embedding models (RooCodeInc#5616) (RooCodeInc#5617)
  fix: [5424] return the cwd in the exec tool's response so that the model is not lost after subsequent calls (RooCodeInc#5667)
  Changeset version bump (RooCodeInc#5670)
  chore: add changeset for v3.23.8 patch release (RooCodeInc#5669)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request lgtm This PR has been approved by a maintainer PR - Needs Review size:L This PR changes 100-499 lines, ignoring generated files.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants