Repository navigation
MCP OAuth: replica-safe authorization codes via mesh-backed store - #609
Merged
Merged
Conversation
The OAuth authorization-code store was an in-memory per-process dictionary,
so with multiple KEDA replicas behind one ingress the /authorize code minted
on pod A could not be exchanged at /token on pod B ("unknown or already
consumed code") — MCP clients could not connect at all.
Codes are now mesh nodes at Admin/OAuthCode/{sha256-prefix} (PG-persisted,
shared by all replicas), mirroring the ApiToken scheme: hash-keyed, raw code
never stored, written under System identity. Single-use is atomic across
replicas — consumption IS the DeleteNode; a lost race maps to invalid_grant,
never a second success. 5-minute expiry checked post-consume, opportunistic
cleanup of expired siblings on the generate path, PKCE unchanged, reactive
surface end-to-end with a single ToTask bridge at the HTTP boundary.
Also fixes the framework defect this exposed: a request delivered to a
per-node hub disposing after its delete was silently dropped, hanging the
sender for its full 30s timeout. MessageService now NACKs the awaiting
sender with DeliveryFailure(NotFound) via the parent hub.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes MCP OAuth authorization codes replica-safe by replacing the per-process in-memory authorization-code store with a mesh-backed (PG-persisted) store, so /authorize and /token can land on different portal replicas without breaking the OAuth code exchange. It also hardens message delivery during hub shutdown to fail fast (instead of silently dropping awaited requests), and adds regression tests + a What’s New entry.
Changes:
- Persist OAuth authorization codes as
Admin/OAuthCode/{sha256-prefix}mesh nodes (raw code never stored) with atomic single-use semantics viaDeleteNode. - Update OAuth controller + tests to use the reactive mesh-backed store and validate cross-instance exchange behavior.
- NACK awaited requests that are dropped during hub shutdown, and document the behavior change.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.Auth.Test/OAuthConnectControllerTests.cs | Updates controller tests to use the mesh-backed OAuthCodeStore and await code-node visibility before exchanging. |
| test/Memex.Portal.Shared.Test/OAuthCodeStoreTest.cs | Converts OAuthCodeStore tests to run on a full monolith mesh and adds a cross-store-instance exchange test (replica simulation). |
| test/Memex.Portal.Shared.Test/Memex.Portal.Shared.Test.csproj | Adds Monolith testbase reference required for mesh-backed OAuthCodeStore tests. |
| src/MeshWeaver.Messaging.Hub/MessageService.cs | Adds shutdown-path NACK behavior to avoid silent drops that can stall callers awaiting responses. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-07-23-mcp-connect-reliability.md | Adds a What’s New entry describing reliable MCP sign-in with multiple replicas. |
| memex/Memex.Portal.Shared/MemexConfiguration.cs | Registers the OAuthCode NodeType/content type in the portal mesh configuration. |
| memex/Memex.Portal.Shared/Authentication/OAuthConnectController.cs | Bridges /authorize to the reactive, mesh-backed code store and updates /token to use the new exchange result shape. |
| memex/Memex.Portal.Shared/Authentication/OAuthCodeStore.cs | Replaces in-memory dictionary with mesh-node persistence and single-use exchange via delete-first semantics. |
| memex/Memex.Portal.Shared/Authentication/OAuthCodeNodeType.cs | Introduces the OAuthCode NodeType definition and registers the AuthorizationCode content type. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
…— sync streams ride it out
Fixes the two CI regressions from the shutdown-drop NACK (run 30003419841:
NodeTypeCompileParkTest.ParkedNodeType_RecycleRetry_AfterFix_SettlesOkAndUnparks,
ExportImportAccessControlTest.Export_EditorFullAccess_ExportsWholeSubtree)
WITHOUT reverting to silent drops (the OAuth consumed-code 30s-hang fix stays).
Root cause: the NACK a disposing hub now posts for a dropped delivery was shaped
as a terminal failure (NotFound). But the very same drop fires when a
SubscribeRequest races a RECYCLE's DisposeRequest — the address is about to
REACTIVATE. The sync-stream subscribe machinery treated the correlated
DeliveryFailure as "owner does not exist": reduced.OnError + keepAlive.Dispose()
tore down exactly the recovery machinery (keep-alive + change-feed resubscribe
latch, which demonstrably re-sends the SubscribeRequest ~2s after a recycle)
that used to ride out the silent drop — so every reader of a mid-recycle
NodeType waited to its timeout and the recycle-retry compile flow wedged.
Fix shape — make the transient nature first-class:
- ErrorType.ShuttingDown (Messaging.Contract): transient rejection; the address
may reactivate on the next probe. Mirrors the Orleans mid-DeactivateOnIdle
reject ("invalid activation. Rejecting now.").
- MessageService.ScheduleNotify stamps the shutdown-drop NACK with it.
- JsonSynchronizationStream's initial-subscribe error path keeps the stream and
keep-alive ALIVE on ShuttingDown (recovery stays event-driven via the
change-feed resubscribe latch — no timer, no re-post); all other failures
remain terminal (never heartbeat a non-existent owner).
- SynchronizationStream's uncorrelated DeliveryFailure handler likewise ignores
ShuttingDown instead of erroring the stream.
- MeshNodeStreamCache.IsTransientOwnerFailure + AreaErrorClassifier
.IsTransientHubFailure recognise the "is shutting down" banner so all layers
classify it transient (cache never negative-caches it; GUI retries).
Verified locally: RecycleRetry green in 5s (was 90s timeout; pre-NACK baseline
bisected green to pin the NACK as the trigger; msg-trace pinned the drop to the
cache's SubscribeRequest racing the recycle DisposeRequest and the +2s latch
resubscribe that the terminal error was killing). Full battery green:
NodeTypeCompileParkTest 6/6, Export_EditorFullAccess 1/1, OAuthCodeStore 11/11,
OAuthConnectController 26/26, MeshWeaver.Messaging.Hub.Test 102/102; Release
-warnaserror clean for Messaging.Contract/Hub, Data, Hosting, Layout,
Memex.Portal.Shared.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… immutability fixes 1. MessageService: the shutdown-drop NACK now fires only for deliveries a caller can actually be awaiting — typed IRequest messages, plus RawJson. RawJson must stay in the gate: cross-hub deliveries reach ScheduleNotify UNDESERIALIZED (the MESHWEAVER_MSG_TRACE captures show the dropped GetDataRequest and SubscribeRequest both as msg=RawJson at this gate), so an IRequest-only gate — the review's literal suggestion — would silently skip exactly the two flows the NACK exists for (the OAuth consumed-code read and the sync-stream subscribe) and reintroduce the 30s hang. RawJson payloads that look like a DeliveryFailure are skipped via a cheap content sniff (a NACK-of-NACK between two concurrently-disposing hubs would ping-pong; a sniff false-positive only reverts that delivery to the historical silent drop). Typed fire-and-forget events (DataChangedEvent & co) revert to the historical silent drop — nobody awaits them, and NACKing them was pure disposal noise. 2. OAuthCodeStore.ExtractEntry doc: AuthorizationCode IS registered (AddOAuthCodeType/WithMeshType); the JsonElement fallback covers hubs that have not registered the type and older persisted rows. 3. OAuthCodeNodeType: ExcludeFromContext now ImmutableHashSet per the collections policy. Verified: NodeTypeCompileParkTest 6/6, Export_EditorFullAccess 1/1, OAuthCodeStore 11/11 (covers the NACK fast-fail via the single-use replay tests), MeshWeaver.Messaging.Hub.Test 102/102; Release -warnaserror clean for MeshWeaver.Messaging.Hub + Memex.Portal.Shared. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 23, 2026
Merged
Flush pending own-node saves on hub dispose (acked-write loss; TwoSiloRecycleConvergence flake)
#627
Merged
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
MCP clients could not connect to memex-cloud: the OAuth authorization-code store was an in-memory per-process dictionary, so with 2–8 KEDA replicas behind one ingress,
/authorizeminted the code on pod A and the client's/tokenexchange landed on pod B →unknown or already consumed code (never issued by this process)(observed on 3 pods in prod Loki, 2026-07-23 09:58–10:01). Pod restarts also wiped pending codes.Fix
Admin/OAuthCode/{sha256-prefix}(PG-persisted, shared across replicas), mirroring the ApiToken scheme: hash-keyed, raw code never stored, System identity writes, newOAuthCodeNodeTyperegistration.DeleteNode— the exchange is honored only when this caller's delete removed the node; a lost race →invalid_grant, never a second success.ToTaskbridge at the HTTP boundary; diagnosableFailureReasonstrings kept.MessageServicenow NACKs the sender withDeliveryFailure(NotFound)via the parent hub (cascade-safe, same exclusions asReportFailure).Verification
Memex.Portal.Shared.TestOAuthCodeStore: 11/11 (incl.TwoStoreInstances_GenerateOnOne_ExchangeOnOther— two store instances on one mesh = two replicas on one PG)MeshWeaver.Auth.TestOAuthConnectController: 26/26MeshWeaver.Messaging.Hub.Testfull regression: 102/102-warnaserrorclean for all touched projects + dependents🤖 Generated with Claude Code