fix: use agent repo root for tests and commits#95
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hi @Bryan-Roe! 👋
Your private repo does not have access to Sourcery.
Please upgrade to continue using Sourcery ✨
There was a problem hiding this comment.
Pull request overview
Updates the autonomous code agent to run tests and create commits relative to the agent’s configured repository root (self.repo.repo_root) rather than the module-level REPO_ROOT, and adds regression tests to ensure the behavior doesn’t silently revert.
Changes:
- Update
CodeAgent.run_tests()to locatescripts/test_runner.pyand run subprocesses withcwd=self.repo.repo_root. - Update
CodeAgent.commit_changes()to stage and commit withcwd=self.repo.repo_root. - Add tests that verify
run_tests()/commit_changes()use the agent repo root even when the globalREPO_ROOTis intentionally wrong.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
scripts/autonomous_code_agent.py |
Uses self.repo.repo_root for test execution and git commit/stage working directory. |
tests/test_autonomous_code_agent.py |
Adds regression tests to confirm repo-root selection for tests and commits. |
| wrong_root = tmp_path / "wrong-root" | ||
| wrong_root.mkdir() | ||
| monkeypatch.setattr(aca, "REPO_ROOT", wrong_root) | ||
| monkeypatch.setattr(aca, "DATA_OUT", tmp_path / "data_out") | ||
| monkeypatch.setattr(aca, "STATUS_FILE", tmp_path / "data_out" / "status.json") | ||
|
|
||
| agent = aca.CodeAgent(llm_type="echo") | ||
| agent.repo.repo_root = tmp_path |
There was a problem hiding this comment.
These new tests patch aca.DATA_OUT/aca.STATUS_FILE but don’t clear the module logger handlers like _configure_agent_repo() does. Since CodeAgent.__init__() always adds a FileHandler, this test (and later tests) will accumulate handlers and can leave open file descriptors across tests (potentially causing tmp dir cleanup issues on some platforms). Consider reusing _configure_agent_repo() here (and/or explicitly closing/removing existing handlers) before constructing CodeAgent.
| wrong_root = tmp_path / "wrong-root" | ||
| wrong_root.mkdir() | ||
| monkeypatch.setattr(aca, "REPO_ROOT", wrong_root) | ||
| monkeypatch.setattr(aca, "DATA_OUT", tmp_path / "data_out") | ||
| monkeypatch.setattr(aca, "STATUS_FILE", tmp_path / "data_out" / "status.json") | ||
|
|
||
| agent = aca.CodeAgent(llm_type="echo") | ||
| agent.repo.repo_root = tmp_path | ||
|
|
There was a problem hiding this comment.
Same issue here: this test patches aca.DATA_OUT/aca.STATUS_FILE but doesn’t clear/close existing _LOGGER handlers before instantiating CodeAgent, so handlers can pile up across tests. Reusing _configure_agent_repo() (or adding a small helper to reset logger handlers safely) would keep tests isolated.
![bryan-roe-bot[bot]](https://avatars.githubusercontent.com/in/1073263?s=60&v=4){kind=small}
| subprocess.run( | ||
| ["git", "add", "--"] + files_to_stage, | ||
| cwd=REPO_ROOT, | ||
| cwd=self.repo.repo_root, | ||
| check=True, | ||
| capture_output=True, | ||
| ) |
There was a problem hiding this comment.
Semgrep identified an issue in your code:
Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.
To resolve this comment:
🔧 No guidance has been designated for this issue. Fix according to your organization's approved methods.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by dangerous-subprocess-use-audit.
You can view more details about this finding in the Semgrep AppSec Platform.
|
Semgrep found 1 Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'. |
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com