fix(runtime): replace tmux pane capture with explicit bounded output (ODY-150)

This commit is contained in:
Alexandre Teixeira
2026-10-01 21:43:17 +01:00
parent 85cfe59b15
commit 4efb85ee33
6 changed files with 45 additions and 456 deletions
+8 -296
View File
@@ -9,16 +9,14 @@ import shutil
import subprocess
import sys
import time
import collections
import json
from typing import Optional, Callable, Awaitable, Tuple, Dict
from typing import Optional
from urllib.parse import urlparse
import httpx
from src import containment
from src.constants import AGENT_ISOLATED_TMP_DIRNAME, MAX_OUTPUT_CHARS, WORKSPACE_MOUNT
from src.agent_runtime.journal import mark_operation_started
logger = logging.getLogger(__name__)
@@ -29,9 +27,6 @@ logger = logging.getLogger(__name__)
DEFAULT_BASH_TIMEOUT = 120
DEFAULT_PYTHON_TIMEOUT = 60 * 60
PROGRESS_INTERVAL_S = 2.0
PROGRESS_TAIL_LINES = 12
TMUX_CAPTURE_LINES = 2000
_HOST_SHELL_BRIDGE_HOSTS = {"127.0.0.1", "localhost", "::1", "host.docker.internal"}
IS_WINDOWS = sys.platform.startswith("win")
_HOST_SHELL_CANCEL_TASKS: set[asyncio.Task] = set()
@@ -221,11 +216,6 @@ def is_host_shell_bridge_url_allowed(url: str) -> bool:
return True
def _tmux_session_name(session_id: Optional[str]) -> str:
raw = re.sub(r"[^A-Za-z0-9_.-]+", "-", str(session_id or "default")).strip("-")
return f"ody-agent-{raw[:80] or 'default'}"
def _replace_workspace_alias(content: str, cwd: str) -> str:
"""Map virtual /workspace paths without corrupting absolute host paths."""
return re.sub(
@@ -361,7 +351,7 @@ def _filesystem_boundary_block(mechanism: str, mode: str, *, confined: bool) ->
Reports the **filesystem dimension only**, deliberately. The probe knows
this host could also give a process group and a real wall clock, but
BashTool and PythonTool still assemble their own ``create_subprocess_*``
Compatibility namespace previews assemble their own ``create_subprocess_*``
call and pass neither ``start_new_session`` nor a group-wide kill, so
listing those dimensions here would be the false claim
:mod:`src.containment` calls worse than an honest absence. They arrive when
@@ -529,288 +519,6 @@ def _wrap_workspace_namespace(
return shlex.join(args)
async def _run_exec(*args: str, timeout: float = 10) -> Tuple[str, str, int]:
proc = await asyncio.create_subprocess_exec(
*args,
stdout=asyncio.subprocess.PIPE,
stderr=asyncio.subprocess.PIPE,
)
try:
out_b, err_b = await asyncio.wait_for(proc.communicate(), timeout=timeout)
except asyncio.TimeoutError:
try:
proc.kill()
except Exception:
pass
return "", "timeout", 124
return (
out_b.decode("utf-8", errors="replace"),
err_b.decode("utf-8", errors="replace"),
proc.returncode or 0,
)
async def _tmux_has_session(name: str) -> bool:
_, _, rc = await _run_exec("tmux", "has-session", "-t", name, timeout=3)
return rc == 0
async def _tmux_capture(name: str) -> str:
out, _, _ = await _run_exec(
"tmux", "capture-pane", "-p", "-J", "-S", f"-{TMUX_CAPTURE_LINES}", "-t", name,
timeout=5,
)
return out
async def _tmux_send_line(name: str, line: str) -> None:
if line:
await _run_exec("tmux", "send-keys", "-t", name, "-l", line, timeout=5)
await _run_exec("tmux", "send-keys", "-t", name, "C-m", timeout=5)
async def _ensure_tmux_session(name: str, cwd: str, env: Optional[dict]) -> None:
# tmux creates child panes from the long-lived server environment, not
# necessarily from the app process that issued ``new-session``. On hosts
# where tmux predates the Odysseus virtualenv this silently resolves
# ``python`` to the system interpreter, losing plotting/PDF dependencies
# and prompting futile pip-install loops. Reassert the small execution
# environment on both new and reused panes.
forwarded_env = {
key: str(env[key])
for key in ("PATH", "VIRTUAL_ENV", "HOME", "TMPDIR")
if env and env.get(key)
}
if await _tmux_has_session(name):
if forwarded_env:
exports = " ".join(
f"{key}={shlex.quote(value)}" for key, value in forwarded_env.items()
)
await _tmux_send_line(name, f"export {exports}")
await _run_exec("tmux", "send-keys", "-t", name, "stty -echo", "C-m", timeout=5)
return
env_args = [f"{key}={value}" for key, value in forwarded_env.items()]
await _run_exec(
"tmux", "new-session", "-d", "-s", name, "-c", cwd,
"env",
*env_args,
f"TERM={env.get('TERM', 'xterm-256color') if env else 'xterm-256color'}",
f"COLUMNS={env.get('COLUMNS', '120') if env else '120'}",
f"LINES={env.get('LINES', '40') if env else '40'}",
"/bin/bash",
"--noprofile",
"--norc",
timeout=10,
)
if not await _tmux_has_session(name):
raise RuntimeError(f"failed to create tmux session {name}")
await _run_exec("tmux", "send-keys", "-t", name, "stty -echo", "C-m", timeout=5)
def _output_after_marker(capture: str, start_marker: str, end_marker: str) -> Tuple[str, bool]:
lines = capture.splitlines()
start_idx = -1
for idx, line in enumerate(lines):
if line.strip() == start_marker:
start_idx = idx
if start_idx < 0:
return capture, False
end_idx = -1
for idx in range(start_idx + 1, len(lines)):
if lines[idx].strip().startswith(end_marker):
end_idx = idx
if end_idx < 0:
return "\n".join(lines[start_idx + 1:]), False
return "\n".join(lines[start_idx + 1:end_idx]), True
def _extract_marker_rc(capture: str, end_marker: str) -> int:
for line in reversed(capture.splitlines()):
stripped = line.strip()
if stripped.startswith(end_marker):
suffix = stripped[len(end_marker):].strip()
if suffix.isdigit():
return int(suffix)
return 0
async def _run_tmux_bash(
content: str,
*,
session_id: str,
cwd: str,
env: Optional[dict],
timeout: float,
progress_cb: Optional[Callable[[Dict], Awaitable[None]]] = None,
) -> Tuple[str, str, Optional[int], bool]:
name = _tmux_session_name(session_id)
await _ensure_tmux_session(name, cwd, env)
stamp = f"{int(time.time() * 1000)}-{abs(hash(content)) % 1000000}"
start_marker = f"__ODYSSEUS_CMD_START_{stamp}__"
end_prefix = f"__ODYSSEUS_CMD_END_{stamp}__:"
# Execute each tool call in a non-interactive child shell. The tmux pane
# is deliberately persistent, but handing its terminal stdin to commands
# lets programs such as ffmpeg block forever on overwrite prompts. EOF is
# the deterministic behavior expected from an agent tool invocation.
child_command = f"/bin/bash -lc {shlex.quote(content)} </dev/null"
wrapped = (
f"printf '\\n{start_marker}\\n'\n"
f"{child_command}\n"
f"__ody_rc=$?\n"
f"printf '\\n{end_prefix}%s\\n' \"$__ody_rc\"\n"
)
for line in wrapped.splitlines():
await _tmux_send_line(name, line)
started = time.time()
last_tail = ""
while True:
capture = await _tmux_capture(name)
body, done = _output_after_marker(capture, start_marker, end_prefix)
tail = "\n".join(body.splitlines()[-PROGRESS_TAIL_LINES:])
if progress_cb and tail != last_tail:
last_tail = tail
try:
await progress_cb({
"elapsed_s": round(time.time() - started, 1),
"tail": tail,
"tmux_session": name,
})
except Exception:
pass
if done:
rc = _extract_marker_rc(capture, end_prefix)
cleaned = _clean_tmux_command_output(body, wrapped)
return cleaned, "", rc, False
if time.time() - started > timeout:
try:
await _run_exec("tmux", "send-keys", "-t", name, "C-c", timeout=3)
except Exception:
pass
# Ctrl-C targets the pane's foreground process group, but a child
# can outlive its wrapper shell and become an orphan. Destroy this
# task-scoped session as the timeout boundary; the next tool call
# recreates it through _ensure_tmux_session.
try:
await _run_exec("tmux", "kill-session", "-t", name, timeout=3)
except Exception:
pass
cleaned = _clean_tmux_command_output(body, wrapped)
return cleaned, "", 124, True
await asyncio.sleep(0.5)
def _clean_tmux_command_output(text: str, wrapped_command: str) -> str:
lines = text.splitlines()
wrapped_lines = {ln.rstrip() for ln in wrapped_command.splitlines() if ln.strip()}
cleaned = []
for line in lines:
raw = line.rstrip()
stripped = raw.strip()
if not stripped:
cleaned.append(raw)
continue
if stripped in wrapped_lines:
continue
if stripped.startswith("__ody_rc=") or stripped.startswith("printf "):
continue
if re.fullmatch(r"(?:bash|sh)-[\d.]+\$ ?", stripped):
continue
if re.fullmatch(r"[\w.@:/~+-]+[#$] ?", stripped):
continue
cleaned.append(raw)
return "\n".join(cleaned).strip()
async def _run_subprocess_streaming(
proc: asyncio.subprocess.Process,
*,
timeout: float,
progress_cb: Optional[Callable[[Dict], Awaitable[None]]] = None,
) -> Tuple[str, str, Optional[int], bool]:
started = time.time()
stdout_full: list[str] = []
stderr_full: list[str] = []
tail = collections.deque(maxlen=PROGRESS_TAIL_LINES)
async def _reader(stream, full_buf, label: str):
if stream is None:
return
while True:
line = await stream.readline()
if not line:
break
decoded = line.decode("utf-8", errors="replace").rstrip("\n")
full_buf.append(decoded)
if label == "err":
tail.append(f"! {decoded}")
else:
tail.append(decoded)
async def _progress_emitter():
await asyncio.sleep(PROGRESS_INTERVAL_S)
while True:
if progress_cb:
try:
await progress_cb({
"elapsed_s": round(time.time() - started, 1),
"tail": "\n".join(list(tail)),
})
except Exception:
pass
await asyncio.sleep(PROGRESS_INTERVAL_S)
rd_out = asyncio.create_task(_reader(proc.stdout, stdout_full, "out"))
rd_err = asyncio.create_task(_reader(proc.stderr, stderr_full, "err"))
prog_task = asyncio.create_task(_progress_emitter()) if progress_cb else None
timed_out = False
try:
await asyncio.wait_for(proc.wait(), timeout=timeout)
except asyncio.TimeoutError:
timed_out = True
try:
proc.kill()
except Exception:
pass
try:
await asyncio.wait_for(proc.wait(), timeout=2)
except Exception:
pass
except asyncio.CancelledError:
try:
proc.kill()
except Exception:
pass
try:
await asyncio.wait_for(proc.wait(), timeout=2)
except Exception:
pass
for t in (rd_out, rd_err):
t.cancel()
if prog_task is not None:
prog_task.cancel()
raise
finally:
if prog_task is not None and not prog_task.done():
prog_task.cancel()
try:
await prog_task
except (asyncio.CancelledError, Exception):
pass
for t in (rd_out, rd_err):
try:
await asyncio.wait_for(t, timeout=1)
except Exception:
pass
return (
"\n".join(stdout_full),
"\n".join(stderr_full),
proc.returncode,
timed_out,
)
def _owned_spec(cwd: str, env: Optional[dict], timeout: int, readonly_extra: tuple = ()) -> containment.ContainmentSpec:
"""Server-defined boundary shared by the native execution tools."""
readonly = []
@@ -853,14 +561,15 @@ async def _run_owned_command(command, ctx: dict, *, tool: str, timeout: int, arg
if result.stderr.rstrip():
output = (output + "\nSTDERR: " + result.stderr.rstrip()).strip()
truncated = result.output_truncated or len(output) > MAX_OUTPUT_CHARS
capture_note = " Captured output was truncated." if truncated else ""
common = {"containment": boundary, "teardown": teardown, "output_truncated": truncated}
if not teardown["dead"]:
return {**common, "error": f"{tool}: process teardown could not verify death",
return {**common, "error": f"{tool}: process teardown could not verify death.{capture_note}",
"failure_kind": "process_teardown_failed", "exit_code": 1,
"stdout": _truncate(result.stdout, MAX_OUTPUT_CHARS),
"stderr": _truncate(result.stderr, MAX_OUTPUT_CHARS)}
if result.timed_out:
return {**common, "error": f"{tool}: timed out after {timeout}s; process tree terminated",
return {**common, "error": f"{tool}: timed out after {timeout}s; process tree terminated.{capture_note}",
"exit_code": 124, "stdout": _truncate(result.stdout, MAX_OUTPUT_CHARS),
"stderr": _truncate(result.stderr, MAX_OUTPUT_CHARS)}
if tool == "python":
@@ -868,6 +577,9 @@ async def _run_owned_command(command, ctx: dict, *, tool: str, timeout: int, arg
if child_failure:
return {**common, "error": _truncate("python: a child operation failed despite a zero Python exit status:\n" + child_failure, MAX_OUTPUT_CHARS),
"exit_code": 1, "stderr": _truncate(result.stderr, MAX_OUTPUT_CHARS)}
if truncated:
note = "\n…[output truncated by containment capture limit]…"
output = output[:MAX_OUTPUT_CHARS - len(note)] + note
return {**common, "output": _truncate(output, MAX_OUTPUT_CHARS) or "(no output)",
"exit_code": result.exit_code if result.exit_code is not None else 1}
-157
View File
@@ -1,74 +1,4 @@
import asyncio
from types import SimpleNamespace
def test_new_tmux_session_forwards_runtime_python_environment(monkeypatch):
from src.agent_tools import subprocess_tools
calls = []
checks = 0
async def fake_has_session(_name):
nonlocal checks
checks += 1
return checks > 1
async def fake_run_exec(*args, **kwargs):
calls.append(args)
if args[:2] == ("tmux", "has-session"):
return "", "", 0
return "", "", 0
monkeypatch.setattr(subprocess_tools, "_tmux_has_session", fake_has_session)
monkeypatch.setattr(subprocess_tools, "_run_exec", fake_run_exec)
asyncio.run(subprocess_tools._ensure_tmux_session(
"ody-test",
"/workspace",
{
"PATH": "/opt/ody/bin:/usr/bin",
"VIRTUAL_ENV": "/opt/ody",
"HOME": "/workspace",
"TMPDIR": "/workspace/.tmp",
"SECRET": "must-not-forward",
},
))
new_session = next(call for call in calls if call[:2] == ("tmux", "new-session"))
assert "PATH=/opt/ody/bin:/usr/bin" in new_session
assert "VIRTUAL_ENV=/opt/ody" in new_session
assert "HOME=/workspace" in new_session
assert "TMPDIR=/workspace/.tmp" in new_session
assert not any("SECRET=" in arg for arg in new_session)
def test_reused_tmux_session_refreshes_runtime_python_environment(monkeypatch):
from src.agent_tools import subprocess_tools
sent = []
async def fake_has_session(_name):
return True
async def fake_send_line(name, line):
sent.append((name, line))
async def fake_run_exec(*_args, **_kwargs):
return "", "", 0
monkeypatch.setattr(subprocess_tools, "_tmux_has_session", fake_has_session)
monkeypatch.setattr(subprocess_tools, "_tmux_send_line", fake_send_line)
monkeypatch.setattr(subprocess_tools, "_run_exec", fake_run_exec)
asyncio.run(subprocess_tools._ensure_tmux_session(
"ody-existing",
"/workspace",
{"PATH": "/path with spaces/bin:/usr/bin", "VIRTUAL_ENV": "/path with spaces"},
))
assert sent == [(
"ody-existing",
"export PATH='/path with spaces/bin:/usr/bin' VIRTUAL_ENV='/path with spaces'",
)]
def test_workspace_alias_rewrite_does_not_duplicate_absolute_host_path():
@@ -97,93 +27,6 @@ def test_workspace_namespace_preserves_literal_paths_inside_scripts(tmp_path):
assert (Path(tmp_path) / "result.txt").read_text() == "ok"
def test_tmux_bash_runs_tool_command_with_closed_stdin(monkeypatch, tmp_path):
from src.agent_tools import subprocess_tools
sent = []
async def fake_ensure(*_args, **_kwargs):
return None
async def fake_send(_name, line):
sent.append(line)
async def fake_capture(_name):
start = next(line for line in sent if "__ODYSSEUS_CMD_START_" in line)
start = start.split("\\n")[1]
end_line = next(line for line in sent if "__ODYSSEUS_CMD_END_" in line)
end = end_line.split("\\n")[1].split("%s")[0]
return f"{start}\ngot-eof\n{end}0\n"
monkeypatch.setattr(subprocess_tools, "_ensure_tmux_session", fake_ensure)
monkeypatch.setattr(subprocess_tools, "_tmux_send_line", fake_send)
monkeypatch.setattr(subprocess_tools, "_tmux_capture", fake_capture)
monkeypatch.setattr(subprocess_tools.time, "time", lambda: 0.123456)
output, _stderr, rc, timed_out = asyncio.run(
subprocess_tools._run_tmux_bash(
"if read answer; then echo unexpected; else echo got-eof; fi",
session_id="test",
cwd=str(tmp_path),
env={},
timeout=2,
)
)
assert output == "got-eof"
assert rc == 0
assert timed_out is False
assert any("/bin/bash -lc" in line and "</dev/null" in line for line in sent)
def test_tmux_bash_timeout_destroys_session_process_tree(monkeypatch, tmp_path):
from src.agent_tools import subprocess_tools
exec_calls = []
sent = []
clock = [-2.0]
async def fake_ensure(*_args, **_kwargs):
return None
async def fake_send(name, line):
sent.append((name, line))
async def fake_capture(_name):
return "still running"
async def fake_run_exec(*args, **kwargs):
exec_calls.append(args)
return "", "", 0
async def fake_sleep(_seconds):
return None
monkeypatch.setattr(subprocess_tools, "_ensure_tmux_session", fake_ensure)
monkeypatch.setattr(subprocess_tools, "_tmux_send_line", fake_send)
monkeypatch.setattr(subprocess_tools, "_tmux_capture", fake_capture)
monkeypatch.setattr(subprocess_tools, "_run_exec", fake_run_exec)
monkeypatch.setattr(subprocess_tools.asyncio, "sleep", fake_sleep)
def fake_time():
clock[0] += 2.0
return clock[0]
monkeypatch.setattr(subprocess_tools.time, "time", fake_time)
_output, _stderr, rc, timed_out = asyncio.run(
subprocess_tools._run_tmux_bash(
"grep -r needle /large/tree",
session_id="timeout-test",
cwd=str(tmp_path),
env={},
timeout=1,
)
)
assert rc == 124 and timed_out is True
assert any(call[:3] == ("tmux", "send-keys", "-t") for call in exec_calls)
assert any(call[:3] == ("tmux", "kill-session", "-t") for call in exec_calls)
def test_direct_bash_subprocess_has_closed_stdin(monkeypatch, tmp_path):
from src.agent_tools import subprocess_tools
from src import tool_execution
+1 -1
View File
@@ -162,7 +162,7 @@ async def test_windows_bash_does_not_use_a_stray_tmux_executable(monkeypatch, tm
async def fail_tmux(*_args, **_kwargs):
pytest.fail("native Windows must not enter the POSIX tmux path")
monkeypatch.setattr(subprocess_tools, "_run_tmux_bash", fail_tmux)
monkeypatch.setattr(subprocess_tools.asyncio, "create_subprocess_shell", fail_tmux)
result = await subprocess_tools.BashTool().execute(
"pwd",
+1 -1
View File
@@ -17,7 +17,7 @@ async def test_a_chat_session_always_uses_the_owned_runner(monkeypatch, tmp_path
monkeypatch.setattr(subprocess_tools.shutil, "which", lambda name: "/fake/tmux" if name == "tmux" else original(name))
async def forbidden(*args, **kwargs):
pytest.fail("native Bash resurrected a persistent tmux shell")
monkeypatch.setattr(subprocess_tools, "_run_tmux_bash", forbidden)
monkeypatch.setattr(subprocess_tools.asyncio, "create_subprocess_shell", forbidden)
result = await subprocess_tools.BashTool().execute("printf ok", {"session_id": "same-chat"})
assert result["output"] == "ok"
assert result["teardown"]["dead"] is True
@@ -151,3 +151,37 @@ async def test_capture_preserves_multibyte_text_across_chunks(native_boundary):
[sys.executable, "-c", "import sys; sys.stdout.write('€' * 30000)"], argv=True)
assert result.stdout == "€" * 30000
assert result.output_truncated is False
async def test_chat_bash_captures_more_than_2000_lines(native_boundary):
result = await subprocess_tools.BashTool().execute(
"printf 'START\\n'; for i in $(seq 1 3000); do printf 'x\\n'; done; printf 'END\\n'",
{"session_id": "large-output-chat"},
)
assert result["output"].startswith("START\n")
assert result["output"].endswith("\nEND")
assert len(result["output"].splitlines()) == 3002
assert result["output_truncated"] is False
assert result["teardown"]["dead"] is True
async def test_chat_bash_reports_capture_limit_as_incomplete(native_boundary):
result = await subprocess_tools.BashTool().execute(
"for i in $(seq 1 12000); do printf 'x\\n'; done", {"session_id": "capped-chat"},
)
assert result["exit_code"] == 0
assert result["output_truncated"] is True
assert "truncated" in result["output"]
assert result["teardown"]["dead"] is True
async def test_repeated_chat_calls_refresh_environment(native_boundary):
first = await subprocess_tools.BashTool().execute('printf "%s" "$VALUE"', {
"session_id": "same-chat", "subproc_env": {"PATH": "/usr/bin:/bin", "VALUE": "first"},
})
second = await subprocess_tools.BashTool().execute('printf "%s" "$VALUE"', {
"session_id": "same-chat", "subproc_env": {"PATH": "/usr/bin:/bin", "VALUE": "second"},
})
assert first["output"] == "first"
assert second["output"] == "second"
assert first["containment"]["id"] != second["containment"]["id"]
+1 -1
View File
@@ -75,7 +75,7 @@ The source tree reads **108** `ODYSSEUS_*` variables: 78 an operator may want to
| `ODYSSEUS_MAX_VISUAL_EVIDENCE_FRAMES` | `'3'` | `src/agent_loop.py:15361` | How many video frames one tool result may contribute. Clamped to 1-8. |
| `ODYSSEUS_MAX_VISUAL_EVIDENCE_IMAGES` | `'1'` | `src/agent_loop.py:15329` | How many images one tool result may contribute to the model turn. Clamped to 1-8. |
| `ODYSSEUS_MCP_ALLOWED_COMMANDS` | `''` | `src/agent_tools/admin_tools.py:140` | Security-relevant. Comma-separated allowlist of MCP launcher basenames the agent may start. Empty by default, and the deny list still wins. |
| `ODYSSEUS_PYTHON_TOOL_SITE_PACKAGES` | `''` | `src/agent_tools/subprocess_tools.py:1145` (+1 more) | Security-relevant. Absolute package roots, separated by the platform path separator, exposed to the sandboxed Python tool. Empty exposes none. |
| `ODYSSEUS_PYTHON_TOOL_SITE_PACKAGES` | `''` | `src/agent_tools/subprocess_tools.py:857` (+1 more) | Security-relevant. Absolute package roots, separated by the platform path separator, exposed to the sandboxed Python tool. Empty exposes none. |
| `ODYSSEUS_SCRIPT_HOST` | `'localhost'` | `src/builtin_actions.py:919` | Default host for the run-script action. `localhost`, `127.0.0.1`, `local` and empty run locally; any other value runs over SSH. |
| `ODYSSEUS_TOOL_APPROVAL_GATE` | `'0'` | `src/tool_capabilities.py:645` | Security-relevant. Truthy makes tool calls pass through the approval gate. Off by default. |