refactor(backend): base owns the gateway-attach flow, fail hard (0081 review)
Addresses review on #519 (@didericis 6143 + the fail-hard direction over the codex skip). Base owns the flow (6143 / ADR 0006). `attach_bottled_agents_to_gateway()` is now a concrete method on `BottleBackend` that delegates to `gateway_attach.reconcile_running_bottles`; backends override only three primitives — `_gateway_attach_resources()`, `_running_bottles()`, `_attach_bottle_to_gateway()`. The shared control flow + error policy live in one place so a backend can't drift onto a bespoke loop or a silent skip. Keeps the reconcile flow + `GatewayAttachResources` out of `backend/base.py` (the size guardrail) in a dedicated `backend/gateway_attach.py`. New ADR 0006 records the "shared behaviour in the base backend, subclasses override primitives" theme. Fail hard, never skip (the maintainer's direction over the codex review's skip). Any attach failure now aborts bring-up instead of being logged and skipped: a bottle that silently can't reach the fresh gateway (its egress just starts failing TLS) is worse than a loud failure. Every bottle is attempted and the failures are raised together (aggregate `InfraLaunchError`) so one bring-up surfaces the full blast radius. Resource gathering (the CA fetch) also fails hard. PRD 0081 goal + design updated to match (reversed from the earlier "tolerate per-bottle failures" draft). Bumps the base.py guardrail cap 580->600 for the new contract surface (the delegator + 3 abstract primitives); the flow itself lives in gateway_attach.py. Refs #516, #519. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,65 @@
|
||||
# ADR 0006: Shared behaviour lives in the base backend
|
||||
|
||||
- **Status:** Accepted
|
||||
- **Date:** 2026-07-27
|
||||
- **Deciders:** didericis
|
||||
|
||||
## Context
|
||||
|
||||
`BottleBackend` has three concrete implementations (Firecracker, docker,
|
||||
macOS-container). Cross-backend operations can be expressed two ways:
|
||||
|
||||
1. **Abstract method per backend** — the base declares `@abstractmethod foo()`
|
||||
and each backend writes its own `foo()` end to end.
|
||||
2. **Template method** — the base owns the *control flow* of `foo()` as a
|
||||
concrete method and each backend overrides small **primitives** it calls.
|
||||
|
||||
Form 1 gives each backend total freedom, which is exactly the problem: the three
|
||||
implementations drift. The bring-up reconcile (PRD 0081) first shipped as form 1
|
||||
and immediately diverged — each backend re-implemented "gather CA → list running
|
||||
bottles → push to each", and one backend's copy silently skipped failures while
|
||||
another's aborted, an inconsistency caught only in review (#519). The behaviour
|
||||
that must be identical (the loop, the error policy, the ordering) was duplicated
|
||||
in the place least likely to stay in sync.
|
||||
|
||||
## Decision
|
||||
|
||||
Prefer the template-method form for cross-backend behaviour: **encode shared
|
||||
control flow and policy in the base backend; backends override primitives, not
|
||||
control flow.** Lean toward *less* freedom in the concrete backends and *more*
|
||||
consistency in the base.
|
||||
|
||||
Concretely, a cross-backend operation on `BottleBackend` should be a concrete
|
||||
method that:
|
||||
|
||||
- owns the sequencing, iteration, and error policy (e.g. fail-hard vs.
|
||||
best-effort, fail-fast vs. attempt-all-then-aggregate); and
|
||||
- delegates only the irreducibly backend-specific steps to `@abstractmethod`
|
||||
primitives with narrow, well-typed signatures.
|
||||
|
||||
The first application is `attach_bottled_agents_to_gateway()` (PRD 0081): the
|
||||
base gathers resources once, attempts every running bottle, and raises an
|
||||
aggregate `InfraLaunchError` if any failed; backends supply only
|
||||
`_gateway_attach_resources()`, `_running_bottles()`, and
|
||||
`_attach_bottle_to_gateway()`.
|
||||
|
||||
A backend-specific `@abstractmethod` with no shared control flow (e.g.
|
||||
`_launch_impl`, `enumerate_active`) stays as form 1 — this ADR is about
|
||||
operations whose *shape* should be uniform, not about banning abstract methods.
|
||||
|
||||
## Consequences
|
||||
|
||||
- The error policy, ordering, and loop for a cross-backend operation have one
|
||||
source of truth; a new backend cannot forget them or diverge.
|
||||
- New backends implement smaller, more focused primitives instead of re-deriving
|
||||
a whole flow.
|
||||
- The base carries more logic and needs generic-enough primitive signatures
|
||||
(e.g. a backend-specific attach-target type parameter on `BottleBackend`).
|
||||
- Genuinely backend-specific one-offs still use a plain abstract method; the
|
||||
guidance is a default, not an absolute.
|
||||
|
||||
## Links
|
||||
|
||||
- PRD 0081 (`docs/prds/0081-reprovision-gateway-state-on-bringup.md`).
|
||||
- Review that prompted this: PR #519.
|
||||
- `bot_bottle/backend/base.py` (`BottleBackend.attach_bottled_agents_to_gateway`).
|
||||
@@ -47,8 +47,13 @@ destroy (the original #450 failure mode).
|
||||
different mechanism per service.
|
||||
- The CA **rotates** on every gateway bring-up (no long-lived CA), distributed to
|
||||
running agents by the same reconcile.
|
||||
- Per-bottle failures are tolerated: one unreachable or malformed bottle does not
|
||||
block the others or the gateway coming up.
|
||||
- Reconcile failures are **loud, never hidden**: if a running bottle can't be
|
||||
re-attached to the fresh gateway, the bring-up fails and names the bottle
|
||||
rather than leaving it silently unable to reach the gateway (a bottle whose
|
||||
egress just starts failing TLS is worse than a loud failure). Every bottle is
|
||||
attempted first and the failures are raised together, so one bring-up surfaces
|
||||
the full blast radius. (Reversed from an earlier "tolerate per-bottle
|
||||
failures" draft after #519 review.)
|
||||
- **All backends** (Firecracker, docker, macOS) reconcile through the *same*
|
||||
contract — an abstract method on the backend base class, so a new backend
|
||||
cannot forget to implement it and none drifts onto a bespoke mechanism.
|
||||
@@ -61,15 +66,27 @@ destroy (the original #450 failure mode).
|
||||
|
||||
## Design
|
||||
|
||||
**The contract — `attach_bottled_agents_to_gateway()` on the backend ABC.**
|
||||
Reconciling running bottles against the current gateway is a backend
|
||||
responsibility (only the backend can enumerate its agents and reach them —
|
||||
firecracker over SSH, docker/macOS over `exec`/`cp`), so it is an
|
||||
`@abc.abstractmethod` on `BottleBackend` (`backend/base.py`). Every backend
|
||||
implements it; the host calls it whenever the gateway is (re)brought up. This is
|
||||
what makes the fix cross-backend by construction rather than a per-backend
|
||||
follow-up. It reprovisions **all** registered bottles' gateway-dependent state:
|
||||
CA, git-gate, and egress tokens.
|
||||
**The contract — `attach_bottled_agents_to_gateway()`, a concrete flow on the
|
||||
backend base.** Reconciling running bottles against the current gateway is a
|
||||
backend responsibility (only the backend can enumerate its agents and reach them
|
||||
— firecracker over SSH, docker/macOS over `exec`/`cp`). To keep every backend
|
||||
reconciling identically, the *control flow* lives in a concrete
|
||||
`attach_bottled_agents_to_gateway()` on `BottleBackend` (`backend/base.py`) and
|
||||
each backend overrides only three primitives:
|
||||
|
||||
1. `_gateway_attach_resources()` — gather what a bottle needs to attach (the
|
||||
gateway CA now; egress secrets + git-gate state as this grows).
|
||||
2. `_running_bottles()` — enumerate the live bottles as backend handles.
|
||||
3. `_attach_bottle_to_gateway(bottle, resources)` — push the resources into one
|
||||
bottle, raising `InfraLaunchError` (naming it) on failure.
|
||||
|
||||
The base gathers resources once, attempts every bottle, and — fail hard, never
|
||||
skip — raises an aggregate `InfraLaunchError` if any failed, so a backend can't
|
||||
drift onto a bespoke loop or a silent skip. (Encoding shared behaviour in the
|
||||
base backend, with subclasses overriding primitives rather than control flow, is
|
||||
[ADR 0006](../decisions/0006-shared-behavior-in-base-backend.md).) The host
|
||||
calls it whenever the gateway is (re)brought up. It reprovisions **all**
|
||||
registered bottles' gateway-dependent state: CA, git-gate, and egress tokens.
|
||||
|
||||
**Trigger — the gateway bring-up path.** The host calls
|
||||
`attach_bottled_agents_to_gateway()` only when the gateway was actually
|
||||
@@ -98,7 +115,9 @@ and its CA is available:
|
||||
+ per-repo creds under `/git/<bottle_id>`.
|
||||
- **egress token:** read the agent's `ENV_VAR_SECRET` and feed
|
||||
`reprovision_bottles`, restoring the orchestrator's in-memory tokens.
|
||||
4. Every per-bottle step is wrapped so one failure is logged and skipped.
|
||||
4. If any per-bottle step fails, the reconcile attempts the rest and then raises
|
||||
an aggregate naming every bottle that failed — bring-up fails loudly rather
|
||||
than leaving a bottle silently unable to reach the gateway.
|
||||
|
||||
**Retire the persistence prototype.** No CA data volume, no git-gate data volume
|
||||
(the abandoned PRs #511 / #513). The launch-time `_reprovision_running_bottles`
|
||||
|
||||
Reference in New Issue
Block a user