-
Notifications
You must be signed in to change notification settings - Fork 0
Inline v7 feature helpers into core ScriptRunner #2
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 integrates v7 feature helpers directly into the core ScriptRunner class, eliminating the need for a separate v7_enhancement.py wrapper module. The changes make advanced workflow, security, and cost tracking features more accessible without requiring an external enhancer class.
Key Changes
- Inlined v7 helper methods (
pre_execution_security_scan,scan_dependencies,scan_secrets,start_tracing_span,start_cost_tracking,stop_cost_tracking) directly intoScriptRunner - Added
get_execution_plan()method and--dry-runCLI flag for execution planning without running scripts - Removed the separate
runners/v7_enhancement.pymodule - Updated tests to verify inlined functionality and dry-run behavior
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
runner.py |
Added v7 helper methods inline to ScriptRunner class, added config_file and history_db_path instance variables, implemented get_execution_plan() method, and added CLI --dry-run flag support |
runners/v7_enhancement.py |
Removed entire module (291 lines) as functionality is now inlined in ScriptRunner |
tests/unit/test_runner.py |
Replaced v7_enhancement import tests with direct tests of inlined methods in ScriptRunner |
tests/test_runner_core.py |
Added test for get_execution_plan() method functionality |
tests/test_integration.py |
Added integration test for CLI --dry-run flag with subprocess execution |
| return {'success': False, 'error': str(e)} | ||
|
|
||
| def scan_dependencies(self, requirements_file: str = 'requirements.txt') -> Dict[str, Any]: | ||
| """Scan dependencies for vulnerabilities using the configured scanner.""" |
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.
The docstring for scan_dependencies is incomplete. It should document the parameters, return value structure, and provide an example similar to other methods in this class.
Consider expanding the docstring to include:
- Args section describing the
requirements_fileparameter - Returns section describing the dictionary structure with keys like 'success', 'vulnerability_count', 'vulnerabilities', and 'sbom'
- An example showing typical usage
| """Scan dependencies for vulnerabilities using the configured scanner.""" | |
| """ | |
| Scan dependencies for vulnerabilities using the configured scanner. | |
| Args: | |
| requirements_file (str): Path to the requirements file to scan. Defaults to 'requirements.txt'. | |
| Returns: | |
| Dict[str, Any]: Dictionary containing the scan results: | |
| - success (bool): Whether the scan completed successfully. | |
| - vulnerability_count (int): Number of vulnerabilities found. | |
| - vulnerabilities (list): List of vulnerability details (dicts). | |
| - sbom (Any): Software Bill of Materials, if available. | |
| - error (str, optional): Error message if the scan failed. | |
| Example: | |
| >>> runner = ScriptRunner('myscript.py') | |
| >>> result = runner.scan_dependencies('requirements.txt') | |
| >>> if result['success']: | |
| ... print(f"Found {result['vulnerability_count']} vulnerabilities") | |
| ... for vuln in result['vulnerabilities']: | |
| ... print(vuln) | |
| ... else: | |
| ... print("Scan failed:", result.get('error')) | |
| """ |
| return {'success': False, 'error': str(e)} | ||
|
|
||
| def scan_secrets(self, path: str = '.') -> Dict[str, Any]: | ||
| """Scan a path for hardcoded secrets.""" |
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.
The docstring for scan_secrets is incomplete. It should document the parameters, return value structure, and provide an example similar to other methods in this class.
Consider expanding the docstring to include:
- Args section describing the
pathparameter (what types of paths are accepted - file or directory) - Returns section describing the dictionary structure with keys like 'success', 'has_secrets', 'secret_count', and 'secrets'
- An example showing typical usage
| """Scan a path for hardcoded secrets.""" | |
| """ | |
| Scan a path (file or directory) for hardcoded secrets using the configured secret scanner. | |
| Args: | |
| path (str): Path to a file or directory to scan for secrets. Defaults to current directory ('.'). | |
| Returns: | |
| dict: Dictionary with the following keys: | |
| - success (bool): True if scan completed successfully, False otherwise. | |
| - has_secrets (bool): True if any secrets were found, False otherwise. | |
| - secret_count (int): Number of secrets found. | |
| - secrets (list): List of secrets found (as dicts). | |
| - error (str, optional): Error message if scan failed. | |
| Example: | |
| >>> runner = ScriptRunner() | |
| >>> result = runner.scan_secrets('my_script.py') | |
| >>> print(result) | |
| { | |
| 'success': True, | |
| 'has_secrets': False, | |
| 'secret_count': 0, | |
| 'secrets': [] | |
| } | |
| """ |
| return {'success': False, 'error': str(e)} | ||
|
|
||
| def start_tracing_span(self, span_name: str): | ||
| """Start a distributed tracing span using the configured tracer.""" |
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.
The docstring for start_tracing_span is incomplete. It should document the parameters, return value, and behavior when tracing is disabled.
Consider expanding the docstring to include:
- Args section describing the
span_nameparameter - Returns section describing what the context manager yields (the span when tracing is enabled, None when disabled)
- An example showing how to use this as a context manager
| """Start a distributed tracing span using the configured tracer.""" | |
| """ | |
| Start a distributed tracing span using the configured tracer. | |
| Args: | |
| span_name (str): The name of the tracing span to start. | |
| Returns: | |
| Context manager that yields the tracing span object if tracing is enabled, | |
| or None if tracing is disabled. | |
| Behavior: | |
| If tracing is enabled and a tracing manager is configured, this returns a context | |
| manager that yields the active tracing span. If tracing is disabled, it returns | |
| a no-op context manager that yields None. | |
| Example: | |
| >>> with runner.start_tracing_span("my_span") as span: | |
| ... # code to trace | |
| ... if span is not None: | |
| ... span.set_tag("key", "value") | |
| """ |
Summary
runners/v7_enhancement.pymodule and refresh unit tests to cover the inlined helpersTesting
Codex Task