Skip to content

feat(export): E3b — device sync, export handlers, bridge client wiring #124

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
simons-plugins merged 2 commits into from
Aug 5, 2026
Merged

feat(export): E3b — device sync, export handlers, bridge client wiring #124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
19c5334
feat(export): E3b — device sync, export handlers, bridge client wiring
simons-plugins Aug 5, 2026
139a7fa
fix(export): harden E3b per PR #124 three-review batch
simons-plugins Aug 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ loop→Indigo writes go straight through `device_sync.apply_states` (thread-safe
| `bridge_client.py` | Bridge-node client (outbound export): hello+attach handshake that fails closed on version skew, endpoint CRUD, fire-and-forget `set_state`, §5 event callbacks. Attach refusals are triaged (§1.1): `version_mismatch`/`mass_removal_refused` halt with a `halted_reason`, `endpoint_map_invalid` holds the socket open un-attached in a `recovery` state so the §3.11 rebuild stays reachable, anything else reconnects on the normal backoff |
| `export_store.py` | The export allow-list (PRD-indigo-matter-export §5.1): `ExportEntry` (device id + role + name override + options) and an `RLock`'d store persisted as ONE JSON string in `pluginPrefs["matterExports"]`, schema-versioned. A blob it cannot parse is moved aside to `matterExports.corrupt` and the store starts empty — user config is never silently discarded |
| `export_catalog.py` | Indigo device → eligible Matter roles, or an `Excluded(reason)` shown in the picker (PRD §5.2, XAC9). The loop guard (XNG3/XAC6) is `pluginId` and nothing else, checked before any type reasoning. Type dispatch walks the IOM **class-name chain**, not `isinstance` — the indigo module is a MagicMock under test |
| `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. E3 roles only: plug, on/off light, dimmable, colour-temp, extended colour. `handler_for` returns `None` for the E4 roles the §5.1 dialog can already put in the allow-list. Hue diffs carry a ±1° tolerance (Matter's 0–254 hue round-trips ±1°); saturation deliberately has none |
| `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 an E4 role (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 |

@coderabbitai coderabbitai Bot Aug 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Line 51 contradicts the code and HANDOVER.md on the loop-thread claim.

The entry ends with "on_command dispatches indigo.* from the loop thread, the same discipline device_sync.apply_states already uses". The two other places that describe this say the opposite:

  • export_bridge.py lines 453-459: issuing indigo.* device commands from a non-Indigo thread is "unverified from the docs", and the apply_states precedent "covers state writes on our own devices, which is not the same claim".
  • HANDOVER.md line 508: "Not 'existing house discipline'".

CLAUDE.md is the architecture map a contributor reads first. Presenting an acknowledged unverified assumption as settled discipline is how the hedge gets dropped in the next module.

📝 Proposed wording 
-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 |
+State pushes are fire-and-forget onto the loop; `on_command` dispatches `indigo.*` from the loop thread, which is **unverified from the docs** (`device_sync.apply_states` only sets a precedent for state *writes* on our own devices). It is kept as a single seam so it can move to `run_in_executor` if the loop is seen stalling |
📝 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
| `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 an E4 role (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 |
| `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 an E4 role (an unknown role fails the *whole* attach). State pushes are fire-and-forget onto the loop; `on_command` dispatches `indigo.*` from the loop thread, which is **unverified from the docs** (`device_sync.apply_states` only sets a precedent for state *writes* on our own devices). It is kept as a single seam so it can move to `run_in_executor` if the loop is seen stalling |
🤖 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 CLAUDE.md entry for export_bridge.py to
remove the assertion that on_command dispatches indigo.* from the loop thread as
established discipline. Describe this behavior as an unverified assumption or
explicitly defer to the qualification in export_bridge.py and HANDOVER.md, while
preserving the surrounding lifecycle and attach-provider documentation.

| `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) |
| `server_process.py` | `ServerProcess` = the matter-server (controller) specialisation of `LaunchAgent`: its prefs, its argv, its pinned version. Gated by the `serverLocation` pref — the config asks "is matter-server on this Mac?"; `local` (turnkey default) manages it here on loopback, `remote` connects to a server elsewhere. `manageLaunchAgent`/host/port are derived from that in `startup` (see `plugin.py:server_location`) |
| `commission_jobs.py` | Commissioning job state machine (API.md §3.2/§3.3) |
Expand Down
16 changes: 14 additions & 2 deletions docs/HANDOVER.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -499,11 +499,23 @@ Domio no longer commissions; it relays a **share code** (Apple Home is admin 1;

**Export side (E1, `docs/BRIDGE_PROTOCOL.md`):** `bridge_protocol.py` the wire contract (envelope, §3 commands, §1.1 error codes, §4.2 roles, normalised `BridgeCommand`/`StatusReport`/`PairingReport`/`FabricInfo`; **no** rename firewall — we own both ends) · `bridge_client.py` the client (hello+attach handshake, **fails closed** on `protocolVersion` skew via `on_version_skew` + halt, endpoint CRUD, fire-and-forget `set_state`, §5 event callbacks) · `bridge-node/` the TypeScript node · `tests/fixtures/bridge_protocol/frames.json` the ONE golden-frame file both suites read (§7; `npm test` copies it into the TS build).

**Export side (E2, allow-list + UI):** `export_store.py` the allow-list model (`ExportEntry` = device id + §4.2 role + name override + options; `RLock`'d, one schema-versioned JSON string in `pluginPrefs["matterExports"]`, unparseable blobs moved aside to `matterExports.corrupt` rather than discarded) · `export_catalog.py` the PRD §5.2 mapping (eligible roles + safe default, or `Excluded(reason)`; loop guard is `pluginId` only — XNG3/XAC6 — and type dispatch walks the IOM class-name chain because `isinstance` is unusable against the MagicMock'd indigo module) · `MenuItems.xml` → **Manage Matter Exports…**, the UI-D dialog: no `<CallbackMethod>` (so it gets a single Close button and the in-dialog buttons do the work), filter textfield + Apply-filter button + single-select device `menu` with `dynamicReload` (a multi-select `list` has NO CallbackMethod, so master-detail is impossible with one) + role menu + name/polarity fields + Add/Remove buttons + a readonly `exportStatus` textfield (Indigo labels cannot change at runtime) + a readonly summary list. `plugin.py` builds the store in `startup` and owns the callbacks. `bridge_client` is still **not** wired to the store — that is E3.
**Export side (E2, allow-list + UI):** `export_store.py` the allow-list model (`ExportEntry` = device id + §4.2 role + name override + options; `RLock`'d, one schema-versioned JSON string in `pluginPrefs["matterExports"]`, unparseable blobs moved aside to `matterExports.corrupt` rather than discarded) · `export_catalog.py` the PRD §5.2 mapping (eligible roles + safe default, or `Excluded(reason)`; loop guard is `pluginId` only — XNG3/XAC6 — and type dispatch walks the IOM class-name chain because `isinstance` is unusable against the MagicMock'd indigo module) · `MenuItems.xml` → **Manage Matter Exports…**, the UI-D dialog: no `<CallbackMethod>` (so it gets a single Close button and the in-dialog buttons do the work), filter textfield + Apply-filter button + single-select device `menu` with `dynamicReload` (a multi-select `list` has NO CallbackMethod, so master-detail is impossible with one) + role menu + name/polarity fields + Add/Remove buttons + a readonly `exportStatus` textfield (Indigo labels cannot change at runtime) + a readonly summary list. `plugin.py` builds the store in `startup` and owns the callbacks.

**Export side (E3b, the outbound pipeline):** `export_handlers.py` the per-**role** handler table — the mirror of `matter_handlers/`, keyed by §4.2 role because outbound there is no cluster, only a user declaration. Three methods per role: `states_for` (the §4.2 snapshot), `diff` (changed keys only; hue carries a ±1° tolerance because Matter's 0–254 hue round-trips ±1°, saturation deliberately does not because 0–100↔0–254 is exact) and `dispatch` (§4.2 command → `indigo.device.turnOn/turnOff`, `indigo.dimmer.setBrightness`, `indigo.dimmer.setColorLevels`). **E3 roles only**; `handler_for` answers `None` for the E4 roles the dialog can already write into the allow-list · `export_bridge.py` the engine: owns the `BridgeClient`, starts it only while the allow-list is non-empty (XG5) and stops it on the deliberate §3.1 `replace_all` attach when it empties; its endpoint provider re-runs `export_catalog.classify` on **every** attach and skips-with-warning anything deleted, excluded, re-typed, or carrying an E4 role (an unknown role fails the *whole* attach with `internal` — E3a); `on_command` resolves through the store and dispatches on the loop thread · `plugin.py` `deviceUpdated`/`deviceDeleted` + the `_exports_changed` seam.

**E3b decisions worth not re-deriving.**
- **`subscribeToChanges` is conditional and one-way.** It subscribes to *every* device on the server ("a significant amount of traffic" per the IOM reference), so it is issued only once the allow-list is non-empty — at `startup`, or from the dialog the first time a user exports something (it is a plain request to the server, not a startup-only registration). There is **no unsubscribe in the canonical reference**, so it is never turned off again; `deviceUpdated`'s guard is a plain `frozenset` attribute on the Plugin (`self._exported_ids`, refreshed only by `_exports_changed`) so a non-exported device costs one hash lookup — no lock, no rebuild, no allocation.
- **`indigo.*` device commands from the loop thread are *unverified from the docs*.** Not "existing house discipline" — `device_sync.apply_states`'s precedent covers state *writes* on our own devices, which is a different claim from `indigo.device.turnOn` on somebody else's. It is left on the loop because `on_command` is a **single seam**: one method, one call site, so moving it to `run_in_executor` is a local change the day the loop is seen stalling. Bulk Indigo IPC does *not* get that latitude — `export_bridge.endpoint_specs` is one blocking `indigo.devices[id]` copy per exported device, and `bridge_client._gather_endpoints` runs it in an executor because this loop is shared with the inbound matter-server client (a slow Indigo server would otherwise stall live Matter updates behind an export reconcile).
- **Attach deadline scales:** `bridge_client.attach_timeout_for(n) = max(8.0, 2.0 + 0.15n)`. The node answers an attach only after its ~100ms-paced removals (§3.3), so ~80 endpoints spend 8s in pacing alone — the flat deadline would have timed out on exactly the databases that need export most. Zero protocol change.
- **…and `n` is the REMOVALS, not the endpoints sent.** For an ordinary reconnect the two track each other (the node's set came from our last attach), so `len(specs)` is a fine default. For `export_bridge._replace_all_then_stop` — the §3.1 un-export — they are opposites: zero sent, *everything* removed. It passes `attach_timeout_for(<allow-list size before it was emptied>)` explicitly, captured in `exports_changed` because the store is already empty by the time the un-export runs. Defaulting there gave a 60-device un-export the 8s floor: timeout mid-reconcile → a false "accessories may linger" warning → `close()` yanking the socket out from under a node that was working fine.
- **Colour is one-of, in both directions.** Matter's `colorMode` makes hue/sat and colour temperature mutually exclusive, and the node's push side picks hue/sat when a device reports both (`bridge-node/src/endpoints.ts` `colorPatch`). So `_set_color_temp` zeroes RGB alongside the white write and `_set_color` zeroes `whiteLevel` alongside the RGB write — each only when the device actually has that channel (`_number(dev, …) is not None`), because an all-zero RGB write to a CT-only driver is its own way to black out a room. A device with **no** white channel skips `setColorTemp` entirely (debug line) rather than inventing `whiteLevel=100` for a channel it does not have. **Needs the live E2E on a real RGBW driver** — this is the one change in the batch whose behaviour is a guess about drivers rather than about our own code. Two things to watch for on that run: (a) whether a driver treats an all-zero RGB write as "off" rather than "no colour"; (b) `colorPatch`'s mode arbitration — an RGBW device pushes `colorTempMireds` *and* `saturation` in the same snapshot, and the node sets `colorMode` from whichever it sees last, so a CT change still lands as `CurrentHueAndCurrentSaturation` with saturation 0. That is pre-existing (it happened with *stale* RGB before, which was worse), it is node-side, and it is deliberately **not** touched here.
- **Hue is omitted below saturation 20** (`export_handlers.SATURATION_HUE_FLOOR`). Indigo stores colour as three integer 0–100 channels, so hue is *recovered*, and near the grey axis those integers carry almost no angular information — measured worst-case round-trip error: 180° at sat 0, 30° at sat 1, 6° at sat 5, 1° at sat 20+. The ±1° `HUE_TOLERANCE_DEGREES` only covers the last of those, so every pastel change was pushing a `hue` nobody asked for; it converges (`setColorLevels` is absolute) but the Home colour wheel visibly jumps on the way. No protocol change — §3.4 state maps are partial by design, and the node's `colorPatch` already handles a saturation-only patch.
- **A role change in the dialog is remove+re-add** (§4.1 refuses a role change in place), and the accessory is new to every ecosystem afterwards — it loses its Home-app name and room. `exportStatus` says so before the user finds out.
- **`bridgeWsPort` is deliberately NOT in `PluginConfig.xml` yet.** PRD §5.5's Export section is a whole panel (enable/disable wholesale, both ports, the pairing readout) and belongs with E6/E7; a lone port field would ship an Export section that cannot start, stop or pair anything. `bridge_client` already reads the pref, so a hand-set `.indiPref` value is the escape hatch for E3's manually-run node.

**E2 hardening (PR #122) — read before building E3.** The store persists *then* commits: `_commit` writes the pref, flushes through the injected `save_prefs` (`indigo.server.savePluginPrefs`), and only then adopts the new map in memory, rolling the pref back if the flush raises — so memory and prefs can never disagree. It holds a `prefs_getter` callable, not the `pluginPrefs` object, because Indigo may rebind that on a PluginConfig save. A load failure is carried in `store.load_error` and shown in the dialog instead of "Nothing is exported yet.", and `matterExports.corrupt` is **first-rescue-wins** (a second corruption never overwrites it).

> **The store is NOT the guard. E3 MUST re-classify at endpoint-build time.** The injected `entry_validator` re-runs the loop guard over entries restored from prefs, and `ExportEntry.from_dict` enforces the options shape per role (`invert` only on `windowCovering`) — but both run at *load*, against the database as it was then. A device can change type, gain our `pluginId`, or be replaced between load and endpoint build. Every endpoint E3 builds must call `export_catalog.classify` again and refuse anything that comes back `Excluded`; treating a store hit as proof of eligibility reintroduces exactly the loop (XNG3/XAC6) the guard exists to prevent.
> **The store is NOT the guard — re-classify at endpoint-build time.** (Honoured by `export_bridge._spec_for` since E3b; keep it that way.) The injected `entry_validator` re-runs the loop guard over entries restored from prefs, and `ExportEntry.from_dict` enforces the options shape per role (`invert` only on `windowCovering`) — but both run at *load*, against the database as it was then. A device can change type, gain our `pluginId`, or be replaced between load and endpoint build. Every endpoint E3 builds must call `export_catalog.classify` again and refuse anything that comes back `Excluded`; treating a store hit as proof of eligibility reintroduces exactly the loop (XNG3/XAC6) the guard exists to prevent.

**Key invariants:** node-details has NO `endpoints` key (derive from flat `attributes`); `attribute_updated` data is `[node_id,"ep/cl/at",value]`; `node_removed` is a bare id; `server_info` is a bare connect frame (`sdk_version`/`fabric_id`). Setpoints/modes are attribute **writes**, not commands.

Expand Down
2 changes: 1 addition & 1 deletion indigo-matter.indigoPlugin/Contents/Info.plist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
<key>IwsApiVersion</key>
<string>1.0.0</string>
<key>PluginVersion</key>
<string>2026.7.27</string>
<string>2026.7.28</string>
<key>ServerApiVersion</key>
<string>3.6</string>
</dict>
Expand Down
Loading
Loading