Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions gcode/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,16 @@ def execute_bash(command: str) -> str:
"""Execute a bash command on the local machine and return its output.

Requires interactive confirmation (y/n) before running unless auto-approve
is enabled. Returns combined stdout and stderr, and reports a non-zero exit
code if the command fails.
is enabled. In non-interactive environments (CI, Docker, pipes) the command
is rejected rather than hanging or crashing; enable auto-approve to run.
Returns combined stdout and stderr, and reports a non-zero exit code if the
command fails.
"""
if not AUTO_APPROVE:
confirm = input(f"GCode wants to run: {command}\nApprove? (y/n): ")
try:
confirm = input(f"GCode wants to run: {command}\nApprove? (y/n): ")

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: on EOFError there was no user, yet the model is told "cancelled by user" — in CI the agent will misread this as the human declining. Consider a distinct message for the non-interactive case, e.g. "Command rejected: no terminal available (non-interactive). Run with --yes to auto-approve.", and keep the "cancelled by user" text for real denials.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks — agreed that in CI there is no human declining, so the current wording could mislead an agent into reading a rejection. This PR merged as submitted (commit 568e62f). If you'd like, I can open a small follow-up that uses a distinct non-interactive rejection message (e.g. no terminal available — run with --yes to auto-approve) so the two cases stay unambiguous.

except (EOFError, KeyboardInterrupt):
return "Command execution cancelled by user."
if confirm.strip().lower() != "y":
return "Command execution cancelled by user."
try:
Expand Down
36 changes: 36 additions & 0 deletions tests/test_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,3 +116,39 @@ def test_grep_python_fallback_skips_binary_files(tmp_path):
out = _grep_python("needle", str(tmp_path), "*")
assert "text.txt" in out
assert "bin.dat" not in out


def test_execute_bash_cancels_on_eof():
from gcode.tools import execute_bash

with patch("builtins.input", side_effect=EOFError):
out = execute_bash.invoke({"command": "echo hi"})
assert out == "Command execution cancelled by user."


def test_execute_bash_cancels_on_keyboard_interrupt():
from gcode.tools import execute_bash

with patch("builtins.input", side_effect=KeyboardInterrupt):
out = execute_bash.invoke({"command": "echo hi"})
assert out == "Command execution cancelled by user."


def test_execute_bash_rejects_non_yes_answer():
from gcode.tools import execute_bash

with patch("builtins.input", return_value="n"):
out = execute_bash.invoke({"command": "echo hi"})
assert out == "Command execution cancelled by user."


def test_execute_bash_auto_approve_skips_prompt(tmp_path):
from gcode.tools import AUTO_APPROVE, execute_bash, set_auto_approve

set_auto_approve(True)
try:
with patch("builtins.input", side_effect=AssertionError("must not prompt")):
out = execute_bash.invoke({"command": "echo auto-approved"})
assert "auto-approved" in out
finally:
set_auto_approve(AUTO_APPROVE)