fix(env): run local commands through bash - #868
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
This PR fixes the LocalEnvironment “bash” tool to actually execute commands with Bash (instead of /bin/sh via shell=True, which is often dash on Debian/Ubuntu), aligning behavior more closely with the Docker environment and enabling bash-only syntax like process substitution.
Changes:
- Add a configurable
interpretertoLocalEnvironmentConfig, defaulting to["bash", "-lc"]. - Switch local command execution from
subprocess.Popen(..., shell=True)to invoking the configured interpreter directly. - Add test coverage ensuring bash-only syntax works by default (skipped on Windows).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/minisweagent/environments/local.py |
Introduces a configurable interpreter (default bash -lc) and runs commands through it instead of /bin/sh via shell=True. |
tests/environments/test_local.py |
Extends config default assertions and adds a bash-only syntax test to prevent regressions. |
Codecov Report✅ All modified and coverable lines are covered by tests.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
|
still thinking about this, your PR is also very clean (thx!), I'm just still wondering if just prefixing |
|
The one thing I don't like with this, is that this will probably not work out of the box on some systems... |
Summary
Fixes #829.
LocalEnvironment advertised and prompted bash commands, but executed them via
subprocess.Popen(..., shell=True). On Debian/Ubuntu, Python uses/bin/shforshell=True, and/bin/shis commonlydash, so bash-only syntax such as process substitution fails even though the tool is namedbash.This changes local command execution to use a configurable interpreter, defaulting to
bash -lc, matching the Docker backend's default interpreter behavior more closely while preserving the existing timeout/process-group handling.Validation
.venv/bin/python -m pytest tests/environments/test_local.py.venv/bin/ruff check src/minisweagent/environments/local.py tests/environments/test_local.pygit diff --check