227 lines
8.5 KiB
Markdown
227 lines
8.5 KiB
Markdown
# PRD 0004: Split out provisioners
|
|
|
|
- **Status:** Draft
|
|
- **Author:** didericis
|
|
- **Created:** 2026-05-11
|
|
|
|
## Summary
|
|
|
|
Break `bot_bottle/backend/docker/backend.py` (664 lines) apart by
|
|
moving the four provisioner methods — `provision_prompt`,
|
|
`provision_skills`, `provision_ssh`, `provision_git` — out of
|
|
`DockerBottleBackend` into their own modules under
|
|
`bot_bottle/backend/docker/provision/`. The abstract base in
|
|
`bot_bottle/backend/__init__.py` keeps the same four-method
|
|
contract; only the Docker implementation changes shape.
|
|
|
|
## Problem
|
|
|
|
`DockerBottleBackend` is doing too much in one file. After PRD 0003
|
|
landed, the class owns:
|
|
|
|
- `prepare` — name resolution, validation, scratch file writes
|
|
- `launch` — image build, network creation, sidecar lifecycle,
|
|
`docker run`, teardown
|
|
- `_run_agent_container` — argv assembly + name-conflict retry
|
|
- `provision_prompt` / `provision_skills` / `provision_ssh` /
|
|
`provision_git` — four host→container copy paths
|
|
- `prepare_cleanup` / `cleanup` / `list_active` — orphan handling
|
|
|
|
The provisioners are the largest single chunk. `provision_ssh` alone
|
|
is ~150 lines because it sets up a root-staged keyring, an in-container
|
|
`ssh-agent`, and a `socat` forwarder so node (uid 1000) can talk to a
|
|
root-owned agent socket without ptrace access. That logic is
|
|
self-contained — it touches the container via `docker exec` and
|
|
`docker cp` and reads from `BottlePlan` — but it sits in the same file
|
|
as image build and cleanup, which makes the file hard to scan and
|
|
invites unrelated changes to land in the same diff.
|
|
|
|
The provisioners are also the most likely place for new backends to
|
|
diverge. A future fly.io backend would not run `ssh-agent` in a
|
|
sidecar this way; an Apple `container` backend might. Pulling each
|
|
provisioner into its own module makes the per-backend variation a
|
|
file boundary, not a method boundary inside a god-class.
|
|
|
|
## Goals / Success Criteria
|
|
|
|
The feature works when all of the following are observable:
|
|
|
|
- `cli.py start` produces a byte-identical container topology, env,
|
|
skills layout, SSH config, and `.git` copy as before the split. No
|
|
user-visible behavior change.
|
|
- `DockerBottleBackend` in `backend.py` is under ~350 lines, with the
|
|
four provisioner methods reduced to thin dispatchers that delegate
|
|
to the per-provisioner modules.
|
|
- The full test suite passes unchanged (unit + integration + canary).
|
|
|
|
The feature is **done** when all of the following ship:
|
|
|
|
- A new `bot_bottle/backend/docker/provision/` subpackage exists
|
|
with one module per provisioner: `prompt.py`, `skills.py`, `ssh.py`,
|
|
`git.py`. Each exports a single top-level function taking
|
|
`(plan: DockerBottlePlan, target: str)` and returning the same type
|
|
the current method returns (`str | None` for prompt, `None` for the
|
|
others).
|
|
- `DockerBottleBackend.provision_prompt` / `provision_skills` /
|
|
`provision_ssh` / `provision_git` each become one-line delegations
|
|
to the new module functions.
|
|
- The abstract `BottleBackend.provision_*` signatures in
|
|
`bot_bottle/backend/__init__.py` are unchanged. The
|
|
`BottleBackend.provision` orchestration in the base class is
|
|
unchanged.
|
|
- No top-level CLI code or other backend gains a direct import of the
|
|
provisioner modules — the only call site is
|
|
`DockerBottleBackend.provision_*`.
|
|
|
|
## Non-goals
|
|
|
|
- No change to *what* the provisioners do. The SSH provisioning's
|
|
root-keyring + ssh-agent + socat-bridge design stays exactly as it
|
|
is. The skills `docker cp <src>/. <dst>/` pattern stays. The
|
|
`.git` copy stays gated on `spec.copy_cwd` + cwd having a `.git`.
|
|
- No replacement of `launch`'s ad-hoc `state: dict[str, str]`
|
|
teardown with `contextlib.ExitStack`. That cleanup is worthwhile
|
|
but is a separate change.
|
|
- No deduplication of the two name-conflict retry loops (one in
|
|
`prepare`, one in `_run_agent_container`).
|
|
- No removal of the `os.environ["CLAUDE_CODE_OAUTH_TOKEN"]` mutation
|
|
in `_run_agent_container`. That's a parent-process side effect
|
|
worth fixing, but it's outside the provisioner split.
|
|
- No new abstract base class for provisioners (no `Provisioner` ABC).
|
|
The four functions stay module-level; the abstract surface is the
|
|
four methods on `BottleBackend`. Introducing a `Provisioner` type
|
|
would be premature with one backend.
|
|
- No change to the `BottleBackend.provision_*` method names or
|
|
signatures. Callers continue to invoke them on the backend
|
|
instance.
|
|
|
|
## Scope
|
|
|
|
### In scope
|
|
|
|
- New `bot_bottle/backend/docker/provision/` subpackage with
|
|
`__init__.py`, `prompt.py`, `skills.py`, `ssh.py`, `git.py`.
|
|
- Moving the four method bodies out of
|
|
`DockerBottleBackend` into the new modules verbatim, adjusting only
|
|
what's needed to make them free functions: `self` becomes implicit
|
|
via the `plan` argument; private helpers move with their primary
|
|
caller; imports of `docker_mod`, `network_mod`, `pipelock`,
|
|
`expand_tilde`, etc. follow them.
|
|
- Reducing the `provision_*` methods on `DockerBottleBackend` to
|
|
one-line delegations.
|
|
- Updating any tests that monkeypatch
|
|
`DockerBottleBackend.provision_*` to monkeypatch the new module
|
|
functions instead (if any do — most existing tests don't reach
|
|
into provisioning).
|
|
|
|
### Out of scope
|
|
|
|
- `prepare`, `launch`, `_run_agent_container`, `prepare_cleanup`,
|
|
`cleanup`, `list_active`. These stay in `backend.py`.
|
|
- The `validate_skills` and `validate_ssh_entries` host-side
|
|
validation methods. They run from `prepare` (before the y/N), not
|
|
from `provision`, so they belong with `prepare` and stay on the
|
|
class.
|
|
- Any change to the abstract `BottleBackend` base in
|
|
`backend/__init__.py`.
|
|
- Cross-backend reuse of provisioner code. There's no second backend
|
|
yet; designing for one before it exists would be premature.
|
|
|
|
## Proposed Design
|
|
|
|
### New layout
|
|
|
|
```
|
|
bot_bottle/backend/docker/
|
|
backend.py # DockerBottleBackend (slimmer)
|
|
bottle.py
|
|
bottle_plan.py
|
|
bottle_cleanup_plan.py
|
|
network.py
|
|
pipelock.py
|
|
util.py
|
|
provision/
|
|
__init__.py # empty; explicit imports per module
|
|
prompt.py # provision_prompt(plan, target) -> str | None
|
|
skills.py # provision_skills(plan, target) -> None
|
|
ssh.py # provision_ssh(plan, target) -> None
|
|
git.py # provision_git(plan, target) -> None
|
|
```
|
|
|
|
### Function signatures
|
|
|
|
Each module exports one top-level function with the same shape:
|
|
|
|
```python
|
|
# prompt.py
|
|
def provision_prompt(plan: DockerBottlePlan, target: str) -> str | None: ...
|
|
|
|
# skills.py
|
|
def provision_skills(plan: DockerBottlePlan, target: str) -> None: ...
|
|
|
|
# ssh.py
|
|
def provision_ssh(plan: DockerBottlePlan, target: str) -> None: ...
|
|
|
|
# git.py
|
|
def provision_git(plan: DockerBottlePlan, target: str) -> None: ...
|
|
```
|
|
|
|
`target` is the resolved container name (same value the current
|
|
methods receive). The functions are free functions, not methods, so
|
|
they don't accept `self`.
|
|
|
|
### Delegation on the backend
|
|
|
|
`DockerBottleBackend.provision_*` shrinks to:
|
|
|
|
```python
|
|
from .provision import prompt as _prompt
|
|
from .provision import skills as _skills
|
|
from .provision import ssh as _ssh
|
|
from .provision import git as _git
|
|
|
|
class DockerBottleBackend(BottleBackend):
|
|
...
|
|
def provision_prompt(self, plan: BottlePlan, target: str) -> str | None:
|
|
assert isinstance(plan, DockerBottlePlan)
|
|
return _prompt.provision_prompt(plan, target)
|
|
|
|
def provision_skills(self, plan: BottlePlan, target: str) -> None:
|
|
assert isinstance(plan, DockerBottlePlan)
|
|
_skills.provision_skills(plan, target)
|
|
|
|
# ...same for ssh, git
|
|
```
|
|
|
|
The `isinstance` assert stays on the method (the abstract base passes
|
|
`BottlePlan`, not `DockerBottlePlan`) so the module functions can
|
|
take the concrete type and skip re-checking.
|
|
|
|
### Existing code touched
|
|
|
|
- **`bot_bottle/backend/docker/backend.py`** — four method
|
|
bodies move out; method definitions stay as one-line delegations.
|
|
Imports for `pipelock_proxy_host_port`, `expand_tilde`, etc., that
|
|
are only used by the moved bodies migrate with them.
|
|
- **`bot_bottle/backend/docker/__init__.py`** — no change. The
|
|
public surface (`DockerBottleBackend`) is unchanged.
|
|
- **`bot_bottle/backend/__init__.py`** — no change.
|
|
- **`tests/`** — no expected change. Existing tests exercise the
|
|
backend via `DockerBottleBackend` or the CLI surface; they don't
|
|
reach into provisioners directly. Verify after the move and only
|
|
update if a test breaks.
|
|
|
|
### Data model changes
|
|
|
|
None.
|
|
|
|
### External dependencies
|
|
|
|
None new.
|
|
|
|
## References
|
|
|
|
- PRD 0003 (`docs/prds/0003-bottle-backend-abstraction.md`) —
|
|
establishes the four-method provisioner contract being preserved
|
|
here.
|