fix(model): provisional conformance no longer implies mandatory - #4167
Open
Apollon77 wants to merge 1 commit into
Open
fix(model): provisional conformance no longer implies mandatory#4167Apollon77 wants to merge 1 commit into
Apollon77 wants to merge 1 commit into
Conversation
An otherwise list whose terms follow a "P" describes the conformance intended once the element is no longer provisional. Applicability caps those terms at optional, matching Conformance.isMandatory and the runtime conformance compiler, which already read "P, M" this way. The variance engine agrees, so generated types declare provisional elements optional rather than always present. GroupcastServer now enables the elements it implements. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aligns the model-layer conformance/applicability logic with Matter spec §7.3 so that provisional (P) conformance no longer implies mandatory for the terms that follow it, and propagates that change through variance inference, generated types, and node behaviors.
Changes:
- Update
Conformance.computeApplicabilityto cap “intended” mandatory conformance followingPat Optional, while preserving feature/condition gating and keeping standalonePOptional (not excluded). - Apply the same “provisional caps at optional” rule in the cluster-variance inference engine, affecting generated typing of provisional elements.
- Add/adjust tests (model + node) and update
GroupcastServerto explicitlyenable()the provisional elements it implements; update changelog to document the breaking behavior change.
Reviewed changes
Copilot reviewed 8 out of 16 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/model/src/aspects/Conformance.ts | Adjusts applicability computation for provisional (P) terms to match spec semantics. |
| packages/model/src/logic/cluster-variance/InferredComponents.ts | Ensures inferred variance treats provisional intended conformance as optional while preserving conditions. |
| packages/model/test/aspects/ConformanceTest.ts | Adds direct test coverage for applicability (including provisional cases). |
| packages/model/test/logic/ClusterVarianceTest.ts | Updates/adds variance tests to reflect provisional-as-optional behavior. |
| packages/node/src/behaviors/groupcast/GroupcastServer.ts | Uses ClusterBehavior.enable() to declare which provisional Groupcast elements are implemented. |
| packages/node/test/behaviors/thread-network-diagnostics/ThreadNetworkDiagnosticsProvisionalTest.ts | Adds node-level contract test asserting provisional ThreadNetworkDiagnostics attrs are absent by default. |
| packages/node/test/behaviors/group-key-management/GroupKeyManagementServerTest.ts | Adds test asserting provisional GroupcastAdoption isn’t implemented by default. |
| packages/types/src/clusters/thread-network-diagnostics.d.ts | Regenerates/updates typings reflecting provisional optionality (note: one interface section appears inconsistent). |
| packages/types/src/clusters/thermostat.d.ts | Marks provisional events as optional in typings. |
| packages/types/src/clusters/microwave-oven-control.d.ts | Marks provisional elements optional in typings. |
| packages/types/src/clusters/groupcast.d.ts | Marks provisional Groupcast elements optional in typings. |
| packages/types/src/clusters/group-key-management.d.ts | Marks provisional groupcastAdoption optional in typings. |
| packages/types/src/clusters/general-commissioning.d.ts | Marks provisional elements optional in typings. |
| packages/types/src/clusters/ambient-context-sensing.d.ts | Marks provisional elements optional in typings. |
| packages/types/src/clusters/access-control.d.ts | Marks provisional elements optional in typings. |
| CHANGELOG.md | Documents breaking changes for model/types/node regarding provisional conformance handling. |
Contributor
|
Tick the box to add this pull request to the merge queue (same as
|
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
Conformance.computeApplicabilitytreated an otherwise list whose first term isPas mandatory-when-the-following-term-applies.GroupKeyManagement.GroupcastAdoption(P, GCAST) therefore resolved to Mandatory with the Groupcast feature enabled, so the managed state layer gave it a default[]and published it — even though no server populates it.Per spec §7.3 (
ref_ProvisionalConformance) the terms following aPdescribe the conformance intended once the element is no longer provisional, not the current one:The model layer was the outlier
Two other engines already read
P, Mcorrectly:P, MConformance.isMandatoryfalseconformance-compilercreateGroupcomputeApplicabilitySo
ClusterBehaviorTypewas auto-supplying a state default for an element the runtime validator never required and the protocol view reported as non-mandatory.Change
Applicability caps at optional for terms following a
P; the terms still contribute their condition, soP, GCASTremains absent without the feature. A mandatory term preceding thePstill wins, matching the spec's[AA], Pexample. A standalonePis now optional rather than excluded — excluding it would drop the element from the conformant view and makeClusterBehavior.enable()throw.The cluster-variance engine gets the same rule, so generated types declare provisional elements optional (
membership?: Membership[]) instead of listing them under "always supports these elements". That is what surfaced unguardedthis.state.membershipreads inGroupcastServer, which now declares the elements it implements viaenable().63 elements across 11 clusters change classification. ~23 are struct/enum fields and inert — TLV optionality comes from
ValueModel.mandatory→isMandatory, which already returnedfalse. The rest are attributes, commands and events.Consequences
ClusterBehavior.enable().Tests
applicabilityForhad no direct coverage; this adds adescribe("applicability")block. Two existing tests pinned the old semantics (ignores provisional, and theP, FOO, BAR, BAZvariance case) and are updated. A new node-level test uses ThreadNetworkDiagnostics — the one affected cluster without a provisional guard — to pin the wire-level contract: the attribute is absent fromattributeListby default and present once the application supplies a value.Clean build, format-verify, lint and the full suite pass.
Known gap, not addressed here
NameDependentElements.extractOtherwiseInterdependenciesandinter-element-conformancere-walk the same otherwise list without the cap, so for conformance referencing a sibling element name thePis ignored and dependency presence forces the element present.CameraAvStreamManagement.ImageRotationDiscreteAnglestakes that path today but is safe because its bracketing yields an optional fallback. An unbracketedP, <ElementName>would hit it. Tracked for the Groupcast follow-up.🤖 Generated with Claude Code