Cleanup backend and agent provider abstractions #216
@@ -20,6 +20,7 @@ Per PRD 0050 the per-provider implementations live under
|
||||
from __future__ import annotations
|
||||
|
||||
import importlib.util
|
||||
import inspect
|
||||
import os
|
||||
import shlex
|
||||
import tempfile
|
||||
@@ -51,7 +52,6 @@ class AgentProviderRuntime:
|
||||
template: str
|
||||
command: str
|
||||
image: str
|
||||
dockerfile: str
|
||||
prompt_mode: PromptMode
|
||||
bypass_args: tuple[str, ...]
|
||||
resume_args: tuple[str, ...]
|
||||
@@ -127,6 +127,18 @@ class AgentProvider(ABC):
|
||||
"""The static command / image / prompt-mode table for this
|
||||
template."""
|
||||
|
||||
@property
|
||||
def dockerfile(self) -> Path | None:
|
||||
|
didericis marked this conversation as resolved
Outdated
|
||||
"""Path to the provider's Dockerfile, or None if no Dockerfile
|
||||
is declared.
|
||||
|
||||
Default: looks for a `Dockerfile` file next to this provider's
|
||||
`agent_provider.py` module. Override to point at a non-standard
|
||||
path, or return None to signal that no Dockerfile exists (the
|
||||
provider relies on a pre-built image)."""
|
||||
path = Path(inspect.getfile(type(self))).parent / "Dockerfile"
|
||||
return path if path.is_file() else None
|
||||
|
||||
@abstractmethod
|
||||
def provision_plan(
|
||||
self,
|
||||
|
||||
@@ -34,6 +34,7 @@ import shutil
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
from ...agent_provider import get_provider
|
||||
from ...log import info, warn
|
||||
from .bottle_state import (
|
||||
mark_preserved,
|
||||
@@ -93,11 +94,11 @@ def fetch_current_dockerfile(slug: str) -> str:
|
||||
override = per_bottle_dockerfile(slug)
|
||||
if override is not None:
|
||||
return override
|
||||
repo_dockerfile = _repo_dockerfile_path()
|
||||
if repo_dockerfile.is_file():
|
||||
repo_dockerfile = get_provider("claude").dockerfile
|
||||
if repo_dockerfile is not None and repo_dockerfile.is_file():
|
||||
return repo_dockerfile.read_text()
|
||||
raise CapabilityApplyError(
|
||||
f"no per-bottle Dockerfile for {slug} and no repo Dockerfile at "
|
||||
f"no per-bottle Dockerfile for {slug} and no provider Dockerfile at "
|
||||
f"{repo_dockerfile}"
|
||||
)
|
||||
|
||||
@@ -125,12 +126,6 @@ def apply_capability_change(slug: str, new_dockerfile: str) -> tuple[str, str]:
|
||||
# --- Internals -------------------------------------------------------------
|
||||
|
||||
|
||||
def _repo_dockerfile_path() -> Path:
|
||||
"""Path to the Claude provider Dockerfile. Resolved at call time so
|
||||
the path is correct regardless of where this module is imported from."""
|
||||
# bot_bottle/backend/docker/ -> bot_bottle/ -> contrib/claude/Dockerfile
|
||||
return Path(__file__).resolve().parent.parent.parent / "contrib" / "claude" / "Dockerfile"
|
||||
|
||||
|
||||
def snapshot_transcript(slug: str) -> None:
|
||||
"""`docker cp` /home/node/.claude out of the agent container into
|
||||
|
||||
@@ -15,7 +15,7 @@ from datetime import datetime, timezone
|
||||
from dataclasses import replace
|
||||
from pathlib import Path
|
||||
|
||||
from ...agent_provider import PROVIDER_TEMPLATES, agent_provision_plan, runtime_for
|
||||
from ...agent_provider import PROVIDER_TEMPLATES, agent_provision_plan, get_provider
|
||||
from ...egress import Egress
|
||||
from ...env import ResolvedEnv, resolve_env
|
||||
from ...git_gate import GitGate
|
||||
@@ -59,7 +59,8 @@ def resolve_plan(
|
||||
agent = manifest.agents[spec.agent_name]
|
||||
bottle = manifest.bottle_for(spec.agent_name)
|
||||
provider = bottle.agent_provider
|
||||
provider_runtime = runtime_for(provider.template)
|
||||
provider_obj = get_provider(provider.template)
|
||||
|
didericis marked this conversation as resolved
Outdated
didericis
commented
We shouldn't be doing this/we already have the provider object on line 61. We shouldn't be doing this/we already have the provider object on line 61.
|
||||
provider_runtime = provider_obj.runtime
|
||||
guest_home = "/home/node"
|
||||
workspace_plan = resolve_workspace_plan(spec, guest_home=guest_home)
|
||||
|
||||
@@ -99,20 +100,16 @@ def resolve_plan(
|
||||
elif provider.dockerfile:
|
||||
image_default = f"bot-bottle-{provider.template}:{slug}"
|
||||
dockerfile_path = _resolve_manifest_dockerfile(provider.dockerfile, spec)
|
||||
elif provider_runtime.dockerfile:
|
||||
image_default = provider_runtime.image
|
||||
dockerfile_path = provider_runtime.dockerfile
|
||||
elif provider.template not in PROVIDER_TEMPLATES:
|
||||
user_dockerfile = (
|
||||
Path.home() / ".bot-bottle" / "contrib" / provider.template / "Dockerfile"
|
||||
)
|
||||
if user_dockerfile.is_file():
|
||||
image_default = f"bot-bottle-{provider.template}:{slug}"
|
||||
dockerfile_path = str(user_dockerfile)
|
||||
else:
|
||||
|
didericis marked this conversation as resolved
Outdated
didericis
commented
A provider should always have a dockerfile now, so this else shouldn't be necessary. A provider should always have a dockerfile now, so this else shouldn't be necessary.
|
||||
p_dockerfile = provider_obj.dockerfile
|
||||
if p_dockerfile is not None:
|
||||
if provider.template in PROVIDER_TEMPLATES:
|
||||
image_default = provider_runtime.image
|
||||
else:
|
||||
image_default = f"bot-bottle-{provider.template}:{slug}"
|
||||
dockerfile_path = str(p_dockerfile)
|
||||
|
didericis marked this conversation as resolved
Outdated
didericis
commented
This rename used to make sense, but not anymore: places where we're assigning This rename used to make sense, but not anymore: places where we're assigning `image_default` should be changed so we assign to `image` and we should just remove this `image = image_default` assignment.
|
||||
else:
|
||||
image_default = provider_runtime.image
|
||||
else:
|
||||
image_default = provider_runtime.image
|
||||
image = os.environ.get("BOT_BOTTLE_IMAGE", image_default)
|
||||
|
didericis marked this conversation as resolved
Outdated
didericis
commented
Are we using this anywhere? Not sure if it's worth keeping this. If it's being used should be renamed to Are we using this anywhere? Not sure if it's worth keeping this. If it's being used should be renamed to `BOT_BOTTLE_AGENT_IMAGE`.
|
||||
derived_image = ""
|
||||
runtime_image = image
|
||||
@@ -216,13 +213,11 @@ def resolve_plan(
|
||||
# moved it behind the `list-egress-routes` MCP tool so the
|
||||
# agent gets live state rather than a launch-time snapshot.)
|
||||
supervise_dockerfile_path = (
|
||||
Path(dockerfile_path)
|
||||
if dockerfile_path
|
||||
else Path(__file__).resolve().parent.parent.parent / "contrib" / "claude" / "Dockerfile"
|
||||
Path(dockerfile_path) if dockerfile_path else provider_obj.dockerfile
|
||||
)
|
||||
dockerfile_content = (
|
||||
supervise_dockerfile_path.read_text(encoding="utf-8")
|
||||
if supervise_dockerfile_path.is_file()
|
||||
if supervise_dockerfile_path is not None and supervise_dockerfile_path.is_file()
|
||||
else ""
|
||||
)
|
||||
supervise_dir = supervise_state_dir(slug)
|
||||
|
||||
@@ -15,7 +15,7 @@ from datetime import datetime, timezone
|
||||
from dataclasses import replace
|
||||
from pathlib import Path
|
||||
|
||||
from ...agent_provider import agent_provision_plan, runtime_for
|
||||
from ...agent_provider import PROVIDER_TEMPLATES, agent_provision_plan, get_provider
|
||||
from ...backend import BottleSpec
|
||||
from ...backend.docker.bottle_state import (
|
||||
BottleMetadata,
|
||||
@@ -57,7 +57,8 @@ def resolve_plan(
|
||||
manifest = spec.manifest
|
||||
bottle = manifest.bottle_for(spec.agent_name)
|
||||
provider = bottle.agent_provider
|
||||
provider_runtime = runtime_for(provider.template)
|
||||
provider_obj = get_provider(provider.template)
|
||||
provider_runtime = provider_obj.runtime
|
||||
guest_home = "/home/node"
|
||||
workspace_plan = resolve_workspace_plan(spec, guest_home=guest_home)
|
||||
|
||||
@@ -122,11 +123,16 @@ def resolve_plan(
|
||||
if provider.dockerfile:
|
||||
agent_dockerfile_path = _resolve_manifest_dockerfile(provider.dockerfile, spec)
|
||||
image_default = f"bot-bottle-{provider.template}:{slug}"
|
||||
elif provider_runtime.dockerfile:
|
||||
agent_dockerfile_path = provider_runtime.dockerfile
|
||||
image_default = provider_runtime.image
|
||||
else:
|
||||
image_default = provider_runtime.image
|
||||
p_dockerfile = provider_obj.dockerfile
|
||||
if p_dockerfile is not None:
|
||||
agent_dockerfile_path = str(p_dockerfile)
|
||||
if provider.template in PROVIDER_TEMPLATES:
|
||||
image_default = provider_runtime.image
|
||||
else:
|
||||
image_default = f"bot-bottle-{provider.template}:{slug}"
|
||||
else:
|
||||
image_default = provider_runtime.image
|
||||
agent_image_ref = os.environ.get("BOT_BOTTLE_IMAGE", image_default)
|
||||
agent_provision = agent_provision_plan(
|
||||
template=provider.template,
|
||||
|
||||
@@ -42,7 +42,6 @@ _RUNTIME = AgentProviderRuntime(
|
||||
template="claude",
|
||||
command="claude",
|
||||
image="bot-bottle-claude:latest",
|
||||
dockerfile=str(Path(__file__).resolve().parent / "Dockerfile"),
|
||||
prompt_mode="append_file",
|
||||
bypass_args=("--dangerously-skip-permissions",),
|
||||
resume_args=("--continue",),
|
||||
|
||||
@@ -50,7 +50,6 @@ _RUNTIME = AgentProviderRuntime(
|
||||
template="codex",
|
||||
command="codex",
|
||||
image="bot-bottle-codex:latest",
|
||||
dockerfile=str(Path(__file__).resolve().parent / "Dockerfile"),
|
||||
prompt_mode="read_prompt_file",
|
||||
bypass_args=("--dangerously-bypass-approvals-and-sandbox",),
|
||||
resume_args=("resume", "--last"),
|
||||
|
||||
@@ -30,7 +30,7 @@ class _Provider(AgentProvider):
|
||||
@property
|
||||
def runtime(self) -> AgentProviderRuntime:
|
||||
return AgentProviderRuntime(
|
||||
template="test", command="test", image="", dockerfile="",
|
||||
template="test", command="test", image="",
|
||||
prompt_mode="append_file", bypass_args=(), resume_args=(),
|
||||
remote_control_args=(),
|
||||
)
|
||||
|
||||
@@ -61,7 +61,6 @@ class TestSmolmachinesResolveEnv(unittest.TestCase):
|
||||
patch(
|
||||
"bot_bottle.backend.smolmachines.prepare.agent_provision_plan"
|
||||
) as mock_app,
|
||||
patch("bot_bottle.backend.smolmachines.prepare.runtime_for"),
|
||||
):
|
||||
mock_gg.return_value.prepare.return_value = MagicMock()
|
||||
mock_eg.return_value.prepare.return_value = MagicMock()
|
||||
|
||||
@@ -43,7 +43,7 @@ class _Provider(AgentProvider):
|
||||
@property
|
||||
def runtime(self) -> AgentProviderRuntime:
|
||||
return AgentProviderRuntime(
|
||||
template="test", command="test", image="", dockerfile="",
|
||||
template="test", command="test", image="",
|
||||
prompt_mode="append_file", bypass_args=(), resume_args=(),
|
||||
remote_control_args=(),
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user
This should always return a docker/should never return
None