feat(server): migrate handlers to _get_app_initialized (#399 phase 2)#410
Merged
feat(server): migrate handlers to _get_app_initialized (#399 phase 2)#410
Conversation
Every @mcp.tool / @mcp.resource / @register handler now fetches the AppContext through ``await _get_app_initialized(ctx)`` instead of the bare ``_get_app(ctx)``. Phase 1 (#399) turned storage/embedder/ index_engine/search_pipeline into properties that read from a lazy ``_components`` slot; phase 3 will drop the eager init call from ``app_lifespan`` so handshake-only MCP sessions stop creating ``~/.memtomem/memtomem.db``. Phase 2 migrates the call sites so that flip is safe — a handler that still used ``_get_app`` would hit the ``_require_initialized`` guard on the first storage read. Behavior is unchanged today because ``app_lifespan`` still calls ``ensure_initialized`` eagerly. The helper is a thin wrapper: it calls ``_get_app`` and awaits ``ensure_initialized`` once (idempotent via ``_init_lock``), so the repeated awaits in the 100+ handler call sites have no cost after the first one. Scope: - server/context.py: new ``async _get_app_initialized`` helper - server/__init__.py: re-export - server/resources.py + 30 files under server/tools/: swap import + change ``app = _get_app(ctx)`` to ``app = await _get_app_initialized(ctx)`` - tests/test_context_window.py: update 7 monkeypatches to target the new helper via ``AsyncMock(return_value=app)`` - tests/test_tools_logic.py + tests/test_trust_ux.py: add ``ensure_initialized=AsyncMock()`` to the SimpleNamespace fakes so the await inside the helper resolves - tests/test_tool_registration.py: new regression guard ``test_handlers_use_get_app_initialized`` — AST-checks that no file under server/tools/ or server/resources.py imports bare ``_get_app``, preventing silent drift on new handlers Out of scope (filed separately): - #409 — ``_revert_to_stored`` writes to the read-only properties the phase 1 conversion introduced. Pre-existing latent bug surfaced by this PR's mypy run; recovery path with no test coverage. Fixing it here would mix concerns. Co-Authored-By: Claude <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@mcp.tool/@mcp.resourcehandler now fetches theAppContextviaawait _get_app_initialized(ctx)instead of the bare_get_app(ctx).ensure_initializedfromapp_lifespan). Without this migration, a handler still reaching through_get_appwould hit the_require_initializedguard the moment Phase 3 flips — handshake-only MCP sessions still need that flip to actually stop creating~/.memtomem/memtomem.db, but handlers have to be ready first.app_lifespancallingensure_initializedeagerly, so every handler still sees populated components. The migration is purely a call-site swap.What changed
server/context.pyasync _get_app_initialized(ctx) -> AppContext— one-line wrapper that calls_get_appand awaitsensure_initialized(idempotent via_init_lock, so repeated awaits across 100+ handler call sites have no cost after the first).server/__init__.pyserver/resources.py@mcp.resourcehandlers migrated.server/tools/*.py(30 files)app = _get_app(ctx)toapp = await _get_app_initialized(ctx). Import swap + single-line swap per handler, no logic changes.tests/test_context_window.pymemtomem.server.tools.search._get_appretargeted to_get_app_initializedviaAsyncMock(return_value=app)(the helper is async; synclambda _: appno longer works).tests/test_tools_logic.py+tests/test_trust_ux.py_fake_ctx/_recall_ctx/_search_ctxSimpleNamespacestubs gainensure_initialized=AsyncMock()so theawaitinside the helper resolves without touching real storage.tests/test_tool_registration.pytest_handlers_use_get_app_initialized— AST-checks that no file underserver/tools/orserver/resources.pyimports bare_get_app, so new handlers can't silently drift back.Deliberately out of scope
_revert_to_storedwrites to read-only AppContext properties (#399 phase 1 regression) #409 —_revert_to_storedwrites to read-onlyAppContextproperties. Pre-existing latent bug introduced by Phase 1's property conversion (embedder/search_pipeline/index_engine).mypy packages/memtomem/src/memtomem/serversurfaces it during this PR's lint pass; runtime impact is onmem_embedding_reset(mode="revert_to_stored")which has no test coverage. Filed as bug(server):_revert_to_storedwrites to read-only AppContext properties (#399 phase 1 regression) #409 and explicitly not fixed here — it's an orthogonal write-path bug, not a handler migration.ensure_initializedcall fromapp_lifespan+ moving watcher/scheduler/watchdog startup into the first-tool-call path is a separate PR. This PR makes that flip safe but doesn't perform it.Test plan
uv run ruff check packages/memtomem/src packages/memtomem/tests— passuv run ruff format --check packages/memtomem/src packages/memtomem/tests— passuv run pytest -m "not ollama"— 2172 passed (2171 pre-existing + 1 new guard). No regressions.test_server_app_context.py(Phase 1 lock-semantics tests) still pass — the helper builds on the sameensure_initializedpath those tests cover.uv run mypy packages/memtomem/src/memtomem/server— no new errors introduced by this PR (3 pre-existing read-only-property writes instatus_config._revert_to_storedtracked in bug(server):_revert_to_storedwrites to read-only AppContext properties (#399 phase 1 regression) #409).🤖 Generated with Claude Code