refactor(bottle): extract claude_docker_argv from exec_claude
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 <path>` 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).
This commit is contained in:
@@ -28,7 +28,15 @@ class DockerBottle(Bottle):
|
|||||||
self._prompt_path = prompt_path_in_container
|
self._prompt_path = prompt_path_in_container
|
||||||
self._closed = False
|
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)
|
full_argv = list(argv)
|
||||||
if self._prompt_path:
|
if self._prompt_path:
|
||||||
full_argv.extend(["--append-system-prompt-file", self._prompt_path])
|
full_argv.extend(["--append-system-prompt-file", self._prompt_path])
|
||||||
@@ -36,7 +44,12 @@ class DockerBottle(Bottle):
|
|||||||
if tty:
|
if tty:
|
||||||
cmd.append("-it")
|
cmd.append("-it")
|
||||||
cmd.extend([self.name, "claude", *full_argv])
|
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:
|
def exec(self, script: str) -> ExecResult:
|
||||||
# Pipe via stdin to `sh -s` so the caller never has to worry
|
# Pipe via stdin to `sh -s` so the caller never has to worry
|
||||||
|
|||||||
@@ -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()
|
||||||
Reference in New Issue
Block a user