Files
bot-bottle/docs/prds/0004-split-out-provisioners.md
T
didericis 47c3ba63f8
test / unit (pull_request) Successful in 36s
test / integration (pull_request) Successful in 58s
test / integration (push) Successful in 54s
test / unit (push) Successful in 32s
docs(prd): mark merged PRDs as Active
Flip Status: Draft -> Active for the 23 PRDs whose work has shipped to
main (including 0027, now that PR #95 has merged). Leaves the
terminal-status PRDs unchanged: 0007 and 0010 (Superseded) and 0014
(Retargeted) were replaced, not shipped as-is.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-05-28 22:12:03 -04:00

227 lines
8.5 KiB
Markdown

# PRD 0004: Split out provisioners
- **Status:** Active
- **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.