Skip to content

refactor(protocol): separate recognized and implemented capabilities - #479

Merged
SunSi12138 merged 1 commit into
devfrom
fix/401-negotiated-capability-authority
Sep 4, 2026
Merged

SunSi12138 merged 1 commit into
devfrom
fix/401-negotiated-capability-authority

Conversation

@SunSi12138

@SunSi12138 SunSi12138 commented Sep 4, 2026 •

Copy link
Copy Markdown
Owner

Fixes #401
Refs #397

Summary

This PR is based on dev.

dev already contained the central ProtocolV2Negotiator, immutable NegotiatedSessionOptions, and RpcSession.TryCompleteHandshake(...) publication boundary. The remaining issue #401 authority leak was narrower: endpoint implementation/advertisement was derived from the protocol-recognition set.

This change:

  • renames the protocol safety set from KnownCapabilities to RecognizedCapabilities
  • makes ProtocolV2Negotiator.AlwaysImplementedCapabilities an explicit declaration rather than deriving it from RecognizedCapabilities
  • keeps Compression conditional on configured provider bindings
  • preserves unknown optional peer-bit forward compatibility and existing negotiated-response validation
  • documents protocol recognition vs endpoint implementation/advertisement as separate facts
  • adds regression coverage so adding a recognized bit cannot silently make endpoints advertise it

Minor/version audit on dev

Current dev has MinorVersion == 5 and MinimumCompatibleMinorVersion == 5; minors 0–4 are outside the current wire generation. No mixed-minor capability-history table is introduced.

location purpose classification
ProtocolV2Constants / handshake fields wire grammar generation protocol boundary
ProtocolV2Negotiator compatibility validation and immutable negotiated options negotiation boundary
RpcSession.TryCompleteHandshake final publication safety check handshake publication boundary
ProtocolV2PayloadCodec serialize/parse minor codec boundary
request/stream/cancel/compression runtime paths no protocol-minor feature gates found negotiated capabilities remain sole feature authority

Capability authority split

  • RpcSessionProtocolRules.RecognizedCapabilities: bits understood by protocol/codec/session validation
  • ProtocolV2Negotiator.AlwaysImplementedCapabilities: capabilities explicitly implemented unconditionally by the endpoint
  • Compression: appended only when local provider bindings exist
  • NegotiatedSessionOptions.Capabilities: sole optional-feature authority after successful handshake

Adding a future bit to RecognizedCapabilities alone therefore cannot make Client/Server advertise it.

Validation

Staging validation on dev@bb717ae:

  • protocol minor/version usage audit — passed
  • diff/whitespace and no-KnownCapabilities guard — passed
  • complete SharpLink.UnitTests suite — passed

Formal PR CI on head 91a6a7a401ebe9e69926c3b90cec437011809727:

Temporary staging workflows/scripts were never present in the PR branch, and all temporary issue-401 staging branches were deleted after validation.

The PR was merged into dev at 2026-09-04 14:04:33Z; GitHub records merged_by: SunSi12138. Extended validation completed successfully on the same head/tree.

@SunSi12138
SunSi12138 marked this pull request as ready for review September 4, 2026 13:01

@SunSi12138 SunSi12138 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall direction looks sound, and the minor-to-capability history matches the commits that introduced minors 1/2/3. I left two non-blocking engineering comments: one about separating protocol-known capabilities from endpoint-advertised support, and one about making the mixed-minor test scope explicit so it is not mistaken for historical wire-interoperability coverage.

Comment thread src/SharpLink.Client/SharpLinkClient.Lifecycle.cs Outdated
Comment thread test/SharpLink.IntegrationTests/ProtocolMinorCapabilityIntegrationTests.cs Outdated
@github-actions
github-actions Bot force-pushed the fix/401-negotiated-capability-authority branch from 786d8aa to 7d452a6 Compare September 4, 2026 13:22
@SunSi12138
SunSi12138 force-pushed the fix/401-negotiated-capability-authority branch from 7d452a6 to 9d7276a Compare September 4, 2026 13:24
@SunSi12138
SunSi12138 marked this pull request as draft September 4, 2026 13:39
@SunSi12138
SunSi12138 force-pushed the fix/401-negotiated-capability-authority branch from 9d7276a to 274ab3b Compare September 4, 2026 13:52
@SunSi12138 SunSi12138 changed the title refactor(protocol): centralize negotiated capability authority refactor(protocol): separate recognized and implemented capabilities Sep 4, 2026
@SunSi12138
SunSi12138 changed the base branch from main to dev September 4, 2026 13:53
@SunSi12138
SunSi12138 force-pushed the fix/401-negotiated-capability-authority branch from 274ab3b to 91a6a7a Compare September 4, 2026 13:55
@SunSi12138
SunSi12138 marked this pull request as ready for review September 4, 2026 13:59
@SunSi12138
SunSi12138 merged commit 9f8c03b into dev Sep 4, 2026
22 checks passed
@SunSi12138
SunSi12138 deleted the fix/401-negotiated-capability-authority branch September 4, 2026 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Protocol Simplification] Make negotiated capabilities the sole runtime feature authority

1 participant