diff --git a/claude_bottle/backend/__init__.py b/claude_bottle/backend/__init__.py index 3e7dcea..01bf5f0 100644 --- a/claude_bottle/backend/__init__.py +++ b/claude_bottle/backend/__init__.py @@ -139,6 +139,21 @@ class Bottle(ABC): name: str + @abstractmethod + def claude_argv( + self, argv: list[str], *, tty: bool = True, + ) -> list[str]: + """Return the host-side argv that runs `claude ` + inside the bottle. Used by `exec_claude` for foreground + handoffs and by the dashboard's tmux `respawn-pane` flow, + which needs the argv up front (it spawns claude in a tmux + pane rather than as a child of the current process). + + Implementations transparently inject + `--append-system-prompt-file` when the bottle was launched + with a provisioned prompt path.""" + ... + @abstractmethod def exec_claude(self, argv: list[str], *, tty: bool = True) -> int: ... diff --git a/claude_bottle/backend/docker/bottle.py b/claude_bottle/backend/docker/bottle.py index e0d421c..02cf4f3 100644 --- a/claude_bottle/backend/docker/bottle.py +++ b/claude_bottle/backend/docker/bottle.py @@ -28,15 +28,9 @@ class DockerBottle(Bottle): self._prompt_path = prompt_path_in_container self._closed = False - def claude_docker_argv( + def claude_argv( self, argv: list[str], *, tty: bool = True, ) -> list[str]: - """Return the full `docker exec` argv for running claude in - this bottle. Public so callers that want to spawn claude - somewhere other than the dashboard's foreground (e.g., - `tmux split-window` / `tmux respawn-pane` from the dashboard - when `$TMUX` is set) can build on the same command without - duplicating the `--append-system-prompt-file` plumbing.""" full_argv = list(argv) if self._prompt_path: full_argv.extend(["--append-system-prompt-file", self._prompt_path]) @@ -48,7 +42,7 @@ class DockerBottle(Bottle): def exec_claude(self, argv: list[str], *, tty: bool = True) -> int: return subprocess.run( - self.claude_docker_argv(argv, tty=tty), check=False, + self.claude_argv(argv, tty=tty), check=False, ).returncode def exec(self, script: str, *, user: str = "node") -> ExecResult: diff --git a/claude_bottle/backend/smolmachines/bottle.py b/claude_bottle/backend/smolmachines/bottle.py index 89c836f..27759b4 100644 --- a/claude_bottle/backend/smolmachines/bottle.py +++ b/claude_bottle/backend/smolmachines/bottle.py @@ -75,6 +75,21 @@ class SmolmachinesBottle(Bottle): # because exec doesn't inherit from machine_create's env. self._guest_env = dict(guest_env or {}) + def claude_argv( + self, argv: list[str], *, tty: bool = True, + ) -> list[str]: + flags = ["smolvm", "machine", "exec", "--name", self.name] + if tty: + flags += ["-i", "-t"] + flags += _env_flags_for("node") + flags += _guest_env_flags(self._guest_env) + claude_tail = ["claude"] + if self._prompt_path: + claude_tail += ["--append-system-prompt-file", self._prompt_path] + claude_tail += argv + flags += ["--", "runuser", "-u", "node", "--", *claude_tail] + return flags + def exec_claude(self, argv: list[str], *, tty: bool = True) -> int: """Run `claude` interactively inside the VM as the `node` user. Inherits the operator's terminal (stdin / stdout / @@ -89,18 +104,9 @@ class SmolmachinesBottle(Bottle): UID switches via `runuser -u node --` (not `-l`) so we avoid login-shell wiring. HOME / USER come from `smolvm -e` instead, which sets them on the process env.""" - flags = ["smolvm", "machine", "exec", "--name", self.name] - if tty: - flags += ["-i", "-t"] - flags += _env_flags_for("node") - flags += _guest_env_flags(self._guest_env) - claude_argv = ["claude"] - if self._prompt_path: - claude_argv += ["--append-system-prompt-file", self._prompt_path] - claude_argv += argv - flags += ["--", "runuser", "-u", "node", "--", *claude_argv] - result = subprocess.run(flags, check=False) - return result.returncode + return subprocess.run( + self.claude_argv(argv, tty=tty), check=False, + ).returncode def exec(self, script: str, *, user: str = "node") -> ExecResult: """Run a POSIX shell script as `user` (default `node`) and diff --git a/claude_bottle/cli/dashboard.py b/claude_bottle/cli/dashboard.py index ebe7560..71fedc8 100644 --- a/claude_bottle/cli/dashboard.py +++ b/claude_bottle/cli/dashboard.py @@ -762,7 +762,7 @@ def _in_tmux() -> bool: def _claude_runtime_args(*, resume: bool, remote_control: bool = False) -> list[str]: - """The argv the dashboard hands to `bottle.claude_docker_argv` + """The argv the dashboard hands to `bottle.claude_argv` on every attach — matches what `attach_claude` builds for the foreground handoff so both surfaces produce the same claude invocation.""" @@ -777,28 +777,35 @@ def _claude_runtime_args(*, resume: bool, remote_control: bool = False) -> list[ def _build_resume_argv_with_fallback( bottle, *, remote_control: bool = False, ) -> list[str]: - """Build a docker-exec argv that runs `claude --continue` and + """Build a backend-exec argv that runs `claude --continue` and falls back to plain `claude` if no prior session exists. `--continue` exits non-zero when an agent has been spun up but never typed at — there's no transcript to resume. The shell-level `||` wrapper makes that case start a fresh session instead of crashing the pane. The trade-off: we - invoke `sh -c` inside the container, so the command is two + invoke `sh -c` inside the bottle, so the command is two `claude` invocations behind a tiny shell rather than one direct exec. Acceptable; the shell adds microseconds and the fallback only kicks in when --continue would have - failed anyway.""" + failed anyway. + + Works across backends because `bottle.claude_argv` always + surfaces the `claude` token preceded by the backend's exec + framing (docker: `docker exec -it `; smolmachines: + `smolvm machine exec --name -- runuser -u node --`). + Splitting at `claude` keeps the framing as the prefix and + wraps just the claude tail in `sh -c`.""" base_args = ["--dangerously-skip-permissions"] if remote_control: base_args.append("--remote-control") - base_docker = bottle.claude_docker_argv(base_args) - # Split docker-prefix from the claude-and-args tail so we - # can compose ` --continue || ` inside + base_exec = bottle.claude_argv(base_args) + # Split exec-framing prefix from the claude-and-args tail so + # we can compose ` --continue || ` inside # `sh -c`. The `claude` token is the marker. - claude_idx = base_docker.index("claude") - prefix = base_docker[:claude_idx] - claude_cmd = " ".join(shlex.quote(a) for a in base_docker[claude_idx:]) + claude_idx = base_exec.index("claude") + prefix = base_exec[:claude_idx] + claude_cmd = " ".join(shlex.quote(a) for a in base_exec[claude_idx:]) return [ *prefix, "sh", "-c", @@ -806,23 +813,23 @@ def _build_resume_argv_with_fallback( ] -def _build_split_pane_argv(docker_argv: list[str]) -> list[str]: - """Pure helper: wrap a docker-exec argv with `tmux split-window +def _build_split_pane_argv(claude_argv: list[str]) -> list[str]: + """Pure helper: wrap a backend-exec argv with `tmux split-window -h -P -F '#{pane_id}'`. The `-P -F` combo tells tmux to print the new pane's id on stdout so we can track it for later `respawn-pane` calls.""" return [ "tmux", "split-window", "-h", "-P", "-F", "#{pane_id}", - *docker_argv, + *claude_argv, ] -def _build_respawn_pane_argv(pane_id: str, docker_argv: list[str]) -> list[str]: - """Pure helper: wrap a docker-exec argv with `tmux respawn-pane +def _build_respawn_pane_argv(pane_id: str, claude_argv: list[str]) -> list[str]: + """Pure helper: wrap a backend-exec argv with `tmux respawn-pane -k -t `. `-k` kills the existing process in the pane before respawning.""" - return ["tmux", "respawn-pane", "-k", "-t", pane_id, *docker_argv] + return ["tmux", "respawn-pane", "-k", "-t", pane_id, *claude_argv] @contextlib.contextmanager @@ -1046,12 +1053,12 @@ def _attach_in_tmux( # exists (agent spun up but never typed at). Wrap with a # shell-level fallback so the pane lands in a fresh # claude instead of crashing. - docker_argv = _build_resume_argv_with_fallback(bottle) + claude_argv = _build_resume_argv_with_fallback(bottle) else: - docker_argv = bottle.claude_docker_argv( + claude_argv = bottle.claude_argv( _claude_runtime_args(resume=False), ) - pane_id = _ensure_right_pane(tmux_state, docker_argv) + pane_id = _ensure_right_pane(tmux_state, claude_argv) if pane_id is None: # tmux failed (missing binary, server died, size error). # One status-line failover to the curses handoff so the diff --git a/tests/unit/test_dashboard_active_agents.py b/tests/unit/test_dashboard_active_agents.py index 55299d7..32f9a79 100644 --- a/tests/unit/test_dashboard_active_agents.py +++ b/tests/unit/test_dashboard_active_agents.py @@ -369,7 +369,7 @@ class TestResumeArgvWithFallback(unittest.TestCase): class TestClaudeRuntimeArgs(unittest.TestCase): - """The argv passed to `bottle.claude_docker_argv` on each + """The argv passed to `bottle.claude_argv` on each attach. Locked here so the tmux + foreground paths build identical claude invocations.""" diff --git a/tests/unit/test_docker_bottle.py b/tests/unit/test_docker_bottle.py index 01bf890..009545b 100644 --- a/tests/unit/test_docker_bottle.py +++ b/tests/unit/test_docker_bottle.py @@ -1,6 +1,6 @@ """Unit: DockerBottle's argv builder (PRD 0021 chunk 1). -`claude_docker_argv` is the pure helper that `exec_claude` and the +`claude_argv` is the pure helper that `exec_claude` and the PRD-0021 tmux helpers both build on. It encodes two non-trivial rules — the optional `--append-system-prompt-file` flag and the optional `-it` for TTY mode — that we lock down here so the tmux @@ -22,16 +22,16 @@ def _bottle(prompt_path: str | None = None) -> DockerBottle: ) -class TestClaudeDockerArgv(unittest.TestCase): +class TestClaudeArgv(unittest.TestCase): def test_minimal_argv_no_prompt(self): - argv = _bottle().claude_docker_argv([]) + argv = _bottle().claude_argv([]) self.assertEqual( ["docker", "exec", "-it", "claude-bottle-dev-abc", "claude"], argv, ) def test_appends_passed_args_after_claude(self): - argv = _bottle().claude_docker_argv( + argv = _bottle().claude_argv( ["--dangerously-skip-permissions", "--continue"], ) self.assertEqual( @@ -41,7 +41,7 @@ class TestClaudeDockerArgv(unittest.TestCase): ) def test_appends_prompt_file_flag_when_set(self): - argv = _bottle("/home/node/.claude-bottle-prompt.txt").claude_docker_argv( + argv = _bottle("/home/node/.claude-bottle-prompt.txt").claude_argv( ["--dangerously-skip-permissions"], ) self.assertEqual( @@ -53,30 +53,30 @@ class TestClaudeDockerArgv(unittest.TestCase): ) def test_no_prompt_flag_when_none(self): - argv = _bottle(None).claude_docker_argv(["--continue"]) + argv = _bottle(None).claude_argv(["--continue"]) self.assertNotIn("--append-system-prompt-file", argv) def test_empty_prompt_string_is_treated_as_no_prompt(self): # Matches the existing exec_claude behavior: falsy # prompt_path means "skip the flag." The synth path in # dashboard.py relies on this when metadata is missing. - argv = _bottle("").claude_docker_argv(["--continue"]) + argv = _bottle("").claude_argv(["--continue"]) self.assertNotIn("--append-system-prompt-file", argv) def test_tty_false_drops_it_flag(self): - argv = _bottle().claude_docker_argv([], tty=False) + argv = _bottle().claude_argv([], tty=False) self.assertEqual( ["docker", "exec", "claude-bottle-dev-abc", "claude"], argv, ) def test_caller_argv_not_mutated(self): - # `claude_docker_argv` builds `full_argv` from a copy, so a + # `claude_argv` builds `full_argv` from a copy, so a # caller passing a long-lived list (e.g., the dashboard's # _claude_args fixture) doesn't get extra flags appended to # it on subsequent calls. original = ["--continue"] - _bottle("/x").claude_docker_argv(original) + _bottle("/x").claude_argv(original) self.assertEqual(["--continue"], original) diff --git a/tests/unit/test_smolmachines_bottle.py b/tests/unit/test_smolmachines_bottle.py new file mode 100644 index 0000000..69e3a1a --- /dev/null +++ b/tests/unit/test_smolmachines_bottle.py @@ -0,0 +1,107 @@ +"""Unit: SmolmachinesBottle's `claude_argv` builder. + +The dashboard's tmux pane-respawn path calls `bottle.claude_argv` +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. +""" + +from __future__ import annotations + +import unittest + +from claude_bottle.backend.smolmachines.bottle import SmolmachinesBottle + + +def _bottle(prompt_path: str | None = None, **env: str) -> SmolmachinesBottle: + return SmolmachinesBottle( + "claude-bottle-dev-abc", + prompt_path=prompt_path, + guest_env=env, + ) + + +class TestClaudeArgv(unittest.TestCase): + def test_minimal_argv_no_prompt(self): + argv = _bottle().claude_argv([]) + self.assertEqual( + [ + "smolvm", "machine", "exec", "--name", + "claude-bottle-dev-abc", + "-i", "-t", + "-e", "HOME=/home/node", + "-e", "USER=node", + "--", + "runuser", "-u", "node", "--", + "claude", + ], + argv, + ) + + def test_appends_passed_args_after_claude(self): + argv = _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"], + ) + self.assertEqual( + [ + "claude", + "--append-system-prompt-file", + "/home/node/.claude-bottle-prompt.txt", + "--dangerously-skip-permissions", + ], + argv[argv.index("claude"):], + ) + + def test_no_prompt_flag_when_none(self): + argv = _bottle(None).claude_argv(["--continue"]) + self.assertNotIn("--append-system-prompt-file", argv) + + def test_empty_prompt_string_is_treated_as_no_prompt(self): + 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( + 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. + self.assertIn("-e", argv) + self.assertIn("HTTPS_PROXY=http://127.0.0.1:1234", argv) + self.assertIn("NO_PROXY=localhost", argv) + + def test_runuser_switch_precedes_claude(self): + # The dashboard's `_build_resume_argv_with_fallback` finds + # the `claude` token to split exec-framing from the claude + # tail. `runuser -u node --` must sit on the prefix side so + # the shell wrap inherits the UID switch. + argv = _bottle().claude_argv([]) + claude_idx = argv.index("claude") + self.assertEqual( + ["runuser", "-u", "node", "--"], + argv[claude_idx - 4:claude_idx], + ) + + +if __name__ == "__main__": + unittest.main()