docs(plan): #1705 Phase 2 scope shape decisions and review findings in the implementation plan - #1862
Conversation
Add amendment M6 (scope shape): a delegate-only IRequestContextScopeFactory (RunAsServiceAsync, RunAsPrincipalAsync, RunInboundAsync, RunRestoredAsync), holder origin and kind that survive invalidation (Q1), an identity- and origin-preserving setter, issuer-stamped identities checked at Resolve and at read time (Q3), per-token claim exclusion (Q2), the accessor-type validator, and the Phase 3 connection-endpoint rules (WebSocket and HubMetadata skip, connection-origin marker, Blazor masking scope, misordered routing logged). Apply section 7 of the PR #1849 design record and both of its reviews, rewrite the Phase 2 tasks and prompt, budget the test migrations, and add the "Review log (Phase 2 scope shape)" with two open maintainer questions. Refs #1705
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request. Check if the Files changed in this pull request are included in default exclusions.
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
⚙️ Run configuration
📒 Files selected for processing (1)
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…se 2 plan MQ-1: skip Accept: text/event-stream requests under the connection marker and answer 500 to every request after the first detected UseEncinaContext-before-UseRouting misordering. MQ-2: the connection-origin marker is a confirmed decision.
|
@coderabbitai review |
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request. Check if the Files changed in this pull request are included in default exclusions.
|
… to the #1705 plan WebSocket upgrades are detected through IHttpUpgradeFeature and IHttpExtendedConnectFeature, the misorder latch trips only on a hub endpoint and lives on the middleware instance, every new context holder inherits the facts of the current one, the tenant rule applies to every accepted explicit context, user ids leave the authorization and EF Core interceptor logs, and ordinary SSE endpoints opt in per event through a public InboundRequestInfo builder. Records D1-D4, the EventIds 172-174, the CS0051 decision, the test budget gaps and a review log; rewrites the Phase 2 and Phase 3 prompts.
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request. Check if the Files changed in this pull request are included in default exclusions.
…#1705 plan E1: the misordering latch needs an unchanged Request.Path; a changed path logs Warning 203 and latches nothing. E2: an External restore ignores the persisted tenant and accepts only a trusted tenant from dispatcher configuration. E3: the connection skip applies only to GET requests that accept text/event-stream. Also applies minors 3 and 5-9, the 174 service name, and the Information log for application calls of RunInboundAsync.
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request. Check if the Files changed in this pull request are included in default exclusions.
|
@coderabbitai review |
|
Summary
This updates the #1705 implementation plan with the Phase 2 scope-shape decisions. The analysis is in PR #1849, kept as the design record and not merged. The maintainer decisions are in the #1705 comments of 2026-10-05.
Decisions applied
IRequestContextScopeFactoryis delegate-only:RunAsServiceAsync,RunAsPrincipalAsync,RunInboundAsync,RunRestoredAsync.RunAsBuiltInAsynclives on an internal interface. Ending a scope only invalidates its holder; the caller's context is restored by the async frame.PerTokenClaimTypesis configurable.auth_timeis no longer excluded;nonce,at_hashandc_hashare added. A validator rejects excluding the subject, role, permission,amrandacrtypes.IdentityIssuertoken, one identity per scope. Liveness is checked both inResolveand when the identity is read. Issuer-less explicit identities are refused. The rule order is stated.TStateoverloads. Phase 3 adds a benchmark of the middleware and the circuit scope.Review findings folded in, from the pr-reviewer and the adversarial review of PR #1849:
HubMetadataendpoints are skipped, under a connection-origin marker scope;TenantResolutionMiddlewareapplies the same skip.Leftbranch runs inside an anonymous masking scope.!IsDispatchInFlight.IsSameAs.SuppressFlow.A new "Review log (Phase 2 scope shape)" table maps every finding to its plan change.
Maintainer decisions: MQ-1 is (b) and (c) together, and MQ-2 is confirmed. D1-D4 come from the adversarial review of this PR. All of them are recorded in the #1705 comments of 2026-10-05 and in the plan.
The plan was self-reviewed with
docs-reviewer: 2 majors and 7 minors, all fixed.Refs #1705