feat(export): E6+E7 — pairing UX, fabric management, bridge LaunchAgent [no-release] - #128
Merged
Merged
Conversation
E6: Pair Matter Bridge / Unpair an Ecosystem menu actions over §3.8/§3.9, pairing code to the event log with an IWS-served page carrying the manual code and MT: payload, PluginConfig Export section with a live fabric readout, §5 fabric events surfaced in the Indigo log. E7: the bridge node becomes a managed LaunchAgent via the AgentSpec machinery extracted in PR #120 — installed from the exact-pinned registry spec indigo-matter-bridge@0.5.0 (no JavaScript ships in the plugin bundle), gated so a fresh install stays inert (XG5/XAC1), started on the first export and stopped when the allow-list empties, with the watchdog branch and failure isolation from the inbound controller. remove_package is now per-package (npm uninstall) rather than wiping the shared node_modules — the TODO(E7) left by the AgentSpec extraction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxqGhP3DcENf68AZK21U4S
📝 WalkthroughWalkthroughChangesThe PR adds a publishable Matter bridge package with bridged-device identity metadata and structured fabric-removal responses. It adds bridge LaunchAgent installation and lifecycle control, export enablement, pairing and unpairing workflows, pairing-page rendering, diagnostics, documentation, and comprehensive tests. Matter bridge integration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Plugin
participant ExportBridge
participant BridgeProcess
participant BridgeClient
Plugin->>ExportBridge: apply export preference
ExportBridge->>BridgeProcess: start or stop bridge agent
ExportBridge->>BridgeClient: connect and open pairing window
BridgeClient-->>ExportBridge: return pairing and fabric state
ExportBridge-->>Plugin: provide status and pairing-page data
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
The E6+E7 rebase onto main conflicted on PluginVersion (main carried 2026.8.2 from the CI change, the branch 2026.8.1) and the markers were committed. Resolved to 2026.8.3, above both. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxqGhP3DcENf68AZK21U4S
Three reviewers converged on one family: claiming success from the wrong
signal. Every message below was true of a nearby fact and false of what
the user was told.
CRITICAL
- A1 First-run dead end: menuInstallBridgeNode went install() → restart()
with no ensure_installed(), but the plist is written only by
_start_bridge_agent, which cannot pass preflight before the package
exists. Result on a fresh Mac: two wrong messages and no bridge. Now
ensure_installed() first, restart() only when it returns False. The old
test asserted against Mock(restart=True); it is now a real BridgeProcess.
- A2 LaunchAgent.run_state()/is_alive() distinguish not_loaded / running /
loaded_not_running / unknown. _start_bridge_agent no longer prints "the
LaunchAgent is running" off `ensure_installed() is not None`; start() and
restart() stop reporting a loaded-but-dead job as started.
- A3 remove_fabric answers {removed, remaining} on both sides (protocol,
golden frames, docs) and emits fabrics_changed even when the index held
nothing — the picker is built from a cached list, so a stale index is the
designed path. The menu branches on it and re-reads the fabric set.
- A4 The §5.5 readout consults client.connected/attached instead of
`bridge.active` ("a client object exists"), and dates its fabric list.
- A5 _stop_bridge_agent uninstalls rather than stops: the plist carries
RunAtLoad, so the old behaviour started an unpaired bridge with an empty
allow-list after every reboot.
- A6/A7/A8/A9 test-only: the bridged identity is now pinned against a real
ServerNode's BasicInformation; the XAC7 debt-path agent stop, the
disconnected-client gates, and closedPrefsConfigUi's exports_changed
re-run all have tests that die when the code goes.
SECURITY
- S1 The pairing page and the menu log line now state that the passcode is
a live credential over IWS, which authenticates only if enabled; INSTALL
gains "Before you pair the export bridge". No auth built into the handler.
IMPORTANT
- I1 Vendor table verified against the CSA DCL and the vendored matter.js:
0x1075 is not an issued id, 0x100B is Signify not Google (0x6006), and
Apple's second fabric 0x1384 was missing. Pinned against matter.js source.
- I2 no-selection row + seeding on the destructive unpair picker.
- I3 AgentSpec.install_menu — messages named the npm package, not the menu.
- I4/I5 stop failures are logged and the two Falses distinguished; the
diagnosis asks preflight() and stops calling never-truncated lines "Recent".
- I6 remove_package reports the truth by looking at the package directory.
- I7 window_expires_at is compared against now and re-derived on attach.
- I8 the un-export warning no longer promises a retry that does not exist.
- I9 clean-reinstall and stop menus for the bridge.
- I10 bundle scan: no .js/.ts ships inside indigo-matter.indigoPlugin.
MEDIUM
- M1 dead Plugin._bridge_agent() removed. M2 the rebuild test now calls
twice with changed prefs. M3/M4/M5 getWebServerURL, note_window_opened,
_escape(None), package.json name, the _agent_started latch and the
UI-thread deadlines are all pinned. M6 PRD/HANDOVER reconciled to
Info.plist 2026.8.3 and to the measured suite counts (the 345 TS claim
was wrong; npm test reported 347 before this batch). M7 bridge-node
README. M8 HANDOVER records that bridgedInfoFor's identity expansion
arrived in this PR.
Suites: 2243 Python, 348 TS. pylint 9.42.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JxqGhP3DcENf68AZK21U4S
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bridge-node/src/node.ts (1)
729-767: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winEmit
fabrics_changedfor successful fabric removal.
fabric.leave()completes beforethis.noteFabrics()runs, butnoteFabrics(undefined)only updates counters and cannot emit the fallback event. InremoveFabric(), emitfabrics_changedafter the removal read succeeds, and keepdocs/BRIDGE_PROTOCOL.md#L220-L234in sync becauseremoveFabric()must not rely only on matter.js observable delivery.🤖 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 `@bridge-node/src/node.ts` around lines 729 - 767, Update removeFabric in bridge-node/src/node.ts (lines 729-767) to explicitly emit the fabrics_changed fallback event after a successful removal read, rather than relying only on noteFabrics(undefined) or matter.js observation; preserve the existing handling for unavailable remaining counts. Update docs/BRIDGE_PROTOCOL.md (lines 220-234) to document this required explicit event emission behavior.
🧹 Nitpick comments (3)
bridge-node/test/stub-bridge.ts (1)
283-285: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMake the stale-index result configurable.
removeFabric()returns the successful-removal fixture for everyfabricIndex. A WebSocket test usingStubBridgecannot receiveremove_fabric_already_gone.Add a configurable
RemoveFabricResultfield. Set it to the stale-index fixture in the relevant test.🤖 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 `@bridge-node/test/stub-bridge.ts` around lines 283 - 285, Add a configurable RemoveFabricResult field to StubBridge and update removeFabric() to return that field instead of always using golden.remove_fabric.response.result. Initialize the field with the current successful-removal result, then set it to the remove_fabric_already_gone fixture in the relevant WebSocket test.indigo-matter.indigoPlugin/Contents/Server Plugin/launch_agent.py (1)
828-844: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
_package_dir()instead of rebuilding the path.
_remove_package_dirrecomputesos.path.join(self.project_dir, "node_modules", self.spec.package), which is exactly what_package_dir()returns.remove_packageverifies the outcome against_package_dir(). Two expressions for one path can diverge: a later change to the layout in_package_dir()alone would make the fallback delete one path while the check inspects another, andremove_packagewould then report a failure over a successful removal.♻️ Proposed fix to use the single derivation
- target = os.path.join(self.project_dir, "node_modules", self.spec.package) + target = self._package_dir() try: if os.path.isdir(target): shutil.rmtree(target)🤖 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/launch_agent.py around lines 828 - 844, Update _remove_package_dir to obtain the deletion target by calling _package_dir() instead of reconstructing it from project_dir, node_modules, and spec.package. Preserve the existing directory/file removal and warning behavior so remove_package validates the same path it deletes.indigo-matter.indigoPlugin/Contents/Server Plugin/export_bridge.py (1)
660-676: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse
_stop_agent_off_loophere.Lines 675-676 repeat the body of
_stop_agent_off_loop(Line 470). One call keeps the off-loop stop in one place, so a later change to the executor choice cannot apply to only one caller.♻️ Proposed consolidation
- loop = asyncio.get_running_loop() - await loop.run_in_executor(None, self._stop_agent) + await self._stop_agent_off_loop()🤖 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/export_bridge.py around lines 660 - 676, Replace the duplicated executor-based stop logic in the shown else branch with a call to the existing _stop_agent_off_loop helper. Preserve the current ordering and await the helper after the un-export attempt, leaving _stop_agent’s implementation and surrounding cleanup unchanged.
🤖 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 `@CLAUDE.md`:
- Line 51: Update the ExportBridge lifecycle description to say the client and
bridge LaunchAgent start/stop on the allow-list’s empty↔non-empty transitions,
rather than only on the dialog’s transitions. Preserve the existing ordering and
session ownership details.
In `@docs/HANDOVER.md`:
- Around line 17-18: Update the pylint score in the handover verification
summary to match the authoritative CI result, using 9.42 if confirmed. Leave the
plugin, bridge-node, and suite counts unchanged.
- Around line 324-326: Update the fenced code block containing the
configurationVersion excerpt to specify the text language identifier, changing
the opening fence to ```text while leaving the excerpt unchanged.
In `@docs/INSTALL.md`:
- Around line 279-280: In the security warning text, update the spelling from
“afterwards” to the selected American-English form “afterward,” preserving the
surrounding wording.
In `@indigo-matter.indigoPlugin/Contents/Server` Plugin/PluginConfig.xml:
- Around line 128-143: Update validatePrefsConfigUi to validate bridgeWsPort and
bridgeMatterPort in addition to the existing remote-host check. Reject blank,
non-numeric, and out-of-range port values before preferences are persisted,
while preserving acceptance of valid port numbers.
---
Outside diff comments:
In `@bridge-node/src/node.ts`:
- Around line 729-767: Update removeFabric in bridge-node/src/node.ts (lines
729-767) to explicitly emit the fabrics_changed fallback event after a
successful removal read, rather than relying only on noteFabrics(undefined) or
matter.js observation; preserve the existing handling for unavailable remaining
counts. Update docs/BRIDGE_PROTOCOL.md (lines 220-234) to document this required
explicit event emission behavior.
---
Nitpick comments:
In `@bridge-node/test/stub-bridge.ts`:
- Around line 283-285: Add a configurable RemoveFabricResult field to StubBridge
and update removeFabric() to return that field instead of always using
golden.remove_fabric.response.result. Initialize the field with the current
successful-removal result, then set it to the remove_fabric_already_gone fixture
in the relevant WebSocket test.
In `@indigo-matter.indigoPlugin/Contents/Server` Plugin/export_bridge.py:
- Around line 660-676: Replace the duplicated executor-based stop logic in the
shown else branch with a call to the existing _stop_agent_off_loop helper.
Preserve the current ordering and await the helper after the un-export attempt,
leaving _stop_agent’s implementation and surrounding cleanup unchanged.
In `@indigo-matter.indigoPlugin/Contents/Server` Plugin/launch_agent.py:
- Around line 828-844: Update _remove_package_dir to obtain the deletion target
by calling _package_dir() instead of reconstructing it from project_dir,
node_modules, and spec.package. Preserve the existing directory/file removal and
warning behavior so remove_package validates the same path it deletes.
🪄 Autofix
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 Plus
Run ID: 8dfaeec3-4160-4873-800f-e4eebd380463
📒 Files selected for processing (38)
CLAUDE.mdbridge-node/README.mdbridge-node/package.jsonbridge-node/src/endpoints.tsbridge-node/src/node.tsbridge-node/src/protocol.tsbridge-node/src/registry.tsbridge-node/src/ws-server.tsbridge-node/test/fixture-shapes.tsbridge-node/test/fixtures.test.tsbridge-node/test/integration.test.tsbridge-node/test/registry.test.tsbridge-node/test/stub-bridge.tsdocs/BRIDGE_PROTOCOL.mddocs/HANDOVER.mddocs/INSTALL.mddocs/PRD-indigo-matter-export.mdindigo-matter.indigoPlugin/Contents/Info.plistindigo-matter.indigoPlugin/Contents/Server Plugin/Actions.xmlindigo-matter.indigoPlugin/Contents/Server Plugin/MenuItems.xmlindigo-matter.indigoPlugin/Contents/Server Plugin/PluginConfig.xmlindigo-matter.indigoPlugin/Contents/Server Plugin/bridge_agent.pyindigo-matter.indigoPlugin/Contents/Server Plugin/bridge_client.pyindigo-matter.indigoPlugin/Contents/Server Plugin/bridge_protocol.pyindigo-matter.indigoPlugin/Contents/Server Plugin/export_bridge.pyindigo-matter.indigoPlugin/Contents/Server Plugin/launch_agent.pyindigo-matter.indigoPlugin/Contents/Server Plugin/plugin.pyindigo-matter.indigoPlugin/Contents/Server Plugin/server_process.pytests/fakes.pytests/fixtures/bridge_protocol/frames.jsontests/test_bridge_agent.pytests/test_export_agent_wiring.pytests/test_export_bridge.pytests/test_launch_agent.pytests/test_pairing_menu.pytests/test_plugin_behaviour.pytests/test_server_process.pytests/test_xac10_no_matter_js.py
| | `export_handlers.py` | The **outbound** handler table, keyed by §4.2 **role** (the inbound registry is keyed by cluster; outbound there is no cluster, only a user-declared role) — `states_for` / `diff` / `dispatch` per role, each taking the export's §4.1 `options`. **Total over the v1 role enum since E4**: plug, on/off light, dimmable, colour-temp, extended colour, covering, lock, the seven sensors, thermostat. Hue diffs carry a ±1° tolerance (Matter's 0–254 hue round-trips ±1°); saturation deliberately has none. `windowCovering` applies the per-export `invert` polarity here so a `position` on the wire always means 100 = open; `doorLock` dispatches `indigo.device.lock`/`unlock` and confirms **nothing** (PRD §7). Indigo declares no units, so sensor/thermostat readings are passed through as already being in the §4.2 unit — documented in the module header as the known gap the device catalog should close. **One exception, `pressureSensor`:** Indigo's barometer convention is hPa (this plugin's own inbound handler writes it, and `export_catalog` routes `hpa`/`mbar` names here) and §4.2's key is `pressureKPa`, so it divides by 10 | | ||
| | `export_bridge.py` | The outbound engine: owns the `BridgeClient` and everything the Indigo callbacks *mean* for it. The client exists **only** while the allow-list is non-empty (XG5) and starts/stops on the dialog's empty↔non-empty transitions. The attach endpoint provider **re-runs `export_catalog.classify` on every attach** — the store is a past user declaration, not a guard — and skips-with-warning anything deleted/excluded/re-typed or carrying a role this build has no handler for — which since E4 means only an allow-list written by a *newer* plugin (an unknown role fails the *whole* attach). State pushes are fire-and-forget onto the loop; `on_command` dispatches `indigo.*` from the loop thread, the same discipline `device_sync.apply_states` already uses | | ||
| | `launch_agent.py` | Generic launchd LaunchAgent machinery (npm/npx/node resolution, plist authoring, applied-plist digest, orphan/EADDRINUSE reaping), driven by a frozen `AgentSpec` that carries one agent's identity. Extracted so the Matter **bridge node** can be a second agent without duplicating it (PRD-indigo-matter-export §4.2 / XOQ3) | | ||
| | `export_bridge.py` | The outbound engine: owns the `BridgeClient` and everything the Indigo callbacks *mean* for it. The client exists **only** while the allow-list is non-empty (XG5) and starts/stops on the dialog's empty↔non-empty transitions — and since E7 so does the **bridge LaunchAgent**, through injected `agent_start`/`agent_stop`/`agent_diagnose` seams: started before the client on empty→non-empty, stopped **after** the un-export has landed on non-empty→empty, and never stopped by a session that did not start it. The §5 pairing events (`fabrics_changed`/`commissioned`/`decommissioned`/`window_closed`) are consumed here since E6 — they were emitted by the node from E5 and read by nobody. PRD §5.5's `exportEnabled` switch fails **open** (absent or null means on, the opposite of the controller's attestation flag) and turning it off deliberately does **not** un-export: it drops the socket and stops the agent, leaving accessories paired-but-unavailable. The attach endpoint provider **re-runs `export_catalog.classify` on every attach** — the store is a past user declaration, not a guard — and skips-with-warning anything deleted/excluded/re-typed or carrying a role this build has no handler for — which since E4 means only an allow-list written by a *newer* plugin (an unknown role fails the *whole* attach). State pushes are fire-and-forget onto the loop; `on_command` dispatches `indigo.*` from the loop thread, the same discipline `device_sync.apply_states` already uses | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Describe the lifecycle trigger as the allow-list transition.
ExportBridge can receive the transition from Indigo callbacks, not only from the configuration dialog. The plugin context describes _start_bridge_agent as running on whichever Indigo thread changed the allow-list. Replace “the dialog’s empty↔non-empty transitions” with “the allow-list’s empty↔non-empty transitions” so the documentation covers non-dialog changes.
🤖 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 `@CLAUDE.md` at line 51, Update the ExportBridge lifecycle description to say
the client and bridge LaunchAgent start/stop on the allow-list’s empty↔non-empty
transitions, rather than only on the dialog’s transitions. Preserve the existing
ordering and session ownership details.
Comment on lines
+17
to
+18
| Plugin `2026.8.3`, bridge-node `0.5.0`. Suites: **2243 Python**, **348 TS** | ||
| (from 2056/344). pylint 9.41. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Reconcile the recorded pylint score.
This section records pylint 9.41, but the PR verification summary reports pylint 9.42. Update the handover after checking the authoritative CI result.
🤖 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 `@docs/HANDOVER.md` around lines 17 - 18, Update the pylint score in the
handover verification summary to match the authoritative CI result, using 9.42
if confirmed. Leave the plugin, bridge-node, and suite counts unchanged.
Comment on lines
+324
to
+326
| ``` | ||
| root.parts.aggregator.parts.indigo-1.bridgedDeviceBasicInformation.configurationVersion = 2 | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language identifier to the fenced block.
Line 324 opens a code block without a language. markdownlint-cli2 reports MD040. Use text for this data excerpt.
Proposed fix
-```
+```text
root.parts.aggregator.parts.indigo-1.bridgedDeviceBasicInformation.configurationVersion = 2📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ``` | |
| root.parts.aggregator.parts.indigo-1.bridgedDeviceBasicInformation.configurationVersion = 2 | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 324-324: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@docs/HANDOVER.md` around lines 324 - 326, Update the fenced code block
containing the configurationVersion excerpt to specify the text language
identifier, changing the opening fence to ```text while leaving the excerpt
unchanged.
Source: Linters/SAST tools
Comment on lines
+279
to
+280
| state and they can operate it. Removing them afterwards means *Plugins ▸ Matter ▸ | ||
| Unpair an Ecosystem…*, and you would first have to notice. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the selected American-English spelling.
Replace afterwards with afterward in the security warning.
Proposed fix
-... Removing them afterwards means *Plugins ▸ Matter ▸ Unpair an Ecosystem…*
+... Removing them afterward means *Plugins ▸ Matter ▸ Unpair an Ecosystem…*📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| state and they can operate it. Removing them afterwards means *Plugins ▸ Matter ▸ | |
| Unpair an Ecosystem…*, and you would first have to notice. | |
| state and they can operate it. Removing them afterward means *Plugins ▸ Matter ▸ | |
| Unpair an Ecosystem…*, and you would first have to notice. |
🧰 Tools
🪛 LanguageTool
[locale-violation] ~279-~279: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ... and they can operate it. Removing them afterwards means *Plugins ▸ Matter ▸ Unpair an Eco...
(AFTERWARDS_US)
🤖 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 `@docs/INSTALL.md` around lines 279 - 280, In the security warning text, update
the spelling from “afterwards” to the selected American-English form
“afterward,” preserving the surrounding wording.
Source: Linters/SAST tools
Comment on lines
+128
to
+143
| <Field id="bridgeWsPort" type="textfield" defaultValue="5581" | ||
| visibleBindingId="showExportAdvanced" visibleBindingValue="true" | ||
| alwaysUseInDialogHeightCalc="true"> | ||
| <Label>Bridge control port (loopback):</Label> | ||
| <Description>How the plugin talks to the bridge node. Loopback only — nothing outside | ||
| this Mac can reach it. Change it only if something else already uses 5581.</Description> | ||
| </Field> | ||
| <Field id="bridgeMatterPort" type="textfield" defaultValue="5540" | ||
| visibleBindingId="showExportAdvanced" visibleBindingValue="true" | ||
| alwaysUseInDialogHeightCalc="true"> | ||
| <Label>Matter port:</Label> | ||
| <Description>The port ecosystems reach the bridge on. 5540 is the Matter default and | ||
| is what Alexa expects; move it only if another Matter stack on this Mac | ||
| (Homebridge 2.x, matterbridge, Home Assistant) already holds it — the bridge's | ||
| log will say so. Changing it takes effect on the next plugin reload.</Description> | ||
| </Field> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether the port prefs are validated or coerced anywhere.
set -euo pipefail
fd -e py . -x rg -n --with-filename 'bridgeWsPort|bridgeMatterPort|PREF_MATTER_PORT|PREF_WS_PORT' {} \;Repository: simons-plugins/indigo-matter
Length of output: 2869
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Relevant source files =="
git ls-files | rg '(^|/)plugin\.py$|bridge_protocol\.py$|bridge_agent\.py$|bridge_client\.py$|PluginConfig\.xml$'
echo
echo "== plugin.py outline matches =="
ast-grep outline indigo-matter.indigoPlugin/Contents/Server\ Plugin/plugin.py --match validatePrefsConfigUi --view expanded || true
echo
echo "== plugin.py validation section =="
rg -n --with-filename 'def validatePrefsConfigUi|bridgeWsPort|bridgeMatterPort|remote host|PrefsConfig' indigo-matter.indigoPlugin/Contents/Server\ Plugin/plugin.py -C 3
echo
echo "== bridge_protocol.py =="
sed -n '1,70p' indigo-matter.indigoPlugin/Contents/Server\ Plugin/bridge_protocol.py
echo
echo "== bridge_agent.py port helpers and BridgeProcess init =="
sed -n '1,260p' indigo-matter.indigoPlugin/Contents/Server\ Plugin/bridge_agent.py
echo
echo "== bridge_client.py defaults/env usage =="
sed -n '150,210p' indigo-matter.indigoPlugin/Contents/Server\ Plugin/bridge_client.py
echo
echo "== PluginConfig.xml field IDs =="
sed -n '120,146p' indigo-matter.indigoPlugin/Contents/Server\ Plugin/PluginConfig.xml
echo
echo "== tests around bad port preference handling =="
sed -n '130,190p' indigo-matter.indigoPlugin/Contents/Server\ Plugin/tests/test_bridge_agent.py
sed -n '585,603p' indigo-matter.indigoPlugin/Contents/Server\ Plugin/tests/test_bridge_client.py
sed -n '275,305p' indigo-matter.indigoPlugin/Contents/Server\ Plugin/tests/test_pairing_menu.pyRepository: simons-plugins/indigo-matter
Length of output: 23796
Validate bridgeWsPort and bridgeMatterPort in validatePrefsConfigUi.
These fields use type="textfield" and validatePrefsConfigUi currently only checks the remote host. As configured, non-numeric, blank, or out-of-range values are saved by Indigo. Add validation here so the dialog rejects invalid ports before the settings are persisted.
🤖 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/PluginConfig.xml around
lines 128 - 143, Update validatePrefsConfigUi to validate bridgeWsPort and
bridgeMatterPort in addition to the existing remote-host check. Reject blank,
non-numeric, and out-of-range port values before preferences are persisted,
while preserving acceptance of valid port numbers.
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
The last functional milestones. E6 gives pairing and fabric management a user surface; E7 puts the bridge node under launchd, so it stops being hand-started.
E6
MT:payloadfabrics_changed/commissioned/decommissionedsurfaced in the Indigo logE7
AgentSpecon thelaunch_agent.pymachinery extracted in refactor(server-process): extract AgentSpec/LaunchAgent for the second agent (XOQ3) #120 — exactly what that refactor was forindigo-matter-bridge@0.5.0; the plugin bundle ships no JavaScriptremove_packageis now per-package rather than wiping the sharednode_modules— theTODO(E7)the AgentSpec extraction left behindNotable: a theory disproved, not implemented
The brief carried my hypothesis that our hard-coded
configurationVersionseed clobbers a value matter.js restores. The build agent implemented the persistence layer, mutation-tested it, found the mutation survived, then probed a real stack: matter.js does persist the attribute and restores it over the seed. No bug. The layer was reverted and a test now pins the measured behaviour so the wrong answer isn't re-derived from the data model.Verification
Python 2171 (from 2056), TypeScript 347 (from 344), pylint 9.41. PluginVersion 2026.8.1, bridge-node 0.5.0.
Blocked on Simon
npm publishofindigo-matter-bridge@0.5.0— the package is publish-ready (privateremoved,files: [dist],prepublishOnly) and a test pins package.json's version to the plugin's install spec. Nothing installs until it's on the registry.🤖 Generated with Claude Code
https://claude.ai/code/session_01JxqGhP3DcENf68AZK21U4S
Summary by CodeRabbit