feat(runner): Add injectable executor backend - #3
Conversation
![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.
Reviewed commit: 92c6db40f0
ℹ️ 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".
|
|
||
| executor_cls = ExecutorPolicies[executor_policy] | ||
| executor = executor_cls(cwd) | ||
| executor = (self.executor_factory or _default_executor_factory)(cwd, executor_policy) |
There was a problem hiding this comment.
If an injected factory raises while initializing its backend (for example, when a container or remote executor cannot start), this call occurs before the try/finally, so provisioner.cleanup() is never reached even when keep_worktree=False. Each failed run therefore leaves its copied temporary repository behind; move executor creation inside the protected region so factory failures follow the normal cleanup path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR adds an opt-in executor_factory to SafeMiniRunner so callers can inject a custom execution backend, and ensures that when a backend is injected, both model actions and final verification run through that same backend while preserving the legacy final verification behavior for existing callers.
Changes:
- Add
executor_factorysupport toSafeMiniRunnerand default it to the existing policy-based executors. - Route final verification through the injected executor when provided, otherwise keep using
final_tests_pass. - Add integration tests covering injected executor behavior and legacy final-check behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| safe_mini/runner.py | Introduces injectable executor factory and routes final verification through injected executors while keeping legacy verification for default callers. |
| tests/test_runner_integration.py | Adds tests ensuring injected executors see both actions and verification, and that legacy final verification remains in place by default. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if self._final_check(cwd, executor): | ||
| success = True | ||
| break | ||
|
|
||
| tests_pass = final_tests_pass(cwd, command=self.final_check_command) | ||
| tests_pass = self._final_check(cwd, executor) |
92c6db4 to
b4b704d
Compare
Allow integrations to supply one executor backend for model actions and final verification. Preserve the legacy final checker when no backend is injected. Co-Authored-By: Codex <noreply@openai.com>
b4b704d to
18f3a68
Compare
Summary
Verification
Configured ruff and mypy are not installed in the checked worktree Python environment.