diff --git a/src/agent_cli/main.py b/src/agent_cli/main.py index 76f845d..7730556 100644 --- a/src/agent_cli/main.py +++ b/src/agent_cli/main.py @@ -33,7 +33,7 @@ from .hub import Hub, HubError from .knock import drain as knock_drain from .knock import listen_once as knock_listen -from .lane import LANE_ROLES, LANE_VENDORS, launch +from .lane import LANE_ROLES, LANE_VENDORS, LaneResult, launch from .pg import PgError, cluster_exists, cluster_running, ensure_cluster, require_loopback_dsn, stop_cluster from .runtime import ( Runtime, @@ -2651,10 +2651,7 @@ def cmd_run(args: list[str]) -> None: cwd=cwd, tmux=tmux, ) - print( - f"lane role={result.role} vendor={result.vendor} " - f"STATUS={result.status} rc={result.returncode}" - ) + _print_lane_result(result) if role == "implementer" and result.status == "complete": working = _find_working_agent( store, tid, role=role, vendor=vendor, round_num=round_num @@ -2833,6 +2830,35 @@ def cmd_mail(args: list[str]) -> None: store.close() +_LANE_CONTROL_CHARS_RE = re.compile(r"[\x00-\x08\x0b\x0c\x0e-\x1f\x7f-\x9f]") + + +def _sanitize_lane_output(text: str) -> str: + # The --no-tmux runner captures raw vendor subprocess output (unlike the tmux + # path, whose capture-pane already drops escape sequences), so an ESC-led CSI/OSC + # sequence (7-bit, \x1b-prefixed) or an 8-bit C1 control byte (\x80-\x9f, e.g. a + # bare CSI at \x9b) from a misbehaving vendor process could otherwise reach the + # terminal verbatim. Stripping C0/C1/DEL control bytes (keeping \t/\n/\r) + # neutralizes that without touching the printable content a human or + # --evidence actually needs. + return _LANE_CONTROL_CHARS_RE.sub("", text) + + +def _print_lane_result(result: LaneResult) -> None: + # The vendor's actual review/implementation text lives only in result.stdout — + # by the time launch() returns, the tmux pane it was captured from is already + # killed, so this is the only remaining chance to surface it. Without this, a + # caller sees STATUS/rc but never the content a gate record --evidence needs. + if result.stdout.strip(): + print(_sanitize_lane_output(result.stdout.rstrip())) + if result.stderr.strip(): + print(_sanitize_lane_output(result.stderr.rstrip()), file=sys.stderr) + print( + f"lane role={result.role} vendor={result.vendor} " + f"STATUS={result.status} rc={result.returncode}" + ) + + def cmd_lane(args: list[str]) -> None: if not args or args[0] != "run": die( @@ -2861,10 +2887,7 @@ def cmd_lane(args: list[str]) -> None: if dry_run: print(" ".join(result.argv)) return - print( - f"lane role={result.role} vendor={result.vendor} " - f"STATUS={result.status} rc={result.returncode}" - ) + _print_lane_result(result) if result.status != "complete": raise SystemExit(2) diff --git a/tests/test_lane.py b/tests/test_lane.py index 1598cde..1a5c62e 100644 --- a/tests/test_lane.py +++ b/tests/test_lane.py @@ -9,6 +9,7 @@ from agent_cli.lane import ( GROK_STRIP_ENV, + LaneResult, _run_in_tmux, codex_argv, grok_argv, @@ -16,7 +17,7 @@ parse_status, tmux_wrap_argv, ) -from agent_cli.main import main +from agent_cli.main import _sanitize_lane_output, main pytestmark = pytest.mark.no_pg @@ -502,6 +503,108 @@ def handler(argv: list[str], _calls: list[list[str]]) -> CompletedProcess[str]: assert any("kill-session" in c for c in calls) +def test_cli_lane_run_prints_vendor_stdout( + tmp_path: Path, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch +) -> None: + spec = tmp_path / "spec.md" + spec.write_text("review this\n", encoding="utf-8") + + def fake_launch(**kwargs): # type: ignore[no-untyped-def] + return LaneResult( + role="pr-reviewer-quality", + vendor="grok", + status="complete", + argv=["grok"], + returncode=0, + stdout="no quality findings, distinctive-marker-abc123\nSTATUS: complete\n", + stderr="", + ) + + monkeypatch.setattr("agent_cli.main.launch", fake_launch) + run( + [ + "lane", + "run", + "--role", + "pr-reviewer-quality", + "--vendor", + "grok", + "--spec-file", + str(spec), + "--cwd", + str(tmp_path), + "--no-tmux", + ] + ) + out = capsys.readouterr().out + assert "distinctive-marker-abc123" in out + assert "STATUS=complete" in out + assert out.index("distinctive-marker-abc123") < out.index("STATUS=complete") + + +def test_cli_lane_run_prints_vendor_stderr( + tmp_path: Path, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch +) -> None: + spec = tmp_path / "spec.md" + spec.write_text("review this\n", encoding="utf-8") + + def fake_launch(**kwargs): # type: ignore[no-untyped-def] + return LaneResult( + role="pr-reviewer-quality", + vendor="grok", + status="unavailable", + argv=["grok"], + returncode=1, + stdout="", + stderr="grok: rate limited, distinctive-marker-xyz789", + ) + + monkeypatch.setattr("agent_cli.main.launch", fake_launch) + with pytest.raises(SystemExit): + run( + [ + "lane", + "run", + "--role", + "pr-reviewer-quality", + "--vendor", + "grok", + "--spec-file", + str(spec), + "--cwd", + str(tmp_path), + "--no-tmux", + ] + ) + err = capsys.readouterr().err + assert "distinctive-marker-xyz789" in err + + +def test_sanitize_lane_output_strips_escape_sequences_keeps_text() -> None: + raw = "before\x1b[31mred\x1b[0m after\x07\ttab\nline2" + cleaned = _sanitize_lane_output(raw) + assert "\x1b" not in cleaned + assert "\x07" not in cleaned + assert "red" in cleaned and "after" in cleaned + assert "\ttab\nline2" in cleaned + + +def test_sanitize_lane_output_strips_c1_control_bytes() -> None: + raw = "before\x9b2Jafter" + cleaned = _sanitize_lane_output(raw) + assert "\x9b" not in cleaned + assert "before" in cleaned and "after" in cleaned + + +def test_sanitize_lane_output_strips_exact_range_boundaries() -> None: + stripped = "\x00\x08\x0b\x0c\x0e\x1f\x7f\x80\x9f" + cleaned = _sanitize_lane_output(stripped) + assert cleaned == "" + + kept = "\t\n\r" + assert _sanitize_lane_output(kept) == kept + + def test_cli_dry_run_implementer_grok(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: spec = tmp_path / "spec.md" spec.write_text("implement me\n", encoding="utf-8") diff --git a/tests/test_run.py b/tests/test_run.py index 9995c0d..efdbf54 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -287,6 +287,44 @@ def fake_exec(argv: list[str], *, cwd: str | None = None) -> Completed: assert called["n"] == 0 +def test_run_prints_vendor_stdout( + tmp_path: Path, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch +) -> None: + tid = _bootstrap_implement(tmp_path, capsys) + spec = tmp_path / "spec.md" + spec.write_text("implement this\n", encoding="utf-8") + + def fake_launch(**kwargs): # type: ignore[no-untyped-def] + return LaneResult( + role="implementer", + vendor="grok", + status="complete", + argv=["grok"], + returncode=0, + stdout="implemented the thing, distinctive-marker-run456\nSTATUS: complete\n", + stderr="", + ) + + monkeypatch.setattr("agent_cli.main.launch", fake_launch) + run( + tmp_path, + [ + "run", + "--task", + tid, + "--spec-file", + str(spec), + "--no-tmux", + "--cwd", + str(tmp_path), + ], + ) + out = capsys.readouterr().out + marker_at = out.index("distinctive-marker-run456") + summary_at = out.index("STATUS=complete") + assert marker_at < summary_at + + def test_run_spec_file_implementer_complete( tmp_path: Path, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch ) -> None: