docs(prd): add 0004 split out provisioners
This commit is contained in:
@@ -0,0 +1,226 @@
|
||||
# PRD 0004: Split out provisioners
|
||||
|
||||
- **Status:** Draft
|
||||
- **Author:** didericis
|
||||
- **Created:** 2026-05-11
|
||||
|
||||
## Summary
|
||||
|
||||
Break `claude_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
|
||||
`claude_bottle/backend/docker/provision/`. The abstract base in
|
||||
`claude_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 `claude_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
|
||||
`claude_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 `claude_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
|
||||
|
||||
```
|
||||
claude_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
|
||||
|
||||
- **`claude_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.
|
||||
- **`claude_bottle/backend/docker/__init__.py`** — no change. The
|
||||
public surface (`DockerBottleBackend`) is unchanged.
|
||||
- **`claude_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.
|
||||
Reference in New Issue
Block a user