From 6eb4db21da4df9376930d277e00f182487033d2b Mon Sep 17 00:00:00 2001 From: Daniel Padrino Date: Wed, 26 Aug 2026 22:13:52 -0300 Subject: [PATCH 1/2] Surface the vendor's actual output from a lane run. `agent lane run` genuinely invokes grok/codex against a real diff, but the CLI only printed a one-line STATUS/rc summary and discarded the vendor's actual stdout/stderr. By the time launch() returns, the tmux pane it was captured from is already killed, so nothing downstream (gate record --evidence, a human watching the run) could ever see what the vendor found. Both call sites (agent lane run and the agent run one-step chain) now print result.stdout/stderr before the summary line via a shared _print_lane_result helper. The --no-tmux runner captures raw vendor subprocess output, unlike the tmux path whose capture-pane already drops escape sequences, so a misbehaving vendor process could otherwise inject ANSI/C1 control sequences into the terminal. _sanitize_lane_output strips C0/C1/DEL control bytes (keeping tab/newline/CR) before printing either stream. --- src/agent_cli/main.py | 41 +++++++++++++++---- tests/test_lane.py | 95 ++++++++++++++++++++++++++++++++++++++++++- tests/test_run.py | 38 +++++++++++++++++ 3 files changed, 164 insertions(+), 10 deletions(-) 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..c790046 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,98 @@ 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 + + +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_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: From 31efc8379b3a34b2fdf993fc821f834e04f61e95 Mon Sep 17 00:00:00 2001 From: Daniel Padrino Date: Wed, 26 Aug 2026 22:32:03 -0300 Subject: [PATCH 2/2] Tighten sanitizer and cmd-lane print-order test coverage. The sanitizer tests only exercised a few representative escape bytes, so an off-by-one edit to the control-char regex's range boundaries could slip through unnoticed. Add a boundary test asserting every edge of each stripped range is actually stripped and \t/\n/\r survive. Also assert stdout-before-summary ordering in the cmd_lane print test, matching the equivalent cmd_run test, so a regression that reordered cmd_lane's output specifically would be caught. --- tests/test_lane.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/test_lane.py b/tests/test_lane.py index c790046..1a5c62e 100644 --- a/tests/test_lane.py +++ b/tests/test_lane.py @@ -539,6 +539,7 @@ def fake_launch(**kwargs): # type: ignore[no-untyped-def] 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( @@ -595,6 +596,15 @@ def test_sanitize_lane_output_strips_c1_control_bytes() -> None: 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")