diff --git a/docs/prds/0004-split-out-provisioners.md b/docs/prds/0004-split-out-provisioners.md new file mode 100644 index 0000000..38bf4f5 --- /dev/null +++ b/docs/prds/0004-split-out-provisioners.md @@ -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 /. /` 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.