-
Notifications
You must be signed in to change notification settings - Fork 0
Add dry-run execution plan support to runner CLI #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds dry-run execution plan support to the runner CLI, allowing users to validate scripts and preview execution configuration without actually running the script.
- Introduces a
--dry-runCLI flag for validating and previewing execution plans - Exposes execution plan details via new
get_execution_plan()method in ScriptRunner - Extends test coverage with unit and integration tests for dry-run functionality
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| runner.py | Adds get_execution_plan() method to ScriptRunner, stores config_file and history_db_path fields, implements --dry-run CLI flag handling |
| tests/test_runner_core.py | Adds unit test for get_execution_plan() method, removes trailing whitespace |
| tests/test_integration.py | Adds integration test for CLI dry-run functionality via subprocess execution |
| if args.dry_run: | ||
| try: | ||
| runner.validate_script() | ||
| except Exception as exc: | ||
| logging.error(f"Dry-run validation failed: {exc}") | ||
| return 1 | ||
|
|
||
| plan = runner.get_execution_plan() | ||
| print("\nDRY-RUN: Execution plan (no script executed)") | ||
| for key, value in plan.items(): | ||
| print(f" {key}: {value}") | ||
| return 0 |
Copilot
AI
Nov 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent exit pattern: Use sys.exit() instead of return to match the pattern used throughout the rest of the main() function. All other early exits in the function use sys.exit() (lines 8440, 8447, 8460, 8476, 8494, etc.).
| if args.dry_run: | ||
| try: | ||
| runner.validate_script() | ||
| except Exception as exc: | ||
| logging.error(f"Dry-run validation failed: {exc}") | ||
| return 1 | ||
|
|
||
| plan = runner.get_execution_plan() | ||
| print("\nDRY-RUN: Execution plan (no script executed)") | ||
| for key, value in plan.items(): | ||
| print(f" {key}: {value}") | ||
| return 0 |
Copilot
AI
Nov 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dry-run execution plan may show incomplete configuration. The dry-run check occurs before CLI arguments like --monitor-interval (line 8524) and retry configuration (lines 8527-8544) are applied to the runner. This means the execution plan will display default values instead of user-specified values for these settings. Consider either:
- Moving the dry-run check after all configuration is applied, or
- Passing these additional arguments to the ScriptRunner constructor
Summary
--dry-runCLI option that validates scripts and prints an execution plan instead of runningTesting
Codex Task