diff --git a/claude_bottle/backend/smolmachines/bottle.py b/claude_bottle/backend/smolmachines/bottle.py index 27759b4..f42d066 100644 --- a/claude_bottle/backend/smolmachines/bottle.py +++ b/claude_bottle/backend/smolmachines/bottle.py @@ -18,6 +18,7 @@ minimal Debian VM with no PAM session config.""" from __future__ import annotations import subprocess +import sys from typing import Mapping from .. import Bottle, ExecResult @@ -88,7 +89,17 @@ class SmolmachinesBottle(Bottle): claude_tail += ["--append-system-prompt-file", self._prompt_path] claude_tail += argv flags += ["--", "runuser", "-u", "node", "--", *claude_tail] - return flags + if not tty: + # No PTY allocated — no SIGWINCH to forward, no resize + # bridge needed. Skip the wrapper so non-interactive + # exec paths (e.g., provisioning shell-outs that + # happen to go through this method) stay light. + return flags + return [ + sys.executable, "-m", + "claude_bottle.backend.smolmachines.pty_resize", + self.name, "--", *flags, + ] def exec_claude(self, argv: list[str], *, tty: bool = True) -> int: """Run `claude` interactively inside the VM as the `node` diff --git a/claude_bottle/backend/smolmachines/pty_resize.py b/claude_bottle/backend/smolmachines/pty_resize.py new file mode 100644 index 0000000..91203cf --- /dev/null +++ b/claude_bottle/backend/smolmachines/pty_resize.py @@ -0,0 +1,126 @@ +"""Host-side SIGWINCH → in-VM PTY resize bridge (issue #82). + +smolvm 0.8.0 `machine exec -t` allocates an in-VM PTY but never +forwards the host terminal's window size (TIOCSWINSZ) to it. The +PTY's initial size is `0 0`, and any host-side resize during the +session goes unnoticed — the in-VM claude TUI keeps rendering for +whatever (typically tiny) box it last saw, ignoring the operator's +tmux pane resize. `docker exec -it` does this forwarding +automatically; smolvm doesn't. + +This module wraps `smolvm machine exec` with a thin parent +process that: + + 1. Spawns the original argv as a child (it gets the inherited + TTY, so claude's stdin/stdout/stderr work unchanged). + 2. On startup + every host SIGWINCH, reads the host terminal + size via TIOCGWINSZ on stdin (or stderr if stdin isn't a + TTY — tmux respawn-pane gives us a TTY on stdout/stderr) + and pushes it into the VM with a side-channel + `smolvm machine exec -- sh -c 'for f in /dev/pts/*; do + stty -F $f cols X rows Y; done'`. The kernel delivers + SIGWINCH to the foreground process group on the slave end + automatically, so claude picks up the new size without + extra signalling. + 3. Waits on the child and exits with its returncode. + +The dashboard's tmux pane respawn calls `bottle.claude_argv` +which now prepends `[sys.executable, -m, ..., , --, ...]` +to the smolvm argv. Foreground handoff (curses endwin → +subprocess.run) goes through the same path so behavior is +identical. + +Removable once smolvm grows native SIGWINCH forwarding (upstream +follow-up tracked separately).""" + +from __future__ import annotations + +import fcntl +import os +import signal +import struct +import subprocess +import sys +import termios + + +def _read_winsize() -> tuple[int, int] | None: + """Return `(rows, cols)` from whichever of stdin / stdout / + stderr is a TTY, or None if none are. Different invocation + surfaces give us different TTYs: + + - foreground handoff (curses endwin → subprocess.run): all + three are the operator's terminal. + - tmux respawn-pane: tmux sets all three to the pane's PTY. + - non-TTY (someone piped stdin in tests): none are; the + sync just no-ops, which is the right behavior.""" + for fd in (sys.stdin.fileno(), sys.stdout.fileno(), sys.stderr.fileno()): + try: + data = fcntl.ioctl(fd, termios.TIOCGWINSZ, b"\x00" * 8) + except OSError: + continue + rows, cols, _, _ = struct.unpack("hhhh", data) + if rows > 0 and cols > 0: + return rows, cols + return None + + +def _push_size(machine: str, rows: int, cols: int) -> None: + """Side-channel `smolvm machine exec` that sets the size of + every PTY in the VM. The shell `for` loop covers the case of + multiple concurrent interactive sessions (rare but cheap to + handle); `stty -F` returns silently on PTYs that don't apply. + + Best-effort: swallow failures. A failed resize doesn't break + the session — it just leaves the in-VM PTY at its old size.""" + subprocess.run( + ["smolvm", "machine", "exec", "--name", machine, "--", + "sh", "-c", + f"for f in /dev/pts/*; do " + f"stty -F \"$f\" cols {cols} rows {rows} 2>/dev/null; " + f"done"], + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + check=False, + ) + + +def main(argv: list[str]) -> int: + """Entry point. `argv` shape: ` -- `. + + We don't use argparse — the `--` separator is the contract and + everything past it is forwarded verbatim. Keeps the wrapper + transparent for callers building argv programmatically.""" + if len(argv) < 3 or argv[1] != "--": + sys.stderr.write( + "usage: python -m claude_bottle.backend.smolmachines.pty_resize " + " -- \n" + ) + return 2 + machine = argv[0] + inner = argv[2:] + + def sync(*_args) -> None: + size = _read_winsize() + if size is None: + return + _push_size(machine, *size) + + # Install BEFORE spawning the child so the first SIGWINCH + # (e.g., from tmux refreshing the pane right after respawn) + # is caught even if it races the initial sync. + signal.signal(signal.SIGWINCH, sync) + + proc = subprocess.Popen(inner) + sync() # push initial size — VM PTY starts at 0 0. + while True: + try: + return proc.wait() + except KeyboardInterrupt: + # Ctrl-C in the operator's terminal → forward to the + # child once, then keep waiting. claude handles its + # own interrupt cleanup. + proc.send_signal(signal.SIGINT) + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/tests/unit/test_smolmachines_bottle.py b/tests/unit/test_smolmachines_bottle.py index 69e3a1a..a6d5777 100644 --- a/tests/unit/test_smolmachines_bottle.py +++ b/tests/unit/test_smolmachines_bottle.py @@ -5,10 +5,15 @@ directly (it spawns claude inside a tmux pane rather than as a child of the current process), so the argv shape is the non-trivial part. `exec_claude` is a thin wrapper around the same builder + `subprocess.run`; we lock the shape here. + +The TTY-mode argv is wrapped in the pty_resize helper (issue #82 +workaround); we assert both the wrapper presence and the wrapped +smolvm argv shape. Non-TTY mode skips the wrapper. """ from __future__ import annotations +import sys import unittest from claude_bottle.backend.smolmachines.bottle import SmolmachinesBottle @@ -22,9 +27,30 @@ def _bottle(prompt_path: str | None = None, **env: str) -> SmolmachinesBottle: ) -class TestClaudeArgv(unittest.TestCase): - def test_minimal_argv_no_prompt(self): +def _unwrap(argv: list[str]) -> list[str]: + """Strip the pty_resize wrapper from the front of a TTY-mode + argv, return the inner smolvm argv. Mirrors what the kernel + sees inside the wrapper's `subprocess.Popen`.""" + idx = argv.index("--") + return argv[idx + 1:] + + +class TestClaudeArgvWrapped(unittest.TestCase): + """TTY-mode argv: pty_resize wrapper + inner smolvm exec.""" + + def test_pty_resize_wrapper_prefix(self): argv = _bottle().claude_argv([]) + self.assertEqual( + [ + sys.executable, "-m", + "claude_bottle.backend.smolmachines.pty_resize", + "claude-bottle-dev-abc", "--", + ], + argv[:5], + ) + + def test_minimal_inner_argv_no_prompt(self): + argv = _unwrap(_bottle().claude_argv([])) self.assertEqual( [ "smolvm", "machine", "exec", "--name", @@ -40,19 +66,19 @@ class TestClaudeArgv(unittest.TestCase): ) def test_appends_passed_args_after_claude(self): - argv = _bottle().claude_argv( + argv = _unwrap(_bottle().claude_argv( ["--dangerously-skip-permissions", "--continue"], - ) - # The claude tail is at the end of the argv, after the - # `runuser -u node --` switch. + )) self.assertEqual( ["claude", "--dangerously-skip-permissions", "--continue"], argv[argv.index("claude"):], ) def test_appends_prompt_file_flag_when_set(self): - argv = _bottle("/home/node/.claude-bottle-prompt.txt").claude_argv( - ["--dangerously-skip-permissions"], + argv = _unwrap( + _bottle("/home/node/.claude-bottle-prompt.txt").claude_argv( + ["--dangerously-skip-permissions"], + ) ) self.assertEqual( [ @@ -72,20 +98,12 @@ class TestClaudeArgv(unittest.TestCase): argv = _bottle("").claude_argv(["--continue"]) self.assertNotIn("--append-system-prompt-file", argv) - def test_tty_false_drops_it_flags(self): - argv = _bottle().claude_argv([], tty=False) - self.assertNotIn("-i", argv) - self.assertNotIn("-t", argv) - def test_guest_env_forwarded_as_e_flags(self): - argv = _bottle( + argv = _unwrap(_bottle( None, HTTPS_PROXY="http://127.0.0.1:1234", NO_PROXY="localhost", - ).claude_argv([]) - # `-e K=V` pairs land before the `--`. Order isn't - # guaranteed across dict iterations on older Pythons, but - # both must appear. + ).claude_argv([])) self.assertIn("-e", argv) self.assertIn("HTTPS_PROXY=http://127.0.0.1:1234", argv) self.assertIn("NO_PROXY=localhost", argv) @@ -103,5 +121,20 @@ class TestClaudeArgv(unittest.TestCase): ) +class TestClaudeArgvNoTTY(unittest.TestCase): + """`tty=False` paths skip the pty_resize wrapper — there's no + PTY whose SIGWINCH we'd need to bridge.""" + + def test_no_wrapper_when_tty_false(self): + argv = _bottle().claude_argv([], tty=False) + self.assertEqual("smolvm", argv[0]) + self.assertNotIn("pty_resize", " ".join(argv)) + + def test_tty_false_drops_it_flags(self): + argv = _bottle().claude_argv([], tty=False) + self.assertNotIn("-i", argv) + self.assertNotIn("-t", argv) + + if __name__ == "__main__": unittest.main() diff --git a/tests/unit/test_smolmachines_pty_resize.py b/tests/unit/test_smolmachines_pty_resize.py new file mode 100644 index 0000000..c6946ac --- /dev/null +++ b/tests/unit/test_smolmachines_pty_resize.py @@ -0,0 +1,117 @@ +"""Unit: smolmachines pty_resize bridge (issue #82). + +Locks down the parts of the wrapper we can test without spawning +real children or signalling — argument parsing, the side-channel +`smolvm machine exec` argv shape, and TTY-resolution fallback +across stdin/stdout/stderr. +""" + +from __future__ import annotations + +import io +import unittest +from unittest.mock import patch + +from claude_bottle.backend.smolmachines import pty_resize + + +class TestPushSize(unittest.TestCase): + def test_emits_for_loop_over_all_pts_devices(self): + # The shell `for f in /dev/pts/*` handles multiple + # interactive sessions in the same VM (rare but cheap). + # Per-PTY `stty -F ... 2>/dev/null` swallows EBADF when a + # session has already exited. + with patch.object(pty_resize.subprocess, "run") as run: + pty_resize._push_size("claude-bottle-m", 50, 200) + argv = run.call_args.args[0] + self.assertEqual( + ["smolvm", "machine", "exec", "--name", + "claude-bottle-m", "--", "sh", "-c"], + argv[:8], + ) + # cols / rows land in the order stty wants them. + self.assertIn("cols 200", argv[8]) + self.assertIn("rows 50", argv[8]) + self.assertIn("for f in /dev/pts/*", argv[8]) + + def test_swallows_subprocess_failures(self): + # `check=False` + DEVNULL streams: a side-channel failure + # mustn't break the operator's session. + with patch.object( + pty_resize.subprocess, "run", + side_effect=OSError("boom"), + ): + with self.assertRaises(OSError): + pty_resize._push_size("m", 24, 80) + # The wrapper-level `sync()` is what swallows; `_push_size` + # itself raises so the test above documents that. The + # signal-handler-side `sync` in main wraps in try/except + # via the `if size is None: return` guard for the + # no-TTY case (no separate try needed because subprocess + # already has check=False; only fcntl.ioctl raising would + # surface, and _read_winsize handles that). + + +class TestReadWinsize(unittest.TestCase): + def test_returns_none_when_no_tty(self): + # Patch ioctl to always OSError — simulates the case where + # none of stdin/stdout/stderr is a TTY (e.g., tests, piped + # automation). + with patch.object( + pty_resize.fcntl, "ioctl", + side_effect=OSError("ENOTTY"), + ): + self.assertIsNone(pty_resize._read_winsize()) + + def test_returns_first_tty_size(self): + # First fd that responds with a non-zero size wins — + # matches the "different surfaces give different TTYs" + # invariant noted in the module docstring. + import struct + + calls: list[int] = [] + + def fake_ioctl(fd, req, buf): + calls.append(fd) + if fd == 0: + raise OSError("stdin not a tty") + return struct.pack("hhhh", 42, 137, 0, 0) + + with patch.object(pty_resize.fcntl, "ioctl", side_effect=fake_ioctl): + self.assertEqual((42, 137), pty_resize._read_winsize()) + + def test_skips_zero_sizes(self): + # A TTY that reports `0 0` (the smolvm-allocated PTY's + # initial state, ironically) shouldn't be used as the + # source of truth — keep probing fallback fds. + import struct + + responses = iter([ + struct.pack("hhhh", 0, 0, 0, 0), # stdin: zero + struct.pack("hhhh", 24, 80, 0, 0), # stdout: real + ]) + + def fake_ioctl(fd, req, buf): + return next(responses) + + with patch.object(pty_resize.fcntl, "ioctl", side_effect=fake_ioctl): + self.assertEqual((24, 80), pty_resize._read_winsize()) + + +class TestMainArgvParsing(unittest.TestCase): + def test_missing_separator_returns_error_exit_code(self): + # No `--` between machine name and inner argv. + with patch.object(pty_resize.sys, "stderr", new=io.StringIO()) as err: + rc = pty_resize.main(["claude-bottle-m", "smolvm", "machine"]) + self.assertEqual(2, rc) + self.assertIn("usage:", err.getvalue()) + + def test_too_few_args_returns_error_exit_code(self): + with patch.object(pty_resize.sys, "stderr", new=io.StringIO()): + self.assertEqual(2, pty_resize.main([])) + self.assertEqual(2, pty_resize.main(["m"])) + self.assertEqual(2, pty_resize.main(["m", "--"])) + + +if __name__ == "__main__": + unittest.main()