feat: introduce auth and server-state protocols for hosted deployment#59
Conversation
### what Move the GraphQL schema and docs index out of per-user client objects into the server-level `ServerState` cache: - Add `ensure_cached_async` to `ServerState` for objects that require async construction - `PlatformClient.get_schema()` delegates to `server_state.ensure_cached_async` with a new `_fetch_schema` helper; removes the per-instance `_schema_cache` - `DocsClient.get_index()` delegates to `server_state.ensure_cached_async` with a new `_fetch_index` helper; removes the per-instance `_index` list - Both clients now receive the `ServerState` at construction time via their `get()` factory methods ### why Per-user client objects are constructed fresh for each user in a hosted deployment, so any cache stored on them is also per-user. The GraphQL schema and docs index are tenant-wide resources that should be fetched once and shared, not re-fetched for every user. ### testing All 215 existing tests pass. ### docs No documentation changes needed. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
- `ensure_cached_async`: fix race condition under concurrent async callers by using per-key `asyncio.Lock` with a double-check pattern; multiple coroutines entering before any result is stored now wait on the lock rather than each issuing a duplicate network request - `PlatformClient.get`, `DocsClient.get`: remove redundant `assert ctx.request_context is not None` inside `construct()`; the outer `server_cached` call already asserts this with a descriptive message, so the inner assert was dead code. Use `# type: ignore[union-attr]` to satisfy mypy instead. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Introduce two protocols that allow the MCP server to be parameterized for a hosted multi-user deployment: **AuthProvider protocol** (`server.py`): - Defines `get_credentials(ctx) -> StackletCredentials` - `LocalAuthProvider` (`auth.py`) implements it for local stdio use, loading credentials from `~/.stacklet/` files or env vars (cached per process lifetime) - `StackletCredentials.get()` now delegates to the auth provider stored on server state, rather than calling `server_cached` directly **ServerStateProtocol** (`lifespan.py`): - Extends the server state interface with `auth_provider`, `ensure_cached`, and `ensure_cached_async` - `ServerState` implements the protocol with the existing dict-based process-lifetime cache, now taking `auth_provider` at construction time - `make_lifespan(auth_provider, state_factory=None)` replaces the bare `lifespan` context manager; a hosted deployment can pass a custom `state_factory` to create per-request or per-user state - `make_server(auth_provider=None, state_factory=None)` wires everything together, defaulting to local behavior Both client `get()` methods (`PlatformClient`, `DocsClient`) now type their `server_state` parameter as `ServerStateProtocol` rather than the concrete `ServerState`. A hosted MCP deployment serves multiple users over HTTP/SSE, so credentials must come from per-request forwarded auth headers rather than a shared local config file. Parameterizing both auth and server state behind protocols lets the hosted deployment provide its own implementations without touching the tool or client code. All 215 existing tests pass. No documentation changes needed. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 804db61732
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Greptile SummaryThis PR introduces two extension points —
Confidence Score: 5/5Safe to merge — the auth and state protocol changes are well-scoped, local behaviour is fully preserved by the defaults, and all 215 tests pass. The refactor correctly threads No files require special attention; the one comment left is a non-blocking resource-management observation about unclosed Important Files Changed
Sequence DiagramsequenceDiagram
participant Tool as MCP Tool
participant SC as StackletCredentials.get()
participant State as ServerState (lifespan context)
participant AP as AuthProvider
participant LC as LocalAuthProvider
participant FS as load_stacklet_auth()
Tool->>SC: get(ctx)
SC->>State: ctx.request_context.lifespan_context
SC->>AP: state.auth_provider.get_credentials(ctx)
AP->>LC: (LocalAuthProvider instance)
LC->>State: server_cached(ctx, "STACKLET_CREDS", load_stacklet_auth)
alt first call (not cached)
State->>FS: load_stacklet_auth()
FS-->>State: StackletCredentials
State-->>LC: cached credentials
else subsequent calls
State-->>LC: cached credentials
end
LC-->>SC: StackletCredentials
SC-->>Tool: StackletCredentials
Note over Tool,FS: PlatformClient / DocsClient / AssetDBClient each call .get() per tool invocation
Tool->>State: ensure_cached("HTTP_TRANSPORT", AsyncHTTPTransport)
State-->>Tool: shared transport (cached)
Tool->>Tool: "httpx.AsyncClient(transport=shared_transport, ...)"
Note over State: At shutdown: state.aclose() -> transport.aclose()
Reviews (4): Last reviewed commit: "Fix the lint." | Re-trigger Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f7fa40cd1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
| ) | ||
|
|
||
| monkeypatch.setattr("stacklet.mcp.stacklet_auth.load_stacklet_auth", lambda: fake_credentials) | ||
| monkeypatch.setattr("stacklet.mcp.auth.load_stacklet_auth", lambda: fake_credentials) |
There was a problem hiding this comment.
Nothing tests the new credential path — every test builds the server with defaults, so StackletCredentials.get routing through auth_provider, and ServerState.aclose() closing the transport, are both unexercised. A regression that broke the provider delegation or skipped cleanup on exit would stay green across the suite. Worth one test that passes a stub provider and asserts it's used, plus one that asserts aclose() closes the cached transport.
| async def get_index(self) -> list[DocFile]: | ||
| """Fetch documents index, using the server-level cache.""" | ||
| return await self.server_state.ensure_cached_async("DOCS_INDEX", self._fetch_index) | ||
| return cast( |
There was a problem hiding this comment.
small one: the protocol types ensure_cached/ensure_cached_async as -> Any, dropping the ServerCached TypeVar the concrete ServerState keeps — that's why get_index needs the cast(list[DocFile], …) here. Giving the protocol methods the TypeVar lets the cast go away and also clears a Returning Any on get_schema in graphql.py. (Only uv run mypy surfaces it — the pre-commit hook doesn't install graphql-core/httpx.)
what
Introduce two protocols that allow the MCP server to be parameterized for a hosted multi-user deployment:
AuthProvider protocol (
server.py):get_credentials(ctx) -> StackletCredentialsLocalAuthProvider(auth.py) implements it for local stdio use, loading credentials from~/.stacklet/files or env vars (cached per process lifetime)StackletCredentials.get()now delegates to the auth provider stored on server state, rather than callingserver_cacheddirectlyServerStateProtocol (
lifespan.py):auth_provider,ensure_cached, andensure_cached_asyncServerStateimplements the protocol with the existing dict-based process-lifetime cache, now takingauth_providerat construction timemake_lifespan(auth_provider, state_factory=None)replaces the barelifespancontext manager; a hosted deployment can pass a customstate_factoryto create per-request or per-user statemake_server(auth_provider=None, state_factory=None)wires everything together, defaulting to local behaviorBoth client
get()methods (PlatformClient,DocsClient) now type theirserver_stateparameter asServerStateProtocolrather than the concreteServerState.why
A hosted MCP deployment serves multiple users over HTTP/SSE, so credentials must come from per-request forwarded auth headers rather than a shared local config file. Parameterizing both auth and server state behind protocols lets the hosted deployment provide its own implementations without touching the tool or client code.
testing
All 215 existing tests pass.
docs
No documentation changes needed.