From 2303cbc0befbd99b96ffc86d895115c5320e40d8 Mon Sep 17 00:00:00 2001 From: didericis Date: Tue, 26 May 2026 14:21:04 -0400 Subject: [PATCH] refactor(bottle): extract claude_docker_argv from exec_claude MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PRD 0021 chunk 1. The tmux split-pane helpers (chunk 2+) need the same docker-exec argv that `exec_claude` builds — including the `--append-system-prompt-file ` flag the bottle's provisioner copies into place. Extract the argv construction into a pure `claude_docker_argv(argv, *, tty)` method so both foreground (`subprocess.run`) and tmux paths (`tmux respawn-pane …`) build from the same source. `exec_claude` becomes a one-liner that runs subprocess.run on the argv. No behavior change; 472 unit tests pass (7 new for the pure builder). --- claude_bottle/backend/docker/bottle.py | 17 +++++- tests/unit/test_docker_bottle.py | 84 ++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 tests/unit/test_docker_bottle.py diff --git a/claude_bottle/backend/docker/bottle.py b/claude_bottle/backend/docker/bottle.py index 0a1b781..4670748 100644 --- a/claude_bottle/backend/docker/bottle.py +++ b/claude_bottle/backend/docker/bottle.py @@ -28,7 +28,15 @@ class DockerBottle(Bottle): self._prompt_path = prompt_path_in_container self._closed = False - def exec_claude(self, argv: list[str], *, tty: bool = True) -> int: + def claude_docker_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]) @@ -36,7 +44,12 @@ class DockerBottle(Bottle): if tty: cmd.append("-it") cmd.extend([self.name, "claude", *full_argv]) - return subprocess.run(cmd, check=False).returncode + return cmd + + def exec_claude(self, argv: list[str], *, tty: bool = True) -> int: + return subprocess.run( + self.claude_docker_argv(argv, tty=tty), check=False, + ).returncode def exec(self, script: str) -> ExecResult: # Pipe via stdin to `sh -s` so the caller never has to worry diff --git a/tests/unit/test_docker_bottle.py b/tests/unit/test_docker_bottle.py new file mode 100644 index 0000000..01bf890 --- /dev/null +++ b/tests/unit/test_docker_bottle.py @@ -0,0 +1,84 @@ +"""Unit: DockerBottle's argv builder (PRD 0021 chunk 1). + +`claude_docker_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 +path can rely on identical behavior. +""" + +from __future__ import annotations + +import unittest + +from claude_bottle.backend.docker.bottle import DockerBottle + + +def _bottle(prompt_path: str | None = None) -> DockerBottle: + return DockerBottle( + container="claude-bottle-dev-abc", + teardown=lambda: None, + prompt_path_in_container=prompt_path, + ) + + +class TestClaudeDockerArgv(unittest.TestCase): + def test_minimal_argv_no_prompt(self): + argv = _bottle().claude_docker_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( + ["--dangerously-skip-permissions", "--continue"], + ) + self.assertEqual( + ["docker", "exec", "-it", "claude-bottle-dev-abc", "claude", + "--dangerously-skip-permissions", "--continue"], + argv, + ) + + def test_appends_prompt_file_flag_when_set(self): + argv = _bottle("/home/node/.claude-bottle-prompt.txt").claude_docker_argv( + ["--dangerously-skip-permissions"], + ) + self.assertEqual( + ["docker", "exec", "-it", "claude-bottle-dev-abc", "claude", + "--dangerously-skip-permissions", + "--append-system-prompt-file", + "/home/node/.claude-bottle-prompt.txt"], + argv, + ) + + def test_no_prompt_flag_when_none(self): + argv = _bottle(None).claude_docker_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"]) + self.assertNotIn("--append-system-prompt-file", argv) + + def test_tty_false_drops_it_flag(self): + argv = _bottle().claude_docker_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 + # 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) + self.assertEqual(["--continue"], original) + + +if __name__ == "__main__": + unittest.main()