refactor(bottles): introduce BottlePlan base + move print onto plan
test / run tests/run_tests.py (pull_request) Successful in 19s
test / run tests/run_tests.py (pull_request) Successful in 19s
- Add BottlePlan (frozen dataclass + ABC) with spec, stage_dir, and an abstract `print(*, remote_control)` method. - DockerBottlePlan now inherits from BottlePlan; spec/stage_dir come from the base, Docker-specific fields stay on the subclass. - Move BottleSpec from bottles/docker.py to bottles/__init__.py so the cross-platform types live together. docker.py pulls them via `from . import ...`. - Move show_plan from cli/start.py to `DockerBottlePlan.print`. Caller becomes `plan.print(remote_control=...)`. The CLI no longer reads any Docker-specific fields. - BottlePlatform.prepare is now typed `Callable[..., BottlePlan]`. cmd_start drops ~46 more lines.
This commit is contained in:
@@ -3,7 +3,7 @@
|
||||
A bottle is a running, isolated environment with claude inside. Each
|
||||
platform exposes two functions:
|
||||
|
||||
prepare(spec, stage_dir=...) -> Plan
|
||||
prepare(spec, stage_dir=...) -> BottlePlan
|
||||
Resolves names, validates host-side prerequisites, and writes
|
||||
scratch files. No remote/runtime resources are created yet.
|
||||
Safe to call before the y/N preflight.
|
||||
@@ -20,14 +20,48 @@ environment picks.
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
from abc import ABC, abstractmethod
|
||||
from contextlib import AbstractContextManager
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
from typing import Callable, Protocol
|
||||
|
||||
from ..log import die
|
||||
from .docker import BottleSpec, launch_docker_bottle, prepare_docker_bottle
|
||||
from ..manifest import Manifest
|
||||
|
||||
__all__ = ["Bottle", "BottlePlatform", "BottleSpec", "get_bottle_platform"]
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class BottleSpec:
|
||||
"""CLI-supplied intent. Platform-agnostic — each platform's prepare
|
||||
step consumes it and produces its own platform-specific plan.
|
||||
Resolved values (image names, container name, scratch paths, runsc
|
||||
availability) live on the plan, not the spec."""
|
||||
|
||||
manifest: Manifest
|
||||
agent_name: str
|
||||
copy_cwd: bool
|
||||
user_cwd: str
|
||||
forward_oauth_token: bool
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class BottlePlan(ABC):
|
||||
"""Base output of a platform's prepare step. Concrete subclasses
|
||||
(e.g. DockerBottlePlan) add platform-specific resolved fields and
|
||||
implement `print`."""
|
||||
|
||||
spec: BottleSpec
|
||||
stage_dir: Path
|
||||
|
||||
@abstractmethod
|
||||
def print(self, *, remote_control: bool) -> None:
|
||||
"""Render the y/N preflight summary to stderr."""
|
||||
|
||||
|
||||
# Import concrete platform factories AFTER the base types are defined,
|
||||
# so each platform module can pull BottleSpec / BottlePlan via
|
||||
# `from . import ...` without hitting a partially-initialized module.
|
||||
from .docker import launch_docker_bottle, prepare_docker_bottle # noqa: E402
|
||||
|
||||
|
||||
class Bottle(Protocol):
|
||||
@@ -50,7 +84,7 @@ class BottlePlatform:
|
||||
"""Bundles a platform's two-phase factory under one selectable name."""
|
||||
|
||||
name: str
|
||||
prepare: Callable[..., object]
|
||||
prepare: Callable[..., BottlePlan]
|
||||
launch: Callable[..., AbstractContextManager[Bottle]]
|
||||
|
||||
|
||||
@@ -72,3 +106,12 @@ def get_bottle_platform() -> BottlePlatform:
|
||||
known = ", ".join(sorted(_PLATFORMS))
|
||||
die(f"unknown CLAUDE_BOTTLE_PLATFORM={name!r}; known platforms: {known}")
|
||||
return _PLATFORMS[name]
|
||||
|
||||
|
||||
__all__ = [
|
||||
"Bottle",
|
||||
"BottlePlan",
|
||||
"BottlePlatform",
|
||||
"BottleSpec",
|
||||
"get_bottle_platform",
|
||||
]
|
||||
|
||||
@@ -34,7 +34,7 @@ from .. import skills as skills_mod
|
||||
from .. import ssh as ssh_mod
|
||||
from ..env_resolve import env_resolve
|
||||
from ..log import die, info
|
||||
from ..manifest import Manifest
|
||||
from . import BottlePlan, BottleSpec
|
||||
|
||||
|
||||
# --- Runtime detection -----------------------------------------------------
|
||||
@@ -51,36 +51,20 @@ def runsc_available() -> bool:
|
||||
return r.returncode == 0 and "runsc" in r.stdout
|
||||
|
||||
|
||||
# --- Spec + Plan -----------------------------------------------------------
|
||||
# --- Plan ------------------------------------------------------------------
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class BottleSpec:
|
||||
"""CLI-supplied intent. Platform-agnostic — each platform's prepare
|
||||
step consumes it and produces its own platform-specific plan.
|
||||
Resolved values (image names, container name, scratch paths, runsc
|
||||
availability) live on the plan, not the spec."""
|
||||
class DockerBottlePlan(BottlePlan):
|
||||
"""Docker-specific resolved fields produced by prepare_docker_bottle.
|
||||
Inherits `spec` and `stage_dir` from BottlePlan."""
|
||||
|
||||
manifest: Manifest
|
||||
agent_name: str
|
||||
copy_cwd: bool
|
||||
user_cwd: str
|
||||
forward_oauth_token: bool
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class DockerBottlePlan:
|
||||
"""Output of prepare_docker_bottle. Frozen; the launch step consumes
|
||||
it without further resolution. show_plan reads from it directly."""
|
||||
|
||||
spec: BottleSpec
|
||||
slug: str
|
||||
container_name: str
|
||||
container_name_pinned: bool
|
||||
image: str
|
||||
derived_image: str # "" -> no derived image
|
||||
runtime_image: str # image actually launched (derived or base)
|
||||
stage_dir: Path
|
||||
env_file: Path
|
||||
args_file: Path
|
||||
prompt_file: Path
|
||||
@@ -89,6 +73,47 @@ class DockerBottlePlan:
|
||||
allowlist_summary: str
|
||||
use_runsc: bool
|
||||
|
||||
def print(self, *, remote_control: bool) -> None:
|
||||
"""Render the y/N preflight summary to stderr. Pure presentation."""
|
||||
spec = self.spec
|
||||
manifest = spec.manifest
|
||||
agent = manifest.agents[spec.agent_name]
|
||||
bottle = manifest.bottle_for(spec.agent_name)
|
||||
|
||||
env_names = list(bottle.env.keys())
|
||||
if spec.forward_oauth_token:
|
||||
env_names.append("CLAUDE_CODE_OAUTH_TOKEN")
|
||||
|
||||
ssh_hosts = [e.Host for e in bottle.ssh]
|
||||
prompt_first_line = agent.prompt.splitlines()[0] if agent.prompt else ""
|
||||
runtime_label = "runsc (gVisor)" if self.use_runsc else "runc (default)"
|
||||
|
||||
print(file=sys.stderr)
|
||||
info(f"agent : {spec.agent_name}")
|
||||
info(f"image : {self.image}")
|
||||
if self.derived_image:
|
||||
info(
|
||||
f"cwd : {spec.user_cwd} -> /home/node/workspace "
|
||||
f"(derived: {self.derived_image})"
|
||||
)
|
||||
info(f"container : {self.container_name}")
|
||||
info(f"stage dir : {self.stage_dir}")
|
||||
info("env (names only): " + (", ".join(env_names) if env_names else "(none)"))
|
||||
info("skills : " + (" ".join(agent.skills) if agent.skills else "(none)"))
|
||||
info(f"docker runtime : {runtime_label}")
|
||||
info(f"bottle : {agent.bottle}")
|
||||
if ssh_hosts:
|
||||
info(f" ssh hosts : {', '.join(ssh_hosts)}")
|
||||
else:
|
||||
info(" ssh hosts : (none)")
|
||||
info(f" egress : {self.allowlist_summary}")
|
||||
info(
|
||||
f"prompt : {len(agent.prompt)} chars; "
|
||||
f"first line: {prompt_first_line or '(empty)'}"
|
||||
)
|
||||
info("remote-control : " + ("enabled" if remote_control else "disabled"))
|
||||
print(file=sys.stderr)
|
||||
|
||||
|
||||
# --- Bottle handle ---------------------------------------------------------
|
||||
|
||||
|
||||
@@ -12,55 +12,11 @@ import tempfile
|
||||
from pathlib import Path
|
||||
|
||||
from ..bottles import BottleSpec, get_bottle_platform
|
||||
from ..bottles.docker import DockerBottlePlan
|
||||
from ..log import info
|
||||
from ..manifest import Manifest
|
||||
from ._common import PROG, USER_CWD, read_tty_line
|
||||
|
||||
|
||||
def show_plan(plan: DockerBottlePlan, *, remote_control: bool) -> None:
|
||||
"""Render the y/N preflight summary to stderr. Reads everything off
|
||||
the plan; pure presentation."""
|
||||
spec = plan.spec
|
||||
manifest = spec.manifest
|
||||
agent = manifest.agents[spec.agent_name]
|
||||
bottle = manifest.bottle_for(spec.agent_name)
|
||||
|
||||
env_names = list(bottle.env.keys())
|
||||
if spec.forward_oauth_token:
|
||||
env_names.append("CLAUDE_CODE_OAUTH_TOKEN")
|
||||
|
||||
ssh_hosts = [e.Host for e in bottle.ssh]
|
||||
prompt_first_line = agent.prompt.splitlines()[0] if agent.prompt else ""
|
||||
runtime_label = "runsc (gVisor)" if plan.use_runsc else "runc (default)"
|
||||
|
||||
print(file=sys.stderr)
|
||||
info(f"agent : {spec.agent_name}")
|
||||
info(f"image : {plan.image}")
|
||||
if plan.derived_image:
|
||||
info(
|
||||
f"cwd : {spec.user_cwd} -> /home/node/workspace "
|
||||
f"(derived: {plan.derived_image})"
|
||||
)
|
||||
info(f"container : {plan.container_name}")
|
||||
info(f"stage dir : {plan.stage_dir}")
|
||||
info("env (names only): " + (", ".join(env_names) if env_names else "(none)"))
|
||||
info("skills : " + (" ".join(agent.skills) if agent.skills else "(none)"))
|
||||
info(f"docker runtime : {runtime_label}")
|
||||
info(f"bottle : {agent.bottle}")
|
||||
if ssh_hosts:
|
||||
info(f" ssh hosts : {', '.join(ssh_hosts)}")
|
||||
else:
|
||||
info(" ssh hosts : (none)")
|
||||
info(f" egress : {plan.allowlist_summary}")
|
||||
info(
|
||||
f"prompt : {len(agent.prompt)} chars; "
|
||||
f"first line: {prompt_first_line or '(empty)'}"
|
||||
)
|
||||
info("remote-control : " + ("enabled" if remote_control else "disabled"))
|
||||
print(file=sys.stderr)
|
||||
|
||||
|
||||
def cmd_start(argv: list[str]) -> int:
|
||||
parser = argparse.ArgumentParser(prog=f"{PROG} start", add_help=True)
|
||||
parser.add_argument("--dry-run", action="store_true")
|
||||
@@ -84,7 +40,7 @@ def cmd_start(argv: list[str]) -> int:
|
||||
try:
|
||||
platform = get_bottle_platform()
|
||||
plan = platform.prepare(spec, stage_dir=stage_dir)
|
||||
show_plan(plan, remote_control=args.remote_control)
|
||||
plan.print(remote_control=args.remote_control)
|
||||
|
||||
if dry_run:
|
||||
info("dry-run requested; not starting container.")
|
||||
|
||||
Reference in New Issue
Block a user