Repository navigation
fix(mesh): GetMeshNode's recycling re-probe carries the read's identity - #5444
Merged
meshweaver-cloud[bot] merged 1 commit intoSep 23, 2026
Merged
Conversation
GetMeshNodeOutcome re-issues its GetDataRequest when the owner NACKs
ShuttingDown. The first probe is posted from Subscribe, where the caller's
identity is ambient; the re-probe is posted from the NACK's OnError callback
(or a pacing timer) and re-read the ambient there. hub.Observe restores the
captured identity on OnNext only, and the Orleans router's NACK for a
deactivating grain (RoutingGrain.PostFailure) carries no AccessContext, so on
portal/reads-{meshId} (a User hub with no user) the re-probe was failed closed
by the never-null guard and the read settled Unavailable.
Capture the identity once at Subscribe and pin it on every probe with
WithAccessContext. A caller with no identity still fails closed as before.
Fixes #5227, #5231, #5232, #5431, #5433, #5434, #5435, #5438, #5439.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
Contributor
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Use required-service resolution for AccessService before approval.
Review effort: Lite
Findings: None
What changed in this PR
Preserves caller access identity across GetMeshNode recycling re-probes.
Changes:
- Pins caller identity on every read probe.
- Adds regression coverage for identity-less shutdown NACKs.
- Documents the retry identity-propagation pattern.
| File | Summary |
|---|---|
test/MeshWeaver.Data.Test/GetMeshNodeReProbeKeepsCallerIdentityTest.cs |
Adds regression coverage for retry identity propagation. |
src/MeshWeaver.Mesh.Contract/MeshNodeStreamExtensions.cs |
Captures and applies caller identity; moderate finding: resolve AccessService as a required dependency. |
src/MeshWeaver.Documentation/Data/Architecture/AccessContextPropagation.md |
Documents the retry/re-probe identity anti-pattern. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
This was referenced Sep 23, 2026
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.
What
GetMeshNodeOutcome(src/MeshWeaver.Mesh.Contract/MeshNodeStreamExtensions.cs) now captures the caller's identity once, at Subscribe, and pins it on everyGetDataRequestit posts witho.WithAccessContext(caller). Before this change only the first probe carried it.Why: the
hub=portal/reads-…, message=GetDataRequestfamilyEvery incident in this family has the same log line:
PostPipeline: AccessContext must never be null … hub=portal/reads-{meshId}, message=GetDataRequest, target=<page path>(Home, Store, AgenticEngineering, Mail, Google, Stripe, ThreeBody, Reinsurance, Doc/AI, …).portal/reads-{meshId}is the mesh's shared read-issuing hub (MeshExtensions.MeshReadHub). It hasPostingIdentity.Userand no user of its own.GetMeshNodeissues from it whenever the caller holds the root mesh hub. One such caller is the prerender readIMeshService.GetPreRenderedHtml(path), whichApplicationPagecalls with the URL path aspath.ShuttingDown(it is recycling),GetMeshNodeOutcomesends the read again. The first probe is posted from Subscribe, where the caller's identity is set. The re-probe is posted from the NACK'sOnErrorcallback (and after the second NACK, from a pacing timer), and it read the identity from the current thread again.hub.Observerestores the captured identity onOnNextonly. So theOnErrorcallback runs under whatever identity the NACK delivery carries. The Orleans router's NACK for a deactivating target grain (RoutingGrain.PostFailure) carries none. The re-probe then had no AccessContext, the never-null guard refused it and logged the error, and the read settledUnavailable. The user's page lost its pre-rendered body.This fits the evidence: the incidents are rare, they come in bursts after rolls and package activity, and #5231 shows the same target on two pods 17 s apart. That is a recycling root read by several readers.
Repro (negative control run)
GetMeshNodeReProbeKeepsCallerIdentityTest(MeshWeaver.Data.Test) sets up a User-identity issuing hub with no standing identity, puts the caller's identity in scope only for the call, and uses an owner that NACKs the first probeShuttingDownwith no AccessContext (the wayRoutingGrain.PostFailuredoes) and answers the second.Unavailable, only the first probe arrived, and the test host logged the production line verbatim:PostPipeline: AccessContext must never be null … hub=client/…, message=GetDataRequest, target=client/… was posted with no AccessContext (no Context, no CircuitContext).ResponseFor(which carries the requester's identity back) passes on unfixed code. That is why the test uses the identity-less NACK the Orleans router actually sends.No identity is invented. A caller with no identity still fails closed, exactly as before. Nothing is stamped as hub-self or System.
Docs
Doc/Architecture/AccessContextPropagationhas a new anti-pattern row: a retry or re-probe that reads the identity from the current thread inside a NACK callback or timer.Verification after deploy
These incidents log only when a page's root is recycling. After the roll, the fingerprints listed below should go quiet across the next rolls or package installs.
Pairs-with: none — no public type or member removed; one private local added inside an existing method.
Fixes #5227
Fixes #5231
Fixes #5232
Fixes #5431
Fixes #5433
Fixes #5434
Fixes #5435
Fixes #5438
Fixes #5439
Fixes #1253
(#1253's original
portal/nodeops-*producer was fixed by #1279. Its reopens since 2026-09-22 are all thisportal/reads-*shape, folded onto the shared fingerprint.)🤖 Generated with Claude Code