fix: add timeout to prompt step subprocess execution - #3768
fix: add timeout to prompt step subprocess execution#3768Quratulain-bilal wants to merge 1 commit into
Conversation
mnriem
left a comment
mnriem
left a comment
There was a problem hiding this comment.
Please make sure the default timeout can be overriden
There was a problem hiding this comment.
Pull request overview
Adds a 300-second timeout to prompt subprocess execution and handles timeout failures gracefully.
Changes:
- Passes a timeout to
subprocess.run(). - Converts
TimeoutExpiredinto a failed prompt result.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/workflows/steps/prompt/__init__.py |
Adds prompt execution timeout handling; currently references undefined config. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Medium
| project_root = ( | ||
| Path(context.project_root) if context.project_root else Path.cwd() | ||
| ) | ||
| timeout = config.get("timeout", 300) |
There was a problem hiding this comment.
Fixed: config was not in scope inside the @staticmethod _try_dispatch. Now timeout is extracted from config in execute() (which has access to the step config dict) and passed as a parameter to _try_dispatch(timeout=timeout). The default remains 300 seconds. Also added subprocess.TimeoutExpired handling that returns a clean error message.
|
Please address Copilot feedback and fix test & lint errors |
The prompt step subprocess.run() had no timeout, allowing a hung LLM invocation to block the entire workflow engine indefinitely. The shell step already defaults to 300s timeout. Add timeout parameter (defaulting to 300s, matching shell step) and handle subprocess.TimeoutExpired gracefully.
88d0272 to
37277a5
Compare
Summary
The prompt step subprocess.run() had no timeout, allowing a hung LLM invocation to block the entire workflow engine indefinitely. The shell step already defaults to a 300s timeout.
Changes
Testing
All 176 workflow tests pass.