Repository navigation
fix: DH-22975: v2 add script exec timeout - #206
Merged
Merged
Conversation
|
Review changes with Changed Files
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Non-finite timeout inputs produce internal errors, and CLI documentation incorrectly promises an indefinite default wait.
2 open issues
What changed in this PR
Adds optional script-execution timeouts across the client, MCP tool, and CLI. Timeouts stop waiting without cancelling the running script.
Changes:
- Adds timeout propagation and a dedicated timeout response.
- Introduces
--script-timeoutand extends the CLI request deadline. - Updates documentation and timeout-focused tests.
| File | Description |
|---|---|
| tests/mcp_systems_server/_tools/test_script.py | Tests timeout forwarding and responses. |
| tests/client/test__session.py | Tests bounded execution and exception handling. |
| tests/cli/_commands/test_session.py | Tests the timeout option and deadline extension. |
| tests/cli/_commands/test__wrapping.py | Tests request-timeout overrides. |
| src/deephaven_mcp/mcp_systems_server/_tools/script.py | Exposes script timeouts and reports expiry. |
| src/deephaven_mcp/client/_session.py | Adds bounded waits using private threads. |
| src/deephaven_mcp/cli/_commands/session.py | Adds --script-timeout. |
| src/deephaven_mcp/cli/_commands/_wrapping.py | Propagates per-call request deadlines. |
| docs/CLI.md | Documents timeout behavior and usage. |
Review effort: Balanced
jnumainville
marked this pull request as ready for review
November 3, 2026 15:23
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add optional timeout to script exec
This will not interrupt the exec, but will stop the session from getting stuck on a long running script that can just execute in the background or be killed instead