fix(dashboard): hoist claude_argv to Bottle ABC so smolmachines pane attach works
Launching a smolmachines agent from the dashboard inside tmux crashed with AttributeError: 'SmolmachinesBottle' object has no attribute 'claude_docker_argv' because the tmux pane-respawn path called `bottle.claude_docker_argv(...)` directly — a method that only existed on DockerBottle. The foreground-handoff path (curses endwin → subprocess.run → restore) doesn't hit it; it goes through `bottle.exec_claude` which is on the ABC. - Move the argv builder onto the `Bottle` ABC as `claude_argv(argv, *, tty=True) -> list[str]`. Both backends implement it; both `exec_claude` impls collapse to `subprocess.run(self.claude_argv(argv, tty=tty), check=False)`. - DockerBottle: rename `claude_docker_argv` → `claude_argv`, body unchanged. - SmolmachinesBottle: extract the argv-building from `exec_claude` into `claude_argv`; the new method returns the full `smolvm machine exec --name … -- runuser -u node -- claude …` argv. The `runuser` switch lives on the exec-framing prefix so the dashboard's `_build_resume_argv_with_fallback` split-at-"claude" trick keeps the UID switch when wrapping the claude tail in `sh -c "… --continue || …"`. - Dashboard: drop the docker-specific wording — local + helper arg names `docker_argv` → `claude_argv`; docstrings on `_build_resume_argv_with_fallback`, `_build_split_pane_argv`, `_build_respawn_pane_argv` now say "backend-exec argv". The shell-fallback wrap is unchanged; the existing logic works for smolmachines because `claude` is still the marker token. Tests: - `tests/unit/test_smolmachines_bottle.py` (new): locks down the smolmachines argv shape — prompt-file flag injection, guest-env `-e K=V` forwarding, TTY toggle, runuser-precedes- claude invariant. - `test_docker_bottle.py`: TestClaudeDockerArgv → TestClaudeArgv; method renames follow. - `test_dashboard_active_agents.py`: docstring follow. 615 unit tests pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit was merged in pull request #81.
This commit is contained in:
@@ -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