Skip to content

App lifecycle: async open/close + DI seam - #5

Merged
sagi5060 merged 2 commits into
devfrom
feat/1-app-lifecycle
Jul 26, 2026
Merged

App lifecycle: async open/close + DI seam#5
sagi5060 merged 2 commits into
devfrom
feat/1-app-lifecycle

Conversation

@sagi5060

Copy link
Copy Markdown
Collaborator

Closes #1

What

  • App.open(): an async classmethod context manager that builds the app,
    runs load(), starts MCPLifecycle, and guarantees aclose() runs on
    exit (even on error).
  • App.aclose(): closes the SessionFactory's Redis client and shuts down
    MCPLifecycle. Idempotent — a second call is a no-op, guarded by a
    _closed flag.
  • App(session_factory=...): the constructor now accepts an optional
    SessionFactory override so tests can inject a fake (e.g. one wrapping
    fakeredis) instead of building a real Redis client from settings.
  • serve.py: create_app() now wires App.open()/aclose() through a
    FastAPI lifespan, so compose stop (SIGTERM) closes the Redis client and
    MCP servers cleanly instead of leaking them.

Why

App had no shutdown path — the Redis client and MCP lifecycle were never
closed, and there was no seam to swap infrastructure in tests. This lands the
minimal DI + lifecycle surface the issue asks for, without adding any new
catalog/execution logic (agentdeck still just owns configuration).

Decisions / notes

  • MCPLifecycle is a process-wide classmethod registry (not per-App), so
    open()/aclose() call its startup()/shutdown() directly — both are
    already individually idempotent, matching App.aclose()'s idempotency.
  • serve.py's lifespan calls deck.load() a second time (after open()
    already ran it once) purely to capture the inventory dict for /health;
    load() is idempotent (refresh=True) and cheap, so this is deliberate,
    not a leftover.

Test plan

  • ruff check agentdeck/ tests/
  • ty check agentdeck
  • pytest tests/ -q (added test_open_close_lifecycle: open → session_for
    plumbing (no live model, SQLite fallback) → aclose, then a second
    aclose() to confirm idempotency)

🤖 Generated with Claude Code

App had no shutdown path: SessionFactory's Redis client and the MCP
lifecycle were never closed, and serve.py had no FastAPI lifespan. App.open()
now loads the project, starts MCPLifecycle, and guarantees aclose() runs on
exit; aclose() closes the Redis session client and MCP servers and is
idempotent. The constructor also accepts an optional session_factory to
inject a fake in tests. serve.py wires this through a FastAPI lifespan so
`compose stop` shuts down cleanly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

…rrect

- aclose(): try/finally so the Redis session client and the MCP lifecycle are
  both torn down even if the first step raises.
- open(): load() and MCPLifecycle.startup() now run inside the try, so a
  failure there no longer leaks an injected session_factory.
- aclose() shuts down the process-wide MCP registry only when this App
  started it (`_started_mcp`), so a nested App or a bare App().aclose() no
  longer kills servers it doesn't own.
- serve: `api.state.deck` is initialized to None and every endpoint answers
  503 before the lifespan starts (/health reports {"status": "starting"})
  instead of raising AttributeError or reporting an empty inventory as ok.
- serve: drop the second load() per boot — load() stashes App.inventory and
  /health reads it, so graphs are compiled once.
- tests: cover the session_factory DI seam (bypasses from_settings, closed
  once, closed when load() raises), the MCP ownership guard, and the
  pre-startup 503s.
- docs: App docstring + README recommend `async with App.open()`.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@sagi5060
sagi5060 merged commit 4e1d46d into dev Jul 26, 2026
1 check passed
@sagi5060
sagi5060 deleted the feat/1-app-lifecycle branch July 27, 2026 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

App lifecycle: async open/close + DI seam

1 participant