refactor(contrib): inline provision steps per-provider, drop shared apply module
Each AgentProvider now owns its skills / prompt / provision / supervise_mcp end-to-end. The base ABC declares all four as abstract; ClaudeAgentProvider and CodexAgentProvider each carry their own copy loop. Per PR review feedback (review #128): the shared _provision_apply.py abstraction was weak — Claude and Codex harnesses already diverge (codex's dummy-auth + login-status verify has no claude analogue) and forcing both onto one helper just postpones the split. Duplication is intentional. Deletes bot_bottle/_provision_apply.py and consolidates testing under tests/unit/test_contrib_{claude,codex}_provider.py (one file per provider, covering all four methods).
This commit is contained in:
@@ -1,117 +0,0 @@
|
||||
"""Shared apply primitives used by every `AgentProvider` (PRD 0050).
|
||||
|
||||
The skills/prompt/provision loops here are backend-agnostic — they
|
||||
only touch the `Bottle.exec`/`Bottle.cp_in` primitives that every
|
||||
backend implements. Each provider's `provision_skills` /
|
||||
`provision_prompt` / `provision` defaults delegate here so the
|
||||
business logic lives in exactly one place."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import shlex
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
from .agent_provider import GUEST_HOME
|
||||
from .log import die, info
|
||||
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from .backend import Bottle, BottlePlan
|
||||
|
||||
|
||||
# In-guest paths agents look at. These are baked into the bot-bottle
|
||||
# Dockerfile and the smolmachines guest image; no env knob exposed,
|
||||
# per the PRD 0050 issue feedback.
|
||||
SKILLS_DIR = f"{GUEST_HOME}/.claude/skills"
|
||||
PROMPT_PATH = f"{GUEST_HOME}/.bot-bottle-prompt.txt"
|
||||
|
||||
|
||||
def apply_skills(plan: "BottlePlan", bottle: "Bottle") -> None:
|
||||
"""Copy each named skill tree from the host into the guest.
|
||||
No-op when the agent has no skills.
|
||||
|
||||
`cp_in` lands files as root in every backend; chown back to
|
||||
node:node so the agent can read its own skills."""
|
||||
from .backend.util import host_skill_dir
|
||||
|
||||
agent = plan.spec.manifest.agents[plan.spec.agent_name]
|
||||
if not agent.skills:
|
||||
return
|
||||
|
||||
bottle.exec(f"mkdir -p {SKILLS_DIR}", user="root")
|
||||
|
||||
for name in agent.skills:
|
||||
src = host_skill_dir(name)
|
||||
if not os.path.isdir(src):
|
||||
die(
|
||||
f"skill {name!r} disappeared from host between "
|
||||
f"validation and copy at {src}."
|
||||
)
|
||||
dst = f"{SKILLS_DIR}/{name}"
|
||||
info(f"copying skill {name} into {bottle.name}:{dst}")
|
||||
bottle.exec(f"rm -rf {dst} && mkdir -p {dst}", user="root")
|
||||
bottle.cp_in(f"{src}/.", f"{dst}/")
|
||||
bottle.exec(f"chown -R node:node {dst}", user="root")
|
||||
|
||||
|
||||
def apply_prompt(plan: "BottlePlan", bottle: "Bottle") -> str | None:
|
||||
"""Copy the prompt file into the guest, fix ownership/mode.
|
||||
Returns the in-guest path iff the agent has a non-empty prompt
|
||||
(drives `--append-system-prompt-file`); the file is copied either
|
||||
way so the path always exists."""
|
||||
bottle.cp_in(str(plan.prompt_file), PROMPT_PATH)
|
||||
bottle.exec(
|
||||
f"chown node:node {PROMPT_PATH} && chmod 600 {PROMPT_PATH}",
|
||||
user="root",
|
||||
)
|
||||
agent = plan.spec.manifest.agents[plan.spec.agent_name]
|
||||
return PROMPT_PATH if agent.prompt else None
|
||||
|
||||
|
||||
def apply_provision(plan: "BottlePlan", bottle: "Bottle") -> None:
|
||||
"""Apply the declarative `dirs`/`pre_copy`/`files`/`verify` steps
|
||||
from the provider's `AgentProvisionPlan`. This is the loop that
|
||||
used to live in
|
||||
`backend/<name>/provision/provider_auth.py:provision_provider_auth`
|
||||
pre-PRD-0050."""
|
||||
provision = plan.agent_provision
|
||||
for d in provision.dirs:
|
||||
path = shlex.quote(d.guest_path)
|
||||
_exec(bottle, f"mkdir -p {path}", f"could not create {d.guest_path}")
|
||||
_exec(
|
||||
bottle,
|
||||
f"chown {shlex.quote(d.owner)} {path}",
|
||||
f"could not chown {d.guest_path}",
|
||||
)
|
||||
_exec(
|
||||
bottle,
|
||||
f"chmod {shlex.quote(d.mode)} {path}",
|
||||
f"could not chmod {d.guest_path}",
|
||||
)
|
||||
for command in provision.pre_copy:
|
||||
_exec(bottle, shlex.join(command.argv), command.error)
|
||||
for f in provision.files:
|
||||
bottle.cp_in(str(f.host_path), f.guest_path)
|
||||
path = shlex.quote(f.guest_path)
|
||||
_exec(
|
||||
bottle,
|
||||
f"chown {shlex.quote(f.owner)} {path}",
|
||||
f"could not chown {f.guest_path}",
|
||||
)
|
||||
_exec(
|
||||
bottle,
|
||||
f"chmod {shlex.quote(f.mode)} {path}",
|
||||
f"could not chmod {f.guest_path}",
|
||||
)
|
||||
for command in provision.verify:
|
||||
_exec(bottle, shlex.join(command.argv), command.error)
|
||||
|
||||
|
||||
def _exec(bottle: "Bottle", script: str, error: str) -> None:
|
||||
result = bottle.exec(script, user="root")
|
||||
if result.returncode != 0:
|
||||
detail = (result.stderr or result.stdout).strip()
|
||||
if detail:
|
||||
detail = f": {detail}"
|
||||
die(f"agent provider provisioning: {error}{detail}")
|
||||
@@ -142,35 +142,27 @@ class AgentProvider(ABC):
|
||||
Backends call this during `prepare` and consume the result as
|
||||
before."""
|
||||
|
||||
@abstractmethod
|
||||
def provision_skills(self, plan: "BottlePlan", bottle: "Bottle") -> None:
|
||||
"""Copy each of the agent's named skills from
|
||||
~/.claude/skills/<name>/ on the host into
|
||||
/home/node/.claude/skills/<name>/ in the guest. No-op when
|
||||
the agent has no skills.
|
||||
|
||||
Default implementation matches the legacy backend-side
|
||||
modules; providers override only if the in-guest layout
|
||||
differs."""
|
||||
from . import _provision_apply
|
||||
_provision_apply.apply_skills(plan, bottle)
|
||||
"""Copy each of the agent's named skills from the host into
|
||||
the guest. No-op when the agent has no skills. The in-guest
|
||||
layout is provider-specific (claude-code's
|
||||
`~/.claude/skills/` today; future providers may differ)."""
|
||||
|
||||
@abstractmethod
|
||||
def provision_prompt(self, plan: "BottlePlan", bottle: "Bottle") -> str | None:
|
||||
"""Copy the prompt file into the guest, fix ownership/mode,
|
||||
and return the in-guest path iff the agent has a non-empty
|
||||
prompt (drives the `--append-system-prompt-file` flag).
|
||||
|
||||
The file is copied either way so the path always exists."""
|
||||
from . import _provision_apply
|
||||
return _provision_apply.apply_prompt(plan, bottle)
|
||||
|
||||
@abstractmethod
|
||||
def provision(self, plan: "BottlePlan", bottle: "Bottle") -> None:
|
||||
"""Apply the declarative `dirs`/`pre_copy`/`files`/`verify`
|
||||
steps from `plan.agent_provision`. Default implementation
|
||||
works for any provider that produces a standard plan; was
|
||||
called `provision_provider_auth` on `BottleBackend` before
|
||||
PRD 0050."""
|
||||
from . import _provision_apply
|
||||
_provision_apply.apply_provision(plan, bottle)
|
||||
"""Apply the provider's declarative
|
||||
`dirs`/`pre_copy`/`files`/`verify` steps from
|
||||
`plan.agent_provision`. Was called `provision_provider_auth`
|
||||
on `BottleBackend` before PRD 0050."""
|
||||
|
||||
@abstractmethod
|
||||
def provision_supervise_mcp(
|
||||
|
||||
@@ -9,6 +9,8 @@ sidecar in claude-code's user config (PRD 0013)."""
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
import shlex
|
||||
from pathlib import Path
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
@@ -20,7 +22,7 @@ from ...agent_provider import (
|
||||
AgentProvisionPlan,
|
||||
)
|
||||
from ...egress import EgressRoute
|
||||
from ...log import info, warn
|
||||
from ...log import die, info, warn
|
||||
|
||||
|
||||
if TYPE_CHECKING:
|
||||
@@ -30,6 +32,8 @@ if TYPE_CHECKING:
|
||||
_REPO_ROOT = Path(__file__).resolve().parents[3]
|
||||
|
||||
_SUPERVISE_MCP_NAME = "supervise"
|
||||
_SKILLS_DIR = f"{GUEST_HOME}/.claude/skills"
|
||||
_PROMPT_PATH = f"{GUEST_HOME}/.bot-bottle-prompt.txt"
|
||||
|
||||
_RUNTIME = AgentProviderRuntime(
|
||||
template="claude",
|
||||
@@ -105,6 +109,79 @@ class ClaudeAgentProvider(AgentProvider):
|
||||
hidden_env_names=hidden_env_names,
|
||||
)
|
||||
|
||||
def provision_skills(self, plan: "BottlePlan", bottle: "Bottle") -> None:
|
||||
"""Copy each named skill tree from `~/.claude/skills/<name>/`
|
||||
on the host into the guest's claude-code skills dir. No-op
|
||||
when the agent has no skills."""
|
||||
from ...backend.util import host_skill_dir
|
||||
|
||||
agent = plan.spec.manifest.agents[plan.spec.agent_name]
|
||||
if not agent.skills:
|
||||
return
|
||||
bottle.exec(f"mkdir -p {_SKILLS_DIR}", user="root")
|
||||
for name in agent.skills:
|
||||
src = host_skill_dir(name)
|
||||
if not os.path.isdir(src):
|
||||
die(
|
||||
f"skill {name!r} disappeared from host between "
|
||||
f"validation and copy at {src}."
|
||||
)
|
||||
dst = f"{_SKILLS_DIR}/{name}"
|
||||
info(f"copying skill {name} into {bottle.name}:{dst}")
|
||||
bottle.exec(f"rm -rf {dst} && mkdir -p {dst}", user="root")
|
||||
bottle.cp_in(f"{src}/.", f"{dst}/")
|
||||
bottle.exec(f"chown -R node:node {dst}", user="root")
|
||||
|
||||
def provision_prompt(self, plan: "BottlePlan", bottle: "Bottle") -> str | None:
|
||||
"""Copy the prompt file into the guest, fix ownership/mode.
|
||||
Returns the in-guest path iff the agent has a non-empty
|
||||
prompt (drives `--append-system-prompt-file`); the file is
|
||||
copied either way so the path always exists."""
|
||||
bottle.cp_in(str(plan.prompt_file), _PROMPT_PATH)
|
||||
bottle.exec(
|
||||
f"chown node:node {_PROMPT_PATH} && chmod 600 {_PROMPT_PATH}",
|
||||
user="root",
|
||||
)
|
||||
agent = plan.spec.manifest.agents[plan.spec.agent_name]
|
||||
return _PROMPT_PATH if agent.prompt else None
|
||||
|
||||
def provision(self, plan: "BottlePlan", bottle: "Bottle") -> None:
|
||||
"""Apply the claude-side declarative provision steps from
|
||||
`plan.agent_provision` — today that's the `claude.json`
|
||||
trust-marker file. Hot-replace this with a richer flow as
|
||||
claude-code's harness shape evolves."""
|
||||
provision = plan.agent_provision
|
||||
for d in provision.dirs:
|
||||
path = shlex.quote(d.guest_path)
|
||||
_exec(bottle, f"mkdir -p {path}", f"could not create {d.guest_path}")
|
||||
_exec(
|
||||
bottle,
|
||||
f"chown {shlex.quote(d.owner)} {path}",
|
||||
f"could not chown {d.guest_path}",
|
||||
)
|
||||
_exec(
|
||||
bottle,
|
||||
f"chmod {shlex.quote(d.mode)} {path}",
|
||||
f"could not chmod {d.guest_path}",
|
||||
)
|
||||
for command in provision.pre_copy:
|
||||
_exec(bottle, shlex.join(command.argv), command.error)
|
||||
for f in provision.files:
|
||||
bottle.cp_in(str(f.host_path), f.guest_path)
|
||||
path = shlex.quote(f.guest_path)
|
||||
_exec(
|
||||
bottle,
|
||||
f"chown {shlex.quote(f.owner)} {path}",
|
||||
f"could not chown {f.guest_path}",
|
||||
)
|
||||
_exec(
|
||||
bottle,
|
||||
f"chmod {shlex.quote(f.mode)} {path}",
|
||||
f"could not chmod {f.guest_path}",
|
||||
)
|
||||
for command in provision.verify:
|
||||
_exec(bottle, shlex.join(command.argv), command.error)
|
||||
|
||||
def provision_supervise_mcp(
|
||||
self,
|
||||
plan: "BottlePlan",
|
||||
@@ -131,3 +208,12 @@ class ClaudeAgentProvider(AgentProvider):
|
||||
f"register manually with: "
|
||||
f"claude mcp add --scope user --transport http supervise {supervise_url}"
|
||||
)
|
||||
|
||||
|
||||
def _exec(bottle: "Bottle", script: str, error: str) -> None:
|
||||
result = bottle.exec(script, user="root")
|
||||
if result.returncode != 0:
|
||||
detail = (result.stderr or result.stdout).strip()
|
||||
if detail:
|
||||
detail = f": {detail}"
|
||||
die(f"agent provider provisioning: {error}{detail}")
|
||||
|
||||
@@ -10,6 +10,7 @@ invocation that registers the supervise sidecar in Codex's
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import shlex
|
||||
from pathlib import Path
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
@@ -25,7 +26,7 @@ from ...agent_provider import (
|
||||
)
|
||||
from ...codex_auth import codex_host_access_token, write_codex_dummy_auth_file
|
||||
from ...egress import CODEX_HOST_CREDENTIAL_TOKEN_REF, EgressRoute
|
||||
from ...log import info, warn
|
||||
from ...log import die, info, warn
|
||||
|
||||
|
||||
if TYPE_CHECKING:
|
||||
@@ -35,6 +36,11 @@ if TYPE_CHECKING:
|
||||
_REPO_ROOT = Path(__file__).resolve().parents[3]
|
||||
|
||||
_SUPERVISE_MCP_NAME = "supervise"
|
||||
# Codex agents still read skills from the claude-code convention
|
||||
# (~/.claude/skills/) — the bot-bottle-codex image follows the same
|
||||
# layout. If Codex grows native skill discovery later, override here.
|
||||
_SKILLS_DIR = f"{GUEST_HOME}/.claude/skills"
|
||||
_PROMPT_PATH = f"{GUEST_HOME}/.bot-bottle-prompt.txt"
|
||||
|
||||
_RUNTIME = AgentProviderRuntime(
|
||||
template="codex",
|
||||
@@ -147,6 +153,79 @@ class CodexAgentProvider(AgentProvider):
|
||||
provisioned_env=provisioned_env,
|
||||
)
|
||||
|
||||
def provision_skills(self, plan: "BottlePlan", bottle: "Bottle") -> None:
|
||||
"""Copy each named skill tree from `~/.claude/skills/<name>/`
|
||||
on the host into the guest. No-op when the agent has no
|
||||
skills."""
|
||||
from ...backend.util import host_skill_dir
|
||||
|
||||
agent = plan.spec.manifest.agents[plan.spec.agent_name]
|
||||
if not agent.skills:
|
||||
return
|
||||
bottle.exec(f"mkdir -p {_SKILLS_DIR}", user="root")
|
||||
for name in agent.skills:
|
||||
src = host_skill_dir(name)
|
||||
if not os.path.isdir(src):
|
||||
die(
|
||||
f"skill {name!r} disappeared from host between "
|
||||
f"validation and copy at {src}."
|
||||
)
|
||||
dst = f"{_SKILLS_DIR}/{name}"
|
||||
info(f"copying skill {name} into {bottle.name}:{dst}")
|
||||
bottle.exec(f"rm -rf {dst} && mkdir -p {dst}", user="root")
|
||||
bottle.cp_in(f"{src}/.", f"{dst}/")
|
||||
bottle.exec(f"chown -R node:node {dst}", user="root")
|
||||
|
||||
def provision_prompt(self, plan: "BottlePlan", bottle: "Bottle") -> str | None:
|
||||
"""Copy the prompt file into the guest, fix ownership/mode.
|
||||
Codex reads it via the agent's `Read and follow the
|
||||
instructions in <path>.` bootstrap (see `prompt_args`); the
|
||||
file is copied either way so the path always exists."""
|
||||
bottle.cp_in(str(plan.prompt_file), _PROMPT_PATH)
|
||||
bottle.exec(
|
||||
f"chown node:node {_PROMPT_PATH} && chmod 600 {_PROMPT_PATH}",
|
||||
user="root",
|
||||
)
|
||||
agent = plan.spec.manifest.agents[plan.spec.agent_name]
|
||||
return _PROMPT_PATH if agent.prompt else None
|
||||
|
||||
def provision(self, plan: "BottlePlan", bottle: "Bottle") -> None:
|
||||
"""Apply the codex-side declarative provision steps from
|
||||
`plan.agent_provision`: the `~/.codex/` dir + config.toml
|
||||
trust marker, plus the dummy-auth.json drop + `codex login
|
||||
status` verify when host-credential forwarding is on."""
|
||||
provision = plan.agent_provision
|
||||
for d in provision.dirs:
|
||||
path = shlex.quote(d.guest_path)
|
||||
_exec(bottle, f"mkdir -p {path}", f"could not create {d.guest_path}")
|
||||
_exec(
|
||||
bottle,
|
||||
f"chown {shlex.quote(d.owner)} {path}",
|
||||
f"could not chown {d.guest_path}",
|
||||
)
|
||||
_exec(
|
||||
bottle,
|
||||
f"chmod {shlex.quote(d.mode)} {path}",
|
||||
f"could not chmod {d.guest_path}",
|
||||
)
|
||||
for command in provision.pre_copy:
|
||||
_exec(bottle, shlex.join(command.argv), command.error)
|
||||
for f in provision.files:
|
||||
bottle.cp_in(str(f.host_path), f.guest_path)
|
||||
path = shlex.quote(f.guest_path)
|
||||
_exec(
|
||||
bottle,
|
||||
f"chown {shlex.quote(f.owner)} {path}",
|
||||
f"could not chown {f.guest_path}",
|
||||
)
|
||||
_exec(
|
||||
bottle,
|
||||
f"chmod {shlex.quote(f.mode)} {path}",
|
||||
f"could not chmod {f.guest_path}",
|
||||
)
|
||||
for command in provision.verify:
|
||||
_exec(bottle, shlex.join(command.argv), command.error)
|
||||
|
||||
def provision_supervise_mcp(
|
||||
self,
|
||||
plan: "BottlePlan",
|
||||
@@ -173,3 +252,12 @@ class CodexAgentProvider(AgentProvider):
|
||||
f"register manually with: "
|
||||
f"codex mcp add --transport http supervise {supervise_url}"
|
||||
)
|
||||
|
||||
|
||||
def _exec(bottle: "Bottle", script: str, error: str) -> None:
|
||||
result = bottle.exec(script, user="root")
|
||||
if result.returncode != 0:
|
||||
detail = (result.stderr or result.stdout).strip()
|
||||
if detail:
|
||||
detail = f": {detail}"
|
||||
die(f"agent provider provisioning: {error}{detail}")
|
||||
|
||||
Reference in New Issue
Block a user