feat: expectedFabricSlots warning + C3 direct-commissioning docs#78
Merged
Merged
Conversation
Domio's C3 direct-commissioning flow (Domio commissions onto its own fabric, opens the window, forwards the code) can send an expectedFabricSlots hint documented in API.md §3.2 but never read. Wire it through: commission_jobs now parses it onto the Job, and after interview compares the node's Operational Credentials SupportedFabrics - CommissionedFabrics against the hint, logging a warning (never failing the job) when the device reports fewer slots than Domio expected. Also rewrites the §3.2 preamble to document both setup-code sources (C4 Apple Home/Alexa share vs. C3 Domio direct) instead of only referencing C3 as a future ADR, and bumps PluginVersion per the version-check CI gate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds an ChangesFabric Slots Warning Feature
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant CommissionAPI as commission_jobs.create_job
participant Worker as commission_jobs._run_job
participant Node as Matter Node Interview
Client->>CommissionAPI: POST /commission with expectedFabricSlots
CommissionAPI->>CommissionAPI: store Job.expected_fabric_slots
Worker->>Node: fetch node interview data
Node-->>Worker: SupportedFabrics, CommissionedFabrics
Worker->>Worker: _fabric_slots_available(node)
Worker->>Worker: _warn_if_fabric_slots_short(job, node, node_id)
Worker-->>Client: log warning if available < expected
Compact metadata:
Poem:
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@indigo-matter.indigoPlugin/Contents/Server` Plugin/commission_jobs.py:
- Around line 70-102: The wire-format Operational Credentials constants and
attribute parsing logic currently live in `_fabric_slots_available` inside
`commission_jobs.py`, but this protocol knowledge should be centralized in
`protocol.py`. Move the cluster/attribute ID handling and the slot গণনা into a
`Protocol` helper such as `Protocol.fabric_slots_available(node)`, keeping
`commission_jobs.py` limited to calling that helper. Update any callers to use
the new `Protocol` method and remove the duplicated wire-semantic constants from
`commission_jobs.py`.
- Around line 84-97: The fabric-slot warning helper currently assumes
node.get("attributes") is a dict, so a truthy non-dict value can make
`_warn_if_fabric_slots_short()` fail inside `_run_job()` and escalate
commissioning to internal_error. Update `_warn_if_fabric_slots_short()` to guard
the `attributes` value before calling `.items()`, only iterating when it is an
actual dict, and otherwise fall back to returning None without raising.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 72ec339f-c212-469d-82ad-a2e6af66cfe3
📒 Files selected for processing (4)
docs/API.mdindigo-matter.indigoPlugin/Contents/Info.plistindigo-matter.indigoPlugin/Contents/Server Plugin/commission_jobs.pytests/test_commission_jobs.py
Comment on lines
+70
to
+102
| # Operational Credentials cluster (0x003E), endpoint 0 — SupportedFabrics | ||
| # (0x0002) minus CommissionedFabrics (0x0003) is the remaining fabric capacity | ||
| # after this join, used for the API.md §3.2 `expectedFabricSlots` warning. | ||
| _OP_CREDS_CLUSTER = 0x003E | ||
| _ATTR_SUPPORTED_FABRICS = 0x0002 | ||
| _ATTR_COMMISSIONED_FABRICS = 0x0003 | ||
|
|
||
|
|
||
| def _fabric_slots_available(node: dict) -> Optional[int]: | ||
| """Best-effort read of remaining fabric capacity from the interview payload. | ||
|
|
||
| Returns None (unknown, never treated as zero) if the node's attribute | ||
| snapshot doesn't include both Operational Credentials attributes. | ||
| """ | ||
| supported = commissioned = None | ||
| for key, value in (node.get("attributes") or {}).items(): | ||
| try: | ||
| endpoint, cluster, attribute = Protocol.parse_attr_key(str(key)) | ||
| except (ValueError, AttributeError): | ||
| continue | ||
| if endpoint != 0 or cluster != _OP_CREDS_CLUSTER: | ||
| continue | ||
| if attribute == _ATTR_SUPPORTED_FABRICS: | ||
| supported = value | ||
| elif attribute == _ATTR_COMMISSIONED_FABRICS: | ||
| commissioned = value | ||
| if supported is None or commissioned is None: | ||
| return None | ||
| try: | ||
| return int(supported) - int(commissioned) | ||
| except (TypeError, ValueError): | ||
| return None | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Wire-format constants belong in protocol.py, not commission_jobs.py.
_OP_CREDS_CLUSTER, _ATTR_SUPPORTED_FABRICS, _ATTR_COMMISSIONED_FABRICS and the attribute-key interpretation logic in _fabric_slots_available encode matter-server wire semantics (cluster/attribute IDs). Per the repo's own firewall convention, this knowledge should live behind protocol.py, exposed as a helper (e.g. Protocol.fabric_slots_available(node)), rather than duplicated here.
As per coding guidelines, "Use protocol.py as the rename firewall — the only place that knows matter-server wire field names."
♻️ Suggested direction
-_OP_CREDS_CLUSTER = 0x003E
-_ATTR_SUPPORTED_FABRICS = 0x0002
-_ATTR_COMMISSIONED_FABRICS = 0x0003
-
-
-def _fabric_slots_available(node: dict) -> Optional[int]:
- ...
+# moved to protocol.py:
+# def fabric_slots_available(node: dict) -> Optional[int]: ...Then in commission_jobs.py: available = Protocol.fabric_slots_available(node).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@indigo-matter.indigoPlugin/Contents/Server` Plugin/commission_jobs.py around
lines 70 - 102, The wire-format Operational Credentials constants and attribute
parsing logic currently live in `_fabric_slots_available` inside
`commission_jobs.py`, but this protocol knowledge should be centralized in
`protocol.py`. Move the cluster/attribute ID handling and the slot গণনা into a
`Protocol` helper such as `Protocol.fabric_slots_available(node)`, keeping
`commission_jobs.py` limited to calling that helper. Update any callers to use
the new `Protocol` method and remove the duplicated wire-semantic constants from
`commission_jobs.py`.
Source: Coding guidelines
Comment on lines
+84
to
+97
| supported = commissioned = None | ||
| for key, value in (node.get("attributes") or {}).items(): | ||
| try: | ||
| endpoint, cluster, attribute = Protocol.parse_attr_key(str(key)) | ||
| except (ValueError, AttributeError): | ||
| continue | ||
| if endpoint != 0 or cluster != _OP_CREDS_CLUSTER: | ||
| continue | ||
| if attribute == _ATTR_SUPPORTED_FABRICS: | ||
| supported = value | ||
| elif attribute == _ATTR_COMMISSIONED_FABRICS: | ||
| commissioned = value | ||
| if supported is None or commissioned is None: | ||
| return None |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE='indigo-matter.indigoPlugin/Contents/Server Plugin/commission_jobs.py'
wc -l "$FILE"
ast-grep outline "$FILE" --view expanded
# Read the relevant function(s) with line numbers
sed -n '1,220p' "$FILE" | cat -n
# Find the caller / exception handling around _warn_if_fabric_slots_short and _run_job
rg -n "def _warn_if_fabric_slots_short|def _run_job|internal_error|except Exception|attributes" "$FILE"Repository: simons-plugins/indigo-matter
Length of output: 13813
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE='indigo-matter.indigoPlugin/Contents/Server Plugin/commission_jobs.py'
sed -n '340,470p' "$FILE" | cat -nRepository: simons-plugins/indigo-matter
Length of output: 8666
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "reconcile_node_added\(" -S .Repository: simons-plugins/indigo-matter
Length of output: 1560
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE='indigo-matter.indigoPlugin/Contents/Server Plugin/app.py'
sed -n '320,380p' "$FILE" | cat -nRepository: simons-plugins/indigo-matter
Length of output: 266
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE='indigo-matter.indigoPlugin/Contents/Server Plugin/plugin.py'
sed -n '340,380p' "$FILE" | cat -nRepository: simons-plugins/indigo-matter
Length of output: 2804
Guard attributes before iterating
_warn_if_fabric_slots_short() is also reached from _run_job(), so a truthy non-dict attributes value will raise on .items() and turn the commission into internal_error instead of staying best-effort. Add an isinstance(attributes, dict) guard before iterating.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@indigo-matter.indigoPlugin/Contents/Server` Plugin/commission_jobs.py around
lines 84 - 97, The fabric-slot warning helper currently assumes
node.get("attributes") is a dict, so a truthy non-dict value can make
`_warn_if_fabric_slots_short()` fail inside `_run_job()` and escalate
commissioning to internal_error. Update `_warn_if_fabric_slots_short()` to guard
the `attributes` value before calling `.items()`, only iterating when it is an
actual dict, and otherwise fall back to returning None without raising.
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
1 participant
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.
Summary
Plugin-side slice of the C3 revival (Domio-native direct Matter commissioning — see domio-code PR #231 and forthcoming workspace ADR-0005). The join path itself needs zero functional change (a Domio-minted commissioning-window code is indistinguishable from an Apple Home share code); this PR makes the long-documented
expectedFabricSlotsparam real and updates the API docs.commission_jobs.py: parseexpectedFabricSlots; after the post-join interview, compute remaining fabric capacity (endpoint 0 Operational CredentialsSupportedFabrics − CommissionedFabrics) and warn (never fail) if it's under Domio's expectation. Silent when the hint is absent or the attributes aren't in the snapshot.docs/API.md§3.2: preamble now describes both setup-code sources (C4 share model default, C3 direct commissioning);expectedFabricSlotsrow documents the exact implemented semantics.PluginVersion2026.3.1 → 2026.3.2.Testing
🤖 Generated with Claude Code
https://claude.ai/code/session_01Arj5R66d73tc3PMPvHS65Z
Summary by CodeRabbit
New Features
Bug Fixes
Documentation