Fix ScriptRunner exception flow for WEBAPI #7
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
|
|
||
| def _validate_script_path(path_str: str) -> Path: | ||
| candidate = Path(path_str).expanduser().resolve() |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the exception flow in ScriptRunner._execute_script to ensure proper cleanup and post-execution operations, and adds WEBAPI integration for cancellation support.
- Reorders exception handlers to place
finallyblock after allexceptblocks, ensuring alerts, history saves, and hooks execute for all outcomes - Adds cancellation support for active script runs via
_active_processtracking and acancel_active_run()method - Introduces a minimal FastAPI-based WEBAPI with a dashboard for triggering and monitoring script executions
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| runner.py | Fixes try/except/finally structure, adds process tracking for cancellation, improves exception handling |
| WEBAPI/api.py | New FastAPI service with endpoints for running/canceling scripts and viewing run history |
| WEBAPI/static/index.html | Dashboard UI for interacting with the WEBAPI |
| WEBAPI/serve.sh | Bash script to launch the WEBAPI service |
| WEBAPI/requirements.txt | Dependencies for the WEBAPI module |
| WEBAPI/README.md | Documentation for WEBAPI setup and usage |
| </tr> | ||
| </thead> | ||
| <tbody id="runs-table"> | ||
| <tr><td colspan="6">No runs yet.</td></tr> |
There was a problem hiding this comment.
The colspan value should be 8 to match the number of table headers (ID, Status, Script, Args, Started, Finished, Exit Code, Actions), not 6. This will cause the cell to not span the full width of the table.
| <tr><td colspan="6">No runs yet.</td></tr> | |
| <tr><td colspan="8">No runs yet.</td></tr> |
| <label style="font-weight:600;">Page Size | ||
| <input type="number" id="page-size" value="50" min="1" max="200" /> | ||
| </label> | ||
| <button id="refresh">Refresh Runs</button> |
There was a problem hiding this comment.
Duplicate button with id 'refresh'. Line 102 already defines a button with the same id. This violates HTML spec and will cause the event listener on line 313 to only attach to the first occurrence. Use a unique id or a class selector instead.
| <button id="refresh">Refresh Runs</button> | |
| <button id="refresh-table">Refresh Runs</button> |
| ... print(f"Success! Took {result['metrics']['execution_time_seconds']}s") | ||
| ... else: | ||
| ... print(f"Failed with exit code {result['returncode']}") | ||
| ... print(f"Failed with exit code {result['returncode']}") |
There was a problem hiding this comment.
[nitpick] The indentation change from the original code appears to add unnecessary whitespace. The docstring example should maintain consistent indentation with the rest of the docstring.
| ... print(f"Failed with exit code {result['returncode']}") | |
| ... print(f"Failed with exit code {result['returncode']}") |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fastapi | ||
| uvicorn | ||
| pydantic |
There was a problem hiding this comment.
Launching the new WEBAPI per the documented quick start (pip install -r WEBAPI/requirements.txt followed by python WEBAPI/api.py) fails in a clean environment because this requirements file installs only FastAPI, Uvicorn, and Pydantic. The API immediately imports runner.ScriptRunner, which depends on modules like psutil, requests, and yaml; with only these three packages installed a ModuleNotFoundError is raised before the server can start. Include the ScriptRunner dependencies (e.g., the root requirements) or update the quick-start steps so the service can actually run.
Useful? React with 👍 / 👎.
Summary
_execute_scripterror-handling to placefinallyafter exceptions and avoid syntax errorsTesting
Codex Task