diff --git a/CLAUDE.md b/CLAUDE.md index 3a892a6..7c4a64d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -85,7 +85,7 @@ Design docs and implementation plans live under `planning/` (not under `docs/`, - **Optional-import guard pattern**: top-level conditional imports (`if import_checker.is_X_installed: import X`) keep optional dependencies actually optional. Code that references `X` is only reached when `check_dependencies()` has already returned True; the runtime invariant is maintained by the inline `is_configured → check_dependencies → instantiate` flow in `BaseBootstrapper.__init__`. See "Type checking" below. - **`from_dict` vs `from_object` accept different shapes for `None`**: `BaseConfig.from_dict({"service_name": None})` succeeds and explicitly overrides the default with `None`. `BaseConfig.from_object(obj)` where `obj.service_name is None` filters the attribute out and the dataclass default takes over. The asymmetry is documented in both methods' docstrings (`instruments/base.py:17, 23`) and pinned by tests in `tests/test_config.py:54-94`. Pick `from_dict` if explicit-None override is the load-bearing semantic. - **`__post_init__` cascade invariant**: every config-class `__post_init__` must call `super().__post_init__()` so MRO chains terminate cleanly. `BaseConfig` has a no-op `__post_init__` as the chain terminator. Required because `OpenTelemetryConfig.__post_init__` (SEC-2 warning), `CorsConfig.__post_init__` (SEC-3 validation), and `FastAPIConfig.__post_init__` (UnsetType app construction) all sit on the same MRO for `FastAPIConfig`/`LitestarConfig`/`FastStreamConfig`/`FreeConfig`; without the cascade, a class that returns early before `super()` blocks the rest of the chain. `FastAPIConfig` uses the explicit `super(FastAPIConfig, self).__post_init__()` form because `@dataclass(slots=True)` replaces the class object after the body compiles and breaks bare `super()`. -- **`_lite_bootstrap_*` prefix for sentinels on user-supplied app instances**: when the bootstrapper needs to tag a user-supplied framework app (FastAPI, FastMCP, Litestar, FastStream) with internal state — e.g., the `_lite_bootstrap_lifespan_attached` marker that prevents double-wrap on FastAPI — store it as a direct attribute on the app instance with a `_lite_bootstrap_` prefix. Read with `getattr(application, "_lite_bootstrap_", False)` (no SLF violation); write with `application._lite_bootstrap_ = value # noqa: SLF001`. Don't squat in framework-provided user namespaces like Starlette's `application.state`. +- **`_lite_bootstrap_*` prefix for sentinels on user-supplied app instances**: when the bootstrapper needs to tag a user-supplied framework app (FastAPI, FastMCP, Litestar, FastStream) with internal state, store it as a direct attribute on the instance with a `_lite_bootstrap_` prefix. The canonical example is the `_lite_bootstrap_teardown_attached` marker that gates the double-attach guard, set once by the shared `BaseBootstrapper._attach_teardown_once(target, attach)` and applied uniformly across all four app-bearing bootstrappers (FastAPI tags the app, Litestar tags its `AppConfig`, FastStream/FastMCP tag the app). Read with `getattr(target, "_lite_bootstrap_", False)` (no SLF violation); write with `setattr`/`target._lite_bootstrap_ = value # noqa: SLF001`. Don't squat in framework-provided user namespaces like Starlette's `application.state`. See `architecture/bootstrappers.md`. ### Type checking diff --git a/architecture/bootstrappers.md b/architecture/bootstrappers.md index 4644427..3f46f43 100644 --- a/architecture/bootstrappers.md +++ b/architecture/bootstrappers.md @@ -65,12 +65,34 @@ The same string is produced by the public `build_summary()` method, callable at any later point (REPL, health endpoint) regardless of log-level filtering. To inspect skips programmatically, iterate `bootstrapper.skipped_instruments`. +## Teardown-on-shutdown attach + +Each app-bearing bootstrapper wires its `teardown` into the framework's shutdown +lifecycle from `__init__`, through one shared seam: +`BaseBootstrapper._attach_teardown_once(target, attach)`. That method owns the +double-attach guard — it tags the attach target with a +`_lite_bootstrap_teardown_attached` marker, so a second bootstrapper on the same +app warns and skips rather than stacking a second teardown hook. Only the `attach` +thunk is framework-specific: + +- **FastAPI** — merge a lifespan context manager (`_wrap_lifespan`); target is the app. +- **Litestar** — append to `application_config.on_shutdown`; target is the + `AppConfig` (the built `Litestar` app is slotted and is never tagged). +- **FastStream** — register via `application.on_shutdown(...)`; target is the app. +- **FastMCP** — add a `_TeardownProvider` whose async lifespan runs teardown; target + is the app (FastMCP exposes no `on_shutdown` API). +- **Free** — no app, no shutdown lifecycle; not wired. + +The guard is uniform: the same marker and warning apply to all four app-bearing +frameworks. `attach` is typed `Callable[[], object]` because some hooks (FastStream's +`on_shutdown`) return the callback. + ## App-tagging sentinel convention When a bootstrapper must tag a user-supplied framework app (FastAPI, FastMCP, Litestar, FastStream) with internal state, it stores a direct attribute prefixed `_lite_bootstrap_` rather than squatting in framework namespaces like Starlette's -`application.state`. Example: FastAPI's lifespan double-wrap guard reads -`getattr(application, "_lite_bootstrap_lifespan_attached", False)` (no SLF -violation) and writes -`application._lite_bootstrap_lifespan_attached = True # noqa: SLF001`. +`application.state`. The canonical example is the teardown guard's +`_lite_bootstrap_teardown_attached` marker, read via +`getattr(target, BaseBootstrapper._TEARDOWN_MARKER, False)` (no SLF violation) and +written via `setattr` inside `_attach_teardown_once`. diff --git a/lite_bootstrap/bootstrappers/base.py b/lite_bootstrap/bootstrappers/base.py index 7af40e8..acfcbc7 100644 --- a/lite_bootstrap/bootstrappers/base.py +++ b/lite_bootstrap/bootstrappers/base.py @@ -24,6 +24,31 @@ class BaseBootstrapper(abc.ABC, typing.Generic[ApplicationT]): skipped_instruments: list[tuple[type[BaseInstrument], str]] bootstrap_config: BaseConfig + # Marker tagged on a user-supplied app (or its config) once this bootstrapper has + # attached its teardown to the framework's shutdown lifecycle. Prevents a second + # bootstrapper on the same app from re-attaching. See architecture/bootstrappers.md. + _TEARDOWN_MARKER: typing.ClassVar[str] = "_lite_bootstrap_teardown_attached" + + def _attach_teardown_once(self, target: object, attach: typing.Callable[[], object]) -> None: + """Run ``attach`` (which wires ``teardown`` into the framework's shutdown) once per target. + + Idempotent across bootstrapper instances: if ``target`` is already tagged, warn + and skip rather than stacking a second teardown hook. ``attach`` is the only + framework-specific part; detection + warning live here. + """ + if getattr(target, self._TEARDOWN_MARKER, False): + warnings.warn( + f"{type(self).__name__} already has a lite-bootstrap teardown hook attached to this " + f"application or its configuration; skipping. This {type(self).__name__}'s teardown " + f"will not run on shutdown — construct one {type(self).__name__} per application.", + stacklevel=3, + ) + return + # Mark only after a successful attach: if attach() raises, the target stays untagged + # so a retry can re-attach rather than silently warning-and-skipping forever. + attach() + setattr(target, self._TEARDOWN_MARKER, True) + def build_summary(self) -> str: """Return a multi-line human-readable summary of configured + skipped instruments. diff --git a/lite_bootstrap/bootstrappers/fastapi_bootstrapper.py b/lite_bootstrap/bootstrappers/fastapi_bootstrapper.py index 112f8cc..e9d51a0 100644 --- a/lite_bootstrap/bootstrappers/fastapi_bootstrapper.py +++ b/lite_bootstrap/bootstrappers/fastapi_bootstrapper.py @@ -197,19 +197,10 @@ async def lifespan_manager(self, _: "fastapi.FastAPI") -> typing.AsyncIterator[d def __init__(self, bootstrap_config: FastAPIConfig) -> None: super().__init__(bootstrap_config) - application = _narrow_app(self.bootstrap_config) - # FastAPI's lifespan_context is opaque after wrap; tag the app instance directly - # rather than squatting in Starlette's user-facing application.state namespace. - if getattr(application, "_lite_bootstrap_lifespan_attached", False): - warnings.warn( - "FastAPI application already has a lite-bootstrap lifespan wrapper attached; " - "skipping re-wrap. This FastAPIBootstrapper's teardown will not be invoked on " - "ASGI shutdown — construct one FastAPIBootstrapper per application.", - stacklevel=2, - ) - return - application._lite_bootstrap_lifespan_attached = True # noqa: SLF001 # ty: ignore[unresolved-attribute] + self._attach_teardown_once(application, lambda: self._wrap_lifespan(application)) + + def _wrap_lifespan(self, application: "fastapi.FastAPI") -> None: old_lifespan_manager = application.router.lifespan_context application.router.lifespan_context = _merge_lifespan_context( old_lifespan_manager, diff --git a/lite_bootstrap/bootstrappers/fastmcp_bootstrapper.py b/lite_bootstrap/bootstrappers/fastmcp_bootstrapper.py index 52135b0..0285d82 100644 --- a/lite_bootstrap/bootstrappers/fastmcp_bootstrapper.py +++ b/lite_bootstrap/bootstrappers/fastmcp_bootstrapper.py @@ -2,7 +2,6 @@ import dataclasses import time import typing -import warnings from lite_bootstrap import import_checker from lite_bootstrap.bootstrappers.base import BaseBootstrapper @@ -152,15 +151,8 @@ def is_ready(self) -> bool: def __init__(self, bootstrap_config: FastMcpConfig) -> None: super().__init__(bootstrap_config) - if any(isinstance(p, _TeardownProvider) for p in self.bootstrap_config.application.providers): - warnings.warn( - "FastMCP application already has a _TeardownProvider attached; skipping re-attachment. " - "This FastMcpBootstrapper's teardown will not be invoked on ASGI shutdown — " - "construct one FastMcpBootstrapper per application.", - stacklevel=2, - ) - return - self.bootstrap_config.application.add_provider(_TeardownProvider(self.teardown)) + application = self.bootstrap_config.application + self._attach_teardown_once(application, lambda: application.add_provider(_TeardownProvider(self.teardown))) def _prepare_application(self) -> "FastMCP[typing.Any]": return self.bootstrap_config.application diff --git a/lite_bootstrap/bootstrappers/faststream_bootstrapper.py b/lite_bootstrap/bootstrappers/faststream_bootstrapper.py index 59287f0..2f795e9 100644 --- a/lite_bootstrap/bootstrappers/faststream_bootstrapper.py +++ b/lite_bootstrap/bootstrappers/faststream_bootstrapper.py @@ -205,7 +205,8 @@ def is_ready(self) -> bool: def __init__(self, bootstrap_config: FastStreamConfig) -> None: super().__init__(bootstrap_config) - self.bootstrap_config.application.on_shutdown(self.teardown) + application = self.bootstrap_config.application + self._attach_teardown_once(application, lambda: application.on_shutdown(self.teardown)) def _prepare_application(self) -> "AsgiFastStream": return self.bootstrap_config.application diff --git a/lite_bootstrap/bootstrappers/litestar_bootstrapper.py b/lite_bootstrap/bootstrappers/litestar_bootstrapper.py index a7411ad..a0edc33 100644 --- a/lite_bootstrap/bootstrappers/litestar_bootstrapper.py +++ b/lite_bootstrap/bootstrappers/litestar_bootstrapper.py @@ -277,8 +277,12 @@ class LitestarBootstrapper(BaseBootstrapper["litestar.Litestar"]): def __init__(self, bootstrap_config: LitestarConfig) -> None: super().__init__(bootstrap_config) - self.bootstrap_config.application_config.debug = bootstrap_config.service_debug - self.bootstrap_config.application_config.on_shutdown.append(self.teardown) + application_config = self.bootstrap_config.application_config + self._attach_teardown_once(application_config, lambda: self._apply_config(application_config)) + + def _apply_config(self, application_config: "AppConfig") -> None: + application_config.debug = self.bootstrap_config.service_debug + application_config.on_shutdown.append(self.teardown) def is_ready(self) -> bool: return import_checker.is_litestar_installed diff --git a/planning/changes/2026-06-24.01-unify-teardown-attach/design.md b/planning/changes/2026-06-24.01-unify-teardown-attach/design.md new file mode 100644 index 0000000..a24c2e2 --- /dev/null +++ b/planning/changes/2026-06-24.01-unify-teardown-attach/design.md @@ -0,0 +1,191 @@ +--- +status: shipped +date: 2026-06-24 +slug: unify-teardown-attach +summary: Move the teardown-on-shutdown attach behind one BaseBootstrapper._attach_teardown_once seam with a uniform marker, extending the double-attach guard to Litestar and FastStream. +supersedes: null +superseded_by: null +pr: 130 +outcome: Shipped as designed — one _attach_teardown_once seam owns detection + warning + skip; FastAPI/FastMCP migrated behavior-preserving; Litestar/FastStream gained the guard; attach typed Callable[[], object]; 100% coverage held. +--- + +# Design: Unify the teardown-on-shutdown attach behind one guarded seam + +## Summary + +"Register this bootstrapper's `teardown` to run on the framework's shutdown" is a +real seam implemented ad-hoc in five `__init__` bodies. The double-attach guard + +warning is copy-pasted between FastAPI and FastMCP (with two different detection +mechanisms) and absent from Litestar and FastStream. This change moves the guard +into one `BaseBootstrapper._attach_teardown_once(target, attach)` method that owns +detection (a uniform `_lite_bootstrap_teardown_attached` marker), the warning, and +the skip; each framework's `__init__` shrinks to "here is my target, here is how I +attach." The guard extends uniformly to Litestar and FastStream, closing a +behavioral inconsistency. + +## Motivation + +The teardown-attach is wired five different ways (verified across +`bootstrappers/*.py`): + +- **FastAPI** (`fastapi_bootstrapper.py:204-217`): reads a marker + `_lite_bootstrap_lifespan_attached`, warns + skips if set, else marks and merges + its lifespan via `_merge_lifespan_context`. +- **FastMCP** (`fastmcp_bootstrapper.py:155-163`): **structural** check + `any(isinstance(p, _TeardownProvider) for p in app.providers)`, warns + skips, + else adds a `_TeardownProvider`. +- **Litestar** (`litestar_bootstrapper.py:281`): bare + `application_config.on_shutdown.append(self.teardown)` — **no guard**. +- **FastStream** (`faststream_bootstrapper.py:208`): bare + `application.on_shutdown(self.teardown)` — **no guard**. +- **Free** (`free_bootstrapper.py`): no app, no shutdown lifecycle — out of scope. + +The same user error — two bootstrappers constructed against one app — is handled +inconsistently: FastAPI/FastMCP warn and skip (their guard is tested at +`test_fastapi_bootstrap.py:130` and `test_fastmcp_bootstrap.py:283`), while +Litestar/FastStream silently double-register `teardown`. The shared logic (check → +warn → skip → else mark + attach) and near-identical warning text live in two +copies; detection is reinvented per framework. Deletion test on a unified guard: +delete it and the guard + warning re-duplicate across FastAPI/FastMCP and the gap +on Litestar/FastStream reopens — it concentrates complexity, so it earns its keep. +(Surfaced as candidate 2 of the 2026-06-23 architecture review; marked "worth +exploring".) + +## Non-goals + +- **Unifying the attach mechanism.** The four mechanisms (lifespan merge, provider, + `on_shutdown` list append, `on_shutdown()` method call) are genuinely different + and stay framework-specific. Only detection + warn + skip are shared. +- **A class-level registry / WeakSet for "already attached".** Considered, rejected: + it would contradict the documented `_lite_bootstrap_*` app-tagging convention + (`architecture/bootstrappers.md:68`) and introduce process-global mutable state + with test-isolation hazards. The marker keeps the "attached" bit local to the + app's lifetime. +- **A free-function helper module.** The guard is bootstrapper-lifecycle logic and + belongs on `BaseBootstrapper` next to `teardown()`, where `type(self).__name__` + is available for the warning. +- **Changing the warn-and-skip policy to raise.** FastAPI/FastMCP keep lenient + warn+skip; Litestar/FastStream adopt the same. No escalation to an error. +- **Bringing Free into the seam.** It has no app to attach to. + +## Design + +### 1. The seam: `BaseBootstrapper._attach_teardown_once` + +One method on the base owns detection, warning, and skip; the marker name is a +class constant: + +```python +class BaseBootstrapper(abc.ABC, typing.Generic[ApplicationT]): + _TEARDOWN_MARKER: typing.ClassVar[str] = "_lite_bootstrap_teardown_attached" + + def _attach_teardown_once(self, target: object, attach: typing.Callable[[], None]) -> None: + if getattr(target, self._TEARDOWN_MARKER, False): + warnings.warn( + f"The application passed to {type(self).__name__} already has a lite-bootstrap " + f"teardown hook attached; skipping. This {type(self).__name__}'s teardown will " + f"not run on shutdown — construct one {type(self).__name__} per application.", + stacklevel=3, + ) + return + setattr(target, self._TEARDOWN_MARKER, True) # noqa: B010 — documented _lite_bootstrap_ tag + attach() +``` + +`stacklevel=3` points the warning at the user's construction site (warn ← +`_attach_teardown_once` ← subclass `__init__` ← user). The warning noun is derived +from `type(self).__name__`, so no per-class label is needed. + +### 2. Each framework's `__init__` becomes target + attach thunk + +The detection and warning vanish from every subclass; each supplies only its +target and how it attaches: + +```python +# FastAPI — target is the app; attach merges the lifespan +self._attach_teardown_once(application, lambda: self._wrap_lifespan(application)) + +# FastMCP — target is the app; attach adds the provider (still the attach mechanism) +self._attach_teardown_once( + self.bootstrap_config.application, + lambda: self.bootstrap_config.application.add_provider(_TeardownProvider(self.teardown)), +) + +# Litestar — target is the AppConfig (the built app is slotted); now guarded +self._attach_teardown_once( + self.bootstrap_config.application_config, + lambda: self.bootstrap_config.application_config.on_shutdown.append(self.teardown), +) + +# FastStream — target is the app; now guarded +self._attach_teardown_once( + self.bootstrap_config.application, + lambda: self.bootstrap_config.application.on_shutdown(self.teardown), +) +``` + +FastAPI's lifespan merge moves to a small `_wrap_lifespan` helper (or stays an +inline lambda). The early-`return`-from-`__init__` guards collapse into the +method's skip; nothing runs after the attach in any subclass `__init__`, so +behavior is preserved. + +### 3. Detection unified to the marker; two consequences + +Detection becomes one marker on the attach target. A probe confirmed all four +targets accept an arbitrary attribute and are weakref-able — including Litestar's +`AppConfig` (the *built* `Litestar` app is slotted, but it is never the attach +target). Two consequences, on the record: + +- **FastMCP** migrates from its structural `isinstance(p, _TeardownProvider)` + detection to the marker. The `_TeardownProvider` class stays — it is how FastMCP + *attaches*; it just stops being how FastMCP *detects*. +- **FastAPI**'s marker renames `_lite_bootstrap_lifespan_attached` → + `_lite_bootstrap_teardown_attached`. It is an internal tag on the user's app, not + a public symbol, so it renames freely. + +### 4. Behavior change: Litestar and FastStream gain the guard + +This is the intended effect of the unification, not a side effect. A second +bootstrapper constructed against the same Litestar `AppConfig` / FastStream app now +emits the warning and skips the second attach, instead of silently registering +`teardown` twice. Because `BaseBootstrapper.teardown()` is idempotent +(`test_free_bootstrap.py:117`), the prior double-register was non-fatal but +silent; the new behavior is consistent and observable. + +## Operations + +None. + +## Testing + +Test-first. Two layers: + +- **Direct unit test of the seam (new — the depth payoff).** On a cheap concrete + `FreeBootstrapper` (no app), call the inherited `_attach_teardown_once` against a + dummy `types.SimpleNamespace()` target with a spy thunk: first call runs the + thunk and sets the marker; second call warns and does **not** run the thunk. No + framework, no ASGI lifespan. +- **Per-framework "routes through the guard" (uniform).** Each of the four asserts + the warning fires on a second construction against the same target. FastAPI / + FastMCP: keep existing double-attach tests, update the `match=` string to the + unified wording, keep their attach-once assertions (lifespan not re-wrapped / + single `_TeardownProvider`). Litestar / FastStream: **new** tests asserting the + warning fires and the hook is registered once (Litestar via + `len(application_config.on_shutdown)`; FastStream via the warning). + +`just test` green at 100%, `just lint-ci` clean (`ty` included). + +## Risk + +- **Behavior change on Litestar/FastStream (med likelihood × low impact).** Code + that knowingly constructs two bootstrappers on one app now gets a warning. This + is the intended consistency, non-fatal (warn + skip), and the first + bootstrapper's teardown still runs. Called out here and for the retro. +- **FastMCP detection migration (low × med).** Switching structural → marker must + preserve the tested outcome (second bootstrapper warns, single `_TeardownProvider` + added). Mitigation: the existing FastMCP double-attach test is retained, only its + message match updated; the ASGI-lifespan teardown test + (`test_fastmcp_bootstrap.py:61`) pins that the provider still fires. +- **Marker rename leaving a stale tag (low × low).** `_lite_bootstrap_lifespan_attached` + disappears; nothing public reads it. Grep confirms it is referenced only in + FastAPI's own code, its test, and the arch docs (all updated here). diff --git a/planning/changes/2026-06-24.01-unify-teardown-attach/plan.md b/planning/changes/2026-06-24.01-unify-teardown-attach/plan.md new file mode 100644 index 0000000..0d83bfb --- /dev/null +++ b/planning/changes/2026-06-24.01-unify-teardown-attach/plan.md @@ -0,0 +1,197 @@ +--- +status: shipped +date: 2026-06-24 +slug: unify-teardown-attach +spec: unify-teardown-attach +pr: 130 +--- + +# unify-teardown-attach — implementation plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use +> superpowers:subagent-driven-development (recommended) or +> superpowers:executing-plans to implement this plan task-by-task. Steps +> use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Move the teardown-on-shutdown guard into one +`BaseBootstrapper._attach_teardown_once`, route all four app-bearing +bootstrappers through it, and extend the double-attach warning uniformly to +Litestar and FastStream. + +**Spec:** [`design.md`](./design.md) + +**Branch:** `feat/unify-teardown-attach` + +**Commit strategy:** Per-task commits (each task leaves the suite green). + +TDD throughout: failing test first, watch it fail for the right reason, then +implement to green. Each task ends green at 100% coverage. + +--- + +### Task 1: `_attach_teardown_once` seam on `BaseBootstrapper` (red → green) + +**Files:** +- Modify: `lite_bootstrap/bootstrappers/base.py` +- Modify: `tests/test_free_bootstrap.py` (de-facto base-behavior test surface) + +Add the guarded seam and unit-test it directly against a dummy target. No +bootstrapper is rewired yet, so the existing suite stays green. + +- [ ] **Step 1: Write the direct seam test, red.** + + On a `FreeBootstrapper` instance, call the inherited + `_attach_teardown_once(target, spy)` with `target = types.SimpleNamespace()`: + - first call runs `spy` exactly once and sets `getattr(target, marker)` truthy + - second call emits `UserWarning` (match `"already has a lite-bootstrap teardown hook"`) + and does **not** run `spy` again + + Run `just test -- -k attach_teardown_once` → fails (method does not exist). + +- [ ] **Step 2: Implement `_attach_teardown_once` + `_TEARDOWN_MARKER`.** + + Add the method and the `_TEARDOWN_MARKER` class constant per the spec + (getattr → warn+skip, else setattr + `attach()`), `stacklevel=3`, warning noun + from `type(self).__name__`. + + Run `just test -- -k attach_teardown_once` → green. + +- [ ] **Step 3: Commit.** + + ```bash + git add lite_bootstrap/bootstrappers/base.py tests/test_free_bootstrap.py + git commit -m "feat: add BaseBootstrapper._attach_teardown_once guarded seam + + Co-Authored-By: Claude Opus 4.8 (1M context) " + ``` + +--- + +### Task 2: Route FastAPI + FastMCP through the seam + +**Files:** +- Modify: `lite_bootstrap/bootstrappers/fastapi_bootstrapper.py` +- Modify: `lite_bootstrap/bootstrappers/fastmcp_bootstrapper.py` +- Modify: `tests/test_fastapi_bootstrap.py`, `tests/test_fastmcp_bootstrap.py` + +Migrate the two already-guarded bootstrappers; behavior preserved (warn + skip), +detection unified to the marker. + +- [ ] **Step 1: Update the existing double-attach tests to the unified wording.** + + In `test_fastapi_bootstrap.py:130` and `test_fastmcp_bootstrap.py:283`, change + the warning `match`/substring to `"already has a lite-bootstrap teardown hook"`. + Keep the attach-once assertions (FastAPI lifespan not re-wrapped; FastMCP single + `_TeardownProvider`). Run those tests → red (old wording gone once impl changes). + +- [ ] **Step 2: Migrate FastAPI.** + + Replace the inline marker-check/wrap in `__init__` with + `self._attach_teardown_once(application, lambda: self._wrap_lifespan(application))`; + move the `_merge_lifespan_context` body into `_wrap_lifespan` (or inline lambda). + Remove the old `_lite_bootstrap_lifespan_attached` get/set. + +- [ ] **Step 3: Migrate FastMCP.** + + Replace the structural `isinstance(p, _TeardownProvider)` guard in `__init__` + with `self._attach_teardown_once(app, lambda: app.add_provider(_TeardownProvider(self.teardown)))`. + Keep the `_TeardownProvider` class (attach mechanism). + + Run `just test -- -k "fastapi or fastmcp"` → green. Confirm + `test_fastmcp_bootstrap.py:61` (ASGI-lifespan teardown) still passes. + +- [ ] **Step 4: Commit.** + + ```bash + git add lite_bootstrap/bootstrappers/fastapi_bootstrapper.py \ + lite_bootstrap/bootstrappers/fastmcp_bootstrapper.py \ + tests/test_fastapi_bootstrap.py tests/test_fastmcp_bootstrap.py + git commit -m "refactor: route FastAPI + FastMCP teardown attach through the seam + + Co-Authored-By: Claude Opus 4.8 (1M context) " + ``` + +--- + +### Task 3: Extend the guard to Litestar + FastStream (red → green) + +**Files:** +- Modify: `lite_bootstrap/bootstrappers/litestar_bootstrapper.py` +- Modify: `lite_bootstrap/bootstrappers/faststream_bootstrapper.py` +- Modify: `tests/test_litestar_bootstrap.py`, `tests/test_faststream_bootstrap.py` + +New behavior: these two gain the double-attach guard. + +- [ ] **Step 1: Write the new double-attach tests, red.** + + - Litestar: construct two `LitestarBootstrapper`s on the same `LitestarConfig` + (shared `application_config`); assert the second warns + (`"already has a lite-bootstrap teardown hook"`) and + `len(application_config.on_shutdown)` increased by exactly 1 total. + - FastStream: construct two `FastStreamBootstrapper`s on the same app; assert the + second warns. + + Run those tests → fail (no guard yet; no warning emitted). + +- [ ] **Step 2: Route Litestar + FastStream through the seam.** + + Litestar `__init__`: keep the `application_config.debug = ...` line, replace the + bare `on_shutdown.append` with + `self._attach_teardown_once(application_config, lambda: application_config.on_shutdown.append(self.teardown))`. + FastStream `__init__`: replace the bare `on_shutdown(self.teardown)` with + `self._attach_teardown_once(application, lambda: application.on_shutdown(self.teardown))`. + + Run `just test -- -k "litestar or faststream"` → green. + +- [ ] **Step 3: Commit.** + + ```bash + git add lite_bootstrap/bootstrappers/litestar_bootstrapper.py \ + lite_bootstrap/bootstrappers/faststream_bootstrapper.py \ + tests/test_litestar_bootstrap.py tests/test_faststream_bootstrap.py + git commit -m "feat: extend teardown double-attach guard to Litestar + FastStream + + Co-Authored-By: Claude Opus 4.8 (1M context) " + ``` + +--- + +### Task 4: Ship-time docs + index + +**Files:** +- Modify: `architecture/bootstrappers.md` +- Modify: `CLAUDE.md` +- Modify: `planning/changes/2026-06-24.01-unify-teardown-attach/design.md` (frontmatter) + +- [ ] **Step 1: Update `architecture/bootstrappers.md`.** + + In the teardown section (§42) and the app-tagging sentinel section (§68): name + the unified `_attach_teardown_once` seam, the `_lite_bootstrap_teardown_attached` + marker (rename from `_lite_bootstrap_lifespan_attached`), and that the + double-attach guard now applies uniformly to all four app-bearing frameworks. + +- [ ] **Step 2: Update `CLAUDE.md`.** + + Update the `_lite_bootstrap_*` convention bullet: the marker is now + `_lite_bootstrap_teardown_attached`, set once via the shared + `BaseBootstrapper._attach_teardown_once`, uniform across FastAPI/Litestar/ + FastStream/FastMCP. + +- [ ] **Step 3: Fill design frontmatter + regenerate index.** + + Set `summary`; set `status` per the merge state (see candidate-1 precedent: + `approved` until a PR exists, `shipped` + `pr` at PR time). Run `just index`. + +- [ ] **Step 4: Final verification.** + + `just lint-ci` clean (incl. `ty`), `just test` green at 100%. + +- [ ] **Step 5: Commit.** + + ```bash + git add architecture/bootstrappers.md CLAUDE.md \ + planning/changes/2026-06-24.01-unify-teardown-attach/ planning/ + git commit -m "docs: record unified teardown-attach seam in architecture + CLAUDE + + Co-Authored-By: Claude Opus 4.8 (1M context) " + ``` diff --git a/planning/decisions/2026-06-24-teardown-marker-accepted-limits.md b/planning/decisions/2026-06-24-teardown-marker-accepted-limits.md new file mode 100644 index 0000000..7c2f9c4 --- /dev/null +++ b/planning/decisions/2026-06-24-teardown-marker-accepted-limits.md @@ -0,0 +1,70 @@ +--- +status: accepted +date: 2026-06-24 +slug: teardown-marker-accepted-limits +summary: Two deliberate limits of the unified teardown marker — FastMCP detects via attribute not provider scan, and Litestar tags the shareable AppConfig. +supersedes: null +superseded_by: null +pr: 130 +--- + +# Accepted limits of the unified teardown-attach marker + +**Decision:** The `_lite_bootstrap_teardown_attached` marker introduced in +[unify-teardown-attach](../changes/2026-06-24.01-unify-teardown-attach/design.md) +(#130) carries two known limits that we accept rather than design around: +FastMCP detects double-attach via the attribute marker (not a provider-list scan), +and Litestar tags the shared `AppConfig` (not a built app). Both surfaced in the +#130 code review and were judged not worth the added complexity. + +## Context + +The unified seam `BaseBootstrapper._attach_teardown_once(target, attach)` detects a +second bootstrapper on the same target by tagging the target with one marker +attribute. Two consequences came up in review: + +1. **FastMCP detection change.** FastMCP previously detected double-attach + structurally — `any(isinstance(p, _TeardownProvider) for p in app.providers)` — + which reads the actual attach state. The unified marker replaces that with an + attribute on the app. If the app's `providers` list is cleared after the first + bootstrap while the marker survives, a second bootstrapper warns-and-skips + instead of re-attaching. +2. **Litestar marker target.** The Litestar app does not exist at `__init__` time + (it is built later via `Litestar.from_config`), so the attach — and therefore + the marker — lands on the `application_config` (`AppConfig`). Two `LitestarConfig` + instances that *share* one `AppConfig` but intend two distinct apps will collide: + the second warns and skips, and its teardown never runs. + +## Decision & rationale + +Both are accepted. Rationale, so they are not re-litigated: + +- **FastMCP (marker over structural):** uniformity across all four app-bearing + bootstrappers is worth more than the sliver of state-accuracy the provider scan + gave. The only scenario the guard exists for — a second bootstrapper on the same + app — is detected identically by the marker. The regression requires user/framework + code to mutate `app.providers` *after* bootstrap, which no supported flow does. + Considered and rejected: keeping FastMCP on a bespoke structural check, which would + re-fragment detection and defeat the seam's whole point. +- **Litestar (config-level marker):** attaching at config level is the *only* option + given the app is built lazily; tagging the built app would require restructuring the + attach to bootstrap time. And sharing one mutable `AppConfig` across two intended + apps is already broken independently of teardown — instrument bootstrap mutates the + shared config's `cors_config`, `route_handlers`, and `openapi_config`. The marker + collision is one symptom of an already-unsupported pattern, not a new hazard. The + #130 warning text was reworded to name "this application or its configuration" so + the remediation is accurate. + +The genuinely actionable review finding from the same pass — marker set before a +fallible `attach()` — was *fixed* in #130 (mark only after `attach()` succeeds), not +accepted; it is out of scope for this decision. + +## Revisit trigger + +- **FastMCP:** a supported flow starts mutating `FastMCP.providers` after bootstrap + (e.g. a documented hot-reload / provider-swap API), making the attribute marker + diverge from real attach state. Then move FastMCP back to structural detection or + reconcile the marker with the provider list. +- **Litestar:** sharing one `AppConfig` across multiple apps becomes a supported, + documented pattern, or the attach is restructured to run at `bootstrap()` time + (when the app exists). Then tag the built `Litestar` app instead of the config. diff --git a/tests/test_fastapi_bootstrap.py b/tests/test_fastapi_bootstrap.py index ee2a048..3b24e05 100644 --- a/tests/test_fastapi_bootstrap.py +++ b/tests/test_fastapi_bootstrap.py @@ -127,8 +127,8 @@ def test_second_fastapi_bootstrapper_on_same_app_warns_not_stacks(fastapi_config warnings.simplefilter("always") FastAPIBootstrapper(bootstrap_config=config_b) - matching = [w for w in caught if "already has a lite-bootstrap lifespan" in str(w.message)] - assert matching, "expected warning about existing lite-bootstrap lifespan" + matching = [w for w in caught if "already has a lite-bootstrap teardown hook" in str(w.message)] + assert matching, "expected warning about existing lite-bootstrap teardown hook" assert application.router.lifespan_context is lifespan_after_first, ( "second bootstrapper must not re-wrap the lifespan" ) diff --git a/tests/test_fastmcp_bootstrap.py b/tests/test_fastmcp_bootstrap.py index 4185557..c775e25 100644 --- a/tests/test_fastmcp_bootstrap.py +++ b/tests/test_fastmcp_bootstrap.py @@ -280,8 +280,8 @@ def test_second_fastmcp_bootstrapper_on_same_app_warns_not_stacks() -> None: warnings.simplefilter("always") FastMcpBootstrapper(bootstrap_config=config_b) - matching = [w for w in caught if "_TeardownProvider" in str(w.message)] - assert matching, "expected warning about existing _TeardownProvider" + matching = [w for w in caught if "already has a lite-bootstrap teardown hook" in str(w.message)] + assert matching, "expected warning about existing lite-bootstrap teardown hook" assert list(application.providers) == providers_after_first, ( "second bootstrapper must not stack another _TeardownProvider" ) diff --git a/tests/test_faststream_bootstrap.py b/tests/test_faststream_bootstrap.py index ba49464..22d9a41 100644 --- a/tests/test_faststream_bootstrap.py +++ b/tests/test_faststream_bootstrap.py @@ -2,6 +2,7 @@ import logging import typing import uuid +import warnings from unittest.mock import AsyncMock, patch import faststream.asgi @@ -67,6 +68,19 @@ def build_faststream_config( ) +def test_second_faststream_bootstrapper_on_same_app_warns(broker: RedisBroker) -> None: + config_a = build_faststream_config(broker=broker) + FastStreamBootstrapper(bootstrap_config=config_a) + + config_b = dataclasses.replace(config_a) # shares the same application + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + FastStreamBootstrapper(bootstrap_config=config_b) + + matching = [w for w in caught if "already has a lite-bootstrap teardown hook" in str(w.message)] + assert matching, "expected warning about existing lite-bootstrap teardown hook" + + async def test_faststream_bootstrap(broker: RedisBroker) -> None: bootstrap_config = build_faststream_config(broker=broker) bootstrapper = FastStreamBootstrapper(bootstrap_config=bootstrap_config) diff --git a/tests/test_free_bootstrap.py b/tests/test_free_bootstrap.py index c891009..e0e3a50 100644 --- a/tests/test_free_bootstrap.py +++ b/tests/test_free_bootstrap.py @@ -1,4 +1,5 @@ import logging +import types import warnings from unittest.mock import MagicMock @@ -10,6 +11,7 @@ FreeConfig, TeardownError, ) +from lite_bootstrap.bootstrappers.base import BaseBootstrapper from lite_bootstrap.instruments.logging_instrument import LoggingInstrument from lite_bootstrap.instruments.pyroscope_instrument import PyroscopeInstrument from tests.conftest import CustomInstrumentor, SentryTestTransport, emulate_package_missing @@ -114,6 +116,41 @@ def test_free_bootstrapper_with_missing_instrument_dependency( FreeBootstrapper(bootstrap_config=free_bootstrapper_config) +def test_attach_teardown_once_attaches_then_warns_and_skips(free_bootstrapper_config: FreeConfig) -> None: + bootstrapper = FreeBootstrapper(bootstrap_config=free_bootstrapper_config) + target = types.SimpleNamespace() + attach = MagicMock() + + with warnings.catch_warnings(): + warnings.simplefilter("error") + bootstrapper._attach_teardown_once(target, attach) # noqa: SLF001 + + attach.assert_called_once() + assert getattr(target, BaseBootstrapper._TEARDOWN_MARKER) is True # noqa: SLF001 + + with pytest.warns(UserWarning, match="already has a lite-bootstrap teardown hook"): + bootstrapper._attach_teardown_once(target, attach) # noqa: SLF001 + + attach.assert_called_once() # still once — second attach skipped + + +def test_attach_teardown_once_does_not_mark_when_attach_raises(free_bootstrapper_config: FreeConfig) -> None: + bootstrapper = FreeBootstrapper(bootstrap_config=free_bootstrapper_config) + target = types.SimpleNamespace() + + failing = MagicMock(side_effect=RuntimeError("attach boom")) + with pytest.raises(RuntimeError, match="attach boom"): + bootstrapper._attach_teardown_once(target, failing) # noqa: SLF001 + + # A failed attach must leave the target untagged so a retry can re-attach. + assert not getattr(target, BaseBootstrapper._TEARDOWN_MARKER, False) # noqa: SLF001 + + retry = MagicMock() + bootstrapper._attach_teardown_once(target, retry) # noqa: SLF001 + retry.assert_called_once() + assert getattr(target, BaseBootstrapper._TEARDOWN_MARKER) is True # noqa: SLF001 + + def test_teardown_is_idempotent(free_bootstrapper_config: FreeConfig) -> None: bootstrapper = FreeBootstrapper(bootstrap_config=free_bootstrapper_config) bootstrapper.bootstrap() diff --git a/tests/test_litestar_bootstrap.py b/tests/test_litestar_bootstrap.py index 03b6496..e39e327 100644 --- a/tests/test_litestar_bootstrap.py +++ b/tests/test_litestar_bootstrap.py @@ -1,5 +1,6 @@ import dataclasses import gc +import warnings import weakref import litestar @@ -47,6 +48,23 @@ def litestar_config() -> LitestarConfig: ) +def test_second_litestar_bootstrapper_on_same_config_warns_not_stacks(litestar_config: LitestarConfig) -> None: + config_a = litestar_config + LitestarBootstrapper(bootstrap_config=config_a) + on_shutdown_after_first = len(config_a.application_config.on_shutdown) + + config_b = dataclasses.replace(litestar_config) # shares the same application_config + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + LitestarBootstrapper(bootstrap_config=config_b) + + matching = [w for w in caught if "already has a lite-bootstrap teardown hook" in str(w.message)] + assert matching, "expected warning about existing lite-bootstrap teardown hook" + assert len(config_a.application_config.on_shutdown) == on_shutdown_after_first, ( + "second bootstrapper must not stack another on_shutdown teardown" + ) + + def test_litestar_bootstrap(litestar_config: LitestarConfig) -> None: bootstrapper = LitestarBootstrapper(bootstrap_config=litestar_config) application = bootstrapper.bootstrap()