fix(dashboard): hoist claude_argv to Bottle ABC so smolmachines pane attach works #81
@@ -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 <argv>`
|
||||
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: ...
|
||||
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 <c>`; smolmachines:
|
||||
`smolvm machine exec --name <m> -- 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 `<claude…> --continue || <claude…>` inside
|
||||
base_exec = bottle.claude_argv(base_args)
|
||||
# Split exec-framing prefix from the claude-and-args tail so
|
||||
# we can compose `<claude…> --continue || <claude…>` 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 <pane_id>`. `-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
|
||||
|
||||
@@ -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."""
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
|
||||
@@ -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()
|
||||
Reference in New Issue
Block a user