Skip to content

Conversation

@Apollon77
Copy link
Collaborator

This PR is a "all and nothing PR" collecting several fixes and optimizations I run across while testing. Some highlights are:

  • When decommisioning a node read the response to know if fabric removal was successful. The response is custom and so not throws and error by default
  • Allow to pass through a custom commissioning flow from options in new CommissioningClient
  • adjust some initialization order for PeerSet and FabricManager and adding checks to prevent duplicate creations in wrong environments
  • export some classes not exported but needed later
  • only arm Failsafe when an incoming PASE session was created, not when we create one
  • Move the Fabric-sensitive data sanitation into ServerEndpointInitializer
  • enhance some behaviors methods by String variants
  • guard double Node.close() calls
  • Remove only subscriptions from same peer for keepSubscriptions=false, not all

Nothing is needed for changelog because most smaller internal optimizations or introduced by recent commits

Copilot AI review requested due to automatic review settings December 9, 2025 16:37
@Apollon77 Apollon77 requested a review from lauckhart as a code owner December 9, 2025 16:37
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR consolidates various internal fixes, optimizations, and preparatory changes to the Matter.js implementation. The changes primarily focus on improving code reliability, proper resource handling, and fixing edge cases in commissioning and subscription management.

  • Fixed subscription clearing logic to only remove subscriptions from the same peer when keepSubscriptions=false
  • Added guard against double Node.close() calls and proper async handling in controller behaviors
  • Moved fabric-sensitive data sanitation from Behaviors to ServerEndpointInitializer for better separation of concerns

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/protocol/src/protocol/MessageExchange.ts Improved comment clarity about resubmission handling
packages/protocol/src/fabric/FabricAuthority.ts Added TODO comment for fabric certificate validation
packages/protocol/src/certificate/CertificateAuthority.ts Enhanced log message for credential loading
packages/node/src/node/server/ServerSubscription.ts Added TODO comment about timing consideration
packages/node/src/node/server/ServerEndpointInitializer.ts Moved fabric-scoped data limitation logic here from Behaviors
packages/node/src/node/server/InteractionServer.ts Fixed subscription clearing to only affect same peer's sessions
packages/node/src/node/index.ts Exported ClientNode class
packages/node/src/node/client/index.ts Exported ClientNodeInteraction
packages/node/src/node/NodePhysicalProperties.ts Optimized by using maybeStateOf and extracting status variable
packages/node/src/node/Node.ts Added guard against double close() calls
packages/node/src/endpoint/properties/Endpoints.ts Renamed parameter from 'number' to 'id' for better clarity
packages/node/src/endpoint/properties/EndpointInitializer.ts Changed behaviorsInitialized return type to MaybePromise
packages/node/src/endpoint/properties/Behaviors.ts Added string overloads for has() and versionOf() methods; moved fabric logic
packages/node/src/behaviors/group-key-management/GroupKeyManagementServer.ts Removed unnecessary MaybePromise return type
packages/node/src/behaviors/general-diagnostics/GeneralDiagnosticsServer.ts Removed unnecessary MaybePromise return type
packages/node/src/behaviors/general-commissioning/GeneralCommissioningServer.ts Added check to only arm failsafe for incoming PASE sessions
packages/node/src/behaviors/basic-information/BasicInformationServer.ts Removed unnecessary MaybePromise return type and blank line
packages/node/src/behaviors/access-control/AccessControlServer.ts Removed unnecessary MaybePromise return type
packages/node/src/behavior/system/network/index.ts Exported NetworkClient
packages/node/src/behavior/system/controller/ControllerBehavior.ts Made nodeOnline async and added fabric label synchronization
packages/node/src/behavior/system/commissioning/index.ts Exported RemoteDescriptor
packages/node/src/behavior/system/commissioning/RemoteDescriptor.ts Fixed toLongForm to return the long object
packages/node/src/behavior/system/commissioning/CommissioningServer.ts Moved FabricManager event registration to enterOnlineMode
packages/node/src/behavior/system/commissioning/CommissioningClient.ts Simplified passcode validation; added removeFabric response validation; added commissioningFlowImpl option
packages/general/src/util/Cancelable.ts Fixed spelling: "cancelation" to "cancellation"

@lauckhart lauckhart merged commit 206ca2d into main Dec 9, 2025
36 checks passed
@lauckhart lauckhart deleted the cutout0912 branch December 9, 2025 21:23
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.

3 participants