Skip to content

Commit a4ec6ec

Browse files
authored
Fix validate_tool hang: pass stdin=DEVNULL to the validation subprocess (#218)
* Fix validate_tool hang: pass stdin=DEVNULL to the validation subprocess `validate_tool` runs each tool's smoke test (e.g. `pi --print "say hi in 5 words or less"`) via `subprocess.run(..., timeout=60)` without setting `stdin`, so the child inherits ucode's own stdin. In print/headless mode some agent CLIs (pi among them) still open stdin and block waiting for input/EOF. When ucode is launched from a non-interactive parent whose stdin is an open pipe with no EOF — e.g. an agent harness spawning `ucode <tool>` as a subprocess — the validation command never sees EOF, blocks for the full 60s, and `validate_tool` returns `(False, "timed out")`. The caller (`_auto_configure_tool` / configure) then reverts the config and aborts with "<tool> validation failed — config reverted", even though the tool is correctly configured and the gateway is reachable. The validation smoke test is one-shot and never needs interactive input, so pass `stdin=subprocess.DEVNULL`. This is benign for every tool (an interactive prompt in a validation run would itself be a bug) and makes validation robust regardless of how ucode was launched. Verified on a managed sandbox: `pi --print "say hi"` returns instantly with `stdin=DEVNULL` but hangs to the timeout when it inherits an open stdin pipe. Signed-off-by: Edwin He <edwin.he@databricks.com> * Remove explanatory comment on the validate_tool stdin=DEVNULL kwarg Per review: the kwarg is self-explanatory; drop the comment block. --------- Signed-off-by: Edwin He <edwin.he@databricks.com>
1 parent eea5ffb commit a4ec6ec

2 files changed

Lines changed: 41 additions & 1 deletion

File tree

src/ucode/agents/__init__.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,13 @@ def validate_tool(tool: str) -> tuple[bool, str]:
466466
env = None
467467
try:
468468
result = subprocess.run(
469-
cmd, check=False, capture_output=True, text=True, timeout=60, env=env
469+
cmd,
470+
check=False,
471+
capture_output=True,
472+
text=True,
473+
timeout=60,
474+
env=env,
475+
stdin=subprocess.DEVNULL,
470476
)
471477
if result.returncode == 0:
472478
return True, ""

tests/test_agents_init.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -567,3 +567,37 @@ def test_low_verbosity_omits_panels(self, monkeypatch, capsys):
567567
assert "Ready" not in out
568568
# Per-tool success line is still printed.
569569
assert "Codex is working" in out
570+
571+
572+
class TestValidateTool:
573+
def test_runs_validate_command_with_stdin_devnull(self, monkeypatch):
574+
# Regression guard: the validation smoke test must never inherit the
575+
# caller's stdin, or it hangs to the timeout when ucode is launched
576+
# from a non-interactive parent whose stdin is an open pipe.
577+
captured: dict = {}
578+
579+
def fake_run(cmd, **kwargs):
580+
captured["cmd"] = cmd
581+
captured["kwargs"] = kwargs
582+
return subprocess.CompletedProcess(cmd, 0, stdout="", stderr="")
583+
584+
monkeypatch.setattr("ucode.agents.subprocess.run", fake_run)
585+
monkeypatch.setattr(agents_mod, "load_state", lambda: {})
586+
587+
ok, err = agents_mod.validate_tool("codex")
588+
589+
assert ok is True
590+
assert err == ""
591+
assert captured["kwargs"].get("stdin") is subprocess.DEVNULL
592+
593+
def test_reports_timed_out_on_timeout(self, monkeypatch):
594+
def fake_run(cmd, **kwargs):
595+
raise subprocess.TimeoutExpired(cmd, 60)
596+
597+
monkeypatch.setattr("ucode.agents.subprocess.run", fake_run)
598+
monkeypatch.setattr(agents_mod, "load_state", lambda: {})
599+
600+
ok, err = agents_mod.validate_tool("codex")
601+
602+
assert ok is False
603+
assert err == "timed out"

0 commit comments

Comments
 (0)