From 053b8782acafb50c5b3c0221846aacaf9b0ff79f Mon Sep 17 00:00:00 2001 From: Simon Clark Date: Wed, 5 Aug 2026 08:42:27 +0100 Subject: [PATCH 1/2] =?UTF-8?q?feat(export):=20E3a=20=E2=80=94=20bridge-no?= =?UTF-8?q?de=20endpoint=20CRUD=20for=20relay/dimmer/colour=20roles?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bridge node's endpoint set is now driven entirely by the protocol (BRIDGE_PROTOCOL §3.1-§3.5). The hard-coded E0 endpoint (indigo-999001) is gone: a node nobody has attached to serves an empty aggregator. The E0 pairing on jarvis survives because matter.js persists endpoint numbers against `Endpoint.id`, not against presence in the running set — verified by a restart smoke test that re-attaches the same devices in a different order and gets the same numbers back. Implements the five E3 roles only — onOffPlugInUnit, onOffLight, dimmableLight, colorTemperatureLight, extendedColorLight. Sensors, thermostat, doorLock and windowCovering are E4 and are refused with an `internal` error naming the gap, rather than silently skipped: `unknown_role` would be a lie (they are in the v1 enum) and a dropped export the user selected is worse than a loud one. New modules: - src/endpoints.ts — role → device type, the §4.2 unit converters next to the clusters they feed, the state writers and the cluster-change listeners. - src/reconcile.ts — matter.js-free arg parsing and the reconcile planner, including the §3.1 mass-removal guard. The test double reuses it, so the guard and the role rules are decided in one place. - src/registry.ts — the live endpoint set: reconcile, upsert, remove, set_state, set_reachable, ~100ms bulk-removal pacing (injectable). Three matter.js 0.17.8 findings, all pinned in comments: - ExtendedColorLightDevice is built with `ColorControlServer.with("Xy", "ColorTemperature")` — no HueSaturation — so §4.2's hue/saturation vocabulary had no attributes to write. The Matter spec makes HS mandatory for device type 0x010D, so the override restores conformance. - ColorControl refuses to initialise without colorMode, enhancedColorMode, colorCapabilities, coupleColorTempToLevelMinMireds and startUpColorTemperatureMireds, none of which the typings mark required. - `currentLevel` is constrained to 1-254 on all four lighting device types, so 0% is written as 1 — lossless, because it converts back to 0. Golden frames: attach_with_endpoints, attach_replace_all, attach_mass_removal_refused, upsert_endpoint, upsert_endpoint_role_change, remove_endpoint, remove_endpoint_absent, set_state, set_state_unknown_device and set_reachable move out of "pending" into real deepEqual assertions on both sides. Two frames had to change to be honourable, per §7: - attach_with_endpoints' second endpoint was a doorLock, which E3 cannot build. It is now a dimmableLight (same device id, same endpoint number); rebuild_endpoint_map follows, and command_lock moves to 900007, the doorLock that attach_all_roles already declares. - get_status carried the E0 999001 endpoint, which nothing creates any more. It now carries the same StatusReport attach_with_endpoints returns — §6.2's invariant is that attach answers with exactly what get_status would, so sharing the payload is the contract. The Python suites' PENDING lookups become name lookups across both sections: which section a frame sits in is a statement about the node, and says nothing about the plugin-side client those tests drive. bridge-node 0.1.0 → 0.2.0; PluginVersion 2026.7.26 → 2026.7.27. No Python source changes — the plugin side is the next PR. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JxqGhP3DcENf68AZK21U4S --- bridge-node/package.json | 2 +- bridge-node/src/endpoints.ts | 524 +++++++++++++++ bridge-node/src/node.ts | 144 ++-- bridge-node/src/protocol.ts | 66 +- bridge-node/src/reconcile.ts | 182 +++++ bridge-node/src/registry.ts | 246 +++++++ bridge-node/src/storage.ts | 8 +- bridge-node/src/ws-server.ts | 53 +- bridge-node/test/fixture-shapes.ts | 78 ++- bridge-node/test/fixtures.test.ts | 24 +- bridge-node/test/protocol.test.ts | 252 ++++++- bridge-node/test/reconcile.test.ts | 168 +++++ bridge-node/test/registry.test.ts | 624 ++++++++++++++++++ bridge-node/test/stub-bridge.ts | 166 ++++- bridge-node/test/units.test.ts | 141 ++++ .../Contents/Info.plist | 2 +- tests/fixtures/bridge_protocol/frames.json | 526 ++++++++------- tests/test_bridge_client.py | 64 +- tests/test_bridge_protocol_frames.py | 22 +- 19 files changed, 2889 insertions(+), 403 deletions(-) create mode 100644 bridge-node/src/endpoints.ts create mode 100644 bridge-node/src/reconcile.ts create mode 100644 bridge-node/src/registry.ts create mode 100644 bridge-node/test/reconcile.test.ts create mode 100644 bridge-node/test/registry.test.ts create mode 100644 bridge-node/test/units.test.ts diff --git a/bridge-node/package.json b/bridge-node/package.json index e5aff5c..a10db2f 100644 --- a/bridge-node/package.json +++ b/bridge-node/package.json @@ -1,6 +1,6 @@ { "name": "indigo-matter-bridge", - "version": "0.1.0", + "version": "0.2.0", "private": true, "description": "Matter bridge node for the indigo-matter Indigo plugin — exports selected Indigo devices as Matter accessories.", "license": "MIT", diff --git a/bridge-node/src/endpoints.ts b/bridge-node/src/endpoints.ts new file mode 100644 index 0000000..76ec2c1 --- /dev/null +++ b/bridge-node/src/endpoints.ts @@ -0,0 +1,524 @@ +/** + * Role → matter.js endpoint construction, the §4.2 unit conversions, and the + * cluster-change listeners that turn ecosystem writes into `command` events. + * + * Everything Matter-shaped about a *bridged child* lives here: one table entry + * per role, carrying its device type, its `set_state` writer and its command + * listeners, so the converter for a role sits next to the cluster it feeds + * (§4.2's "exactly one converter per role, in the node"). + * + * E3 implements the relay/dimmer/colour roles. The remaining §4.2 roles + * (sensors, thermostat, doorLock, windowCovering) are E4 and are refused here + * rather than silently dropped — see {@link UNSUPPORTED_ROLE_DETAILS}. + */ + +import { Endpoint, type EndpointType } from "@matter/main"; +import { BridgedDeviceBasicInformationServer } from "@matter/main/behaviors/bridged-device-basic-information"; +import { ColorControlServer } from "@matter/main/behaviors/color-control"; +import { ColorControl } from "@matter/main/clusters/color-control"; +import { ColorTemperatureLightDevice } from "@matter/main/devices/color-temperature-light"; +import { DimmableLightDevice } from "@matter/main/devices/dimmable-light"; +import { ExtendedColorLightDevice } from "@matter/main/devices/extended-color-light"; +import { OnOffLightDevice } from "@matter/main/devices/on-off-light"; +import { OnOffPlugInUnitDevice } from "@matter/main/devices/on-off-plug-in-unit"; + +import { + type CommandEventData, + type EndpointSpec, + ErrorCode, + ProtocolError, + Role, + type RoleValue, +} from "./protocol.js"; + +/** Bridged Device Basic Information `UniqueID`, stable across restarts. */ +export function uniqueIdFor(indigoDeviceId: number): string { + return `indigo-${indigoDeviceId}`; +} + +/** + * `Endpoint.id` derivation — the identity key of BRIDGE_PROTOCOL §4.1/§6.3. + * Deliberately the *same* value as {@link uniqueIdFor}: one derivation means the + * two can never drift apart, and §6.3's one-way identity flow reads directly. + * matter.js keys persisted endpoint numbers on this string alone (PRD §4.3), so + * it must never be reused or mutated. + */ +export const endpointIdFor = uniqueIdFor; + +/** + * `SerialNumber` must differ from `UniqueID` (Matter rejects equal values), so + * the device id goes in bare here and prefixed there. + */ +export function serialNumberFor(indigoDeviceId: number): string { + return String(indigoDeviceId); +} + +// --------------------------------------------------------------------------- +// §4.2 unit conversions +// --------------------------------------------------------------------------- + +/** Matter LevelControl and ColorControl 8-bit ranges. */ +export const MATTER_LEVEL_MAX = 254; +/** §4.2: mireds are clamped to the range we advertise as physically supported. */ +export const MIREDS_MIN = 153; +export const MIREDS_MAX = 500; +/** Matter `CurrentHue` spans 0-254 for a full 0-360° turn. */ +export const HUE_DEGREES_MAX = 360; + +function clamp(value: number, min: number, max: number): number { + return Math.min(max, Math.max(min, value)); +} + +/** + * Round half away from zero. `Math.round` is half-*up* (−0.5 → −0), which + * differs for negatives; every quantity here is non-negative, but naming the + * rule keeps §4.2's "round-half-up" honest if a signed unit ever arrives. + */ +function roundHalfUp(value: number): number { + return Math.floor(value + 0.5); +} + +/** + * Indigo 0-100 → Matter 0-254 (§4.2). 0 maps to 0 exactly, so "off" survives + * the conversion rather than becoming the dimmest on-level. + */ +export function percentToMatter(percent: number): number { + return clamp(roundHalfUp((clamp(percent, 0, 100) * MATTER_LEVEL_MAX) / 100), 0, MATTER_LEVEL_MAX); +} + +/** Matter 0-254 → Indigo 0-100. Inverse of {@link percentToMatter} for all 101 inputs. */ +export function matterToPercent(level: number): number { + return clamp(roundHalfUp((clamp(level, 0, MATTER_LEVEL_MAX) * 100) / MATTER_LEVEL_MAX), 0, 100); +} + +/** + * The value actually written to `currentLevel`. + * + * The Lighting feature constrains `currentLevel` to 1-254 on all four lighting + * device types (verified against the 0.17.8 device definitions, which `alter` + * the attribute to `min: 1`), so a literal 0 fails validation. Writing 1 + * instead is lossless at the protocol boundary: {@link matterToPercent}(1) is 0. + * Zero brightness is expressed to ecosystems as OnOff = false, which is what + * the Lighting feature intends. + */ +export function percentToCurrentLevel(percent: number): number { + return Math.max(1, percentToMatter(percent)); +} + +/** §4.2: colour temperature is clamped to the advertised physical bounds. */ +export function clampMireds(mireds: number): number { + return clamp(roundHalfUp(mireds), MIREDS_MIN, MIREDS_MAX); +} + +/** Indigo hue 0-360° → Matter `CurrentHue` 0-254. */ +export function degreesToMatterHue(degrees: number): number { + return clamp( + roundHalfUp((clamp(degrees, 0, HUE_DEGREES_MAX) * MATTER_LEVEL_MAX) / HUE_DEGREES_MAX), + 0, + MATTER_LEVEL_MAX, + ); +} + +/** + * Matter `CurrentHue` 0-254 → Indigo hue 0-360°. + * + * Not an exact inverse, and cannot be: 361 degrees do not fit in 255 steps, so + * a round trip is accurate to within one Matter step (~1.42°). Level and + * saturation *do* round-trip exactly because 254 steps span only 101 values. + */ +export function matterHueToDegrees(hue: number): number { + return clamp( + roundHalfUp((clamp(hue, 0, MATTER_LEVEL_MAX) * HUE_DEGREES_MAX) / MATTER_LEVEL_MAX), + 0, + HUE_DEGREES_MAX, + ); +} + +// --------------------------------------------------------------------------- +// The role table +// --------------------------------------------------------------------------- + +/** Matter behaviour ids, as they key `Endpoint.state` / `Endpoint.eventsOf`. */ +const ON_OFF = "onOff"; +const LEVEL_CONTROL = "levelControl"; +const COLOR_CONTROL = "colorControl"; +const BRIDGED_INFO = "bridgedDeviceBasicInformation"; + +/** + * Colour attributes matter.js requires up front. + * + * `colorMode`, `enhancedColorMode`, `colorCapabilities`, + * `coupleColorTempToLevelMinMireds` and `startUpColorTemperatureMireds` are all + * mandatory-on-conformance for a ColorControl server with the CT feature, and + * matter.js 0.17.8 refuses to initialise the behaviour without them (verified: + * the endpoint fails construction with a `conformance` error naming each in + * turn). Nothing in the matter.js docs says so — the typings mark them + * optional, which is why they are pinned here with a reason rather than + * discovered again. + */ +function colorControlDefaults(hueSaturation: boolean): Record { + return { + colorMode: ColorControl.ColorMode.ColorTemperatureMireds, + enhancedColorMode: ColorControl.EnhancedColorMode.ColorTemperatureMireds, + colorCapabilities: { colorTemperature: true, xy: true, hueSaturation }, + colorTempPhysicalMinMireds: MIREDS_MIN, + colorTempPhysicalMaxMireds: MIREDS_MAX, + coupleColorTempToLevelMinMireds: MIREDS_MIN, + startUpColorTemperatureMireds: null, + ...(hueSaturation ? { currentHue: 0, currentSaturation: 0 } : {}), + }; +} + +/** The subset of §4.2 roles this bridge version can construct. */ +export const SUPPORTED_ROLES: readonly RoleValue[] = [ + Role.onOffPlugInUnit, + Role.onOffLight, + Role.dimmableLight, + Role.colorTemperatureLight, + Role.extendedColorLight, +]; + +export function isSupportedRole(role: RoleValue): boolean { + return SUPPORTED_ROLES.includes(role); +} + +/** + * Why a lawful §4.2 role can still be refused. + * + * `unknown_role` would be a lie (the role *is* in the v1 enum) and silently + * skipping the export would be worse — the user selected the device and would + * see nothing appear. So it is an `internal` refusal that names the milestone. + */ +export const UNSUPPORTED_ROLE_DETAILS = (role: RoleValue): string => + `role ${role} is in the v1 enum but not implemented by this bridge version`; + +interface RoleDefinition { + /** Extra behaviour state supplied at construction, beyond bridged info. */ + initialState: () => Record; + /** Build the state patch for a `set_state` (§3.4) in this role's vocabulary. */ + statePatch: (states: Record) => Record; + /** Attribute → `command` event, for changes an ecosystem made (§4.2). */ + watch: readonly WatchSpec[]; + /** + * The matter.js device type, already `.with()`-ed for a bridged child. + * + * Typed as the base {@link EndpointType} on purpose: each role's `.with()` + * produces a *different* concrete type, and the registry holds them all as + * plain `Endpoint`s. Keeping the per-role types would buy nothing — the + * state patches are keyed by behaviour id, which is untyped either way. + */ + deviceType: () => EndpointType; +} + +/** One observable to subscribe: a behaviour id, an attribute, and its command. */ +interface WatchSpec { + behavior: string; + attribute: string; + /** Build the §5 `command` payload; `undefined` means "nothing to report". */ + command: ( + value: unknown, + state: Readonly>, + ) => { command: string; args: Record } | undefined; +} + +function numberOr(value: unknown, fallback: number): number { + return typeof value === "number" && Number.isFinite(value) ? value : fallback; +} + +/** `onOff` → §4.2 `onOff {"value": bool}`. Shared by every role that has it. */ +const WATCH_ON_OFF: WatchSpec = { + behavior: ON_OFF, + attribute: "onOff", + command: value => ({ command: "onOff", args: { value: value === true } }), +}; + +/** + * `currentLevel` → §4.2 `setLevel {"level": 0-100}`. Matter's `moveToLevel`, + * `step`, `move` and the Lighting on/off transitions all land here as an + * attribute change, which is why one listener covers the whole command family. + */ +const WATCH_LEVEL: WatchSpec = { + behavior: LEVEL_CONTROL, + attribute: "currentLevel", + command: value => + typeof value === "number" ? { command: "setLevel", args: { level: matterToPercent(value) } } : undefined, +}; + +const WATCH_COLOR_TEMP: WatchSpec = { + behavior: COLOR_CONTROL, + attribute: "colorTemperatureMireds", + command: value => + typeof value === "number" + ? { command: "setColorTemp", args: { colorTempMireds: clampMireds(value) } } + : undefined, +}; + +/** + * Hue and saturation are one §4.2 command (`setColor` carries both), so each + * observable reads its partner out of the endpoint's current state. Matter + * changes them in one transaction for `moveToHueAndSaturation`, which yields two + * events and therefore two `setColor` emissions with the same final values — + * harmless (the plugin applies the same colour twice) and much simpler than + * coalescing. + */ +const WATCH_HUE: WatchSpec = { + behavior: COLOR_CONTROL, + attribute: "currentHue", + command: (value, state) => ({ + command: "setColor", + args: { + hue: matterHueToDegrees(numberOr(value, 0)), + saturation: matterToPercent(numberOr(state.currentSaturation, 0)), + }, + }), +}; + +const WATCH_SATURATION: WatchSpec = { + behavior: COLOR_CONTROL, + attribute: "currentSaturation", + command: (value, state) => ({ + command: "setColor", + args: { + hue: matterHueToDegrees(numberOr(state.currentHue, 0)), + saturation: matterToPercent(numberOr(value, 0)), + }, + }), +}; + +function onOffPatch(states: Record): Record { + return typeof states.onOff === "boolean" ? { [ON_OFF]: { onOff: states.onOff } } : {}; +} + +function levelPatch(states: Record): Record { + if (typeof states.level !== "number") { + return {}; + } + return { [LEVEL_CONTROL]: { currentLevel: percentToCurrentLevel(states.level) } }; +} + +/** + * Colour patch, including the `colorMode` bookkeeping Matter requires. + * + * An ecosystem reads `colorMode` to decide which attributes to believe. Indigo + * devices happily report hue/saturation *and* a colour temperature at the same + * time (the fixture `set_state_extended_color_light` does exactly that), so a + * rule is needed: hue/saturation win when present, because a user who set a + * colour expects to see that colour, not the white point left over from before. + */ +function colorPatch(states: Record, hueSaturation: boolean): Record { + const color: Record = {}; + if (typeof states.colorTempMireds === "number") { + color.colorTemperatureMireds = clampMireds(states.colorTempMireds); + color.colorMode = ColorControl.ColorMode.ColorTemperatureMireds; + color.enhancedColorMode = ColorControl.EnhancedColorMode.ColorTemperatureMireds; + } + if (hueSaturation) { + let touched = false; + if (typeof states.hue === "number") { + color.currentHue = degreesToMatterHue(states.hue); + touched = true; + } + if (typeof states.saturation === "number") { + color.currentSaturation = percentToMatter(states.saturation); + touched = true; + } + if (touched) { + color.colorMode = ColorControl.ColorMode.CurrentHueAndCurrentSaturation; + color.enhancedColorMode = ColorControl.EnhancedColorMode.CurrentHueAndCurrentSaturation; + } + } + return Object.keys(color).length > 0 ? { [COLOR_CONTROL]: color } : {}; +} + +const ROLE_DEFINITIONS: Partial> = { + [Role.onOffPlugInUnit]: { + deviceType: () => OnOffPlugInUnitDevice.with(BridgedDeviceBasicInformationServer), + initialState: () => ({}), + statePatch: onOffPatch, + watch: [WATCH_ON_OFF], + }, + [Role.onOffLight]: { + deviceType: () => OnOffLightDevice.with(BridgedDeviceBasicInformationServer), + initialState: () => ({}), + statePatch: onOffPatch, + watch: [WATCH_ON_OFF], + }, + [Role.dimmableLight]: { + deviceType: () => DimmableLightDevice.with(BridgedDeviceBasicInformationServer), + initialState: () => ({ [LEVEL_CONTROL]: { currentLevel: 1 } }), + statePatch: states => ({ ...onOffPatch(states), ...levelPatch(states) }), + watch: [WATCH_ON_OFF, WATCH_LEVEL], + }, + [Role.colorTemperatureLight]: { + deviceType: () => ColorTemperatureLightDevice.with(BridgedDeviceBasicInformationServer), + initialState: () => ({ + [LEVEL_CONTROL]: { currentLevel: 1 }, + [COLOR_CONTROL]: { ...colorControlDefaults(false), colorTemperatureMireds: MIREDS_MIN }, + }), + statePatch: states => ({ ...onOffPatch(states), ...levelPatch(states), ...colorPatch(states, false) }), + watch: [WATCH_ON_OFF, WATCH_LEVEL, WATCH_COLOR_TEMP], + }, + [Role.extendedColorLight]: { + // matter.js 0.17.8 builds ExtendedColorLightDevice with + // `ColorControlServer.with("Xy", "ColorTemperature")` — no HueSaturation + // — so `currentHue`/`currentSaturation` would not exist and §4.2's + // hue/saturation vocabulary would be unimplementable. The Matter spec + // makes HS mandatory for device type 0x010D, so this override restores + // conformance rather than extending it. + deviceType: () => + ExtendedColorLightDevice.with( + BridgedDeviceBasicInformationServer, + ColorControlServer.with("HueSaturation", "Xy", "ColorTemperature"), + ), + initialState: () => ({ + [LEVEL_CONTROL]: { currentLevel: 1 }, + [COLOR_CONTROL]: { ...colorControlDefaults(true), colorTemperatureMireds: MIREDS_MIN }, + }), + statePatch: states => ({ ...onOffPatch(states), ...levelPatch(states), ...colorPatch(states, true) }), + watch: [WATCH_ON_OFF, WATCH_LEVEL, WATCH_COLOR_TEMP, WATCH_HUE, WATCH_SATURATION], + }, +}; + +function definitionFor(role: RoleValue): RoleDefinition { + const definition = ROLE_DEFINITIONS[role]; + if (definition === undefined) { + throw new ProtocolError(ErrorCode.internal, UNSUPPORTED_ROLE_DETAILS(role)); + } + return definition; +} + +/** + * Merge two behaviour-keyed patches one level deep. + * + * A plain spread is wrong here and fails loudly: `{...defaults, ...patch}` + * *replaces* the whole `colorControl` object, dropping the mandatory + * `colorCapabilities`/`colorTempPhysicalMinMireds`/… that + * {@link colorControlDefaults} supplies, and matter.js then refuses to + * construct the endpoint with a bare "Behaviors have errors". Merging per + * behaviour is what lets a spec's initial `states` coexist with them. + */ +function mergeBehaviors(...patches: Record[]): Record { + const merged: Record> = {}; + for (const patch of patches) { + for (const [behavior, values] of Object.entries(patch)) { + merged[behavior] = { ...merged[behavior], ...(values as Record) }; + } + } + return merged; +} + +/** The Bridged Device Basic Information every child carries (PRD §5.3). */ +function bridgedInfoFor(spec: EndpointSpec, productName: string): Record { + return { + nodeLabel: spec.label, + productName, + productLabel: spec.label, + serialNumber: serialNumberFor(spec.indigoDeviceId), + uniqueId: uniqueIdFor(spec.indigoDeviceId), + reachable: spec.reachable, + // Optional on bridged devices, and matter.js only lets + // `increaseConfigurationVersion` run when an initial value exists — so + // it is seeded here rather than discovered as a throw on the first bump. + configurationVersion: 1, + }; +} + +/** + * Build the bridged child endpoint for one spec. Not yet added to a parent; + * `Endpoint.id` is fixed at construction and is the identity matter.js keys its + * persisted endpoint number on. + */ +export function createEndpoint(spec: EndpointSpec, productName: string): Endpoint { + const definition = definitionFor(spec.role); + return new Endpoint(definition.deviceType() as never, { + id: endpointIdFor(spec.indigoDeviceId), + ...mergeBehaviors( + { [BRIDGED_INFO]: bridgedInfoFor(spec, productName) }, + definition.initialState(), + definition.statePatch(spec.states), + ), + } as never); +} + +/** Apply a §3.4 `set_state` as a local (offline-context) write. */ +export async function applyStates( + endpoint: Endpoint, + role: RoleValue, + states: Record, +): Promise { + const patch = definitionFor(role).statePatch(states); + if (Object.keys(patch).length === 0) { + return; + } + await endpoint.set(patch as never); +} + +/** §3.5 / PRD §5.3 — `Reachable` tracks the Indigo device's enabled state. */ +export async function applyReachable(endpoint: Endpoint, reachable: boolean): Promise { + await endpoint.set({ [BRIDGED_INFO]: { reachable } } as never); +} + +/** §4.1 — `NodeLabel` follows the export's display name. */ +export async function applyLabel(endpoint: Endpoint, label: string): Promise { + await endpoint.set({ [BRIDGED_INFO]: { nodeLabel: label, productLabel: label } } as never); +} + +/** + * True when a cluster change came from our own `set_state` rather than from an + * ecosystem (§6.4). matter.js runs local agent writes in a `LocalActorContext`, + * whose `offline` is the literal `true`; a remote (network) action carries a + * `RemoteActorContext` with `offline` absent or `false`. + * + * Exported because it is the whole echo guard, and a pure predicate is testable + * without persuading a real Matter stack to originate a remote write. + */ +export function isEcosystemChange(context: unknown): boolean { + return (context as { offline?: boolean } | undefined)?.offline !== true; +} + +/** + * Subscribe the role's cluster-change listeners and return their teardown. + * + * The teardown matters: a removed endpoint whose observables still hold our + * handler would keep emitting `command` events for a device the plugin no + * longer exports, and would keep the closure (and the endpoint) alive. + */ +export function watchCommands( + endpoint: Endpoint, + spec: { indigoDeviceId: number; role: RoleValue }, + emit: (data: CommandEventData) => void, +): () => void { + const teardown: (() => void)[] = []; + + for (const watch of definitionFor(spec.role).watch) { + const events = endpoint.eventsOf(watch.behavior) as Record< + string, + { on(handler: (...args: unknown[]) => void): void; off(handler: (...args: unknown[]) => void): void } + >; + const observable = events[`${watch.attribute}$Changed`]; + if (observable === undefined) { + continue; + } + const handler = (...args: unknown[]): void => { + const [value, , context] = args; + if (!isEcosystemChange(context)) { + return; + } + const state = endpoint.stateOf(watch.behavior) as Readonly>; + const command = watch.command(value, state); + if (command === undefined) { + return; + } + emit({ indigoDeviceId: spec.indigoDeviceId, command: command.command, args: command.args }); + }; + observable.on(handler); + teardown.push(() => observable.off(handler)); + } + + return () => { + for (const off of teardown) { + off(); + } + teardown.length = 0; + }; +} diff --git a/bridge-node/src/node.ts b/bridge-node/src/node.ts index 188705b..42e1992 100644 --- a/bridge-node/src/node.ts +++ b/bridge-node/src/node.ts @@ -1,15 +1,21 @@ /** * The Matter side of the bridge: a ServerNode with an Aggregator at endpoint 1 - * and (in E0) a single hard-coded bridged child endpoint. + * and one bridged child endpoint per exported Indigo device. * - * All matter.js coupling lives here and in main.ts — ADR-0006's binding - * constraint keeps it out of the Indigo plugin entirely, and this module keeps - * it out of the protocol layer so the protocol is testable on its own. + * The child set is entirely protocol-driven (§3.1-§3.3) and lives in + * {@link EndpointRegistry}; a node that has never been attached to serves an + * empty aggregator. Endpoint *numbers* are persisted by matter.js against + * `Endpoint.id`, so a device that comes back with the same id comes back with + * the same number — presence in the running set is not what preserves identity. + * + * All matter.js coupling lives here, in `endpoints.ts`/`registry.ts` and in + * main.ts — ADR-0006's binding constraint keeps it out of the Indigo plugin + * entirely, and this module keeps it out of the protocol layer so the protocol + * is testable on its own. */ import { Endpoint, Environment, ServerNode, VendorId, version as matterJsVersion } from "@matter/main"; -import { BridgedDeviceBasicInformationServer } from "@matter/main/behaviors/bridged-device-basic-information"; -import { OnOffPlugInUnitDevice } from "@matter/main/devices/on-off-plug-in-unit"; +import { BasicInformationServer } from "@matter/main/behaviors/basic-information"; import { AggregatorEndpoint } from "@matter/main/endpoints/aggregator"; import { Crypto } from "@matter/main"; import { DeviceCommissioner, PaseClient, PaseServer, SessionManager } from "@matter/main/protocol"; @@ -18,17 +24,21 @@ import { CommissioningFlowType, ManualPairingCodeCodec, QrPairingCodeCodec } fro import type { BridgeConfig } from "./config.js"; import { type BridgeFacade, + type CommandEventData, type CommissioningWindowResult, describeError, describeErrorWithStack, + type EndpointSpec, ErrorCode, type FabricInfo, type PairingReport, ProtocolError, - Role, + type RemoveResult, type StatusReport, + type UpsertResult, type WindowClosedReason, } from "./protocol.js"; +import { EndpointRegistry } from "./registry.js"; import { type BridgeIdentity, nodeUniqueIdFor, serialNumberFor } from "./storage.js"; import { CommissioningWindow } from "./window.js"; @@ -38,10 +48,8 @@ export const PRODUCT_ID = 0x8000; export const VENDOR_NAME = "simons-plugins"; export const PRODUCT_NAME = "Indigo Matter Bridge"; -/** The one hard-coded export of E0. E1 replaces this with protocol-driven CRUD. */ -const E0_DEVICE_ID = 999001; -const E0_LABEL = "Indigo E0 Test"; -const E0_ROLE = Role.onOffPlugInUnit; +/** PRD §5.3: no hard cap, but past this many exports the log says so. */ +export const ENDPOINT_COUNT_WARNING = 100; /** PBKDF iteration count for enhanced-window verifiers. Spec floor is 1000. */ const PBKDF_ITERATIONS = 1000; @@ -49,21 +57,10 @@ const PBKDF_SALT_BYTES = 32; export { matterJsVersion }; -/** Bridged Device Basic Information `UniqueID`, stable across restarts. */ -export function uniqueIdFor(indigoDeviceId: number): string { - return `indigo-${indigoDeviceId}`; -} - -/** - * `Endpoint.id` derivation — the identity key of BRIDGE_PROTOCOL §4.1/§6.3. - * Deliberately the *same* value as {@link uniqueIdFor}: one derivation means the - * two can never drift apart, and §6.3's one-way identity flow reads directly. - */ -export const endpointIdFor = uniqueIdFor; - export class BridgeNode implements BridgeFacade { #server?: ServerNode; - #child?: Endpoint; + #registry?: EndpointRegistry; + #command?: (data: CommandEventData) => void; readonly #window: CommissioningWindow; constructor( @@ -85,8 +82,10 @@ export class BridgeNode implements BridgeFacade { } /** - * Build the node and bring it online. Aggregator is added first so it takes - * endpoint 1; the bridged child then lands at 2. + * Build the node and bring it online. The aggregator is added first so it + * takes endpoint 1; bridged children land at 2 and up, in the order the + * first `attach` creates them (and thereafter at whatever number matter.js + * has persisted against their id). */ async start(): Promise { const environment = Environment.default; @@ -134,19 +133,13 @@ export class BridgeNode implements BridgeFacade { const aggregator = new Endpoint(AggregatorEndpoint, { id: "aggregator" }); await server.add(aggregator); - const child = new Endpoint(OnOffPlugInUnitDevice.with(BridgedDeviceBasicInformationServer), { - id: endpointIdFor(E0_DEVICE_ID), - bridgedDeviceBasicInformation: { - nodeLabel: E0_LABEL, - productName: PRODUCT_NAME, - productLabel: E0_LABEL, - serialNumber: String(E0_DEVICE_ID), - uniqueId: uniqueIdFor(E0_DEVICE_ID), - reachable: true, - }, + this.#registry = new EndpointRegistry({ + aggregator, + productName: PRODUCT_NAME, + log: message => this.log(message), + emit: data => this.#command?.(data), + onConfigurationChange: () => this.bumpConfigurationVersion(), }); - await aggregator.add(child); - this.#child = child; server.events.commissioning.fabricsChanged.on((fabricIndex, action) => { // A throw here propagates straight into matter.js's observable, and @@ -196,28 +189,84 @@ export class BridgeNode implements BridgeFacade { })); } + private get registry(): EndpointRegistry { + if (this.#registry === undefined) { + throw new ProtocolError(ErrorCode.internal, "Matter node not started"); + } + return this.#registry; + } + getStatus(): StatusReport { - const child = this.#child; - const endpoints = - child === undefined - ? [] - : [{ indigoDeviceId: E0_DEVICE_ID, endpointNumber: Number(child.number), role: E0_ROLE }]; + const endpoints = this.#registry?.summaries() ?? []; return { commissioned: this.server.lifecycle.isCommissioned, fabrics: this.fabrics(), endpointCount: endpoints.length, endpoints, - // E6 introduces the persisted endpoint-number allocator; until then - // there is no baseline to drift from. + // E5 introduces the persisted endpoint-number map and its drift + // detector; until then there is no baseline to drift from. drift: [], }; } + /** §3.1 — reconcile the live endpoint set, then answer with the new status. */ + async reconcile(endpoints: readonly EndpointSpec[], replaceAll: boolean): Promise { + await this.registry.reconcile(endpoints, replaceAll); + if (this.registry.size > ENDPOINT_COUNT_WARNING) { + this.log( + `${this.registry.size} exported endpoints exceeds the ${ENDPOINT_COUNT_WARNING} advisory limit; ` + + "ecosystem per-home accessory caps will bite before memory does", + ); + } + return this.getStatus(); + } + + /** §3.2 */ + async upsertEndpoint(spec: EndpointSpec): Promise { + return this.registry.upsert(spec); + } + + /** §3.3 */ + async removeEndpoint(indigoDeviceId: number): Promise { + return this.registry.remove(indigoDeviceId); + } + + /** §3.4 */ + async setState(indigoDeviceId: number, states: Record): Promise { + await this.registry.setState(indigoDeviceId, states); + } + + /** §3.5 */ + async setReachable(indigoDeviceId: number, reachable: boolean): Promise { + await this.registry.setReachable(indigoDeviceId, reachable); + } + + /** + * PRD §5.3 / Matter 1.5: a changed bridged-node set is a configuration + * change of the bridge. Bumping the root's `ConfigurationVersion` also + * covers the children — matter.js's own + * `BridgedDeviceBasicInformationServer.increaseConfigurationVersion` + * increments the root as well, so doing it once per batch is both cheaper + * and truer to "one logical change, one increment". + */ + private async bumpConfigurationVersion(): Promise { + const server = this.#server; + if (server === undefined) { + return; + } + await server.act(agent => agent.get(BasicInformationServer).increaseConfigurationVersion()); + } + /** §5: the sink for `window_closed`, wired up by the protocol server. */ onWindowClosed(listener: (reason: WindowClosedReason) => void): void { this.#window.onClosed(listener); } + /** §5: the sink for `command`. One listener, last registration wins. */ + onCommand(listener: (data: CommandEventData) => void): void { + this.#command = listener; + } + getPairing(): PairingReport { const commissioned = this.server.lifecycle.isCommissioned; const window = this.#window.current; @@ -268,7 +317,7 @@ export class BridgeNode implements BridgeFacade { * `adminFabricIndex`) and therefore cannot be invoked from an offline agent. * The consequence is that the cluster's `windowStatus`/`adminFabricIndex` * attributes do not reflect a locally-opened window — a conformance gap for - * E7 to close, not something E0's pairing flow depends on. + * E7 to close, not something the pairing flow depends on. */ async openCommissioningWindow(durationSeconds: number): Promise { // Before anything Matter-side: `allowEnhancedCommissioning` swaps the PASE @@ -340,7 +389,8 @@ export class BridgeNode implements BridgeFacade { this.#window.clear(); const server = this.#server; this.#server = undefined; - this.#child = undefined; + this.#registry?.close(); + this.#registry = undefined; if (server !== undefined) { await server.close(); } diff --git a/bridge-node/src/protocol.ts b/bridge-node/src/protocol.ts index 6087919..d236239 100644 --- a/bridge-node/src/protocol.ts +++ b/bridge-node/src/protocol.ts @@ -72,10 +72,10 @@ export interface ErrorFrame { export type WindowClosedReason = "expired" | "commissioned"; /** - * The complete §5 event name domain. Only `window_closed` is *emitted* in E0 — - * the rest arrive with endpoint CRUD — but the whole set is declared here so the - * fixture mirror can catch a misspelt name at compile time rather than shipping - * an event the plugin logs as unknown and drops. + * The complete §5 event name domain. `window_closed` and `command` are emitted + * today; the rest arrive with fabric/drift reporting. The whole set is declared + * here so the fixture mirror can catch a misspelt name at compile time rather + * than shipping an event the plugin logs as unknown and drops. */ export const EventName = { command: "command", @@ -119,6 +119,46 @@ export const Role = { export type RoleValue = (typeof Role)[keyof typeof Role]; +/** Runtime membership test for the §4.2 role enum — the `unknown_role` gate. */ +export function isRole(value: unknown): value is RoleValue { + return typeof value === "string" && Object.prototype.hasOwnProperty.call(Role, value); +} + +/** + * §3.1's opt-in for a reconcile that would empty the live endpoint set. The + * literal is named because both the guard and its refusal message quote it. + */ +export const INTENT_REPLACE_ALL = "replace_all"; + +/** §4.1 — the desired state of one exported device, as the plugin declares it. */ +export interface EndpointSpec { + indigoDeviceId: number; + role: RoleValue; + label: string; + reachable: boolean; + /** Role-specific state keys (§4.2). Values are Indigo-natural units. */ + states: Record; + /** Role-specific extras (e.g. window-covering polarity). Unused by E3 roles. */ + options: Record; +} + +/** §3.2 */ +export interface UpsertResult { + endpointNumber: number; +} + +/** §3.3 */ +export interface RemoveResult { + removed: boolean; +} + +/** §5 `command` — the `data` of an ecosystem-originated action. */ +export interface CommandEventData extends Record { + indigoDeviceId: number; + command: string; + args: Record; +} + /** §4.3 */ export interface FabricInfo { fabricIndex: number; @@ -126,7 +166,7 @@ export interface FabricInfo { vendorId: number; } -/** §4.3 — the E0 subset; `drift` is always empty until E6 adds the allocator. */ +/** §4.3. `drift` is always empty until E5 adds the persisted endpoint-number map. */ export interface StatusReport { commissioned: boolean; fabrics: FabricInfo[]; @@ -172,12 +212,28 @@ export interface BridgeFacade { getStatus(): StatusReport; getPairing(): PairingReport; openCommissioningWindow(durationSeconds: number): Promise; + /** + * §3.1's reconcile. `replaceAll` is the parsed `intent: "replace_all"`; the + * mass-removal guard lives behind this seam because only the implementation + * knows the live set. + */ + reconcile(endpoints: readonly EndpointSpec[], replaceAll: boolean): Promise; + /** §3.2 — create-or-update. Rejects a role change with `role_change` (§4.1). */ + upsertEndpoint(spec: EndpointSpec): Promise; + /** §3.3 — idempotent; `{removed: false}` for a device with no live endpoint. */ + removeEndpoint(indigoDeviceId: number): Promise; + /** §3.4 — local (offline-context) writes, so they do not echo as `command`. */ + setState(indigoDeviceId: number, states: Record): Promise; + /** §3.5 — Bridged Device Basic Information `Reachable`. */ + setReachable(indigoDeviceId: number, reachable: boolean): Promise; /** * Register the sink for `window_closed` (§3.8/§5). One listener, last * registration wins — this seam is what lets the protocol server emit the * event without importing the Matter stack, and lets tests fire it. */ onWindowClosed(listener: (reason: WindowClosedReason) => void): void; + /** The same seam for §5 `command`, emitted when an ecosystem acts. */ + onCommand(listener: (data: CommandEventData) => void): void; } /** A protocol-level failure a command handler can throw to shape its response. */ diff --git a/bridge-node/src/reconcile.ts b/bridge-node/src/reconcile.ts new file mode 100644 index 0000000..74c9265 --- /dev/null +++ b/bridge-node/src/reconcile.ts @@ -0,0 +1,182 @@ +/** + * Argument parsing and the reconcile planner for the endpoint-CRUD commands + * (BRIDGE_PROTOCOL §3.1-§3.5, §4.1). + * + * Deliberately matter.js-free, like `protocol.ts`: every §1.1 refusal the + * endpoint commands can produce — `malformed_args`, `unknown_role`, + * `mass_removal_refused` — is decided here, so the decisions are unit-testable + * without a Matter stack and the WebSocket server and the test double reach the + * same verdicts from the same code. + */ + +import { + type EndpointSpec, + ErrorCode, + INTENT_REPLACE_ALL, + isRole, + ProtocolError, + type RoleValue, +} from "./protocol.js"; + +/** True for a plain JSON object — the shape `states`/`options` must have. */ +function isStruct(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function requireStruct(value: unknown, what: string): Record { + if (value === undefined) { + return {}; + } + if (!isStruct(value)) { + throw new ProtocolError(ErrorCode.malformedArgs, `${what} must be an object`); + } + return value; +} + +/** + * §1.1 gate for the identity key. Every endpoint command carries one, and it is + * always the same refusal, so it is one function rather than four copies. + */ +export function parseDeviceId(value: unknown): number { + if (typeof value !== "number" || !Number.isInteger(value)) { + throw new ProtocolError(ErrorCode.malformedArgs, "indigoDeviceId must be an integer"); + } + return value; +} + +/** + * Parse one wire `EndpointSpec` (§4.1). + * + * A role outside the §4.2 enum is `unknown_role`, not `malformed_args`: §1.1 + * gives it its own code precisely so the plugin can tell "you sent nonsense" + * from "this node is older than your role vocabulary". + */ +export function parseEndpointSpec(value: unknown): EndpointSpec { + if (!isStruct(value)) { + throw new ProtocolError(ErrorCode.malformedArgs, "endpoint must be an object"); + } + const indigoDeviceId = parseDeviceId(value.indigoDeviceId); + if (typeof value.role !== "string") { + throw new ProtocolError(ErrorCode.malformedArgs, "endpoint.role must be a string"); + } + if (!isRole(value.role)) { + throw new ProtocolError(ErrorCode.unknownRole, `role ${value.role} is not in the v1 role enum (§4.2)`); + } + if (typeof value.label !== "string") { + throw new ProtocolError(ErrorCode.malformedArgs, "endpoint.label must be a string"); + } + // `reachable` defaults to true: an omitted flag means "nothing said about + // availability", and an accessory that greys itself out by default would be + // the worse reading. + const reachable = value.reachable === undefined ? true : value.reachable; + if (typeof reachable !== "boolean") { + throw new ProtocolError(ErrorCode.malformedArgs, "endpoint.reachable must be a boolean"); + } + return { + indigoDeviceId, + role: value.role, + label: value.label, + reachable, + states: requireStruct(value.states, "endpoint.states"), + options: requireStruct(value.options, "endpoint.options"), + }; +} + +/** Parse the `endpoints` array of an `attach` (§3.1). */ +export function parseEndpointSpecs(value: unknown): EndpointSpec[] { + if (value === undefined) { + return []; + } + if (!Array.isArray(value)) { + throw new ProtocolError(ErrorCode.malformedArgs, "endpoints must be an array"); + } + const specs = value.map(parseEndpointSpec); + const seen = new Set(); + for (const spec of specs) { + if (seen.has(spec.indigoDeviceId)) { + throw new ProtocolError( + ErrorCode.malformedArgs, + `endpoints contains indigoDeviceId ${spec.indigoDeviceId} twice`, + ); + } + seen.add(spec.indigoDeviceId); + } + return specs; +} + +/** §3.1: the opt-in that makes emptying the endpoint set deliberate. */ +export function parseReplaceAll(intent: unknown): boolean { + if (intent === undefined) { + return false; + } + if (typeof intent !== "string") { + throw new ProtocolError(ErrorCode.malformedArgs, "intent must be a string"); + } + return intent === INTENT_REPLACE_ALL; +} + +/** + * What an `attach` reconcile has to do to the live endpoint set. + * + * `recreate` is the role-change case: §4.1 rejects a role change through + * `upsert_endpoint`, but `attach` is a *full reconcile* from the peer that owns + * the export set (§6.2), and failing the whole attach would leave the plugin no + * way to correct a role at all. So attach removes and re-adds instead. It is + * still an accessory-identity change in every paired ecosystem, hence its own + * bucket and its own log line rather than hiding inside `update`. + */ +export interface ReconcilePlan { + create: EndpointSpec[]; + update: EndpointSpec[]; + recreate: EndpointSpec[]; + remove: number[]; +} + +/** + * Diff the desired set against the live one, enforcing the §3.1 mass-removal + * guard. + * + * The guard is about the *effect*, not the literal empty array: an `attach` + * carrying a completely disjoint device set would also remove every live + * endpoint, and that is just as much "every exported accessory disappears from + * every paired ecosystem" as `endpoints: []` is. + */ +export function planReconcile( + live: ReadonlyMap, + desired: readonly EndpointSpec[], + replaceAll: boolean, +): ReconcilePlan { + const plan: ReconcilePlan = { create: [], update: [], recreate: [], remove: [] }; + const desiredIds = new Set(); + + for (const spec of desired) { + desiredIds.add(spec.indigoDeviceId); + const liveRole = live.get(spec.indigoDeviceId); + if (liveRole === undefined) { + plan.create.push(spec); + } else if (liveRole === spec.role) { + plan.update.push(spec); + } else { + plan.recreate.push(spec); + } + } + + for (const indigoDeviceId of live.keys()) { + if (!desiredIds.has(indigoDeviceId)) { + plan.remove.push(indigoDeviceId); + } + } + + // A `recreate` is not a removal: the device stays exported, it just changes + // accessory type. Counting it as one would refuse a lawful role correction + // on a single-export bridge. + const survivors = live.size - plan.remove.length; + if (live.size > 0 && survivors === 0 && !replaceAll) { + throw new ProtocolError( + ErrorCode.massRemovalRefused, + `attach would remove all ${live.size} live endpoints without intent: ${INTENT_REPLACE_ALL}`, + ); + } + + return plan; +} diff --git a/bridge-node/src/registry.ts b/bridge-node/src/registry.ts new file mode 100644 index 0000000..80c4791 --- /dev/null +++ b/bridge-node/src/registry.ts @@ -0,0 +1,246 @@ +/** + * The live bridged-endpoint set: create, update, remove, and the §3.1 reconcile. + * + * Split out of `node.ts` so that module stays what it is — the Matter *node* + * (identity, commissioning, fabrics) — while this one owns the *children*. + * Both are matter.js-coupled; the protocol layer reaches them only through + * `BridgeFacade`. + */ + +import type { Endpoint } from "@matter/main"; + +import { + applyLabel, + applyReachable, + applyStates, + createEndpoint, + isSupportedRole, + UNSUPPORTED_ROLE_DETAILS, + watchCommands, +} from "./endpoints.js"; +import { + type CommandEventData, + type EndpointSpec, + type EndpointSummary, + ErrorCode, + ProtocolError, + type RemoveResult, + type RoleValue, + type UpsertResult, +} from "./protocol.js"; +import { planReconcile } from "./reconcile.js"; + +/** + * PRD §5.3: bulk removals are paced so controllers see one subscription update + * each rather than a burst they may coalesce or drop. + */ +export const REMOVAL_PACING_MS = 100; + +interface LiveEndpoint { + endpoint: Endpoint; + role: RoleValue; + unwatch: () => void; +} + +export interface EndpointRegistryOptions { + /** The aggregator every bridged child is added to. */ + aggregator: Endpoint; + /** Sink for §5 `command` events. */ + emit: (data: CommandEventData) => void; + log?: (message: string) => void; + /** `ProductName` on every child's Bridged Device Basic Information. */ + productName: string; + /** Injectable so the pacing test does not take a real 100ms per removal. */ + removalPacingMs?: number; + /** + * Called once per add/remove batch so the caller can bump + * `ConfigurationVersion`. Optional because the pure-registry tests have no + * root node to bump. + */ + onConfigurationChange?: () => Promise; +} + +export class EndpointRegistry { + readonly #live = new Map(); + readonly #log: (message: string) => void; + readonly #pacingMs: number; + + constructor(private readonly options: EndpointRegistryOptions) { + this.#log = options.log ?? (() => {}); + this.#pacingMs = options.removalPacingMs ?? REMOVAL_PACING_MS; + } + + /** §4.3 `StatusReport.endpoints`, in device-id order for a stable readout. */ + summaries(): EndpointSummary[] { + return [...this.#live.entries()] + .map(([indigoDeviceId, live]) => ({ + indigoDeviceId, + endpointNumber: Number(live.endpoint.number), + role: live.role, + })) + .sort((a, b) => a.indigoDeviceId - b.indigoDeviceId); + } + + get size(): number { + return this.#live.size; + } + + /** The live role of a device, or undefined — what the reconcile planner diffs. */ + liveRoles(): Map { + return new Map([...this.#live.entries()].map(([id, live]) => [id, live.role])); + } + + /** + * §3.1: full reconcile against the desired set. Throws + * `mass_removal_refused` before touching anything — the guard has to be a + * gate, not a rollback. + */ + async reconcile(desired: readonly EndpointSpec[], replaceAll: boolean): Promise { + for (const spec of desired) { + this.assertSupported(spec.role); + } + const plan = planReconcile(this.liveRoles(), desired, replaceAll); + + for (const indigoDeviceId of plan.recreate.map(spec => spec.indigoDeviceId)) { + // §4.1 rejects a role change through `upsert_endpoint`; attach is the + // reconcile path and does it the only way Matter allows, which is a + // new accessory. Loud, because ecosystems lose the old one's name. + this.#log(`Recreating endpoint ${indigoDeviceId}: role changed`); + } + await this.removeMany([...plan.remove, ...plan.recreate.map(spec => spec.indigoDeviceId)]); + + for (const spec of [...plan.create, ...plan.recreate]) { + await this.create(spec); + } + for (const spec of plan.update) { + await this.update(spec); + } + + if (plan.create.length > 0 || plan.recreate.length > 0 || plan.remove.length > 0) { + await this.noteConfigurationChange(); + } + this.#log( + `Reconciled endpoints: ${plan.create.length} created, ${plan.update.length} updated, ` + + `${plan.recreate.length} recreated, ${plan.remove.length} removed (${this.#live.size} live)`, + ); + } + + /** §3.2 — create-or-update, idempotent, `role_change` on a role mismatch. */ + async upsert(spec: EndpointSpec): Promise { + this.assertSupported(spec.role); + const existing = this.#live.get(spec.indigoDeviceId); + if (existing === undefined) { + const created = await this.create(spec); + await this.noteConfigurationChange(); + return { endpointNumber: Number(created.number) }; + } + if (existing.role !== spec.role) { + throw new ProtocolError( + ErrorCode.roleChange, + `endpoint ${spec.indigoDeviceId} is ${existing.role}; remove and re-add to change role`, + ); + } + await this.update(spec); + return { endpointNumber: Number(existing.endpoint.number) }; + } + + /** §3.3 — idempotent removal. The endpoint-number allocation is retained. */ + async remove(indigoDeviceId: number): Promise { + const live = this.#live.get(indigoDeviceId); + if (live === undefined) { + return { removed: false }; + } + await this.closeOne(indigoDeviceId, live); + await this.noteConfigurationChange(); + return { removed: true }; + } + + /** §3.4 — role-specific state keys, applied as local writes. */ + async setState(indigoDeviceId: number, states: Record): Promise { + const live = this.require(indigoDeviceId); + await applyStates(live.endpoint, live.role, states); + } + + /** §3.5 */ + async setReachable(indigoDeviceId: number, reachable: boolean): Promise { + const live = this.require(indigoDeviceId); + await applyReachable(live.endpoint, reachable); + } + + /** Drop every listener. The endpoints themselves die with the ServerNode. */ + close(): void { + for (const live of this.#live.values()) { + live.unwatch(); + } + this.#live.clear(); + } + + private require(indigoDeviceId: number): LiveEndpoint { + const live = this.#live.get(indigoDeviceId); + if (live === undefined) { + throw new ProtocolError( + ErrorCode.unknownDevice, + `no live endpoint for indigoDeviceId ${indigoDeviceId}`, + ); + } + return live; + } + + private assertSupported(role: RoleValue): void { + if (!isSupportedRole(role)) { + throw new ProtocolError(ErrorCode.internal, UNSUPPORTED_ROLE_DETAILS(role)); + } + } + + private async create(spec: EndpointSpec): Promise { + const endpoint = createEndpoint(spec, this.options.productName); + await this.options.aggregator.add(endpoint); + const unwatch = watchCommands(endpoint, spec, this.options.emit); + this.#live.set(spec.indigoDeviceId, { endpoint, role: spec.role, unwatch }); + this.#log(`Endpoint ${spec.indigoDeviceId} (${spec.role}) added as number ${Number(endpoint.number)}`); + return endpoint; + } + + /** Label, reachability and state — everything an existing endpoint can change. */ + private async update(spec: EndpointSpec): Promise { + const live = this.require(spec.indigoDeviceId); + const info = live.endpoint.stateOf("bridgedDeviceBasicInformation") as { nodeLabel?: string }; + if (info.nodeLabel !== spec.label) { + await applyLabel(live.endpoint, spec.label); + } + await applyReachable(live.endpoint, spec.reachable); + await applyStates(live.endpoint, live.role, spec.states); + } + + private async closeOne(indigoDeviceId: number, live: LiveEndpoint): Promise { + live.unwatch(); + this.#live.delete(indigoDeviceId); + await live.endpoint.close(); + this.#log(`Endpoint ${indigoDeviceId} removed`); + } + + /** PRD §5.3: ~100ms apart, so each removal is its own subscription update. */ + private async removeMany(indigoDeviceIds: readonly number[]): Promise { + for (const [index, indigoDeviceId] of indigoDeviceIds.entries()) { + const live = this.#live.get(indigoDeviceId); + if (live === undefined) { + continue; + } + await this.closeOne(indigoDeviceId, live); + if (index < indigoDeviceIds.length - 1 && this.#pacingMs > 0) { + await new Promise(resolve => setTimeout(resolve, this.#pacingMs)); + } + } + } + + private async noteConfigurationChange(): Promise { + try { + await this.options.onConfigurationChange?.(); + } catch (error) { + // A ConfigurationVersion bump is a hint to controllers, not a + // correctness requirement: failing the whole command over it would + // turn a cosmetic problem into a broken export. + this.#log(`Could not bump ConfigurationVersion: ${String(error)}`); + } + } +} diff --git a/bridge-node/src/storage.ts b/bridge-node/src/storage.ts index 14a20a2..d16946a 100644 --- a/bridge-node/src/storage.ts +++ b/bridge-node/src/storage.ts @@ -111,9 +111,9 @@ function writeIdentity(file: string, identity: BridgeIdentity): void { * Read `identity.json` from {@link storagePath}, creating it (and the directory) * with freshly randomised values on first run. An unreadable or invalid file is * replaced — a bridge that cannot advertise is worse than one that needs - * re-pairing, and E0 has nothing paired to protect yet. + * re-pairing, and nothing is paired to protect until the first commissioning. * - * TODO(E1): a corrupt-but-present identity must refuse to start (PRD §4.3) — + * TODO(E5): a corrupt-but-present identity must refuse to start (PRD §4.3) — * regeneration un-pairs every ecosystem. Only the missing-file branch may mint. */ export function loadOrCreateIdentity( @@ -124,7 +124,7 @@ export function loadOrCreateIdentity( const file = join(storagePath, IDENTITY_FILE); // `undefined` means "no file at all"; anything else is why we are replacing - // a file that does exist — the distinction E1 turns into refuse-to-start. + // a file that does exist — the distinction E5 turns into refuse-to-start. let problem: string | undefined; try { const parsed: unknown = JSON.parse(readFileSync(file, "utf8")); @@ -155,7 +155,7 @@ export function loadOrCreateIdentity( /** * Matter `SerialNumber` is capped at 32 characters, so the UUID is used with its * dashes stripped. Matter requires `UniqueID` and `SerialNumber` to differ, so - * {@link uniqueIdFor} takes the full value and this takes the first half. + * the node's UniqueID takes the full value and this takes the first half. */ export function serialNumberFor(identity: BridgeIdentity): string { return identity.installId.replace(/-/g, "").slice(0, 16); diff --git a/bridge-node/src/ws-server.ts b/bridge-node/src/ws-server.ts index c29c2ae..5f7a6cd 100644 --- a/bridge-node/src/ws-server.ts +++ b/bridge-node/src/ws-server.ts @@ -26,6 +26,7 @@ import { WINDOW_DURATION_MAX_SECONDS, WINDOW_DURATION_MIN_SECONDS, } from "./protocol.js"; +import { parseDeviceId, parseEndpointSpec, parseEndpointSpecs, parseReplaceAll } from "./reconcile.js"; export const LOOPBACK_HOST = "127.0.0.1"; @@ -51,7 +52,11 @@ interface ClientState { pending: Promise; } -/** Commands whose handlers exist in E0. Endpoint CRUD arrives in E1. */ +/** + * A §3 command handler. Argument shape is validated here (that is what + * `malformed_args`/`unknown_role` mean); everything that needs to know the live + * endpoint set is decided behind {@link BridgeFacade}. + */ type CommandHandler = (args: Record, socket: WebSocket, state: ClientState) => Promise; export class BridgeWsServer { @@ -67,7 +72,16 @@ export class BridgeWsServer { this.#handlers.set("get_status", async () => this.options.bridge.getStatus()); this.#handlers.set("get_pairing", async () => this.options.bridge.getPairing()); this.#handlers.set("open_commissioning_window", async args => this.handleOpenWindow(args)); + this.#handlers.set("upsert_endpoint", async args => + this.options.bridge.upsertEndpoint(parseEndpointSpec(args.endpoint)), + ); + this.#handlers.set("remove_endpoint", async args => + this.options.bridge.removeEndpoint(parseDeviceId(args.indigoDeviceId)), + ); + this.#handlers.set("set_state", async args => this.handleSetState(args)); + this.#handlers.set("set_reachable", async args => this.handleSetReachable(args)); options.bridge.onWindowClosed(reason => this.sendEvent(EventName.windowClosed, { reason })); + options.bridge.onCommand(data => this.sendEvent(EventName.command, data)); } /** @@ -254,6 +268,12 @@ export class BridgeWsServer { ); } + // §3.1: parse before attaching state changes hands, so a malformed + // endpoint set cannot supersede a healthy incumbent on its way to being + // rejected. + const endpoints = parseEndpointSpecs(args.endpoints); + const replaceAll = parseReplaceAll(args.intent); + // §2: exactly one attached client; a new attach supersedes the incumbent, // which is how we recover from a half-open socket left by a plugin crash. const incumbent = this.#attached; @@ -272,13 +292,32 @@ export class BridgeWsServer { this.#attached = socket; const pluginVersion = typeof args.pluginVersion === "string" ? args.pluginVersion : "unknown"; - this.#log(`Client attached (plugin ${pluginVersion})`); + this.#log(`Client attached (plugin ${pluginVersion}), reconciling ${endpoints.length} endpoint(s)`); + + // §3.1: a fresh connection is always a full reconcile. The mass-removal + // guard lives behind the facade because only it knows the live set; a + // refusal leaves this client attached but the endpoint set untouched, + // which is what lets the plugin retry with an explicit intent. + return this.options.bridge.reconcile(endpoints, replaceAll); + } - // E1 reconciles `args.endpoints` here; E0 serves a fixed endpoint set. - // The §3.1 mass-removal guard (`mass_removal_refused` unless - // `intent: "replace_all"`) belongs with that reconcile — it is E1 scope, - // and until then there is no client-supplied set that could empty. - return this.options.bridge.getStatus(); + private async handleSetState(args: Record): Promise { + const indigoDeviceId = parseDeviceId(args.indigoDeviceId); + const states = args.states; + if (typeof states !== "object" || states === null || Array.isArray(states)) { + throw new ProtocolError(ErrorCode.malformedArgs, "states must be an object"); + } + await this.options.bridge.setState(indigoDeviceId, states as Record); + return {}; + } + + private async handleSetReachable(args: Record): Promise { + const indigoDeviceId = parseDeviceId(args.indigoDeviceId); + if (typeof args.reachable !== "boolean") { + throw new ProtocolError(ErrorCode.malformedArgs, "reachable must be a boolean"); + } + await this.options.bridge.setReachable(indigoDeviceId, args.reachable); + return {}; } private async handleOpenWindow(args: Record): Promise { diff --git a/bridge-node/test/fixture-shapes.ts b/bridge-node/test/fixture-shapes.ts index bbded27..8ae9b79 100644 --- a/bridge-node/test/fixture-shapes.ts +++ b/bridge-node/test/fixture-shapes.ts @@ -1,6 +1,7 @@ /** - * Compile-time mirror of the E0 half of `../tests/fixtures/bridge_protocol/frames.json` - * — the repo-root golden file shared with the Python suite (§7 testing contract). + * Compile-time mirror of the implemented half of + * `../tests/fixtures/bridge_protocol/frames.json` — the repo-root golden file + * shared with the Python suite (§7 testing contract). * * `JSON.parse` erases types, so the golden file alone cannot fail `tsc` when a * shape in `protocol.ts` drifts. Each payload is restated here bound with @@ -17,11 +18,17 @@ import type { CommissioningWindowResult, ErrorFrame, EventFrame, + FabricInfo, HandshakeFrame, PairingReport, + RemoveResult, StatusReport, + UpsertResult, } from "../src/protocol.js"; +/** The one paired ecosystem the populated fixtures assume. */ +const APPLE_HOME = { fabricIndex: 1, label: "Apple Home", vendorId: 4937 } satisfies FabricInfo; + /** Versions are placeholders: the real ones track package.json / matter.js. */ export const handshake = { protocolVersion: 1, @@ -29,22 +36,27 @@ export const handshake = { matterJsVersion: "0.0.0-test", } satisfies HandshakeFrame; -/** What the E0 node actually serves: one hard-coded endpoint, ignoring `attach`. */ +/** + * The status of a bridge that has reconciled the {@link attachWithEndpoints} + * set — what `get_status` answers once `attach` has run. Both frames share it, + * which is exactly the §6.2 invariant: `attach` returns the same StatusReport + * `get_status` would. + */ export const status = { - commissioned: false, - fabrics: [], - endpointCount: 1, - endpoints: [{ indigoDeviceId: 999001, endpointNumber: 2, role: "onOffPlugInUnit" }], + commissioned: true, + fabrics: [APPLE_HOME], + endpointCount: 2, + endpoints: [ + { indigoDeviceId: 123456789, endpointNumber: 2, role: "onOffLight" }, + { indigoDeviceId: 123456790, endpointNumber: 3, role: "dimmableLight" }, + ], drift: [], } satisfies StatusReport; /** - * The lawful §3.1 answer to the golden `attach` REQUEST, which carries - * `endpoints: []` — an empty desired set reconciles to an empty live set. - * - * The E0 node does not reconcile yet (it ignores the requested set and serves - * {@link status}), so `protocol.test.ts` asserts attach against the live status - * instead. The two converge when E2 makes `attach` do the reconcile for real. + * The §3.1 answer to an `attach` carrying `endpoints: []` against a node with + * nothing live — an empty desired set reconciles to an empty live set. The + * mass-removal guard does not fire: there was nothing to remove. */ export const statusEmpty = { commissioned: false, @@ -54,6 +66,46 @@ export const statusEmpty = { drift: [], } satisfies StatusReport; +/** §3.1 with `intent: "replace_all"`: the live set is emptied deliberately. */ +export const statusReplaceAll = { + commissioned: true, + fabrics: [APPLE_HOME], + endpointCount: 0, + endpoints: [], + drift: [], +} satisfies StatusReport; + +/** §3.2 — the live endpoint's Matter number, for the plugin's own records. */ +export const upsertResult = { endpointNumber: 2 } satisfies UpsertResult; + +/** §3.3 — the two idempotent outcomes. */ +export const removeResult = { removed: true } satisfies RemoveResult; +export const removeAbsentResult = { removed: false } satisfies RemoveResult; + +/** §3.4/§3.5 both answer with an empty result on success. */ +export const emptyResult = {}; + +/** §3.1: emptying a non-empty live set needs `intent: "replace_all"`. */ +export const massRemovalRefused = { + message_id: "m12", + error_code: "mass_removal_refused", + details: "attach would remove all 2 live endpoints without intent: replace_all", +} satisfies ErrorFrame; + +/** §4.1: ecosystems cache device types per endpoint, so a role change is a refusal. */ +export const roleChange = { + message_id: "m14", + error_code: "role_change", + details: "endpoint 123456789 is onOffLight; remove and re-add to change role", +} satisfies ErrorFrame; + +/** §3.4 against a device with no live endpoint. */ +export const setStateUnknownDevice = { + message_id: "m18", + error_code: "unknown_device", + details: "no live endpoint for indigoDeviceId 123456791", +} satisfies ErrorFrame; + /** §3.7 state 1: never commissioned — the basic window with the persisted codes. */ export const pairingUncommissioned = { commissioned: false, diff --git a/bridge-node/test/fixtures.test.ts b/bridge-node/test/fixtures.test.ts index 0598039..fa3bcb0 100644 --- a/bridge-node/test/fixtures.test.ts +++ b/bridge-node/test/fixtures.test.ts @@ -34,10 +34,19 @@ describe("golden fixtures match their typed mirror", () => { const cases: [string, unknown, unknown][] = [ ["handshake", golden.handshake, shapes.handshake], // §3.1: the golden attach REQUEST carries `endpoints: []`, so the lawful - // answer is an empty live set. The E0 node ignores the requested set and - // serves its fixed endpoint — see protocol.test.ts, and E2. + // answer against a node with nothing live is an empty live set. ["attach result", golden.attach.response.result, shapes.statusEmpty], ["get_status result", golden.get_status.response.result, shapes.status], + ["attach_with_endpoints result", golden.attach_with_endpoints.response.result, shapes.status], + ["attach_replace_all result", golden.attach_replace_all.response.result, shapes.statusReplaceAll], + ["attach mass_removal_refused", golden.attach_mass_removal_refused.response, shapes.massRemovalRefused], + ["upsert_endpoint result", golden.upsert_endpoint.response.result, shapes.upsertResult], + ["upsert_endpoint role_change", golden.upsert_endpoint_role_change.response, shapes.roleChange], + ["remove_endpoint result", golden.remove_endpoint.response.result, shapes.removeResult], + ["remove_endpoint (absent) result", golden.remove_endpoint_absent.response.result, shapes.removeAbsentResult], + ["set_state result", golden.set_state.response.result, shapes.emptyResult], + ["set_state unknown_device", golden.set_state_unknown_device.response, shapes.setStateUnknownDevice], + ["set_reachable result", golden.set_reachable.response.result, shapes.emptyResult], ["get_pairing (uncommissioned)", golden.get_pairing_uncommissioned.response.result, shapes.pairingUncommissioned], ["get_pairing (commissioned)", golden.get_pairing_commissioned.response.result, shapes.pairingCommissioned], [ @@ -65,11 +74,11 @@ describe("golden fixtures match their typed mirror", () => { }); } - describe("pending E1 exchanges", () => { + describe("pending exchanges (§3.9-§3.11 and the E4 roles)", () => { const names = Object.keys(golden.pending).filter(key => !key.startsWith("_")); it("every pending entry is a well-formed exchange", () => { - assert.ok(names.length > 0, "pending section should not be empty while E1 is in flight"); + assert.ok(names.length > 0, "pending section should not be empty while E4 is outstanding"); for (const name of names) { const exchange = golden.pending[name]; assert.ok(exchange?.request !== undefined, `${name} has no request`); @@ -82,9 +91,10 @@ describe("golden fixtures match their typed mirror", () => { } }); - // Shape assertions wait for the node-side handlers: `protocol.ts` has no - // types for these results yet, so there is nothing to mirror. The Python - // client suite asserts them today — that half of E1 ships now. + // Shape assertions wait for the node-side handlers. The Python client + // suite asserts them today — the plugin half shipped at E1 — and a + // frame graduates to a real deepEqual above when the node grows its + // handler, which is what E3 did to the endpoint-CRUD family. for (const name of names) { it.skip(`${name} — node handler not implemented yet`, () => {}); } diff --git a/bridge-node/test/protocol.test.ts b/bridge-node/test/protocol.test.ts index 5331608..0aeb6de 100644 --- a/bridge-node/test/protocol.test.ts +++ b/bridge-node/test/protocol.test.ts @@ -1,6 +1,6 @@ /** - * BRIDGE_PROTOCOL.md conformance for the E0 command subset, run against the - * real ws-server with the Matter node stubbed out. + * BRIDGE_PROTOCOL.md conformance for the implemented command set, run against + * the real ws-server with the Matter node stubbed out. */ import assert from "node:assert/strict"; @@ -14,6 +14,9 @@ import { golden, StubBridge } from "./stub-bridge.js"; const BRIDGE_VERSION = "0.1.0-test"; const MATTER_JS_VERSION = "0.17.8"; +/** The paired ecosystem the populated golden statuses assume. */ +const APPLE_HOME = { fabricIndex: 1, label: "Apple Home", vendorId: 4937 }; + const bridge = new StubBridge(); const server = new BridgeWsServer({ port: 0, @@ -23,31 +26,54 @@ const server = new BridgeWsServer({ log: () => {}, }); +/** + * A second server on its own double, for the tests that need a *cold* endpoint + * set. The shared one above is deliberately kept warm at the + * `attach_with_endpoints` set — that is what makes `get_status` answer the + * golden populated StatusReport — so a test about an empty or emptied bridge + * cannot use it without wrecking the ones around it. + */ +async function withColdBridge( + run: (bridge: StubBridge, connect: () => Promise) => Promise, +): Promise { + const cold = new StubBridge(); + const coldServer = new BridgeWsServer({ + port: 0, + bridge: cold, + bridgeVersion: BRIDGE_VERSION, + matterJsVersion: MATTER_JS_VERSION, + log: () => {}, + }); + await coldServer.listen(); + try { + return await run(cold, async () => { + const client = await TestClient.connect(coldServer.port); + await client.next(); // handshake + return client; + }); + } finally { + await coldServer.close(); + } +} + async function connect(): Promise { const client = await TestClient.connect(server.port); await client.next(); // consume the handshake return client; } -async function attach(client: TestClient): Promise> { - return client.request(golden.attach.request); -} - /** - * What the E0 node answers an `attach` with. - * - * NOT `golden.attach.response`: that frame is the lawful §3.1 pair for a request - * carrying `endpoints: []` (empty desired set → empty live set), which is what - * the plugin asserts against. E0 does not reconcile the requested set at all — - * it serves one hard-coded endpoint and returns that status — so this is the - * live truth until E2 makes attach reconcile. Deliberate, and the fixture's - * _comment says so. + * The shared server's `attach` — the populated one, so the live endpoint set + * matches `golden.get_status`. §3.1 makes attach a full reconcile, and + * re-sending the same set is all-updates, so this is safe to call repeatedly. */ -function e0AttachResponse(messageId: string): Record { - return { message_id: messageId, result: golden.get_status.response.result }; +async function attach(client: TestClient): Promise> { + return client.request(golden.attach_with_endpoints.request); } before(async () => { + bridge.statusCommissioned = true; + bridge.statusFabrics = [APPLE_HOME]; await server.listen(); }); @@ -70,13 +96,101 @@ describe("handshake (§2)", () => { }); describe("attach (§3.1)", () => { - it("accepts a matching protocol version and returns a StatusReport", async () => { + it("accepts a matching protocol version and reconciles the endpoint set", async () => { const client = await connect(); const response = await attach(client); - assert.deepEqual(response, e0AttachResponse(golden.attach.request.message_id as string)); + assert.deepEqual(response, golden.attach_with_endpoints.response); client.close(); }); + it("answers an empty desired set with an empty live set", async () => { + // The golden `attach` pair: nothing live, nothing desired. The + // mass-removal guard cannot fire — there is nothing to remove. + await withColdBridge(async (_cold, connectCold) => { + const client = await connectCold(); + assert.deepEqual(await client.request(golden.attach.request), golden.attach.response); + client.close(); + }); + }); + + it("refuses an attach that would remove every live endpoint", async () => { + await withColdBridge(async (cold, connectCold) => { + cold.statusCommissioned = true; + cold.statusFabrics = [APPLE_HOME]; + const client = await connectCold(); + await client.request(golden.attach_with_endpoints.request); + + assert.deepEqual( + await client.request(golden.attach_mass_removal_refused.request), + golden.attach_mass_removal_refused.response, + ); + // §3.1 is a gate, not a rollback: the live set is untouched, and the + // client is still attached so it can retry with the intent. + assert.deepEqual( + await client.request(golden.get_status.request), + golden.get_status.response, + ); + client.close(); + }); + }); + + it("empties the live set when the client says intent: replace_all", async () => { + await withColdBridge(async (cold, connectCold) => { + cold.statusCommissioned = true; + cold.statusFabrics = [APPLE_HOME]; + const client = await connectCold(); + await client.request(golden.attach_with_endpoints.request); + + assert.deepEqual( + await client.request(golden.attach_replace_all.request), + golden.attach_replace_all.response, + ); + client.close(); + }); + }); + + it("rejects a malformed endpoint set without disturbing the live one", async () => { + await withColdBridge(async (cold, connectCold) => { + cold.statusCommissioned = true; + cold.statusFabrics = [APPLE_HOME]; + const client = await connectCold(); + await client.request(golden.attach_with_endpoints.request); + + for (const endpoints of [ + "not-an-array", + [{ role: "onOffLight", label: "x" }], // no indigoDeviceId + [{ indigoDeviceId: 1, role: "onOffLight" }], // no label + [{ indigoDeviceId: 1, role: 7, label: "x" }], + [ + { indigoDeviceId: 1, role: "onOffLight", label: "a" }, + { indigoDeviceId: 1, role: "onOffLight", label: "b" }, + ], + ]) { + const response = await client.request({ + message_id: "bad-set", + command: "attach", + args: { protocolVersion: PROTOCOL_VERSION, pluginVersion: "t", endpoints }, + }); + assert.equal(response.error_code, ErrorCode.malformedArgs, JSON.stringify(endpoints)); + } + + // §1.1: a lawful shape carrying a role outside §4.2 is its own code. + const unknownRole = await client.request({ + message_id: "bad-role", + command: "attach", + args: { + protocolVersion: PROTOCOL_VERSION, + pluginVersion: "t", + endpoints: [{ indigoDeviceId: 1, role: "airPurifier", label: "x" }], + }, + }); + assert.equal(unknownRole.error_code, ErrorCode.unknownRole); + + assert.deepEqual(await client.request(golden.get_status.request), golden.get_status.response); + client.close(); + }); + }); + it("rejects a mismatched protocolVersion and closes the socket", async () => { const client = await connect(); const response = await client.request(golden.attach_version_mismatch.request); @@ -118,7 +232,7 @@ describe("attach (§3.1)", () => { const client = await connect(); await attach(client); const again = await attach(client); - assert.deepEqual(again, e0AttachResponse(golden.attach.request.message_id as string)); + assert.deepEqual(again, golden.attach_with_endpoints.response); assert.equal(client.closed, false); const status = await client.request(golden.get_status.request); @@ -171,6 +285,106 @@ describe("gating (§1.1)", () => { }); }); +describe("endpoint CRUD (§3.2-§3.5)", () => { + /** + * One walk of the golden sequence on a cold bridge: attach the two-endpoint + * set, then every CRUD frame in the order that makes its payload true. They + * share a bridge because they *are* a sequence — `upsert_endpoint`'s + * `{endpointNumber: 2}` is only correct for the endpoint `attach` created, + * and `remove_endpoint` only returns `{removed: true}` once. + */ + it("answers every golden endpoint exchange verbatim", async () => { + await withColdBridge(async (cold, connectCold) => { + cold.statusCommissioned = true; + cold.statusFabrics = [APPLE_HOME]; + const client = await connectCold(); + await client.request(golden.attach_with_endpoints.request); + + // §3.2 idempotent update of a live endpoint, answering with its number. + assert.deepEqual(await client.request(golden.upsert_endpoint.request), golden.upsert_endpoint.response); + // §4.1: ecosystems cache device types, so a role change is refused. + assert.deepEqual( + await client.request(golden.upsert_endpoint_role_change.request), + golden.upsert_endpoint_role_change.response, + ); + // §3.4/§3.5 against a live device, and against one that is not. + assert.deepEqual(await client.request(golden.set_state.request), golden.set_state.response); + assert.deepEqual( + await client.request(golden.set_state_unknown_device.request), + golden.set_state_unknown_device.response, + ); + assert.deepEqual(await client.request(golden.set_reachable.request), golden.set_reachable.response); + assert.equal(cold.lastReachable, false); + // §3.3, then the same removal again — idempotent both ways. + assert.deepEqual(await client.request(golden.remove_endpoint.request), golden.remove_endpoint.response); + assert.deepEqual( + await client.request(golden.remove_endpoint_absent.request), + golden.remove_endpoint_absent.response, + ); + assert.deepEqual( + await client.request({ ...golden.remove_endpoint.request, message_id: "again" }), + { message_id: "again", result: { removed: false } }, + ); + client.close(); + }); + }); + + it("creates an absent endpoint on upsert and reports its new number", async () => { + await withColdBridge(async (_cold, connectCold) => { + const client = await connectCold(); + await client.request(golden.attach.request); // empty live set + const response = await client.request(golden.upsert_endpoint.request); + assert.deepEqual(response.result, { endpointNumber: 2 }); + client.close(); + }); + }); + + it("refuses malformed CRUD args (§1.1)", async () => { + await withColdBridge(async (_cold, connectCold) => { + const client = await connectCold(); + await client.request(golden.attach.request); + + const malformed: [string, Record][] = [ + ["upsert_endpoint", {}], + ["upsert_endpoint", { endpoint: 7 }], + ["remove_endpoint", {}], + ["remove_endpoint", { indigoDeviceId: "123" }], + ["set_state", { indigoDeviceId: 1 }], + ["set_state", { indigoDeviceId: 1, states: [] }], + ["set_reachable", { indigoDeviceId: 1 }], + ["set_reachable", { indigoDeviceId: 1, reachable: "no" }], + ]; + for (const [command, args] of malformed) { + const response = await client.request({ message_id: `bad-${command}`, command, args }); + assert.equal( + response.error_code, + ErrorCode.malformedArgs, + `${command} ${JSON.stringify(args)} was accepted`, + ); + } + client.close(); + }); + }); +}); + +describe("command event (§5)", () => { + it("forwards every §4.2 command payload to the attached client", async () => { + const client = await connect(); + await attach(client); + + for (const frame of [ + golden.command_on_off, + golden.command_set_level, + golden.command_set_color_temp, + golden.command_set_color, + ]) { + bridge.emitCommand(frame.data as never); + assert.deepEqual(await client.next(), frame); + } + client.close(); + }); +}); + describe("get_pairing (§3.7)", () => { it("reports the initial window as open with the persisted codes", async () => { bridge.commissioned = false; diff --git a/bridge-node/test/reconcile.test.ts b/bridge-node/test/reconcile.test.ts new file mode 100644 index 0000000..04a0dd8 --- /dev/null +++ b/bridge-node/test/reconcile.test.ts @@ -0,0 +1,168 @@ +/** + * §3.1-§3.5 argument parsing and the reconcile planner — the refusals that are + * decided before any Matter object is touched. + */ + +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; + +import { type EndpointSpec, ErrorCode, ProtocolError, Role, type RoleValue } from "../src/protocol.js"; +import { + parseDeviceId, + parseEndpointSpec, + parseEndpointSpecs, + parseReplaceAll, + planReconcile, +} from "../src/reconcile.js"; + +function spec(indigoDeviceId: number, role: RoleValue = Role.onOffLight, label = "L"): EndpointSpec { + return { indigoDeviceId, role, label, reachable: true, states: {}, options: {} }; +} + +function live(...entries: [number, RoleValue][]): Map { + return new Map(entries); +} + +function refusal(run: () => unknown): ProtocolError { + try { + run(); + } catch (error) { + assert.ok(error instanceof ProtocolError, `expected a ProtocolError, got ${String(error)}`); + return error; + } + throw new Error("expected a refusal"); +} + +describe("parseEndpointSpec (§4.1)", () => { + it("accepts the golden shape and defaults the optional fields", () => { + assert.deepEqual(parseEndpointSpec({ indigoDeviceId: 7, role: "onOffLight", label: "Lamp" }), { + indigoDeviceId: 7, + role: "onOffLight", + label: "Lamp", + // An absent `reachable` means "nothing said", not "unavailable" — + // defaulting to false would grey out every accessory on a client + // that simply omits the field. + reachable: true, + states: {}, + options: {}, + }); + }); + + it("rejects a role outside §4.2 with unknown_role, not malformed_args", () => { + const error = refusal(() => parseEndpointSpec({ indigoDeviceId: 1, role: "airPurifier", label: "x" })); + assert.equal(error.code, ErrorCode.unknownRole); + assert.equal(error.message, "role airPurifier is not in the v1 role enum (§4.2)"); + }); + + it("does not accept an Object.prototype key as a role", () => { + // `role in Role` would say yes to "constructor"; membership has to be an + // own-property check or the enum has a hole in it. + assert.equal(refusal(() => parseEndpointSpec({ indigoDeviceId: 1, role: "constructor", label: "x" })).code, + ErrorCode.unknownRole); + }); + + for (const [name, value] of [ + ["a non-object", 7], + ["a missing id", { role: "onOffLight", label: "x" }], + ["a fractional id", { indigoDeviceId: 1.5, role: "onOffLight", label: "x" }], + ["a non-string role", { indigoDeviceId: 1, role: 7, label: "x" }], + ["a missing label", { indigoDeviceId: 1, role: "onOffLight" }], + ["a non-boolean reachable", { indigoDeviceId: 1, role: "onOffLight", label: "x", reachable: 1 }], + ["array states", { indigoDeviceId: 1, role: "onOffLight", label: "x", states: [] }], + ] as [string, unknown][]) { + it(`rejects ${name} as malformed_args`, () => { + assert.equal(refusal(() => parseEndpointSpec(value)).code, ErrorCode.malformedArgs); + }); + } +}); + +describe("parseEndpointSpecs / parseReplaceAll (§3.1)", () => { + it("treats an absent endpoints array as an empty desired set", () => { + assert.deepEqual(parseEndpointSpecs(undefined), []); + }); + + it("rejects a duplicated indigoDeviceId", () => { + const error = refusal(() => parseEndpointSpecs([{ indigoDeviceId: 1, role: "onOffLight", label: "a" }, + { indigoDeviceId: 1, role: "onOffLight", label: "b" }])); + assert.equal(error.code, ErrorCode.malformedArgs); + assert.match(error.message, /twice/); + }); + + it("only honours the exact replace_all literal", () => { + assert.equal(parseReplaceAll(undefined), false); + assert.equal(parseReplaceAll("replace_all"), true); + assert.equal(parseReplaceAll("REPLACE_ALL"), false); + assert.equal(parseReplaceAll("replace-all"), false); + assert.equal(refusal(() => parseReplaceAll(true)).code, ErrorCode.malformedArgs); + }); + + it("rejects a non-array endpoints", () => { + assert.equal(refusal(() => parseEndpointSpecs("nope")).code, ErrorCode.malformedArgs); + }); +}); + +describe("parseDeviceId", () => { + it("takes an integer and nothing else", () => { + assert.equal(parseDeviceId(123456789), 123456789); + for (const bad of ["1", 1.5, null, undefined, {}]) { + assert.equal(refusal(() => parseDeviceId(bad)).code, ErrorCode.malformedArgs); + } + }); +}); + +describe("planReconcile (§3.1)", () => { + it("creates everything against an empty live set", () => { + const plan = planReconcile(live(), [spec(1), spec(2)], false); + assert.deepEqual(plan.create.map(s => s.indigoDeviceId), [1, 2]); + assert.deepEqual([plan.update, plan.recreate, plan.remove], [[], [], []]); + }); + + it("splits an incremental change into create / update / remove", () => { + const plan = planReconcile(live([1, Role.onOffLight], [2, Role.onOffLight]), [spec(1), spec(3)], false); + assert.deepEqual(plan.create.map(s => s.indigoDeviceId), [3]); + assert.deepEqual(plan.update.map(s => s.indigoDeviceId), [1]); + assert.deepEqual(plan.remove, [2]); + }); + + it("buckets a role change as a recreate rather than an update", () => { + const plan = planReconcile(live([1, Role.onOffLight]), [spec(1, Role.dimmableLight)], false); + assert.deepEqual(plan.recreate.map(s => s.indigoDeviceId), [1]); + assert.deepEqual(plan.update, []); + }); + + it("does not count a recreate as a removal for the guard", () => { + // The device stays exported; refusing this would make a single-export + // bridge unable to correct a role at all. + const plan = planReconcile(live([1, Role.onOffLight]), [spec(1, Role.dimmableLight)], false); + assert.deepEqual(plan.remove, []); + }); + + it("refuses to empty a non-empty live set without the intent", () => { + const error = refusal(() => planReconcile(live([1, Role.onOffLight], [2, Role.onOffLight]), [], false)); + assert.equal(error.code, ErrorCode.massRemovalRefused); + assert.equal(error.message, "attach would remove all 2 live endpoints without intent: replace_all"); + }); + + it("refuses a wholly disjoint desired set too, not just an empty one", () => { + // §3.1 is about the effect: swapping every device out removes every + // accessory from every paired ecosystem just as surely as `[]` does. + assert.equal( + refusal(() => planReconcile(live([1, Role.onOffLight]), [spec(9)], false)).code, + ErrorCode.massRemovalRefused, + ); + }); + + it("allows the emptying once intent: replace_all is present", () => { + const plan = planReconcile(live([1, Role.onOffLight], [2, Role.onOffLight]), [], true); + assert.deepEqual(plan.remove, [1, 2]); + }); + + it("does not fire the guard when nothing was live", () => { + assert.deepEqual(planReconcile(live(), [], false), { create: [], update: [], recreate: [], remove: [] }); + }); + + it("does not fire when one endpoint survives", () => { + const plan = planReconcile(live([1, Role.onOffLight], [2, Role.onOffLight]), [spec(1)], false); + assert.deepEqual(plan.remove, [2]); + }); +}); diff --git a/bridge-node/test/registry.test.ts b/bridge-node/test/registry.test.ts new file mode 100644 index 0000000..cfdf661 --- /dev/null +++ b/bridge-node/test/registry.test.ts @@ -0,0 +1,624 @@ +/** + * Endpoint CRUD against a **real, un-commissioned** matter.js ServerNode. + * + * The protocol tests deliberately stub the Matter stack out; these do not, + * because the things E3 has to get right — that each role's device type builds + * at all, that `Endpoint.id` fixes the number, that a local `set_state` write + * arrives in an offline context — are all properties of matter.js 0.17.8 and of + * nothing we wrote. A double would only ever confirm our own assumptions. + * + * Each node gets its own {@link Environment} and its own scratch storage + * directory: matter.js takes an exclusive lock per storage path, and + * `Environment.default` is a process-wide singleton whose `storage.path` the + * nodes would otherwise fight over. + */ + +import assert from "node:assert/strict"; +import { mkdirSync, mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { after, describe, it } from "node:test"; + +import { Endpoint, Environment, Logger, ServerNode, VendorId } from "@matter/main"; +import { BasicInformationServer } from "@matter/main/behaviors/basic-information"; +import { AggregatorEndpoint } from "@matter/main/endpoints/aggregator"; + +import { endpointIdFor, uniqueIdFor } from "../src/endpoints.js"; +import { + type CommandEventData, + type EndpointSpec, + ErrorCode, + ProtocolError, + Role, + type RoleValue, +} from "../src/protocol.js"; +import { EndpointRegistry } from "../src/registry.js"; + +const PRODUCT_NAME = "Indigo Matter Bridge"; +const scratchRoots: string[] = []; + +/** + * Scratch storage root. Honour the harness-provided scratchpad when the runner + * sets one; otherwise a temp directory, so `npm test` works anywhere. + */ +const SCRATCH_ROOT = process.env.INDIGO_MATTER_TEST_SCRATCH ?? tmpdir(); +mkdirSync(SCRATCH_ROOT, { recursive: true }); + +// matter.js logs a screenful per endpoint at its default level, which buries the +// TAP output. Fatal keeps a genuine stack visible without the narration. +Logger.level = "fatal"; + +after(() => { + for (const root of scratchRoots) { + rmSync(root, { recursive: true, force: true }); + } +}); + +interface Harness { + registry: EndpointRegistry; + aggregator: Endpoint; + node: ServerNode; + commands: CommandEventData[]; + logs: string[]; + close: () => Promise; +} + +/** + * Build a live node. The Matter port is 0 — matter.js binds an ephemeral one, + * so a parallel run cannot collide — and the node is never started, because + * nothing here needs the network: endpoint numbers, state and change events are + * all assigned at `add()` time. + */ +async function harness(options: { storagePath?: string; removalPacingMs?: number } = {}): Promise { + const storagePath = options.storagePath ?? mkdtempSync(join(SCRATCH_ROOT, "indigo-matter-registry-")); + if (options.storagePath === undefined) { + scratchRoots.push(storagePath); + } + + const environment = new Environment("test", Environment.default); + environment.vars.set("storage.path", storagePath); + environment.vars.set("runtime.signals", false); + + const node = await ServerNode.create({ + id: "indigo-matter-bridge", + environment, + network: { port: 0 }, + commissioning: { passcode: 20202021, discriminator: 3840 }, + productDescription: { name: PRODUCT_NAME, deviceType: AggregatorEndpoint.deviceType }, + basicInformation: { + vendorName: "simons-plugins", + vendorId: VendorId(0xfff1), + productName: PRODUCT_NAME, + productId: 0x8000, + serialNumber: "testserial000001", + uniqueId: "testuniqueid00000000000000000001", + }, + }); + const aggregator = new Endpoint(AggregatorEndpoint, { id: "aggregator" }); + await node.add(aggregator); + + const commands: CommandEventData[] = []; + const logs: string[] = []; + const registry = new EndpointRegistry({ + aggregator, + productName: PRODUCT_NAME, + emit: data => commands.push(data), + log: message => logs.push(message), + removalPacingMs: options.removalPacingMs ?? 0, + // The real wiring from node.ts, not a counter: the point of testing it + // here is that matter.js accepts the call at all. + onConfigurationChange: async () => { + await node.act(agent => agent.get(BasicInformationServer).increaseConfigurationVersion()); + }, + }); + + return { + registry, + aggregator, + node, + commands, + logs, + close: async () => { + registry.close(); + await node.close(); + }, + }; +} + +function spec(indigoDeviceId: number, role: RoleValue, overrides: Partial = {}): EndpointSpec { + return { + indigoDeviceId, + role, + label: `Device ${indigoDeviceId}`, + reachable: true, + states: {}, + options: {}, + ...overrides, + }; +} + +async function rejects(run: () => Promise, code: string): Promise { + try { + await run(); + } catch (error) { + assert.ok(error instanceof ProtocolError, `expected a ProtocolError, got ${String(error)}`); + assert.equal(error.code, code); + return error; + } + throw new Error(`expected a ${code} refusal`); +} + +describe("role factory", () => { + it("builds every E3 role with bridged info and its own clusters", async () => { + const h = await harness(); + try { + const roles: RoleValue[] = [ + Role.onOffPlugInUnit, + Role.onOffLight, + Role.dimmableLight, + Role.colorTemperatureLight, + Role.extendedColorLight, + ]; + await h.registry.reconcile( + roles.map((role, index) => spec(900_001 + index, role, { label: `L${index}` })), + false, + ); + + const byRole = new Map(h.registry.summaries().map(s => [s.role, s])); + assert.deepEqual([...byRole.keys()].sort(), [...roles].sort()); + + for (const [index, role] of roles.entries()) { + const id = 900_001 + index; + const endpoint = [...h.aggregator.parts].find(part => part.id === endpointIdFor(id)); + assert.ok(endpoint !== undefined, `${role} endpoint missing`); + + const info = endpoint.stateOf("bridgedDeviceBasicInformation") as Record; + assert.equal(info.nodeLabel, `L${index}`); + assert.equal(info.uniqueId, uniqueIdFor(id)); + assert.equal(info.reachable, true); + // Matter rejects a UniqueID equal to the SerialNumber. + assert.notEqual(info.uniqueId, info.serialNumber); + // ConfigurationVersion must be seeded or matter.js refuses to + // increment it later. + assert.equal(info.configurationVersion, 1); + + const behaviors = Object.keys(endpoint.state); + assert.ok(behaviors.includes("onOff"), `${role} has no OnOff`); + const needsLevel = role !== Role.onOffPlugInUnit && role !== Role.onOffLight; + assert.equal(behaviors.includes("levelControl"), needsLevel, `${role} levelControl`); + const needsColor = role === Role.colorTemperatureLight || role === Role.extendedColorLight; + assert.equal(behaviors.includes("colorControl"), needsColor, `${role} colorControl`); + } + } finally { + await h.close(); + } + }); + + it("gives extendedColorLight the HueSaturation feature matter.js omits", async () => { + // matter.js 0.17.8 builds ExtendedColorLightDevice with Xy+ColorTemperature + // only, so without the override §4.2's hue/saturation vocabulary would + // have no attributes to write. + const h = await harness(); + try { + await h.registry.reconcile([spec(1, Role.extendedColorLight)], false); + const endpoint = [...h.aggregator.parts][0]; + assert.ok(endpoint !== undefined); + const features = endpoint.featuresOf("colorControl") as Record; + assert.equal(features.hueSaturation, true); + assert.equal(features.colorTemperature, true); + const color = endpoint.stateOf("colorControl") as Record; + assert.equal(typeof color.currentHue, "number"); + assert.equal(typeof color.currentSaturation, "number"); + } finally { + await h.close(); + } + }); + + it("builds a role whose spec already carries its full initial state", async () => { + // A plain object spread would replace the whole colorControl patch here, + // dropping the mandatory attributes matter.js needs, and the endpoint + // would fail construction with a bare "Behaviors have errors". Every + // other test in this file passes `states: {}`, which is why this one + // exists: the failure only shows when a spec supplies colour state. + const h = await harness(); + try { + await h.registry.reconcile( + [ + spec(1, Role.dimmableLight, { states: { onOff: true, level: 60 } }), + spec(2, Role.colorTemperatureLight, { states: { onOff: true, level: 30, colorTempMireds: 250 } }), + spec(3, Role.extendedColorLight, { + states: { onOff: true, level: 100, colorTempMireds: 320, hue: 210, saturation: 80 }, + }), + ], + false, + ); + assert.equal(h.registry.size, 3); + + const ext = [...h.aggregator.parts].find(part => part.id === endpointIdFor(3)); + const color = ext?.stateOf("colorControl") as Record; + assert.equal(color.currentHue, 148); + assert.equal(color.currentSaturation, 203); + assert.equal(color.colorTempPhysicalMinMireds, 153); + assert.equal(color.colorTempPhysicalMaxMireds, 500); + assert.equal((ext?.stateOf("levelControl") as Record).currentLevel, 254); + } finally { + await h.close(); + } + }); + + it("refuses a lawful §4.2 role this bridge version cannot build", async () => { + const h = await harness(); + try { + const error = await rejects( + () => h.registry.reconcile([spec(1, Role.doorLock)], false), + ErrorCode.internal, + ); + assert.match(error.message, /not implemented by this bridge version/); + // Nothing partially applied: the refusal is a gate. + assert.equal(h.registry.size, 0); + } finally { + await h.close(); + } + }); +}); + +describe("reconcile (§3.1)", () => { + it("creates, updates and removes in one pass", async () => { + const h = await harness(); + try { + await h.registry.reconcile([spec(1, Role.onOffLight), spec(2, Role.onOffLight)], false); + assert.deepEqual(h.registry.summaries().map(s => [s.indigoDeviceId, s.endpointNumber]), [ + [1, 2], + [2, 3], + ]); + + await h.registry.reconcile( + [spec(1, Role.onOffLight, { label: "Renamed", reachable: false }), spec(3, Role.dimmableLight)], + false, + ); + assert.deepEqual(h.registry.summaries().map(s => s.indigoDeviceId), [1, 3]); + + const renamed = [...h.aggregator.parts].find(part => part.id === endpointIdFor(1)); + const info = renamed?.stateOf("bridgedDeviceBasicInformation") as Record; + assert.equal(info.nodeLabel, "Renamed"); + assert.equal(info.reachable, false); + } finally { + await h.close(); + } + }); + + it("refuses to empty a non-empty live set, and changes nothing when it does", async () => { + const h = await harness(); + try { + await h.registry.reconcile([spec(1, Role.onOffLight), spec(2, Role.onOffLight)], false); + const error = await rejects(() => h.registry.reconcile([], false), ErrorCode.massRemovalRefused); + assert.equal(error.message, "attach would remove all 2 live endpoints without intent: replace_all"); + assert.equal(h.registry.size, 2); + assert.equal([...h.aggregator.parts].length, 2); + } finally { + await h.close(); + } + }); + + it("empties it when the client asks for replace_all", async () => { + const h = await harness(); + try { + await h.registry.reconcile([spec(1, Role.onOffLight), spec(2, Role.onOffLight)], false); + await h.registry.reconcile([], true); + assert.equal(h.registry.size, 0); + assert.equal([...h.aggregator.parts].length, 0); + } finally { + await h.close(); + } + }); + + it("recreates an endpoint whose role changed", async () => { + const h = await harness(); + try { + await h.registry.reconcile([spec(1, Role.onOffLight), spec(2, Role.onOffLight)], false); + await h.registry.reconcile([spec(1, Role.dimmableLight), spec(2, Role.onOffLight)], false); + + assert.deepEqual(h.registry.summaries().map(s => [s.indigoDeviceId, s.role]), [ + [1, Role.dimmableLight], + [2, Role.onOffLight], + ]); + assert.ok(h.logs.some(line => line.includes("Recreating endpoint 1"))); + // The id is the identity, so the recreated endpoint keeps its number. + assert.equal(h.registry.summaries()[0]?.endpointNumber, 2); + } finally { + await h.close(); + } + }); + + it("paces bulk removals, and the pacing is injectable", async () => { + const h = await harness({ removalPacingMs: 20 }); + try { + await h.registry.reconcile([1, 2, 3].map(id => spec(id, Role.onOffLight)), false); + const started = Date.now(); + await h.registry.reconcile([], true); + // Three removals, two gaps. Asserted as a floor only: the point is + // that the delay exists and is the injected one, not its precision. + assert.ok(Date.now() - started >= 30, `bulk removal took ${Date.now() - started}ms`); + } finally { + await h.close(); + } + }); +}); + +describe("upsert / remove (§3.2, §3.3)", () => { + it("creates when absent and returns the live endpoint number", async () => { + const h = await harness(); + try { + assert.deepEqual(await h.registry.upsert(spec(1, Role.onOffLight)), { endpointNumber: 2 }); + assert.deepEqual(await h.registry.upsert(spec(1, Role.onOffLight, { label: "Again" })), { + endpointNumber: 2, + }); + } finally { + await h.close(); + } + }); + + it("rejects a role change with role_change (§4.1)", async () => { + const h = await harness(); + try { + await h.registry.upsert(spec(123456789, Role.onOffLight)); + const error = await rejects( + () => h.registry.upsert(spec(123456789, Role.dimmableLight)), + ErrorCode.roleChange, + ); + assert.equal(error.message, "endpoint 123456789 is onOffLight; remove and re-add to change role"); + // Unchanged: the refusal must not half-apply the new role. + assert.equal(h.registry.summaries()[0]?.role, Role.onOffLight); + } finally { + await h.close(); + } + }); + + it("removes idempotently", async () => { + const h = await harness(); + try { + await h.registry.upsert(spec(1, Role.onOffLight)); + assert.deepEqual(await h.registry.remove(1), { removed: true }); + assert.deepEqual(await h.registry.remove(1), { removed: false }); + assert.deepEqual(await h.registry.remove(999), { removed: false }); + } finally { + await h.close(); + } + }); + + it("reports unknown_device for set_state and set_reachable", async () => { + const h = await harness(); + try { + const error = await rejects(() => h.registry.setState(123456791, { onOff: true }), ErrorCode.unknownDevice); + assert.equal(error.message, "no live endpoint for indigoDeviceId 123456791"); + await rejects(() => h.registry.setReachable(123456791, false), ErrorCode.unknownDevice); + } finally { + await h.close(); + } + }); +}); + +describe("set_state (§3.4)", () => { + it("writes each role's state keys through its converter", async () => { + const h = await harness(); + try { + await h.registry.reconcile( + [spec(1, Role.dimmableLight), spec(2, Role.extendedColorLight)], + false, + ); + await h.registry.setState(1, { onOff: true, level: 60 }); + await h.registry.setState(2, { onOff: true, level: 100, colorTempMireds: 9999, hue: 210, saturation: 80 }); + + const dim = [...h.aggregator.parts].find(part => part.id === endpointIdFor(1)); + assert.equal((dim?.stateOf("onOff") as Record).onOff, true); + // 60% → round(60 × 254 / 100) = 152. + assert.equal((dim?.stateOf("levelControl") as Record).currentLevel, 152); + + const ext = [...h.aggregator.parts].find(part => part.id === endpointIdFor(2)); + const color = ext?.stateOf("colorControl") as Record; + assert.equal(color.colorTemperatureMireds, 500, "mireds should clamp to the physical maximum"); + assert.equal(color.currentHue, 148); // 210° → round(210 × 254 / 360) + assert.equal(color.currentSaturation, 203); // 80% → round(80 × 254 / 100) + // Hue and saturation win over a colour temperature sent alongside. + assert.equal(color.colorMode, 0); + } finally { + await h.close(); + } + }); + + it("writes 0% as currentLevel 1, which reads back as 0%", async () => { + const h = await harness(); + try { + await h.registry.reconcile([spec(1, Role.dimmableLight, { states: { level: 100 } })], false); + await h.registry.setState(1, { level: 0 }); + const dim = [...h.aggregator.parts][0]; + assert.equal((dim?.stateOf("levelControl") as Record).currentLevel, 1); + } finally { + await h.close(); + } + }); + + it("ignores state keys outside the role's vocabulary", async () => { + const h = await harness(); + try { + await h.registry.reconcile([spec(1, Role.onOffLight)], false); + // An onOffLight has no LevelControl; a stray `level` must be a no-op, + // not a crash — the plugin and node version-skew across a release. + await h.registry.setState(1, { level: 50, occupied: true }); + assert.deepEqual(h.commands, []); + } finally { + await h.close(); + } + }); + + it("applies §3.5 reachability to Bridged Device Basic Information", async () => { + const h = await harness(); + try { + await h.registry.reconcile([spec(1, Role.onOffLight)], false); + await h.registry.setReachable(1, false); + const endpoint = [...h.aggregator.parts][0]; + const info = endpoint?.stateOf("bridgedDeviceBasicInformation") as Record; + assert.equal(info.reachable, false); + } finally { + await h.close(); + } + }); +}); + +/** A matter.js `$Changed` observable, as much of it as these tests drive. */ +interface ChangeObservable { + emit(value: unknown, oldValue: unknown, context: unknown): void; +} + +/** + * Stand in for a controller writing an attribute. + * + * A real remote write needs a commissioned fabric and a session, which is a + * disproportionate amount of machinery for what is being checked: that our + * handler is on the *real* observable and reads the context matter.js passes + * it. Emitting on that observable with a non-offline context exercises exactly + * that path — `RemoteActorContext` declares `offline?: false`. + */ +function ecosystemWrite( + endpoint: Endpoint, + behavior: string, + attribute: string, + value: unknown, + previous: unknown, +): void { + const events = endpoint.eventsOf(behavior) as unknown as Record; + const observable = events[`${attribute}$Changed`]; + assert.ok(observable !== undefined, `${behavior}.${attribute}$Changed does not exist`); + observable.emit(value, previous, { offline: false }); +} + +describe("command events and the echo guard (§4.2, §6.4)", () => { + it("emits no command event for our own set_state writes", async () => { + const h = await harness(); + try { + await h.registry.reconcile([spec(1, Role.extendedColorLight)], false); + await h.registry.setState(1, { onOff: true, level: 60, colorTempMireds: 320 }); + await h.registry.setState(1, { hue: 210, saturation: 80 }); + await h.registry.setState(1, { onOff: false }); + assert.deepEqual(h.commands, [], "a local write echoed back as a command event"); + } finally { + await h.close(); + } + }); + + it("emits the §4.2 payload for each attribute an ecosystem changes", async () => { + const h = await harness(); + try { + await h.registry.reconcile([spec(7, Role.extendedColorLight)], false); + const endpoint = [...h.aggregator.parts][0]; + assert.ok(endpoint !== undefined); + // Seed hue/saturation locally so the paired reads have known values. + await h.registry.setState(7, { hue: 210, saturation: 80 }); + h.commands.length = 0; + + ecosystemWrite(endpoint, "onOff", "onOff", true, false); + ecosystemWrite(endpoint, "levelControl", "currentLevel", 152, 1); + ecosystemWrite(endpoint, "colorControl", "colorTemperatureMireds", 320, 153); + ecosystemWrite(endpoint, "colorControl", "currentHue", 148, 0); + + assert.deepEqual(h.commands, [ + { indigoDeviceId: 7, command: "onOff", args: { value: true } }, + { indigoDeviceId: 7, command: "setLevel", args: { level: 60 } }, + { indigoDeviceId: 7, command: "setColorTemp", args: { colorTempMireds: 320 } }, + // setColor carries both halves; the saturation comes from state. + { indigoDeviceId: 7, command: "setColor", args: { hue: 210, saturation: 80 } }, + ]); + } finally { + await h.close(); + } + }); + + it("stops listening once the endpoint is removed", async () => { + const h = await harness(); + try { + await h.registry.reconcile([spec(1, Role.onOffLight)], false); + const endpoint = [...h.aggregator.parts][0]; + assert.ok(endpoint !== undefined); + + ecosystemWrite(endpoint, "onOff", "onOff", true, false); + assert.equal(h.commands.length, 1, "the listener should be live before removal"); + + await h.registry.remove(1); + assert.equal(h.registry.size, 0); + + // The endpoint object survives in this test's hand; the listener must + // not, or a removed export would keep talking to the plugin. + ecosystemWrite(endpoint, "onOff", "onOff", false, true); + assert.equal(h.commands.length, 1, "a removed endpoint still emitted"); + } finally { + await h.close(); + } + }); + + it("stops listening when the registry closes", async () => { + const h = await harness(); + try { + await h.registry.reconcile([spec(1, Role.onOffLight)], false); + const endpoint = [...h.aggregator.parts][0]; + assert.ok(endpoint !== undefined); + h.registry.close(); + ecosystemWrite(endpoint, "onOff", "onOff", true, false); + assert.deepEqual(h.commands, []); + } finally { + await h.close(); + } + }); +}); + +describe("ConfigurationVersion (PRD §5.3)", () => { + it("bumps the bridge's version when the endpoint set changes, and not otherwise", async () => { + const h = await harness(); + const version = (): number => + (h.node.state.basicInformation as { configurationVersion?: number }).configurationVersion ?? 0; + try { + const start = version(); + await h.registry.reconcile([spec(1, Role.onOffLight)], false); + assert.equal(version(), start + 1, "an added endpoint is a configuration change"); + + // A label/state update is not a configuration change: the bridged + // node set is the same, and controllers do not need to re-read it. + await h.registry.reconcile([spec(1, Role.onOffLight, { label: "Renamed" })], false); + assert.equal(version(), start + 1); + + await h.registry.remove(1); + assert.equal(version(), start + 2, "a removed endpoint is a configuration change"); + } finally { + await h.close(); + } + }); +}); + +describe("endpoint-number stability (PRD §4.3 / XAC5)", () => { + it("gives an id the same number across a node restart", async () => { + const storagePath = mkdtempSync(join(SCRATCH_ROOT, "indigo-matter-restart-")); + scratchRoots.push(storagePath); + + const first = await harness({ storagePath }); + let before: [number, number][]; + try { + await first.registry.reconcile([1, 2, 3].map(id => spec(id, Role.onOffLight)), false); + before = first.registry.summaries().map(s => [s.indigoDeviceId, s.endpointNumber]); + } finally { + await first.close(); + } + + const second = await harness({ storagePath }); + try { + // Deliberately a different creation order: if numbers came from + // iteration order rather than from the persisted id map, this is + // where the accessories would swap identities. + await second.registry.reconcile([3, 1, 2].map(id => spec(id, Role.onOffLight)), false); + assert.deepEqual( + second.registry.summaries().map(s => [s.indigoDeviceId, s.endpointNumber]), + before, + ); + } finally { + await second.close(); + } + }); +}); diff --git a/bridge-node/test/stub-bridge.ts b/bridge-node/test/stub-bridge.ts index eb1b4e8..4c6c7a4 100644 --- a/bridge-node/test/stub-bridge.ts +++ b/bridge-node/test/stub-bridge.ts @@ -9,13 +9,22 @@ import { readFileSync } from "node:fs"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; -import type { - BridgeFacade, - CommissioningWindowResult, - PairingReport, - StatusReport, - WindowClosedReason, +import { + type BridgeFacade, + type CommandEventData, + type CommissioningWindowResult, + type EndpointSpec, + ErrorCode, + type FabricInfo, + type PairingReport, + ProtocolError, + type RemoveResult, + type RoleValue, + type StatusReport, + type UpsertResult, + type WindowClosedReason, } from "../src/protocol.js"; +import { planReconcile } from "../src/reconcile.js"; const here = dirname(fileURLToPath(import.meta.url)); @@ -47,7 +56,21 @@ export interface GoldenFrames { open_window_internal: GoldenExchange; window_closed_expired: GoldenEvent; window_closed_commissioned: GoldenEvent; - /** §3.1-§3.11 exchanges awaiting node-side handlers — skipped by this suite. */ + command_on_off: GoldenEvent; + command_set_level: GoldenEvent; + command_set_color_temp: GoldenEvent; + command_set_color: GoldenEvent; + attach_with_endpoints: GoldenExchange; + attach_replace_all: GoldenExchange; + attach_mass_removal_refused: GoldenExchange; + upsert_endpoint: GoldenExchange; + upsert_endpoint_role_change: GoldenExchange; + remove_endpoint: GoldenExchange; + remove_endpoint_absent: GoldenExchange; + set_state: GoldenExchange; + set_state_unknown_device: GoldenExchange; + set_reachable: GoldenExchange; + /** §3.9-§3.11 and the E4 roles — exchanges awaiting node-side handlers. */ pending: Record; /** * §5 event frames the node does not emit yet — they exist so the plugin can @@ -67,10 +90,93 @@ export const golden: GoldenFrames = JSON.parse( readFileSync(join(here, "fixtures", "bridge_protocol", "frames.json"), "utf8"), ) as GoldenFrames; +/** + * The endpoint set the double models, so `attach`/`upsert`/`remove` answer with + * something a real reconcile could have produced rather than a canned blob. + * + * It reuses the production planner (`planReconcile`) on purpose: the §3.1 + * mass-removal guard and the §4.1 role rules are decided once, in `src/`, and + * the double inherits them. What it fakes is only the Matter part — endpoint + * numbers are handed out in creation order from 2, exactly as matter.js does + * for a freshly built aggregator. + */ +class EndpointModel { + readonly #endpoints = new Map(); + #next = 2; + + roles(): Map { + return new Map([...this.#endpoints.entries()].map(([id, entry]) => [id, entry.role])); + } + + summaries(): StatusReport["endpoints"] { + return [...this.#endpoints.entries()] + .map(([indigoDeviceId, entry]) => ({ indigoDeviceId, ...entry })) + .sort((a, b) => a.indigoDeviceId - b.indigoDeviceId); + } + + has(indigoDeviceId: number): boolean { + return this.#endpoints.has(indigoDeviceId); + } + + reconcile(desired: readonly EndpointSpec[], replaceAll: boolean): void { + const plan = planReconcile(this.roles(), desired, replaceAll); + for (const indigoDeviceId of plan.remove) { + this.#endpoints.delete(indigoDeviceId); + } + for (const spec of [...plan.create, ...plan.recreate]) { + this.#endpoints.delete(spec.indigoDeviceId); + this.add(spec); + } + } + + upsert(spec: EndpointSpec): UpsertResult { + const existing = this.#endpoints.get(spec.indigoDeviceId); + if (existing === undefined) { + return { endpointNumber: this.add(spec) }; + } + if (existing.role !== spec.role) { + throw new ProtocolError( + ErrorCode.roleChange, + `endpoint ${spec.indigoDeviceId} is ${existing.role}; remove and re-add to change role`, + ); + } + return { endpointNumber: existing.endpointNumber }; + } + + remove(indigoDeviceId: number): RemoveResult { + return { removed: this.#endpoints.delete(indigoDeviceId) }; + } + + require(indigoDeviceId: number): void { + if (!this.#endpoints.has(indigoDeviceId)) { + throw new ProtocolError( + ErrorCode.unknownDevice, + `no live endpoint for indigoDeviceId ${indigoDeviceId}`, + ); + } + } + + private add(spec: EndpointSpec): number { + const endpointNumber = this.#next++; + this.#endpoints.set(spec.indigoDeviceId, { role: spec.role, endpointNumber }); + return endpointNumber; + } +} + export class StubBridge implements BridgeFacade { commissioned = false; /** Only meaningful while {@link commissioned}: selects the 3rd §3.7 state. */ windowOpen = false; + /** + * The commissioning half of the §4.3 StatusReport. Separate from + * {@link commissioned}, which the §3.7 pairing tests toggle: a test that is + * about pairing states must not silently rewrite what `get_status` answers. + */ + statusCommissioned = false; + statusFabrics: FabricInfo[] = []; + readonly model = new EndpointModel(); + /** Every §5 `command` the double was asked to emit — for the event tests. */ + readonly commands: CommandEventData[] = []; openWindowError?: Error; /** * Makes `openCommissioningWindow` genuinely slow, for the ordering test. @@ -81,9 +187,53 @@ export class StubBridge implements BridgeFacade { delayOpenWindowMs = 0; readonly openWindowCalls: number[] = []; #windowClosed?: (reason: WindowClosedReason) => void; + #command?: (data: CommandEventData) => void; getStatus(): StatusReport { - return structuredClone(golden.get_status.response.result) as StatusReport; + const endpoints = this.model.summaries(); + return { + commissioned: this.statusCommissioned, + fabrics: structuredClone(this.statusFabrics), + endpointCount: endpoints.length, + endpoints, + drift: [], + }; + } + + async reconcile(endpoints: readonly EndpointSpec[], replaceAll: boolean): Promise { + this.model.reconcile(endpoints, replaceAll); + return this.getStatus(); + } + + async upsertEndpoint(spec: EndpointSpec): Promise { + return this.model.upsert(spec); + } + + async removeEndpoint(indigoDeviceId: number): Promise { + return this.model.remove(indigoDeviceId); + } + + async setState(indigoDeviceId: number, states: Record): Promise { + this.model.require(indigoDeviceId); + this.lastStates = states; + } + + async setReachable(indigoDeviceId: number, reachable: boolean): Promise { + this.model.require(indigoDeviceId); + this.lastReachable = reachable; + } + + lastStates?: Record; + lastReachable?: boolean; + + onCommand(listener: (data: CommandEventData) => void): void { + this.#command = listener; + } + + /** Stand in for an ecosystem acting on an endpoint. */ + emitCommand(data: CommandEventData): void { + this.commands.push(data); + this.#command?.(data); } getPairing(): PairingReport { diff --git a/bridge-node/test/units.test.ts b/bridge-node/test/units.test.ts new file mode 100644 index 0000000..d461c3e --- /dev/null +++ b/bridge-node/test/units.test.ts @@ -0,0 +1,141 @@ +/** + * §4.2 unit conversions and the echo-guard predicate. + * + * Kept separate from `endpoints.test.ts` (which builds real Matter endpoints) + * because these are pure arithmetic: the round-trip properties are the contract + * the plugin relies on, and they should fail loudly without a Matter stack in + * the way. + */ + +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; + +import { + clampMireds, + degreesToMatterHue, + HUE_DEGREES_MAX, + isEcosystemChange, + MATTER_LEVEL_MAX, + matterHueToDegrees, + matterToPercent, + MIREDS_MAX, + MIREDS_MIN, + percentToCurrentLevel, + percentToMatter, +} from "../src/endpoints.js"; + +/** 0..100 inclusive — the whole §4.2 percent domain, not a sample of it. */ +const ALL_PERCENTS = Array.from({ length: 101 }, (_, index) => index); +const ALL_MATTER_LEVELS = Array.from({ length: MATTER_LEVEL_MAX + 1 }, (_, index) => index); + +describe("level and saturation conversion (§4.2)", () => { + it("round-trips every one of the 101 percentages exactly", () => { + for (const percent of ALL_PERCENTS) { + assert.equal(matterToPercent(percentToMatter(percent)), percent, `percent ${percent}`); + } + }); + + it("pins the endpoints of the range", () => { + assert.equal(percentToMatter(0), 0); + assert.equal(percentToMatter(100), MATTER_LEVEL_MAX); + assert.equal(matterToPercent(0), 0); + assert.equal(matterToPercent(MATTER_LEVEL_MAX), 100); + }); + + it("rounds half up", () => { + // 50% is 127 exactly; the halves either side are what the rule decides. + assert.equal(percentToMatter(50), 127); + // 0.5 steps: matter 127.5 would come from 50.196…%; check the inverse + // direction where a genuine .5 occurs (level 89 → 35.039…, level 127 → + // 50.0). Exact halves exist for the hue scale, tested below. + assert.equal(matterToPercent(127), 50); + assert.equal(matterToPercent(128), 50); + }); + + it("clamps out-of-range inputs rather than producing illegal Matter values", () => { + assert.equal(percentToMatter(-5), 0); + assert.equal(percentToMatter(500), MATTER_LEVEL_MAX); + assert.equal(matterToPercent(-1), 0); + assert.equal(matterToPercent(9999), 100); + }); + + it("never emits a percentage outside 0-100 for any Matter level", () => { + for (const level of ALL_MATTER_LEVELS) { + const percent = matterToPercent(level); + assert.ok(Number.isInteger(percent) && percent >= 0 && percent <= 100, `level ${level} → ${percent}`); + } + }); + + it("writes currentLevel as at least 1, which still reads back as 0%", () => { + // The Lighting feature constrains currentLevel to 1-254 on all four + // lighting device types, so 0% cannot be written literally. 1 is the + // lossless substitute: it converts back to 0. + assert.equal(percentToCurrentLevel(0), 1); + assert.equal(matterToPercent(percentToCurrentLevel(0)), 0); + for (const percent of ALL_PERCENTS) { + const written = percentToCurrentLevel(percent); + assert.ok(written >= 1 && written <= MATTER_LEVEL_MAX, `percent ${percent} → ${written}`); + assert.equal(matterToPercent(written), percent, `percent ${percent} did not survive the floor`); + } + }); +}); + +describe("colour temperature conversion (§4.2)", () => { + it("clamps to the advertised physical bounds", () => { + assert.equal(clampMireds(100), MIREDS_MIN); + assert.equal(clampMireds(153), MIREDS_MIN); + assert.equal(clampMireds(320), 320); + assert.equal(clampMireds(500), MIREDS_MAX); + assert.equal(clampMireds(10_000), MIREDS_MAX); + }); + + it("rounds a fractional mired to an integer", () => { + assert.equal(clampMireds(319.4), 319); + assert.equal(clampMireds(319.5), 320); + }); +}); + +describe("hue conversion (§4.2)", () => { + it("pins the endpoints of the range", () => { + assert.equal(degreesToMatterHue(0), 0); + assert.equal(degreesToMatterHue(HUE_DEGREES_MAX), MATTER_LEVEL_MAX); + assert.equal(matterHueToDegrees(0), 0); + assert.equal(matterHueToDegrees(MATTER_LEVEL_MAX), HUE_DEGREES_MAX); + }); + + it("round-trips every degree to within one Matter step", () => { + // 361 degrees do not fit in 255 steps, so this is a bounded-error + // property, not an identity — unlike level and saturation. One step is + // 360/254 ≈ 1.42°, so the tolerance is 1° after rounding. + for (let degrees = 0; degrees <= HUE_DEGREES_MAX; degrees++) { + const back = matterHueToDegrees(degreesToMatterHue(degrees)); + assert.ok(Math.abs(back - degrees) <= 1, `hue ${degrees} came back as ${back}`); + } + }); + + it("clamps out-of-range degrees", () => { + assert.equal(degreesToMatterHue(-1), 0); + assert.equal(degreesToMatterHue(720), MATTER_LEVEL_MAX); + }); + + it("never emits a hue outside 0-360 for any Matter value", () => { + for (const hue of ALL_MATTER_LEVELS) { + const degrees = matterHueToDegrees(hue); + assert.ok(Number.isInteger(degrees) && degrees >= 0 && degrees <= HUE_DEGREES_MAX, `hue ${hue}`); + } + }); +}); + +describe("echo guard (§6.4)", () => { + it("treats a matter.js LocalActorContext as our own write", () => { + assert.equal(isEcosystemChange({ offline: true }), false); + }); + + it("treats anything else as ecosystem-originated", () => { + // RemoteActorContext declares `offline?: false`, so both the explicit + // false and the absent property are network actions. + assert.equal(isEcosystemChange({ offline: false }), true); + assert.equal(isEcosystemChange({}), true); + assert.equal(isEcosystemChange(undefined), true); + }); +}); diff --git a/indigo-matter.indigoPlugin/Contents/Info.plist b/indigo-matter.indigoPlugin/Contents/Info.plist index ac6ae95..b61ee3e 100644 --- a/indigo-matter.indigoPlugin/Contents/Info.plist +++ b/indigo-matter.indigoPlugin/Contents/Info.plist @@ -20,7 +20,7 @@ IwsApiVersion 1.0.0 PluginVersion - 2026.7.26 + 2026.7.27 ServerApiVersion 3.6 diff --git a/tests/fixtures/bridge_protocol/frames.json b/tests/fixtures/bridge_protocol/frames.json index 441cebb..dc59dd6 100644 --- a/tests/fixtures/bridge_protocol/frames.json +++ b/tests/fixtures/bridge_protocol/frames.json @@ -9,25 +9,28 @@ "Shapes only — bridgeVersion/matterJsVersion are substituted at assert time,", "since they track package.json and the pinned matter.js release.", "", - "Top level: the handshake, the exchanges the E0 node implements, and every", - "§5 event. 'pending' holds exchanges for commands the node does not implement", - "yet (endpoint CRUD, §3.2-§3.5, plus §3.9-§3.11 and the §3.1 mass-removal", - "guard); the TS suite skips those, the Python client suite does not — the", - "plugin half of E1 is what this PR builds.", + "Top level: the handshake, the exchanges the node implements, and every §5", + "event. 'pending' holds exchanges for commands the node does not implement", + "yet (the E4 roles' set_state family, §3.9-§3.11); the TS suite skips those,", + "the Python client suite does not — the plugin half shipped at E1.", "", - "Three frames are deliberately not what the live E0 node produces, and the", - "suites know it:", + "Two frames are deliberately not what a live node produces from a cold start,", + "and the suites know it:", " - 'attach' answers with an EMPTY endpoint set, because the request it", - " answers carries `endpoints: []`. That is the lawful §3.1 pair. The E0", - " node ignores the requested set and serves its hard-coded 999001 endpoint", - " (see 'get_status'), so its attach test asserts the live status instead;", - " the two converge when E2 makes attach reconcile for real.", + " answers carries `endpoints: []`. That is the lawful §3.1 pair, and it is", + " also what a node with nothing live does — the mass-removal guard only", + " fires against a non-empty live set.", " - 'unknown_command' uses a name that is not in §3 at all. It used to use", " upsert_endpoint 'because the node does not know it yet', which made the", - " fixture expire the moment E1 landed the handler.", - " - 'command_unexported_device' targets an indigoDeviceId in no endpoint set,", - " for the E3 path where the allow-list no longer contains the device an", - " ecosystem just acted on (§5 / PRD §7 race row): no action, one warning." + " fixture expire the moment the handler landed.", + "", + "'get_status' carries the same StatusReport 'attach_with_endpoints' returns:", + "§6.2's invariant is that attach answers with exactly what get_status would,", + "so the two frames sharing a payload is the contract, not a copy-paste.", + "", + "'command_unexported_device' targets an indigoDeviceId in no endpoint set, for", + "the path where the allow-list no longer contains the device an ecosystem just", + "acted on (§5 / PRD §7 race row): no action, one warning." ], "handshake": { "protocolVersion": 1, @@ -104,14 +107,25 @@ "response": { "message_id": "m5", "result": { - "commissioned": false, - "fabrics": [], - "endpointCount": 1, + "commissioned": true, + "fabrics": [ + { + "fabricIndex": 1, + "label": "Apple Home", + "vendorId": 4937 + } + ], + "endpointCount": 2, "endpoints": [ { - "indigoDeviceId": 999001, + "indigoDeviceId": 123456789, "endpointNumber": 2, - "role": "onOffPlugInUnit" + "role": "onOffLight" + }, + { + "indigoDeviceId": 123456790, + "endpointNumber": 3, + "role": "dimmableLight" } ], "drift": [] @@ -243,6 +257,235 @@ "details": "mDNS advertiser is down" } }, + "attach_with_endpoints": { + "request": { + "message_id": "m10", + "command": "attach", + "args": { + "protocolVersion": 1, + "pluginVersion": "2026.8.1", + "endpoints": [ + { + "indigoDeviceId": 123456789, + "role": "onOffLight", + "label": "Kitchen Lamp", + "reachable": true, + "states": { + "onOff": true + }, + "options": {} + }, + { + "indigoDeviceId": 123456790, + "role": "dimmableLight", + "label": "Lounge Lamp", + "reachable": true, + "states": { + "onOff": true, + "level": 60 + }, + "options": {} + } + ] + } + }, + "response": { + "message_id": "m10", + "result": { + "commissioned": true, + "fabrics": [ + { + "fabricIndex": 1, + "label": "Apple Home", + "vendorId": 4937 + } + ], + "endpointCount": 2, + "endpoints": [ + { + "indigoDeviceId": 123456789, + "endpointNumber": 2, + "role": "onOffLight" + }, + { + "indigoDeviceId": 123456790, + "endpointNumber": 3, + "role": "dimmableLight" + } + ], + "drift": [] + } + } + }, + "attach_replace_all": { + "request": { + "message_id": "m11", + "command": "attach", + "args": { + "protocolVersion": 1, + "pluginVersion": "2026.8.1", + "endpoints": [], + "intent": "replace_all" + } + }, + "response": { + "message_id": "m11", + "result": { + "commissioned": true, + "fabrics": [ + { + "fabricIndex": 1, + "label": "Apple Home", + "vendorId": 4937 + } + ], + "endpointCount": 0, + "endpoints": [], + "drift": [] + } + } + }, + "attach_mass_removal_refused": { + "request": { + "message_id": "m12", + "command": "attach", + "args": { + "protocolVersion": 1, + "pluginVersion": "2026.8.1", + "endpoints": [] + } + }, + "response": { + "message_id": "m12", + "error_code": "mass_removal_refused", + "details": "attach would remove all 2 live endpoints without intent: replace_all" + } + }, + "upsert_endpoint": { + "request": { + "message_id": "m13", + "command": "upsert_endpoint", + "args": { + "endpoint": { + "indigoDeviceId": 123456789, + "role": "onOffLight", + "label": "Kitchen Lamp", + "reachable": true, + "states": { + "onOff": true + }, + "options": {} + } + } + }, + "response": { + "message_id": "m13", + "result": { + "endpointNumber": 2 + } + } + }, + "upsert_endpoint_role_change": { + "request": { + "message_id": "m14", + "command": "upsert_endpoint", + "args": { + "endpoint": { + "indigoDeviceId": 123456789, + "role": "dimmableLight", + "label": "Kitchen Lamp", + "reachable": true, + "states": { + "onOff": true, + "level": 60 + }, + "options": {} + } + } + }, + "response": { + "message_id": "m14", + "error_code": "role_change", + "details": "endpoint 123456789 is onOffLight; remove and re-add to change role" + } + }, + "remove_endpoint": { + "request": { + "message_id": "m15", + "command": "remove_endpoint", + "args": { + "indigoDeviceId": 123456789 + } + }, + "response": { + "message_id": "m15", + "result": { + "removed": true + } + } + }, + "remove_endpoint_absent": { + "request": { + "message_id": "m16", + "command": "remove_endpoint", + "args": { + "indigoDeviceId": 123456791 + } + }, + "response": { + "message_id": "m16", + "result": { + "removed": false + } + } + }, + "set_state": { + "request": { + "message_id": "m17", + "command": "set_state", + "args": { + "indigoDeviceId": 123456789, + "states": { + "onOff": true + } + } + }, + "response": { + "message_id": "m17", + "result": {} + } + }, + "set_state_unknown_device": { + "request": { + "message_id": "m18", + "command": "set_state", + "args": { + "indigoDeviceId": 123456791, + "states": { + "onOff": true + } + } + }, + "response": { + "message_id": "m18", + "error_code": "unknown_device", + "details": "no live endpoint for indigoDeviceId 123456791" + } + }, + "set_reachable": { + "request": { + "message_id": "m19", + "command": "set_reachable", + "args": { + "indigoDeviceId": 123456789, + "reachable": false + } + }, + "response": { + "message_id": "m19", + "result": {} + } + }, "window_closed_expired": { "event": "window_closed", "data": { @@ -278,7 +521,7 @@ "command_lock": { "event": "command", "data": { - "indigoDeviceId": 123456790, + "indigoDeviceId": 900007, "command": "lock", "args": {} } @@ -412,245 +655,20 @@ }, "pending": { "_comment": [ - "E1 commands the bridge node does not implement yet. The TS suite skips", - "these (and asserts they are skipped for a reason); the Python client", - "suite asserts them in full, because the plugin side ships now.", + "Commands and roles the bridge node does not implement yet. The TS suite skips", + "these (and asserts they are skipped for a reason); the Python client suite", + "asserts them in full, because the plugin side ships first.", "", "'attach_all_roles' and the 'set_state_*' family exist so every §4.2 role,", "state key and command name is carried by a real frame rather than a list in", "a test file — tests/test_bridge_protocol_frames.py enumerates the", "vocabularies from bridge_protocol.ROLE_STATE_KEYS / ROLE_COMMANDS and fails", "if any of them has no frame. onOffLight's set_state is the plain 'set_state'", - "entry above; the rest are named per role." + "entry at the top level; the rest are named per role.", + "", + "E3 implements the relay/dimmer/colour roles only, so attach_all_roles stays", + "here until E4 adds sensors, thermostat, doorLock and windowCovering." ], - "attach_with_endpoints": { - "request": { - "message_id": "m10", - "command": "attach", - "args": { - "protocolVersion": 1, - "pluginVersion": "2026.8.1", - "endpoints": [ - { - "indigoDeviceId": 123456789, - "role": "onOffLight", - "label": "Kitchen Lamp", - "reachable": true, - "states": { - "onOff": true - }, - "options": {} - }, - { - "indigoDeviceId": 123456790, - "role": "doorLock", - "label": "Front Door", - "reachable": true, - "states": { - "locked": true - }, - "options": {} - } - ] - } - }, - "response": { - "message_id": "m10", - "result": { - "commissioned": true, - "fabrics": [ - { - "fabricIndex": 1, - "label": "Apple Home", - "vendorId": 4937 - } - ], - "endpointCount": 2, - "endpoints": [ - { - "indigoDeviceId": 123456789, - "endpointNumber": 2, - "role": "onOffLight" - }, - { - "indigoDeviceId": 123456790, - "endpointNumber": 3, - "role": "doorLock" - } - ], - "drift": [] - } - } - }, - "attach_replace_all": { - "request": { - "message_id": "m11", - "command": "attach", - "args": { - "protocolVersion": 1, - "pluginVersion": "2026.8.1", - "endpoints": [], - "intent": "replace_all" - } - }, - "response": { - "message_id": "m11", - "result": { - "commissioned": true, - "fabrics": [ - { - "fabricIndex": 1, - "label": "Apple Home", - "vendorId": 4937 - } - ], - "endpointCount": 0, - "endpoints": [], - "drift": [] - } - } - }, - "attach_mass_removal_refused": { - "request": { - "message_id": "m12", - "command": "attach", - "args": { - "protocolVersion": 1, - "pluginVersion": "2026.8.1", - "endpoints": [] - } - }, - "response": { - "message_id": "m12", - "error_code": "mass_removal_refused", - "details": "attach would remove all 2 live endpoints without intent: replace_all" - } - }, - "upsert_endpoint": { - "request": { - "message_id": "m13", - "command": "upsert_endpoint", - "args": { - "endpoint": { - "indigoDeviceId": 123456789, - "role": "onOffLight", - "label": "Kitchen Lamp", - "reachable": true, - "states": { - "onOff": true - }, - "options": {} - } - } - }, - "response": { - "message_id": "m13", - "result": { - "endpointNumber": 2 - } - } - }, - "upsert_endpoint_role_change": { - "request": { - "message_id": "m14", - "command": "upsert_endpoint", - "args": { - "endpoint": { - "indigoDeviceId": 123456789, - "role": "dimmableLight", - "label": "Kitchen Lamp", - "reachable": true, - "states": { - "onOff": true, - "level": 60 - }, - "options": {} - } - } - }, - "response": { - "message_id": "m14", - "error_code": "role_change", - "details": "endpoint 123456789 is onOffLight; remove and re-add to change role" - } - }, - "remove_endpoint": { - "request": { - "message_id": "m15", - "command": "remove_endpoint", - "args": { - "indigoDeviceId": 123456789 - } - }, - "response": { - "message_id": "m15", - "result": { - "removed": true - } - } - }, - "remove_endpoint_absent": { - "request": { - "message_id": "m16", - "command": "remove_endpoint", - "args": { - "indigoDeviceId": 123456791 - } - }, - "response": { - "message_id": "m16", - "result": { - "removed": false - } - } - }, - "set_state": { - "request": { - "message_id": "m17", - "command": "set_state", - "args": { - "indigoDeviceId": 123456789, - "states": { - "onOff": true - } - } - }, - "response": { - "message_id": "m17", - "result": {} - } - }, - "set_state_unknown_device": { - "request": { - "message_id": "m18", - "command": "set_state", - "args": { - "indigoDeviceId": 123456791, - "states": { - "onOff": true - } - } - }, - "response": { - "message_id": "m18", - "error_code": "unknown_device", - "details": "no live endpoint for indigoDeviceId 123456791" - } - }, - "set_reachable": { - "request": { - "message_id": "m19", - "command": "set_reachable", - "args": { - "indigoDeviceId": 123456789, - "reachable": false - } - }, - "response": { - "message_id": "m19", - "result": {} - } - }, "remove_fabric": { "request": { "message_id": "m20", @@ -717,7 +735,7 @@ { "indigoDeviceId": 123456790, "endpointNumber": 5, - "role": "doorLock" + "role": "dimmableLight" } ], "drift": [] diff --git a/tests/test_bridge_client.py b/tests/test_bridge_client.py index b30751b..9e6d5af 100644 --- a/tests/test_bridge_client.py +++ b/tests/test_bridge_client.py @@ -24,7 +24,15 @@ from fakes import FakeWebSocket, returns FRAMES = load_bridge_frames() -PENDING = FRAMES["pending"] +#: Every request/response exchange in the golden file, keyed by name. +#: Which section a frame sits in ("pending" or top level) is a statement about +#: what the BRIDGE NODE implements — E3 promoted the endpoint-CRUD frames when +#: it grew their handlers — and says nothing about the plugin-side client these +#: tests drive, which has spoken all of them since E1. +EXCHANGES = { + **{k: v for k, v in FRAMES.items() if isinstance(v, dict) and "request" in v}, + **{k: v for k, v in FRAMES["pending"].items() if not k.startswith("_")}, +} HELLO = FRAMES["handshake"] SKEWED_HELLO = {**HELLO, "protocolVersion": bridge_protocol.PROTOCOL_VERSION + 1} @@ -35,13 +43,13 @@ bridge_protocol.CMD_GET_STATUS: FRAMES["get_status"]["response"], bridge_protocol.CMD_GET_PAIRING: FRAMES["get_pairing_commissioned"]["response"], bridge_protocol.CMD_OPEN_WINDOW: FRAMES["open_commissioning_window"]["response"], - bridge_protocol.CMD_UPSERT_ENDPOINT: PENDING["upsert_endpoint"]["response"], - bridge_protocol.CMD_REMOVE_ENDPOINT: PENDING["remove_endpoint"]["response"], - bridge_protocol.CMD_SET_STATE: PENDING["set_state"]["response"], - bridge_protocol.CMD_SET_REACHABLE: PENDING["set_reachable"]["response"], - bridge_protocol.CMD_REMOVE_FABRIC: PENDING["remove_fabric"]["response"], - bridge_protocol.CMD_FACTORY_RESET: PENDING["factory_reset"]["response"], - bridge_protocol.CMD_REBUILD_ENDPOINT_MAP: PENDING["rebuild_endpoint_map"]["response"], + bridge_protocol.CMD_UPSERT_ENDPOINT: EXCHANGES["upsert_endpoint"]["response"], + bridge_protocol.CMD_REMOVE_ENDPOINT: EXCHANGES["remove_endpoint"]["response"], + bridge_protocol.CMD_SET_STATE: EXCHANGES["set_state"]["response"], + bridge_protocol.CMD_SET_REACHABLE: EXCHANGES["set_reachable"]["response"], + bridge_protocol.CMD_REMOVE_FABRIC: EXCHANGES["remove_fabric"]["response"], + bridge_protocol.CMD_FACTORY_RESET: EXCHANGES["factory_reset"]["response"], + bridge_protocol.CMD_REBUILD_ENDPOINT_MAP: EXCHANGES["rebuild_endpoint_map"]["response"], } KITCHEN_LAMP = EndpointSpec( @@ -248,23 +256,23 @@ async def scenario(): def test_upsert_endpoint(self, mock_logger): result = self._exchange( - mock_logger, lambda c: c.upsert_endpoint(KITCHEN_LAMP), "upsert_endpoint", PENDING) + mock_logger, lambda c: c.upsert_endpoint(KITCHEN_LAMP), "upsert_endpoint", EXCHANGES) assert result == 2 def test_upsert_endpoint_accepts_a_wire_dict(self, mock_logger): - wire = PENDING["upsert_endpoint"]["request"]["args"]["endpoint"] + wire = EXCHANGES["upsert_endpoint"]["request"]["args"]["endpoint"] assert self._exchange(mock_logger, lambda c: c.upsert_endpoint(wire), - "upsert_endpoint", PENDING) == 2 + "upsert_endpoint", EXCHANGES) == 2 def test_remove_endpoint(self, mock_logger): assert self._exchange(mock_logger, lambda c: c.remove_endpoint(123456789), - "remove_endpoint", PENDING) is True + "remove_endpoint", EXCHANGES) is True def test_remove_endpoint_absent_is_not_an_error(self, mock_logger): # §3.3: idempotent — removing what is not there succeeds with removed=false. async def scenario(): fake = _fake(responder=golden_responder( - {bridge_protocol.CMD_REMOVE_ENDPOINT: PENDING["remove_endpoint_absent"]["response"]})) + {bridge_protocol.CMD_REMOVE_ENDPOINT: EXCHANGES["remove_endpoint_absent"]["response"]})) client = _client(mock_logger, fake) task = asyncio.create_task(client.run()) await client.wait_connected(timeout=2) @@ -275,12 +283,12 @@ async def scenario(): def test_set_reachable(self, mock_logger): self._exchange(mock_logger, lambda c: c.set_reachable(123456789, False), - "set_reachable", PENDING) + "set_reachable", EXCHANGES) def test_get_status(self, mock_logger): status = self._exchange(mock_logger, lambda c: c.get_status(), "get_status", FRAMES) - assert status.endpoint_count == 1 - assert status.endpoints[0].role == "onOffPlugInUnit" + assert status.endpoint_count == 2 + assert status.endpoints[0].role == "onOffLight" def test_get_pairing(self, mock_logger): pairing = self._exchange(mock_logger, lambda c: c.get_pairing(), @@ -299,18 +307,18 @@ def test_open_commissioning_window_defaults_to_the_matter_maximum(self, mock_log "open_commissioning_window", FRAMES) def test_remove_fabric(self, mock_logger): - self._exchange(mock_logger, lambda c: c.remove_fabric(2), "remove_fabric", PENDING) + self._exchange(mock_logger, lambda c: c.remove_fabric(2), "remove_fabric", EXCHANGES) def test_factory_reset_preserves_endpoint_numbers_by_default(self, mock_logger): - self._exchange(mock_logger, lambda c: c.factory_reset(), "factory_reset", PENDING) + self._exchange(mock_logger, lambda c: c.factory_reset(), "factory_reset", EXCHANGES) def test_factory_reset_can_discard_the_map(self, mock_logger): self._exchange(mock_logger, lambda c: c.factory_reset(False), - "factory_reset_discard_map", PENDING) + "factory_reset_discard_map", EXCHANGES) def test_rebuild_endpoint_map(self, mock_logger): status = self._exchange(mock_logger, lambda c: c.rebuild_endpoint_map(), - "rebuild_endpoint_map", PENDING) + "rebuild_endpoint_map", EXCHANGES) assert status.endpoint_count == 2 # §3.11 REallocates — the numbers differ from the ones attach reported. assert status.endpoints[1].endpoint_number == 5 @@ -318,7 +326,7 @@ def test_rebuild_endpoint_map(self, mock_logger): def test_error_response_raises(self, mock_logger): async def scenario(): fake = _fake(responder=golden_responder( - {bridge_protocol.CMD_UPSERT_ENDPOINT: PENDING["upsert_endpoint_role_change"]["response"]})) + {bridge_protocol.CMD_UPSERT_ENDPOINT: EXCHANGES["upsert_endpoint_role_change"]["response"]})) client = _client(mock_logger, fake) task = asyncio.create_task(client.run()) await client.wait_connected(timeout=2) @@ -344,7 +352,7 @@ async def scenario(): await asyncio.wait_for(client.set_state(123456789, {"onOff": True}), timeout=0.5) frame = sent(fake, bridge_protocol.CMD_SET_STATE) - assert frame["args"] == PENDING["set_state"]["request"]["args"] + assert frame["args"] == EXCHANGES["set_state"]["request"]["args"] assert client._pending == {}, "set_state must not register a pending future" await client.close() @@ -356,7 +364,7 @@ def test_unmatched_error_response_is_logged(self, mock_logger): # unnoticed failure looks exactly like "the ecosystem shows stale state". async def scenario(): fake = _fake(responder=golden_responder( - {bridge_protocol.CMD_SET_STATE: PENDING["set_state_unknown_device"]["response"]})) + {bridge_protocol.CMD_SET_STATE: EXCHANGES["set_state_unknown_device"]["response"]})) client = _client(mock_logger, fake) task = asyncio.create_task(client.run()) await client.wait_connected(timeout=2) @@ -423,7 +431,7 @@ def test_command_events_reach_on_command(self, mock_logger): assert [(c.indigo_device_id, c.command, c.args) for c in received] == [ (123456789, "onOff", {"value": True}), (123456789, "setLevel", {"level": 60}), - (123456790, "lock", {}), + (900007, "lock", {}), ] def test_fabric_events(self, mock_logger): @@ -691,7 +699,7 @@ class TestEndpointMapInvalid: def _recovering(self, mock_logger, overrides=None): table = {bridge_protocol.CMD_ATTACH: error_response( bridge_protocol.ERR_ENDPOINT_MAP_INVALID, - PENDING["endpoint_map_invalid"]["response"]["details"])} + EXCHANGES["endpoint_map_invalid"]["response"]["details"])} table.update(overrides or {}) fake = _fake(responder=golden_responder(table)) return fake, _client(mock_logger, fake) @@ -723,7 +731,7 @@ async def scenario(): task = asyncio.create_task(client.run()) await settle(lambda: client.recovery) - assert (await client.get_status()).endpoint_count == 1 + assert (await client.get_status()).endpoint_count == 2 assert (await client.get_pairing()).commissioned is True await client.close() @@ -831,7 +839,7 @@ def test_an_unmatched_error_names_the_device(self, mock_logger): # bare message_id is not something a user can act on. async def scenario(): fake = _fake(responder=golden_responder( - {bridge_protocol.CMD_SET_STATE: PENDING["set_state_unknown_device"]["response"]})) + {bridge_protocol.CMD_SET_STATE: EXCHANGES["set_state_unknown_device"]["response"]})) client = _client(mock_logger, fake) task = asyncio.create_task(client.run()) await client.wait_connected(timeout=2) @@ -942,7 +950,7 @@ def responder(frame): assert client.status.endpoint_count == 0 # The node now serves the two-endpoint set. - answers[bridge_protocol.CMD_ATTACH] = PENDING["attach_with_endpoints"]["response"] + answers[bridge_protocol.CMD_ATTACH] = EXCHANGES["attach_with_endpoints"]["response"] refreshed = await client.attach([KITCHEN_LAMP]) assert client.status is refreshed diff --git a/tests/test_bridge_protocol_frames.py b/tests/test_bridge_protocol_frames.py index 9302a1f..b990f61 100644 --- a/tests/test_bridge_protocol_frames.py +++ b/tests/test_bridge_protocol_frames.py @@ -33,6 +33,10 @@ def _events() -> dict: EXCHANGES = _exchanges() +#: The same exchanges keyed by bare name, for tests that name one frame. A frame +#: moves out of "pending" when the NODE implements it (E3 moved the endpoint-CRUD +#: family); the plugin-side shapes asserted here do not change when it does. +BY_NAME = {name.removeprefix("pending:"): exchange for name, exchange in EXCHANGES.items()} EVENTS = _events() #: Fixtures with no public builder, and the assertion that replaces the @@ -113,7 +117,7 @@ def test_intent_is_absent_unless_replace_all(self): assert bridge_protocol.ARG_INTENT not in plain["args"] deliberate = BridgeProtocol().build_attach("2026.8.1", [], replace_all=True, message_id="x") assert deliberate["args"][bridge_protocol.ARG_INTENT] == bridge_protocol.INTENT_REPLACE_ALL - assert deliberate == PENDING["attach_replace_all"]["request"] | {"message_id": "x"} + assert deliberate == BY_NAME["attach_replace_all"]["request"] | {"message_id": "x"} def test_factory_reset_preserves_endpoint_numbers_by_default(self): # §3.10: a reset must not scramble identities unless asked to. @@ -140,12 +144,12 @@ def test_response_parses_or_raises(self, name): def test_status_report_is_normalised(self): report = bridge_protocol.parse_status( - PENDING["attach_with_endpoints"]["response"]["result"]) + BY_NAME["attach_with_endpoints"]["response"]["result"]) assert report.commissioned is True assert report.endpoint_count == 2 assert [ep.indigo_device_id for ep in report.endpoints] == [123456789, 123456790] assert [ep.endpoint_number for ep in report.endpoints] == [2, 3] - assert report.endpoints[1].role == "doorLock" + assert report.endpoints[1].role == "dimmableLight" assert report.fabrics[0].fabric_index == 1 assert report.fabrics[0].label == "Apple Home" assert report.fabrics[0].vendor_id == 4937 @@ -179,9 +183,9 @@ def test_commissioning_window(self): assert window.window_expires_at.endswith("Z") def test_removal_results(self): - assert PENDING["remove_endpoint"]["response"]["result"]["removed"] is True + assert BY_NAME["remove_endpoint"]["response"]["result"]["removed"] is True # §3.3: removing an absent endpoint SUCCEEDS — idempotent, not an error. - absent = PENDING["remove_endpoint_absent"]["response"] + absent = BY_NAME["remove_endpoint_absent"]["response"] assert bridge_protocol.KEY_ERROR_CODE not in absent assert absent["result"]["removed"] is False @@ -326,16 +330,16 @@ def test_rebuild_reallocates_rather_than_echoing_the_previous_numbers(self): # accessories). A fixture that echoed attach's numbers demonstrated the # opposite of the command's whole reason to exist. before = {ep["indigoDeviceId"]: ep["endpointNumber"] - for ep in PENDING["attach_with_endpoints"]["response"]["result"]["endpoints"]} + for ep in BY_NAME["attach_with_endpoints"]["response"]["result"]["endpoints"]} after = {ep["indigoDeviceId"]: ep["endpointNumber"] - for ep in PENDING["rebuild_endpoint_map"]["response"]["result"]["endpoints"]} + for ep in BY_NAME["rebuild_endpoint_map"]["response"]["result"]["endpoints"]} assert set(before) == set(after) assert all(after[dev] != before[dev] for dev in before), (before, after) def test_endpoint_spec_round_trips(self): - wire = PENDING["upsert_endpoint"]["request"]["args"]["endpoint"] + wire = BY_NAME["upsert_endpoint"]["request"]["args"]["endpoint"] assert EndpointSpec.from_wire(wire).to_wire() == wire def test_every_role_spec_round_trips(self): - for spec in PENDING["attach_all_roles"]["request"]["args"]["endpoints"]: + for spec in BY_NAME["attach_all_roles"]["request"]["args"]["endpoints"]: assert EndpointSpec.from_wire(spec).to_wire() == spec From 6d807ddf388de38a925903858683d5ef8ea2484d Mon Sep 17 00:00:00 2001 From: Simon Clark Date: Wed, 5 Aug 2026 09:22:41 +0100 Subject: [PATCH 2/2] fix(export): harden E3a registry/reconcile per PR #123 three-review batch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Silent-failure findings from the three reviews, with fail-before regression tests for each: - closeOne evicted the endpoint before awaiting close(), so a throwing close left an accessory live in every ecosystem that the registry denied existed; the retry then answered {removed: false}, which §3.3 defines as success. Close first, restore the listeners on failure, surface `internal`. - No mutex on the registry: a superseding attach on a new socket (the plugin-crash-restart window) planned its diff from a snapshot the incumbent's paced reconcile was still mutating. All mutating facade calls now run on one promise chain, and a queued operation says so. - A reconcile that failed after mutation started skipped the batched ConfigurationVersion bump and answered a bare `internal`. It now bumps if anything changed and logs how far it got and what is live. - set_state answered success for keys the role does not speak, so a whole role's writes could go nowhere unnoticed (the plugin does not await it). Keys given but none consumed is now malformed_args naming them; an empty states stays a lawful no-op, and a partly-understood one still applies. - A missing $Changed observable was skipped silently, which would leave a command family permanently dead with no evidence; it now throws at construction. The handler body is guarded so a throw cannot escape into matter.js's commit path and take the process down. - managedTransitionTimeHandling pinned false on the level-bearing roles: managed transition steps arrive as offline contexts and the echo guard would eat every brightness change. Also: dropped events and non-OPEN sends are logged; a result that cannot be serialised still gets a response (§1); StatusReport gains `driftChecked` so an empty `drift` is not read as an all-clear; label drift compares productLabel too; clamps log at debug. The stub bridge now retains endpoint numbers as matter.js does, upsert_endpoint_unknown_role graduates to a live golden frame, and set_state_bad_keys joins it on both suites. Tests: node 193 (173 pass, 20 skipped), python 1633. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JxqGhP3DcENf68AZK21U4S --- bridge-node/src/endpoints.ts | 177 ++++++++-- bridge-node/src/node.ts | 5 +- bridge-node/src/protocol.ts | 11 + bridge-node/src/registry.ts | 240 ++++++++++--- bridge-node/src/ws-server.ts | 43 ++- bridge-node/test/fixture-shapes.ts | 28 ++ bridge-node/test/fixtures.test.ts | 2 + bridge-node/test/integration.test.ts | 169 +++++++++ bridge-node/test/protocol.test.ts | 163 ++++++++- bridge-node/test/registry.test.ts | 390 ++++++++++++++++++++- bridge-node/test/stub-bridge.ts | 31 +- docs/BRIDGE_PROTOCOL.md | 13 +- tests/fixtures/bridge_protocol/frames.json | 77 ++-- tests/test_bridge_protocol_frames.py | 4 +- 14 files changed, 1241 insertions(+), 112 deletions(-) create mode 100644 bridge-node/test/integration.test.ts diff --git a/bridge-node/src/endpoints.ts b/bridge-node/src/endpoints.ts index 76ec2c1..843445b 100644 --- a/bridge-node/src/endpoints.ts +++ b/bridge-node/src/endpoints.ts @@ -12,7 +12,7 @@ * rather than silently dropped — see {@link UNSUPPORTED_ROLE_DETAILS}. */ -import { Endpoint, type EndpointType } from "@matter/main"; +import { Endpoint, type EndpointType, Logger } from "@matter/main"; import { BridgedDeviceBasicInformationServer } from "@matter/main/behaviors/bridged-device-basic-information"; import { ColorControlServer } from "@matter/main/behaviors/color-control"; import { ColorControl } from "@matter/main/clusters/color-control"; @@ -24,6 +24,7 @@ import { OnOffPlugInUnitDevice } from "@matter/main/devices/on-off-plug-in-unit" import { type CommandEventData, + describeErrorWithStack, type EndpointSpec, ErrorCode, ProtocolError, @@ -31,6 +32,13 @@ import { type RoleValue, } from "./protocol.js"; +/** + * Clamping is silent by design at the protocol boundary (§4.2 says the node + * owns bounds), but a value that had to be clamped is the fingerprint of a + * plugin-side unit bug, so it is worth a debug line naming what moved. + */ +const logger = Logger.get("indigo-matter-endpoints"); + /** Bridged Device Basic Information `UniqueID`, stable across restarts. */ export function uniqueIdFor(indigoDeviceId: number): string { return `indigo-${indigoDeviceId}`; @@ -69,6 +77,19 @@ function clamp(value: number, min: number, max: number): number { return Math.min(max, Math.max(min, value)); } +/** + * {@link clamp}, but says so. Only the *inbound* clamp of each converter uses + * this: the outbound one exists to satisfy the type range and can never fire + * once the input is already in range, so logging it would only ever be noise. + */ +function clampLogged(value: number, min: number, max: number, what: string): number { + const clamped = clamp(value, min, max); + if (clamped !== value) { + logger.debug(`Clamped ${what} ${value} to ${clamped} (supported range ${min}-${max})`); + } + return clamped; +} + /** * Round half away from zero. `Math.round` is half-*up* (−0.5 → −0), which * differs for negatives; every quantity here is non-negative, but naming the @@ -83,7 +104,7 @@ function roundHalfUp(value: number): number { * the conversion rather than becoming the dimmest on-level. */ export function percentToMatter(percent: number): number { - return clamp(roundHalfUp((clamp(percent, 0, 100) * MATTER_LEVEL_MAX) / 100), 0, MATTER_LEVEL_MAX); + return clamp(roundHalfUp((clampLogged(percent, 0, 100, "percentage") * MATTER_LEVEL_MAX) / 100), 0, MATTER_LEVEL_MAX); } /** Matter 0-254 → Indigo 0-100. Inverse of {@link percentToMatter} for all 101 inputs. */ @@ -98,8 +119,11 @@ export function matterToPercent(level: number): number { * device types (verified against the 0.17.8 device definitions, which `alter` * the attribute to `min: 1`), so a literal 0 fails validation. Writing 1 * instead is lossless at the protocol boundary: {@link matterToPercent}(1) is 0. - * Zero brightness is expressed to ecosystems as OnOff = false, which is what - * the Lighting feature intends. + * + * Note that this does **not** also write OnOff false: {@link levelPatch} never + * touches the OnOff cluster. Turning a light off at 0% is the plugin's call — + * it owns the Indigo device's on/off state and sends `onOff` in the same + * `set_state` when it means it (§4.2 lists them as independent state keys). */ export function percentToCurrentLevel(percent: number): number { return Math.max(1, percentToMatter(percent)); @@ -107,13 +131,13 @@ export function percentToCurrentLevel(percent: number): number { /** §4.2: colour temperature is clamped to the advertised physical bounds. */ export function clampMireds(mireds: number): number { - return clamp(roundHalfUp(mireds), MIREDS_MIN, MIREDS_MAX); + return clampLogged(roundHalfUp(mireds), MIREDS_MIN, MIREDS_MAX, "colour temperature (mireds)"); } /** Indigo hue 0-360° → Matter `CurrentHue` 0-254. */ export function degreesToMatterHue(degrees: number): number { return clamp( - roundHalfUp((clamp(degrees, 0, HUE_DEGREES_MAX) * MATTER_LEVEL_MAX) / HUE_DEGREES_MAX), + roundHalfUp((clampLogged(degrees, 0, HUE_DEGREES_MAX, "hue (degrees)") * MATTER_LEVEL_MAX) / HUE_DEGREES_MAX), 0, MATTER_LEVEL_MAX, ); @@ -169,6 +193,25 @@ function colorControlDefaults(hueSaturation: boolean): Record { }; } +/** + * The LevelControl state every level-bearing role starts from. + * + * `managedTransitionTimeHandling` is pinned `false` rather than left to + * matter.js's default because the echo guard depends on it. With managed + * transitions ON, matter.js drives a `moveToLevel` to its target in steps of + * its own making, and those steps are written by matter.js itself — i.e. in an + * *offline* context. {@link isEcosystemChange} would read every one of them as + * our own write and drop it, so an ecosystem dimming a lamp would move the + * Matter attribute and the plugin would never learn the level changed at all. + * Unmanaged, the remote `moveToLevel` lands as one attribute change in a remote + * context, which is exactly what §4.2's `setLevel` is supposed to report. + * + * 0.17.8 already defaults it to `false` (`LevelControlServer.State`), so this + * pin changes no behaviour today — it is here so that a future default flip + * cannot silently sever the whole `setLevel` command family. + */ +const LEVEL_CONTROL_INITIAL = { currentLevel: 1, managedTransitionTimeHandling: false }; + /** The subset of §4.2 roles this bridge version can construct. */ export const SUPPORTED_ROLES: readonly RoleValue[] = [ Role.onOffPlugInUnit, @@ -345,14 +388,14 @@ const ROLE_DEFINITIONS: Partial> = { }, [Role.dimmableLight]: { deviceType: () => DimmableLightDevice.with(BridgedDeviceBasicInformationServer), - initialState: () => ({ [LEVEL_CONTROL]: { currentLevel: 1 } }), + initialState: () => ({ [LEVEL_CONTROL]: { ...LEVEL_CONTROL_INITIAL } }), statePatch: states => ({ ...onOffPatch(states), ...levelPatch(states) }), watch: [WATCH_ON_OFF, WATCH_LEVEL], }, [Role.colorTemperatureLight]: { deviceType: () => ColorTemperatureLightDevice.with(BridgedDeviceBasicInformationServer), initialState: () => ({ - [LEVEL_CONTROL]: { currentLevel: 1 }, + [LEVEL_CONTROL]: { ...LEVEL_CONTROL_INITIAL }, [COLOR_CONTROL]: { ...colorControlDefaults(false), colorTemperatureMireds: MIREDS_MIN }, }), statePatch: states => ({ ...onOffPatch(states), ...levelPatch(states), ...colorPatch(states, false) }), @@ -371,7 +414,7 @@ const ROLE_DEFINITIONS: Partial> = { ColorControlServer.with("HueSaturation", "Xy", "ColorTemperature"), ), initialState: () => ({ - [LEVEL_CONTROL]: { currentLevel: 1 }, + [LEVEL_CONTROL]: { ...LEVEL_CONTROL_INITIAL }, [COLOR_CONTROL]: { ...colorControlDefaults(true), colorTemperatureMireds: MIREDS_MIN }, }), statePatch: states => ({ ...onOffPatch(states), ...levelPatch(states), ...colorPatch(states, true) }), @@ -435,24 +478,76 @@ export function createEndpoint(spec: EndpointSpec, productName: string): Endpoin ...mergeBehaviors( { [BRIDGED_INFO]: bridgedInfoFor(spec, productName) }, definition.initialState(), - definition.statePatch(spec.states), + statePatchOrRefuse(spec.role, spec.states), ), } as never); } -/** Apply a §3.4 `set_state` as a local (offline-context) write. */ +/** + * Which of the given keys this role's converter would not consume. + * + * Derived by running the converter on one key at a time rather than from a + * second table of key names and types, because a second table is a thing that + * can disagree with the first: every §4.2 state key contributes to the patch + * independently (there is no key that is only meaningful alongside another), so + * "did the converter produce anything from this key alone" *is* the definition + * of "consumed", and it cannot drift from {@link RoleDefinition.statePatch}. + * + * This catches a misspelt key, a key belonging to another role, and a key of + * the wrong type (`{"onOff": "true"}`) with the same test — all three are + * cases the patch builders skip. + */ +export function rejectedStateKeys(role: RoleValue, states: Record): string[] { + const { statePatch } = definitionFor(role); + return Object.keys(states).filter(key => Object.keys(statePatch({ [key]: states[key] })).length === 0); +} + +/** + * Apply a §3.4 `set_state` as a local (offline-context) write. + * + * An empty `states` is a lawful no-op and succeeds. Keys the role does not + * speak are a different thing entirely: answering `{}` to + * `{"level": 55}` on an `onOffLight` reports success for a write that never + * happened, and the symptom the user sees — the ecosystem showing stale state — + * is indistinguishable from a bridge that is simply down. So a `states` that + * yields *nothing at all* is `malformed_args` naming the keys that were + * dropped. + * + * A *partly* consumed `states` still succeeds, and that asymmetry is deliberate: + * the plugin and the node version-skew across a release (PM-B keeps an old node + * running), so a newer plugin sending a key this bridge does not know yet + * alongside keys it does must still apply the ones it understands. + */ export async function applyStates( endpoint: Endpoint, role: RoleValue, states: Record, ): Promise { - const patch = definitionFor(role).statePatch(states); + const patch = statePatchOrRefuse(role, states); if (Object.keys(patch).length === 0) { return; } await endpoint.set(patch as never); } +/** + * The role's state patch, refusing a `states` that produced nothing from + * something. Shared by {@link applyStates} and {@link createEndpoint} so a set + * of keys refused on update is not quietly accepted on create — an `attach` + * reconcile runs both paths and they must agree on what is lawful. + */ +function statePatchOrRefuse(role: RoleValue, states: Record): Record { + const patch = definitionFor(role).statePatch(states); + if (Object.keys(states).length > 0 && Object.keys(patch).length === 0) { + throw new ProtocolError( + ErrorCode.malformedArgs, + `role ${role} consumed none of the states given; ` + + `rejected key(s): ${rejectedStateKeys(role, states).join(", ")} (§4.2)`, + ); + } + return patch; +} + /** §3.5 / PRD §5.3 — `Reachable` tracks the Indigo device's enabled state. */ export async function applyReachable(endpoint: Endpoint, reachable: boolean): Promise { await endpoint.set({ [BRIDGED_INFO]: { reachable } } as never); @@ -469,6 +564,13 @@ export async function applyLabel(endpoint: Endpoint, label: string): Promise void, + log: (message: string) => void = () => {}, ): () => void { const teardown: (() => void)[] = []; @@ -497,19 +608,41 @@ export function watchCommands( >; const observable = events[`${watch.attribute}$Changed`]; if (observable === undefined) { - continue; + throw new ProtocolError( + ErrorCode.internal, + `endpoint ${spec.indigoDeviceId} (${spec.role}): matter.js exposes no ` + + `${watch.behavior}.${watch.attribute}$Changed observable, so this role's command family ` + + "could never be reported", + ); } const handler = (...args: unknown[]): void => { - const [value, , context] = args; - if (!isEcosystemChange(context)) { - return; - } - const state = endpoint.stateOf(watch.behavior) as Readonly>; - const command = watch.command(value, state); - if (command === undefined) { - return; + // Everything below runs *inside* matter.js's observable, which is + // itself inside the commit of a remote write. A throw here does not + // fail the write — it escapes as an unhandled rejection, and this + // process exits on those, so one bad payload would take the whole + // bridge down and every exported accessory with it. Mirrors the + // fabricsChanged hardening in node.ts: log, never rethrow. + try { + const [value, , context] = args; + if (!isEcosystemChange(context)) { + return; + } + const state = endpoint.stateOf(watch.behavior) as Readonly>; + const command = watch.command(value, state); + if (command === undefined) { + return; + } + emit({ indigoDeviceId: spec.indigoDeviceId, command: command.command, args: command.args }); + } catch (error) { + try { + log( + `Dropped a ${watch.behavior}.${watch.attribute} change for endpoint ` + + `${spec.indigoDeviceId}: ${describeErrorWithStack(error)}`, + ); + } catch { + // The logger itself failed; there is nowhere left to report. + } } - emit({ indigoDeviceId: spec.indigoDeviceId, command: command.command, args: command.args }); }; observable.on(handler); teardown.push(() => observable.off(handler)); diff --git a/bridge-node/src/node.ts b/bridge-node/src/node.ts index 42e1992..e6e2967 100644 --- a/bridge-node/src/node.ts +++ b/bridge-node/src/node.ts @@ -204,8 +204,11 @@ export class BridgeNode implements BridgeFacade { endpointCount: endpoints.length, endpoints, // E5 introduces the persisted endpoint-number map and its drift - // detector; until then there is no baseline to drift from. + // detector; until then there is no baseline to drift from, which is + // what `driftChecked: false` says out loud (§4.3). An empty `drift` + // here means "not looked", never "looked and found nothing". drift: [], + driftChecked: false, }; } diff --git a/bridge-node/src/protocol.ts b/bridge-node/src/protocol.ts index d236239..8ce1615 100644 --- a/bridge-node/src/protocol.ts +++ b/bridge-node/src/protocol.ts @@ -173,6 +173,17 @@ export interface StatusReport { endpointCount: number; endpoints: EndpointSummary[]; drift: DriftEntry[]; + /** + * Whether `drift` is an answer or an absence. + * + * `drift: []` alone is ambiguous, and the two readings could not be further + * apart: "checked, nothing has moved" versus "there is no persisted map to + * check against yet". Until E5 builds that map this is always `false`, and + * saying so is what stops an empty list being read as an all-clear. The + * plugin ignores unknown fields (§1), so it can start honouring this in E3b + * without a protocol version bump. + */ + driftChecked: boolean; } export interface EndpointSummary { diff --git a/bridge-node/src/registry.ts b/bridge-node/src/registry.ts index 80c4791..8e3223f 100644 --- a/bridge-node/src/registry.ts +++ b/bridge-node/src/registry.ts @@ -20,6 +20,8 @@ import { } from "./endpoints.js"; import { type CommandEventData, + describeError, + describeErrorWithStack, type EndpointSpec, type EndpointSummary, ErrorCode, @@ -39,6 +41,7 @@ export const REMOVAL_PACING_MS = 100; interface LiveEndpoint { endpoint: Endpoint; role: RoleValue; + /** Reassigned when a failed close forces the listeners to be restored. */ unwatch: () => void; } @@ -64,12 +67,63 @@ export class EndpointRegistry { readonly #live = new Map(); readonly #log: (message: string) => void; readonly #pacingMs: number; + /** Tail of the mutation chain — see {@link serialize}. */ + #queue: Promise = Promise.resolve(); + /** + * Names of the mutations on the chain, head first. Pushed *synchronously* + * on the way in, because the whole point is to notice a second caller that + * arrived before the first one's continuation has even been scheduled — + * a flag set inside the queued callback would still read "idle" then. + */ + readonly #waiting: string[] = []; constructor(private readonly options: EndpointRegistryOptions) { this.#log = options.log ?? (() => {}); this.#pacingMs = options.removalPacingMs ?? REMOVAL_PACING_MS; } + /** + * Run one mutation with the registry to itself. + * + * Every mutating entry point goes through here, because two of them running + * at once share both `#live` and the aggregator and neither is transactional. + * The case that forced it is not exotic: a plugin crash leaves a half-open + * socket, launchd restarts the plugin, and the new process `attach`es on a + * *new* socket while the incumbent's reconcile is still mid-flight — and a + * reconcile that paces bulk removals (PRD §5.3, ~100ms each) holds the + * registry for seconds. Without this, the second reconcile plans its diff + * from a snapshot the first is still mutating underneath it, and the two + * interleave into a live set that matches neither request. + * + * A plain promise chain rather than a lock class: there is exactly one + * critical section (the whole registry), so ordering is the only thing to + * decide, and FIFO is what §1's in-receipt-order guarantee already promises + * the plugin. + */ + private serialize(what: string, run: () => Promise): Promise { + const ahead = this.#waiting[0]; + if (ahead !== undefined) { + // Loud on purpose: this is the plugin-crash-restart window, and it + // is the only explanation for an attach that appears to hang. + this.#log(`${what} is waiting for an unfinished ${ahead} to release the registry`); + } + this.#waiting.push(what); + const result = this.#queue.then(async () => { + try { + return await run(); + } finally { + this.#waiting.shift(); + } + }); + // The chain must never be left rejected, or one refusal would poison + // every command behind it. The caller still gets the rejection. + this.#queue = result.then( + () => undefined, + () => undefined, + ); + return result; + } + /** §4.3 `StatusReport.endpoints`, in device-id order for a stable readout. */ summaries(): EndpointSummary[] { return [...this.#live.entries()] @@ -91,11 +145,25 @@ export class EndpointRegistry { } /** - * §3.1: full reconcile against the desired set. Throws - * `mass_removal_refused` before touching anything — the guard has to be a - * gate, not a rollback. + * §3.1: full reconcile against the desired set. + * + * Refusals that can be decided from the plan alone — an unsupported role, + * `mass_removal_refused` — are thrown before anything is touched: those are + * gates, and a gate that half-applies is not a gate. + * + * Once mutation starts there is no such promise. matter.js gives us no + * transaction across several `add`/`close` calls, so a failure part-way + * through leaves a live set that is neither the old one nor the requested + * one. What we owe the plugin then is an honest account of it, which is the + * `reconcile aborted after N/M` line below plus the `internal` refusal — + * **the plugin should follow a failed `attach` with `get_status`** rather + * than assume either set. */ async reconcile(desired: readonly EndpointSpec[], replaceAll: boolean): Promise { + return this.serialize("reconcile", () => this.reconcileNow(desired, replaceAll)); + } + + private async reconcileNow(desired: readonly EndpointSpec[], replaceAll: boolean): Promise { for (const spec of desired) { this.assertSupported(spec.role); } @@ -107,13 +175,39 @@ export class EndpointRegistry { // new accessory. Loud, because ecosystems lose the old one's name. this.#log(`Recreating endpoint ${indigoDeviceId}: role changed`); } - await this.removeMany([...plan.remove, ...plan.recreate.map(spec => spec.indigoDeviceId)]); - for (const spec of [...plan.create, ...plan.recreate]) { - await this.create(spec); - } - for (const spec of plan.update) { - await this.update(spec); + const removals = [...plan.remove, ...plan.recreate.map(spec => spec.indigoDeviceId)]; + const total = removals.length + plan.create.length + plan.recreate.length + plan.update.length; + let done = 0; + let mutated = false; + + try { + await this.removeMany(removals, () => { + mutated = true; + done += 1; + }); + for (const spec of [...plan.create, ...plan.recreate]) { + await this.create(spec); + mutated = true; + done += 1; + } + for (const spec of plan.update) { + await this.update(spec); + done += 1; + } + } catch (error) { + // A part-applied reconcile still changed the bridged-node set, so + // controllers still need the nudge — skipping the bump because the + // batch failed is how a half-applied set becomes an invisible one. + if (mutated) { + await this.noteConfigurationChange(); + } + this.#log( + `reconcile aborted after ${done}/${total} operations; ` + + `live set now [${[...this.#live.keys()].sort((a, b) => a - b).join(", ")}] ` + + `(requested [${desired.map(spec => spec.indigoDeviceId).join(", ")}])`, + ); + throw error; } if (plan.create.length > 0 || plan.recreate.length > 0 || plan.remove.length > 0) { @@ -127,44 +221,52 @@ export class EndpointRegistry { /** §3.2 — create-or-update, idempotent, `role_change` on a role mismatch. */ async upsert(spec: EndpointSpec): Promise { - this.assertSupported(spec.role); - const existing = this.#live.get(spec.indigoDeviceId); - if (existing === undefined) { - const created = await this.create(spec); - await this.noteConfigurationChange(); - return { endpointNumber: Number(created.number) }; - } - if (existing.role !== spec.role) { - throw new ProtocolError( - ErrorCode.roleChange, - `endpoint ${spec.indigoDeviceId} is ${existing.role}; remove and re-add to change role`, - ); - } - await this.update(spec); - return { endpointNumber: Number(existing.endpoint.number) }; + return this.serialize("upsert_endpoint", async () => { + this.assertSupported(spec.role); + const existing = this.#live.get(spec.indigoDeviceId); + if (existing === undefined) { + const created = await this.create(spec); + await this.noteConfigurationChange(); + return { endpointNumber: Number(created.number) }; + } + if (existing.role !== spec.role) { + throw new ProtocolError( + ErrorCode.roleChange, + `endpoint ${spec.indigoDeviceId} is ${existing.role}; remove and re-add to change role`, + ); + } + await this.update(spec); + return { endpointNumber: Number(existing.endpoint.number) }; + }); } /** §3.3 — idempotent removal. The endpoint-number allocation is retained. */ async remove(indigoDeviceId: number): Promise { - const live = this.#live.get(indigoDeviceId); - if (live === undefined) { - return { removed: false }; - } - await this.closeOne(indigoDeviceId, live); - await this.noteConfigurationChange(); - return { removed: true }; + return this.serialize("remove_endpoint", async () => { + const live = this.#live.get(indigoDeviceId); + if (live === undefined) { + return { removed: false }; + } + await this.closeOne(indigoDeviceId, live); + await this.noteConfigurationChange(); + return { removed: true }; + }); } /** §3.4 — role-specific state keys, applied as local writes. */ async setState(indigoDeviceId: number, states: Record): Promise { - const live = this.require(indigoDeviceId); - await applyStates(live.endpoint, live.role, states); + return this.serialize("set_state", async () => { + const live = this.require(indigoDeviceId); + await applyStates(live.endpoint, live.role, states); + }); } /** §3.5 */ async setReachable(indigoDeviceId: number, reachable: boolean): Promise { - const live = this.require(indigoDeviceId); - await applyReachable(live.endpoint, reachable); + return this.serialize("set_reachable", async () => { + const live = this.require(indigoDeviceId); + await applyReachable(live.endpoint, reachable); + }); } /** Drop every listener. The endpoints themselves die with the ServerNode. */ @@ -195,7 +297,17 @@ export class EndpointRegistry { private async create(spec: EndpointSpec): Promise { const endpoint = createEndpoint(spec, this.options.productName); await this.options.aggregator.add(endpoint); - const unwatch = watchCommands(endpoint, spec, this.options.emit); + let unwatch: () => void; + try { + unwatch = watchCommands(endpoint, spec, this.options.emit, this.#log); + } catch (error) { + // The endpoint is already in the Matter tree but will never be in + // `#live`: take it back out rather than leave an accessory the + // registry does not know about (the same zombie {@link closeOne} + // exists to prevent, from the other end). + await endpoint.close().catch(() => undefined); + throw error; + } this.#live.set(spec.indigoDeviceId, { endpoint, role: spec.role, unwatch }); this.#log(`Endpoint ${spec.indigoDeviceId} (${spec.role}) added as number ${Number(endpoint.number)}`); return endpoint; @@ -204,29 +316,75 @@ export class EndpointRegistry { /** Label, reachability and state — everything an existing endpoint can change. */ private async update(spec: EndpointSpec): Promise { const live = this.require(spec.indigoDeviceId); - const info = live.endpoint.stateOf("bridgedDeviceBasicInformation") as { nodeLabel?: string }; - if (info.nodeLabel !== spec.label) { + // Both labels, because {@link applyLabel} writes both: comparing only + // `nodeLabel` would call an endpoint up to date while its `productLabel` + // still showed the name from two renames ago. + const info = live.endpoint.stateOf("bridgedDeviceBasicInformation") as { + nodeLabel?: string; + productLabel?: string; + }; + if (info.nodeLabel !== spec.label || info.productLabel !== spec.label) { await applyLabel(live.endpoint, spec.label); } await applyReachable(live.endpoint, spec.reachable); await applyStates(live.endpoint, live.role, spec.states); } + /** + * Close one endpoint and, only if that worked, forget it. + * + * The order is the whole point. Evicting first and closing after means a + * throwing `close()` leaves an accessory that is still in the Matter tree + * and still visible in every paired ecosystem, but that the registry denies + * exists: taps on it produce no `command` events, and the plugin's retry + * gets `{removed: false}` — a §3.3-idempotent *success* — so nothing ever + * reports the zombie. Closing first makes the failure a failure: the entry + * stays, its listeners are put back, and the plugin sees `internal` on a + * call it can honestly retry. + */ private async closeOne(indigoDeviceId: number, live: LiveEndpoint): Promise { + // Unwatch first so a close-time attribute change cannot emit a command + // for an endpoint on its way out; restored below if the close fails. live.unwatch(); + try { + await live.endpoint.close(); + } catch (error) { + live.unwatch = watchCommands( + live.endpoint, + { indigoDeviceId, role: live.role }, + this.options.emit, + this.#log, + ); + this.#log( + `Endpoint ${indigoDeviceId} failed to close; registry and Matter tree may disagree: ` + + describeErrorWithStack(error), + ); + throw new ProtocolError( + ErrorCode.internal, + `endpoint ${indigoDeviceId} failed to close: ${describeError(error)}`, + ); + } this.#live.delete(indigoDeviceId); - await live.endpoint.close(); this.#log(`Endpoint ${indigoDeviceId} removed`); } - /** PRD §5.3: ~100ms apart, so each removal is its own subscription update. */ - private async removeMany(indigoDeviceIds: readonly number[]): Promise { + /** + * PRD §5.3: ~100ms apart, so each removal is its own subscription update. + * + * `onRemoved` rather than a return count because the count matters most + * when this *throws*: a mid-batch failure aborts the remaining removals, + * and a returned tally would be thrown away exactly when §3.1's partial- + * failure account needs it. The state {@link closeOne} restored is intact + * either way; `reconcile` reports how far it got. + */ + private async removeMany(indigoDeviceIds: readonly number[], onRemoved: () => void): Promise { for (const [index, indigoDeviceId] of indigoDeviceIds.entries()) { const live = this.#live.get(indigoDeviceId); if (live === undefined) { continue; } await this.closeOne(indigoDeviceId, live); + onRemoved(); if (index < indigoDeviceIds.length - 1 && this.#pacingMs > 0) { await new Promise(resolve => setTimeout(resolve, this.#pacingMs)); } diff --git a/bridge-node/src/ws-server.ts b/bridge-node/src/ws-server.ts index 5f7a6cd..fb260f7 100644 --- a/bridge-node/src/ws-server.ts +++ b/bridge-node/src/ws-server.ts @@ -85,13 +85,19 @@ export class BridgeWsServer { } /** - * Push an unsolicited event (§1/§5) to the attached client. Dropped silently - * when nobody is attached: the plugin re-`attach`es on reconnect and gets a - * full reconcile, so a missed event has nothing to recover. + * Push an unsolicited event (§1/§5) to the attached client. + * + * Dropped when nobody is attached — the plugin re-`attach`es on reconnect + * and gets a full reconcile, so a missed event has nothing to recover — but + * never dropped *silently*: a run of these is how "I pressed the switch in + * Apple Home and Indigo did nothing" looks from the node's side, and + * without the line there is no way to tell it from a broken listener. */ sendEvent(event: EventNameValue, data: Record): void { const socket = this.#attached; if (socket === undefined) { + const device = typeof data.indigoDeviceId === "number" ? ` for device ${data.indigoDeviceId}` : ""; + this.#log(`Dropping ${event} event${device}: no client is attached`); return; } const frame: EventFrame = { event, data }; @@ -235,7 +241,13 @@ export class BridgeWsServer { try { const result = await handler(commandArgs, socket, state); - this.send(socket, { message_id: messageId, result }); + if (!this.send(socket, { message_id: messageId, result })) { + // §1 promises exactly one response per request. A result we + // could not put on the wire — a value that will not stringify, + // a socket that failed mid-write — still owes the plugin an + // answer, and `message_id` is still in scope to address it. + this.sendError(socket, messageId, ErrorCode.internal, `Could not send the result of ${command}`); + } } catch (error) { if (error instanceof ProtocolError) { this.sendError(socket, messageId, error.code, error.message); @@ -340,9 +352,28 @@ export class BridgeWsServer { this.send(socket, { message_id: messageId, error_code: code, details }); } - private send(socket: WebSocket, frame: unknown): void { - if (socket.readyState === socket.OPEN) { + /** + * Write one frame, reporting whether it went. + * + * Never throws: a serialisation failure here would otherwise escape into + * whatever was mid-flight (an observable, a handler chain) rather than into + * the response the caller is trying to send. + */ + private send(socket: WebSocket, frame: unknown): boolean { + if (socket.readyState !== socket.OPEN) { + // Almost always the superseded incumbent: §2 closes it the moment a + // new client attaches, and anything already queued for it lands in + // that CLOSING window. Worth a line — it is also what a genuinely + // wedged socket looks like. + this.#log(`Dropping a frame for a socket in readyState ${socket.readyState} (not OPEN)`); + return false; + } + try { socket.send(JSON.stringify(frame)); + return true; + } catch (error) { + this.#log(`Failed to send frame: ${describeErrorWithStack(error)}`); + return false; } } } diff --git a/bridge-node/test/fixture-shapes.ts b/bridge-node/test/fixture-shapes.ts index 8ae9b79..371c2d6 100644 --- a/bridge-node/test/fixture-shapes.ts +++ b/bridge-node/test/fixture-shapes.ts @@ -51,6 +51,9 @@ export const status = { { indigoDeviceId: 123456790, endpointNumber: 3, role: "dimmableLight" }, ], drift: [], + // §4.3: false until E5 persists the endpoint-number map — an empty `drift` + // on its own would read as an all-clear nobody has actually checked. + driftChecked: false, } satisfies StatusReport; /** @@ -64,6 +67,7 @@ export const statusEmpty = { endpointCount: 0, endpoints: [], drift: [], + driftChecked: false, } satisfies StatusReport; /** §3.1 with `intent: "replace_all"`: the live set is emptied deliberately. */ @@ -73,6 +77,7 @@ export const statusReplaceAll = { endpointCount: 0, endpoints: [], drift: [], + driftChecked: false, } satisfies StatusReport; /** §3.2 — the live endpoint's Matter number, for the plugin's own records. */ @@ -106,6 +111,29 @@ export const setStateUnknownDevice = { details: "no live endpoint for indigoDeviceId 123456791", } satisfies ErrorFrame; +/** + * §3.4 against a live device, with keys its role does not speak. + * + * The sibling of {@link setStateUnknownDevice}, and the more dangerous of the + * two: the device exists, so nothing looks wrong. Answering `{}` here would + * report success for a write that produced no patch at all, and the plugin — + * which does not await `set_state` — would never find out. `level` on an + * `onOffLight` is the shape of it that actually happens: a role edited in the + * export dialog while the plugin keeps pushing the old role's states. + */ +export const setStateBadKeys = { + message_id: "m44", + error_code: "malformed_args", + details: "role onOffLight consumed none of the states given; rejected key(s): level (§4.2)", +} satisfies ErrorFrame; + +/** §4.2/§1.1: a lawful frame naming a role outside the v1 enum. */ +export const upsertUnknownRole = { + message_id: "m29", + error_code: "unknown_role", + details: "role airPurifier is not in the v1 role enum (§4.2)", +} satisfies ErrorFrame; + /** §3.7 state 1: never commissioned — the basic window with the persisted codes. */ export const pairingUncommissioned = { commissioned: false, diff --git a/bridge-node/test/fixtures.test.ts b/bridge-node/test/fixtures.test.ts index fa3bcb0..d3798ae 100644 --- a/bridge-node/test/fixtures.test.ts +++ b/bridge-node/test/fixtures.test.ts @@ -42,10 +42,12 @@ describe("golden fixtures match their typed mirror", () => { ["attach mass_removal_refused", golden.attach_mass_removal_refused.response, shapes.massRemovalRefused], ["upsert_endpoint result", golden.upsert_endpoint.response.result, shapes.upsertResult], ["upsert_endpoint role_change", golden.upsert_endpoint_role_change.response, shapes.roleChange], + ["upsert_endpoint unknown_role", golden.upsert_endpoint_unknown_role.response, shapes.upsertUnknownRole], ["remove_endpoint result", golden.remove_endpoint.response.result, shapes.removeResult], ["remove_endpoint (absent) result", golden.remove_endpoint_absent.response.result, shapes.removeAbsentResult], ["set_state result", golden.set_state.response.result, shapes.emptyResult], ["set_state unknown_device", golden.set_state_unknown_device.response, shapes.setStateUnknownDevice], + ["set_state malformed_args (bad keys)", golden.set_state_bad_keys.response, shapes.setStateBadKeys], ["set_reachable result", golden.set_reachable.response.result, shapes.emptyResult], ["get_pairing (uncommissioned)", golden.get_pairing_uncommissioned.response.result, shapes.pairingUncommissioned], ["get_pairing (commissioned)", golden.get_pairing_commissioned.response.result, shapes.pairingCommissioned], diff --git a/bridge-node/test/integration.test.ts b/bridge-node/test/integration.test.ts new file mode 100644 index 0000000..78cabcd --- /dev/null +++ b/bridge-node/test/integration.test.ts @@ -0,0 +1,169 @@ +/** + * The three layers wired together: a real {@link BridgeNode} on a real Matter + * stack, behind a real {@link BridgeWsServer}, talked to over a real socket. + * + * Every other suite cuts one of those seams — `protocol.test.ts` stubs the + * bridge, `registry.test.ts` drives the registry directly with no protocol at + * all — which leaves the wiring *between* them untested: that `node.ts` routes + * its `#command` sink into the WebSocket server's `command` event at all, that + * `attach` composes reconcile with `getStatus`, that a `set_state` arriving as + * a wire frame reaches a matter.js attribute. Each of those is one line of glue + * that no unit test can fail on. + * + * This file gets its own process (node's test runner forks per file), which is + * what lets it use `Environment.default` — `BridgeNode.start` sets `storage.path` + * on it, and matter.js takes an exclusive lock per storage path. + */ + +import assert from "node:assert/strict"; +import { mkdirSync, mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { after, describe, it } from "node:test"; + +import { type Endpoint, Logger } from "@matter/main"; + +import { endpointIdFor } from "../src/endpoints.js"; +import { BridgeNode, matterJsVersion } from "../src/node.js"; +import { PROTOCOL_VERSION } from "../src/protocol.js"; +import { BridgeWsServer } from "../src/ws-server.js"; +import { TestClient } from "./client.js"; + +const BRIDGE_VERSION = "0.1.0-test"; +const KITCHEN = 123456789; +const LOUNGE = 123456790; + +Logger.level = "fatal"; + +const SCRATCH_ROOT = process.env.INDIGO_MATTER_TEST_SCRATCH ?? tmpdir(); +mkdirSync(SCRATCH_ROOT, { recursive: true }); +const storagePath = mkdtempSync(join(SCRATCH_ROOT, "indigo-matter-integration-")); + +after(() => rmSync(storagePath, { recursive: true, force: true })); + +/** A matter.js `$Changed` observable, as much of it as this test drives. */ +interface ChangeObservable { + emit(value: unknown, oldValue: unknown, context: unknown): void; +} + +/** + * Stand in for a controller writing an attribute — the same lever + * `registry.test.ts` uses, and for the same reason: a genuine remote write + * needs a commissioned fabric and a session, which is a great deal of + * machinery to prove that a non-offline context reaches our handler. + */ +function ecosystemWrite(endpoint: Endpoint, behavior: string, attribute: string, value: unknown, previous: unknown) { + const events = endpoint.eventsOf(behavior) as unknown as Record; + const observable = events[`${attribute}$Changed`]; + assert.ok(observable !== undefined, `${behavior}.${attribute}$Changed does not exist`); + observable.emit(value, previous, { offline: false }); +} + +describe("plugin ⇄ node, end to end", () => { + it("attaches, applies a set_state, and reports an ecosystem write on the wire", async () => { + const bridge = new BridgeNode( + // Port 0 on both: matter.js and `ws` each bind an ephemeral port, so + // a parallel run cannot collide with this one. + { storagePath, matterPort: 0, wsPort: 0 }, + { installId: "integration0001", passcode: 20202021, discriminator: 3840 }, + BRIDGE_VERSION, + () => {}, + ); + await bridge.start(); + + const server = new BridgeWsServer({ + port: 0, + bridge, + bridgeVersion: BRIDGE_VERSION, + matterJsVersion, + log: () => {}, + }); + await server.listen(); + + const client = await TestClient.connect(server.port); + try { + // §2: the bare handshake, from the real node's real versions. + const handshake = await client.next(); + assert.equal(handshake.protocolVersion, PROTOCOL_VERSION); + assert.equal(handshake.bridgeVersion, BRIDGE_VERSION); + assert.equal(handshake.matterJsVersion, matterJsVersion); + + // §3.1 — attach is a full reconcile whose result is a StatusReport. + const attached = await client.request({ + message_id: "i1", + command: "attach", + args: { + protocolVersion: PROTOCOL_VERSION, + pluginVersion: "2026.8.1", + endpoints: [ + { + indigoDeviceId: KITCHEN, + role: "onOffLight", + label: "Kitchen Lamp", + reachable: true, + states: { onOff: false }, + options: {}, + }, + { + indigoDeviceId: LOUNGE, + role: "dimmableLight", + label: "Lounge Lamp", + reachable: true, + states: { onOff: true, level: 60 }, + options: {}, + }, + ], + }, + }); + const status = attached.result as Record; + assert.equal(status.endpointCount, 2); + assert.equal(status.driftChecked, false, "§4.3: an empty drift is not an all-clear"); + assert.deepEqual( + (status.endpoints as { indigoDeviceId: number }[]).map(endpoint => endpoint.indigoDeviceId), + [KITCHEN, LOUNGE], + ); + + // §6.2: attach answers exactly what get_status would. + const queried = await client.request({ message_id: "i2", command: "get_status", args: {} }); + assert.deepEqual(queried.result, status); + + // §3.4 — a wire frame reaching a matter.js attribute. + assert.deepEqual( + await client.request({ + message_id: "i3", + command: "set_state", + args: { indigoDeviceId: LOUNGE, states: { level: 100 } }, + }), + { message_id: "i3", result: {} }, + ); + + const aggregator = [...bridge.server.parts].find(part => part.id === "aggregator"); + assert.ok(aggregator !== undefined); + const lounge = [...aggregator.parts].find(part => part.id === endpointIdFor(LOUNGE)); + assert.ok(lounge !== undefined, "the attach did not build the lounge endpoint"); + assert.equal((lounge.stateOf("levelControl") as Record).currentLevel, 254); + + // §6.4: our own write must not come back as a command. + await assert.rejects(client.next(200), "a local set_state echoed onto the wire"); + + // §5 — an ecosystem acting on the endpoint arrives as a `command` + // frame. This is the whole point of the file: the path runs through + // watchCommands → registry emit → BridgeNode#command → sendEvent. + ecosystemWrite(lounge, "levelControl", "currentLevel", 152, 254); + assert.deepEqual(await client.next(), { + event: "command", + data: { indigoDeviceId: LOUNGE, command: "setLevel", args: { level: 60 } }, + }); + + ecosystemWrite(lounge, "onOff", "onOff", false, true); + assert.deepEqual(await client.next(), { + event: "command", + data: { indigoDeviceId: LOUNGE, command: "onOff", args: { value: false } }, + }); + } finally { + client.close(); + await server.close(); + await bridge.close(); + } + }); +}); diff --git a/bridge-node/test/protocol.test.ts b/bridge-node/test/protocol.test.ts index 0aeb6de..9a97ab4 100644 --- a/bridge-node/test/protocol.test.ts +++ b/bridge-node/test/protocol.test.ts @@ -149,6 +149,44 @@ describe("attach (§3.1)", () => { }); }); + it("keeps an endpoint's number across a removal and a role change", async () => { + // The double models this because the real registry does: matter.js keys + // its persisted number on `Endpoint.id`, so a device that goes away and + // comes back — or changes role, which §3.1 implements as remove+re-add — + // comes back as the same accessory number. A double that renumbered + // would have made the golden StatusReports true only on a cold bridge. + await withColdBridge(async (cold, connectCold) => { + cold.statusCommissioned = true; + cold.statusFabrics = [APPLE_HOME]; + const client = await connectCold(); + await client.request(golden.attach_with_endpoints.request); + + // Empty it, then bring the same two devices back — with the first + // one's role changed, which is the recreate path. + await client.request(golden.attach_replace_all.request); + const specs = golden.attach_with_endpoints.request.args as { endpoints: Record[] }; + const rerun = await client.request({ + message_id: "renumber", + command: "attach", + args: { + protocolVersion: PROTOCOL_VERSION, + pluginVersion: "t", + endpoints: [{ ...specs.endpoints[0], role: "dimmableLight", states: {} }, specs.endpoints[1]], + }, + }); + const endpoints = (rerun.result as { endpoints: { indigoDeviceId: number; endpointNumber: number }[] }) + .endpoints; + assert.deepEqual( + endpoints.map(endpoint => [endpoint.indigoDeviceId, endpoint.endpointNumber]), + [ + [123456789, 2], + [123456790, 3], + ], + ); + client.close(); + }); + }); + it("rejects a malformed endpoint set without disturbing the live one", async () => { await withColdBridge(async (cold, connectCold) => { cold.statusCommissioned = true; @@ -225,6 +263,46 @@ describe("attach (§3.1)", () => { second.close(); }); + it("keeps the incumbent when a second socket's attach is refused", async () => { + // §3.1 parses the endpoint set *before* attaching state changes hands. + // If that order ever inverts, a plugin sending one bad spec would hang + // up on the healthy connection currently carrying every command event + // and then fail to attach itself, leaving the bridge with no client at + // all — worse than the malformed attach it was rejecting. + await withColdBridge(async (cold, connectCold) => { + cold.statusCommissioned = true; + cold.statusFabrics = [APPLE_HOME]; + const incumbent = await connectCold(); + await incumbent.request(golden.attach_with_endpoints.request); + + const usurper = await connectCold(); + for (const endpoints of [ + [{ indigoDeviceId: 1, role: "airPurifier", label: "x" }], // unknown_role + "not-an-array", // malformed_args + ]) { + const refusal = await usurper.request({ + message_id: "usurp", + command: "attach", + args: { protocolVersion: PROTOCOL_VERSION, pluginVersion: "t", endpoints }, + }); + assert.ok(refusal.error_code !== undefined, JSON.stringify(endpoints)); + assert.equal(incumbent.closed, false, "the incumbent was hung up on by a refused attach"); + } + + // Still *the* attached client: events go to it, and commands work. + cold.emitCommand(golden.command_on_off.data as never); + assert.deepEqual(await incumbent.next(), golden.command_on_off); + assert.deepEqual(await incumbent.request(golden.get_status.request), golden.get_status.response); + + // And the refused socket never became attached in passing. + const gated = await usurper.request({ message_id: "g", command: "get_status", args: {} }); + assert.equal(gated.error_code, ErrorCode.notAttached); + + usurper.close(); + incumbent.close(); + }); + }); + it("re-attaches the same socket without closing it", async () => { // §2 supersession must not fire on the incumbent when it *is* the // socket attaching: a plugin that re-attaches to refresh its endpoint @@ -265,6 +343,29 @@ describe("gating (§1.1)", () => { client.close(); }); + it("refuses every §3 command before attach, not just the one with a golden frame", async () => { + // Enumerated rather than spot-checked: `not_attached` is a per-command + // gate in `onMessage`, and a command added to the handler map without a + // thought for the gate would otherwise be reachable on an un-attached + // socket. Every §3 command the node implements, minus `attach` itself. + const gated: [string, Record][] = [ + ["get_status", {}], + ["get_pairing", {}], + ["open_commissioning_window", { durationSeconds: 900 }], + ["upsert_endpoint", golden.upsert_endpoint.request.args as Record], + ["remove_endpoint", { indigoDeviceId: 123456789 }], + ["set_state", { indigoDeviceId: 123456789, states: { onOff: true } }], + ["set_reachable", { indigoDeviceId: 123456789, reachable: false }], + ]; + const client = await connect(); + for (const [command, args] of gated) { + const response = await client.request({ message_id: `gate-${command}`, command, args }); + assert.equal(response.error_code, ErrorCode.notAttached, `${command} answered before attach`); + assert.equal(response.message_id, `gate-${command}`); + } + client.close(); + }); + it("returns malformed_args when command is missing", async () => { const client = await connect(); const response = await client.request({ message_id: "nocmd" }); @@ -307,6 +408,14 @@ describe("endpoint CRUD (§3.2-§3.5)", () => { await client.request(golden.upsert_endpoint_role_change.request), golden.upsert_endpoint_role_change.response, ); + // §1.1: a lawfully-shaped endpoint naming a role outside §4.2 gets + // its own code, decided in `parseEndpointSpec` before the facade is + // reached — which is why the live set is untouched below. + assert.deepEqual( + await client.request(golden.upsert_endpoint_unknown_role.request), + golden.upsert_endpoint_unknown_role.response, + ); + assert.equal(cold.model.has(900099), false, "an unknown role must not create an endpoint"); // §3.4/§3.5 against a live device, and against one that is not. assert.deepEqual(await client.request(golden.set_state.request), golden.set_state.response); assert.deepEqual( @@ -537,6 +646,31 @@ describe("frame hygiene (§1)", () => { }); }); +describe("exactly one response (§1)", () => { + it("answers internal when the result itself cannot be put on the wire", async () => { + // A handler can succeed and the response still fail — a value that will + // not serialise, a socket that dies mid-write. §1 promises one response + // per request either way, and the plugin correlates on message_id, so + // silence here would strand a future until its timeout. + const client = await connect(); + await attach(client); + bridge.poisonStatus = true; + try { + const response = await client.request({ message_id: "poison", command: "get_status", args: {} }); + assert.deepEqual(response, { + message_id: "poison", + error_code: ErrorCode.internal, + details: "Could not send the result of get_status", + }); + } finally { + bridge.poisonStatus = false; + } + // And the socket is still usable afterwards. + assert.deepEqual(await client.request(golden.get_status.request), golden.get_status.response); + client.close(); + }); +}); + describe("ordering (§1)", () => { it("answers pipelined frames in receipt order even when the first awaits", async () => { // open_commissioning_window genuinely awaits (crypto); a get_pairing @@ -585,12 +719,13 @@ describe("window_closed event (§3.8/§5)", () => { // Its own server and stub: `#attached` is server-wide state, and the // shared server has attached clients throughout this file. const lonelyBridge = new StubBridge(); + const logs: string[] = []; const lonely = new BridgeWsServer({ port: 0, bridge: lonelyBridge, bridgeVersion: BRIDGE_VERSION, matterJsVersion: MATTER_JS_VERSION, - log: () => {}, + log: message => logs.push(message), }); await lonely.listen(); try { @@ -607,5 +742,31 @@ describe("window_closed event (§3.8/§5)", () => { } finally { await lonely.close(); } + + // Dropped, but not in silence: a run of these is what "the ecosystem + // does nothing" looks like from here, and it must be distinguishable + // from a listener that was never wired up. + assert.equal(logs.filter(line => line.includes("Dropping window_closed event")).length, 2, logs.join("\n")); + }); + + it("names the device when it drops a command event with nobody attached", async () => { + const orphanBridge = new StubBridge(); + const logs: string[] = []; + const orphan = new BridgeWsServer({ + port: 0, + bridge: orphanBridge, + bridgeVersion: BRIDGE_VERSION, + matterJsVersion: MATTER_JS_VERSION, + log: message => logs.push(message), + }); + await orphan.listen(); + try { + orphanBridge.emitCommand(golden.command_on_off.data as never); + } finally { + await orphan.close(); + } + const dropped = logs.filter(line => line.includes("Dropping command event")); + assert.equal(dropped.length, 1, logs.join("\n")); + assert.match(dropped[0] ?? "", /for device 123456789/); }); }); diff --git a/bridge-node/test/registry.test.ts b/bridge-node/test/registry.test.ts index cfdf661..6118988 100644 --- a/bridge-node/test/registry.test.ts +++ b/bridge-node/test/registry.test.ts @@ -23,7 +23,7 @@ import { Endpoint, Environment, Logger, ServerNode, VendorId } from "@matter/mai import { BasicInformationServer } from "@matter/main/behaviors/basic-information"; import { AggregatorEndpoint } from "@matter/main/endpoints/aggregator"; -import { endpointIdFor, uniqueIdFor } from "../src/endpoints.js"; +import { endpointIdFor, uniqueIdFor, watchCommands } from "../src/endpoints.js"; import { type CommandEventData, type EndpointSpec, @@ -69,7 +69,9 @@ interface Harness { * nothing here needs the network: endpoint numbers, state and change events are * all assigned at `add()` time. */ -async function harness(options: { storagePath?: string; removalPacingMs?: number } = {}): Promise { +async function harness( + options: { storagePath?: string; removalPacingMs?: number; emitThrows?: boolean } = {}, +): Promise { const storagePath = options.storagePath ?? mkdtempSync(join(SCRATCH_ROOT, "indigo-matter-registry-")); if (options.storagePath === undefined) { scratchRoots.push(storagePath); @@ -102,7 +104,12 @@ async function harness(options: { storagePath?: string; removalPacingMs?: number const registry = new EndpointRegistry({ aggregator, productName: PRODUCT_NAME, - emit: data => commands.push(data), + emit: data => { + commands.push(data); + if (options.emitThrows === true) { + throw new Error("the command sink blew up"); + } + }, log: message => logs.push(message), removalPacingMs: options.removalPacingMs ?? 0, // The real wiring from node.ts, not a counter: the point of testing it @@ -254,7 +261,9 @@ describe("role factory", () => { ErrorCode.internal, ); assert.match(error.message, /not implemented by this bridge version/); - // Nothing partially applied: the refusal is a gate. + // A role refusal is decided from the plan alone, so it is a genuine + // gate — nothing was touched. (Failures *after* mutation starts get + // no such promise; see the partial-reconcile test below.) assert.equal(h.registry.size, 0); } finally { await h.close(); @@ -330,6 +339,76 @@ describe("reconcile (§3.1)", () => { } }); + it("refuses a mixed set whole, leaving even the planned removals in place", async () => { + // The dangerous shape: one supported role and one E4 role, against a + // POPULATED live set. If the role check ever moved after planning — or + // worse, into the per-spec loop — the removals would run first and the + // user would lose accessories to a request that was refused anyway. + const h = await harness(); + try { + await h.registry.reconcile([spec(1, Role.onOffLight), spec(2, Role.dimmableLight)], false); + const before = h.registry.summaries(); + + await rejects( + // 2 is dropped from the desired set, so it is a planned removal; + // 3 is a lawful v1 role this bridge cannot build. + () => h.registry.reconcile([spec(1, Role.onOffLight), spec(3, Role.doorLock)], false), + ErrorCode.internal, + ); + + assert.deepEqual(h.registry.summaries(), before, "the live set moved under a refused reconcile"); + assert.equal([...h.aggregator.parts].length, 2, "the Matter tree moved under a refused reconcile"); + } finally { + await h.close(); + } + }); + + it("refuses an upsert of an E4 role without touching the live set", async () => { + const h = await harness(); + try { + await h.registry.reconcile([spec(1, Role.onOffLight)], false); + const before = h.registry.summaries(); + await rejects(() => h.registry.upsert(spec(2, Role.thermostat)), ErrorCode.internal); + assert.deepEqual(h.registry.summaries(), before); + assert.equal([...h.aggregator.parts].length, 1); + } finally { + await h.close(); + } + }); + + it("accounts for a reconcile that fails after it has started mutating", async () => { + // Once matter.js has been asked to add or close anything there is no + // transaction to roll back to, so the contract is honesty rather than + // atomicity: bump ConfigurationVersion because the set really did + // change, and say how far it got and what is live now. + const h = await harness(); + const version = (): number => + (h.node.state.basicInformation as { configurationVersion?: number }).configurationVersion ?? 0; + try { + await h.registry.reconcile([spec(1, Role.onOffLight), spec(2, Role.onOffLight)], false); + const start = version(); + + // 2 is removed (mutation happens), then 3 is created with states no + // onOffLight can consume — a failure with the batch half-applied. + await rejects( + () => + h.registry.reconcile( + [spec(1, Role.onOffLight), spec(3, Role.onOffLight, { states: { level: 50 } })], + false, + ), + ErrorCode.malformedArgs, + ); + + assert.deepEqual(h.registry.summaries().map(s => s.indigoDeviceId), [1]); + assert.equal(version(), start + 1, "a part-applied reconcile still changed the bridged-node set"); + const aborted = h.logs.filter(line => line.startsWith("reconcile aborted after")); + assert.equal(aborted.length, 1, h.logs.join("\n")); + assert.equal(aborted[0], "reconcile aborted after 1/3 operations; live set now [1] (requested [1, 3])"); + } finally { + await h.close(); + } + }); + it("paces bulk removals, and the pacing is injectable", async () => { const h = await harness({ removalPacingMs: 20 }); try { @@ -398,6 +477,149 @@ describe("upsert / remove (§3.2, §3.3)", () => { }); }); +/** Make one endpoint's `close()` fail, and return the switch that stops it. */ +function breakClose(endpoint: Endpoint): { restore: () => void } { + const real = endpoint.close.bind(endpoint); + let broken = true; + (endpoint as unknown as { close: () => Promise }).close = async () => { + if (broken) { + throw new Error("matter.js refused to close the endpoint"); + } + await real(); + }; + return { + restore: () => { + broken = false; + }, + }; +} + +describe("a close that fails (§3.3)", () => { + it("keeps the endpoint rather than orphaning it in the Matter tree", async () => { + // The zombie this prevents: evict-then-close meant a throwing close left + // an accessory that every paired ecosystem still shows and still routes + // taps to, while the registry denied it existed — so its commands went + // nowhere and the plugin's retry got `{removed: false}`, which §3.3 + // defines as *success*. Nothing anywhere would ever have reported it. + const h = await harness(); + try { + await h.registry.reconcile([spec(1, Role.onOffLight)], false); + const endpoint = [...h.aggregator.parts][0]; + assert.ok(endpoint !== undefined); + const close = breakClose(endpoint); + + const error = await rejects(() => h.registry.remove(1), ErrorCode.internal); + assert.match(error.message, /endpoint 1 failed to close/); + assert.ok( + h.logs.some(line => line.includes("registry and Matter tree may disagree")), + h.logs.join("\n"), + ); + + // Still known, so a retry is an honest retry rather than a lie. + assert.equal(h.registry.size, 1); + // And still listening: the accessory is live in the ecosystem, so + // the commands it produces must still reach the plugin. + ecosystemWrite(endpoint, "onOff", "onOff", true, false); + assert.deepEqual(h.commands, [{ indigoDeviceId: 1, command: "onOff", args: { value: true } }]); + + close.restore(); + assert.deepEqual(await h.registry.remove(1), { removed: true }); + assert.equal(h.registry.size, 0); + } finally { + await h.close(); + } + }); + + it("aborts a paced batch mid-way with the failed endpoint still accounted for", async () => { + const h = await harness({ removalPacingMs: 0 }); + try { + await h.registry.reconcile([1, 2, 3].map(id => spec(id, Role.onOffLight)), false); + const second = [...h.aggregator.parts].find(part => part.id === endpointIdFor(2)); + assert.ok(second !== undefined); + breakClose(second); + + await rejects(() => h.registry.reconcile([], true), ErrorCode.internal); + + // 1 was removed, 2 failed and stayed, 3 was never attempted — and + // the log says exactly that rather than implying a clean rollback. + assert.deepEqual(h.registry.summaries().map(s => s.indigoDeviceId), [2, 3]); + assert.ok( + h.logs.includes("reconcile aborted after 1/3 operations; live set now [2, 3] (requested [])"), + h.logs.join("\n"), + ); + } finally { + await h.close(); + } + }); +}); + +describe("one mutation at a time (§3.1)", () => { + it("makes a second reconcile wait for the one in flight", async () => { + // The real window: a plugin crash leaves a half-open socket, launchd + // restarts the plugin, and its attach lands on a new socket while the + // incumbent's paced reconcile is still seconds from finishing. Both + // then diff and mutate the same live set. Asserted by log order, + // because that is what shows the second one genuinely waited rather + // than interleaving into a set that happens to look right at the end. + const h = await harness({ removalPacingMs: 25 }); + try { + await h.registry.reconcile([1, 2, 3].map(id => spec(id, Role.onOffLight)), false); + h.logs.length = 0; + + const first = h.registry.reconcile([spec(1, Role.onOffLight)], false); + const second = h.registry.reconcile( + [spec(1, Role.onOffLight), spec(4, Role.dimmableLight)], + false, + ); + await Promise.all([first, second]); + + assert.deepEqual(h.registry.summaries().map(s => s.indigoDeviceId), [1, 4]); + assert.ok( + h.logs.some(line => line === "reconcile is waiting for an unfinished reconcile to release the registry"), + h.logs.join("\n"), + ); + + const firstDone = h.logs.findIndex(line => line.startsWith("Reconciled endpoints:")); + const secondStarted = h.logs.findIndex(line => line.startsWith("Endpoint 4 (dimmableLight) added")); + assert.ok(firstDone >= 0 && secondStarted >= 0, h.logs.join("\n")); + assert.ok( + firstDone < secondStarted, + `the second reconcile mutated before the first finished:\n${h.logs.join("\n")}`, + ); + } finally { + await h.close(); + } + }); + + it("queues set_state and remove behind an in-flight reconcile", async () => { + const h = await harness({ removalPacingMs: 25 }); + try { + await h.registry.reconcile([1, 2, 3].map(id => spec(id, Role.dimmableLight)), false); + h.logs.length = 0; + + const reconcile = h.registry.reconcile([spec(1, Role.dimmableLight)], false); + // 2 is about to be removed by the reconcile above. Without the + // queue this races it; with it, the removal is simply already done + // and §3.3 answers the idempotent `false`. + const removal = h.registry.remove(2); + const write = h.registry.setState(1, { level: 40 }); + await reconcile; + assert.deepEqual(await removal, { removed: false }); + await write; + + const dim = [...h.aggregator.parts].find(part => part.id === endpointIdFor(1)); + assert.equal((dim?.stateOf("levelControl") as Record).currentLevel, 102); + assert.equal( + h.logs.filter(line => line.includes("waiting for an unfinished reconcile")).length, + 2, + h.logs.join("\n"), + ); + } finally { + await h.close(); + } + }); +}); + describe("set_state (§3.4)", () => { it("writes each role's state keys through its converter", async () => { const h = await harness(); @@ -438,19 +660,68 @@ describe("set_state (§3.4)", () => { } }); - it("ignores state keys outside the role's vocabulary", async () => { + it("refuses a set_state whose keys the role does not speak", async () => { + // This used to answer success. An onOffLight has no LevelControl, so + // `{level: 50}` produced an empty patch, `applyStates` returned early + // and §3.4 replied `{}` — and because the plugin does not await + // `set_state`, a whole role's worth of writes could go nowhere with + // nothing anywhere saying so. The symptom (ecosystem shows stale state) + // is identical to the bridge being down. const h = await harness(); try { await h.registry.reconcile([spec(1, Role.onOffLight)], false); - // An onOffLight has no LevelControl; a stray `level` must be a no-op, - // not a crash — the plugin and node version-skew across a release. - await h.registry.setState(1, { level: 50, occupied: true }); + for (const states of [ + { level: 50, occupied: true }, // another role's vocabulary + { level: 55 }, // the golden set_state_bad_keys frame + { onOf: true }, // misspelt + { onOff: "true" }, // right key, wrong type + ]) { + const error = await rejects(() => h.registry.setState(1, states), ErrorCode.malformedArgs); + for (const key of Object.keys(states)) { + assert.ok(error.message.includes(key), `${error.message} should name ${key}`); + } + } assert.deepEqual(h.commands, []); } finally { await h.close(); } }); + it("produces the golden set_state_bad_keys details verbatim (§7)", async () => { + // The golden file carries this refusal for both suites; the string in + // it has to be the one the node really builds. + const h = await harness(); + try { + await h.registry.reconcile([spec(123456789, Role.onOffLight)], false); + const error = await rejects( + () => h.registry.setState(123456789, { level: 55 }), + ErrorCode.malformedArgs, + ); + assert.equal( + error.message, + "role onOffLight consumed none of the states given; rejected key(s): level (§4.2)", + ); + } finally { + await h.close(); + } + }); + + it("still accepts an empty states, and a partly-understood one", async () => { + const h = await harness(); + try { + await h.registry.reconcile([spec(1, Role.dimmableLight)], false); + // §3.4 with nothing to say is a lawful no-op, not a refusal. + await h.registry.setState(1, {}); + // Version skew: a newer plugin may send a key this bridge does not + // know yet. The keys it does know must still be applied. + await h.registry.setState(1, { onOff: true, futureKey: 1 }); + const dim = [...h.aggregator.parts][0]; + assert.equal((dim?.stateOf("onOff") as Record).onOff, true); + } finally { + await h.close(); + } + }); + it("applies §3.5 reachability to Bridged Device Basic Information", async () => { const h = await harness(); try { @@ -570,6 +841,103 @@ describe("command events and the echo guard (§4.2, §6.4)", () => { }); }); +describe("listener wiring is not allowed to fail quietly (§4.2)", () => { + it("throws at construction when a watched observable is missing", async () => { + // Simulates the regression the throw exists for: matter.js renames an + // attribute, or drops one with a feature, and the observable this role + // subscribes to is simply not there. Skipping it — which is what the + // code used to do — leaves the accessory fully controllable in the + // ecosystem with its entire `setLevel` family silently disconnected, + // and nothing in any log to say why Indigo stopped responding. + const h = await harness(); + try { + await h.registry.reconcile([spec(1, Role.dimmableLight)], false); + const endpoint = [...h.aggregator.parts][0]; + assert.ok(endpoint !== undefined); + + // A Proxy, not a spread: matter.js serves these observables from the + // prototype, so a spread would drop *all* of them and the test would + // pass for the wrong reason (on the first watch, not the one meant). + const realEventsOf = endpoint.eventsOf.bind(endpoint); + (endpoint as unknown as { eventsOf: (behavior: string) => unknown }).eventsOf = (behavior: string) => { + const events = realEventsOf(behavior) as Record; + if (behavior !== "levelControl") { + return events; + } + return new Proxy(events, { + get: (target, property, receiver) => + property === "currentLevel$Changed" ? undefined : Reflect.get(target, property, receiver), + }); + }; + + assert.throws( + () => watchCommands(endpoint, { indigoDeviceId: 1, role: Role.dimmableLight }, () => {}), + (error: unknown) => { + assert.ok(error instanceof ProtocolError); + assert.equal(error.code, ErrorCode.internal); + assert.match(error.message, /endpoint 1 \(dimmableLight\)/); + assert.match(error.message, /levelControl\.currentLevel\$Changed/); + return true; + }, + ); + } finally { + await h.close(); + } + }); + + it("contains a throwing command sink instead of taking the process down", async () => { + // The handler runs inside matter.js's observable, inside the commit of + // a remote write. An escaping throw becomes an unhandled rejection, and + // this process exits on those — one bad payload would take the bridge + // and every exported accessory with it. + const h = await harness({ emitThrows: true }); + try { + await h.registry.reconcile([spec(1, Role.onOffLight)], false); + const endpoint = [...h.aggregator.parts][0]; + assert.ok(endpoint !== undefined); + + assert.doesNotThrow(() => ecosystemWrite(endpoint, "onOff", "onOff", true, false)); + + const dropped = h.logs.filter(line => line.includes("Dropped a onOff.onOff change for endpoint 1")); + assert.equal(dropped.length, 1, h.logs.join("\n")); + // The stack, not just the message: this is a node-side bug report. + assert.match(dropped[0] ?? "", /the command sink blew up/); + assert.match(dropped[0] ?? "", /at /); + + // And the endpoint is still usable afterwards. + assert.doesNotThrow(() => ecosystemWrite(endpoint, "onOff", "onOff", false, true)); + assert.equal(h.commands.length, 2); + } finally { + await h.close(); + } + }); +}); + +describe("LevelControl transition handling (§6.4)", () => { + it("pins managedTransitionTimeHandling off on every level-bearing role", async () => { + // With managed transitions on, matter.js drives a moveToLevel in steps + // of its own — written offline — and the echo guard would eat every one + // of them, so an ecosystem dimming a lamp would never reach Indigo. + // 0.17.8 already defaults this off; the pin is what stops a future + // default flip severing the whole setLevel family in silence. + const h = await harness(); + try { + const roles = [Role.dimmableLight, Role.colorTemperatureLight, Role.extendedColorLight]; + await h.registry.reconcile( + roles.map((role, index) => spec(700_001 + index, role)), + false, + ); + for (const [index, role] of roles.entries()) { + const endpoint = [...h.aggregator.parts].find(part => part.id === endpointIdFor(700_001 + index)); + const level = endpoint?.stateOf("levelControl") as Record; + assert.equal(level.managedTransitionTimeHandling, false, `${role} manages transitions`); + } + } finally { + await h.close(); + } + }); +}); + describe("ConfigurationVersion (PRD §5.3)", () => { it("bumps the bridge's version when the endpoint set changes, and not otherwise", async () => { const h = await harness(); @@ -617,6 +985,12 @@ describe("endpoint-number stability (PRD §4.3 / XAC5)", () => { second.registry.summaries().map(s => [s.indigoDeviceId, s.endpointNumber]), before, ); + // Nothing on the wire from a restart. Rebuilding the endpoints + // writes their initial state, and if any of those writes reached + // the plugin as a `command` the bridge would replay the whole + // export back at Indigo on every node restart — every exported + // device commanded to whatever the node happened to have persisted. + assert.deepEqual(second.commands, [], "a restart sprayed command events"); } finally { await second.close(); } diff --git a/bridge-node/test/stub-bridge.ts b/bridge-node/test/stub-bridge.ts index 4c6c7a4..f4898e1 100644 --- a/bridge-node/test/stub-bridge.ts +++ b/bridge-node/test/stub-bridge.ts @@ -65,10 +65,12 @@ export interface GoldenFrames { attach_mass_removal_refused: GoldenExchange; upsert_endpoint: GoldenExchange; upsert_endpoint_role_change: GoldenExchange; + upsert_endpoint_unknown_role: GoldenExchange; remove_endpoint: GoldenExchange; remove_endpoint_absent: GoldenExchange; set_state: GoldenExchange; set_state_unknown_device: GoldenExchange; + set_state_bad_keys: GoldenExchange; set_reachable: GoldenExchange; /** §3.9-§3.11 and the E4 roles — exchanges awaiting node-side handlers. */ pending: Record; @@ -96,12 +98,19 @@ export const golden: GoldenFrames = JSON.parse( * * It reuses the production planner (`planReconcile`) on purpose: the §3.1 * mass-removal guard and the §4.1 role rules are decided once, in `src/`, and - * the double inherits them. What it fakes is only the Matter part — endpoint - * numbers are handed out in creation order from 2, exactly as matter.js does - * for a freshly built aggregator. + * the double inherits them. What it fakes is only the Matter part — a *first* + * endpoint number is handed out in creation order from 2, as matter.js does for + * a freshly built aggregator, and thereafter the number belongs to the device id + * for good ({@link #allocated}), as matter.js's persisted `Endpoint.id` map does. + * Retention is not a detail: it is what makes a role-change recreate keep its + * accessory identity (`registry.test.ts` asserts exactly that against the real + * stack), and a double that renumbered would have quietly disagreed with the + * thing it stands in for. */ class EndpointModel { readonly #endpoints = new Map(); + /** Every number ever handed out, by device id. Never pruned — see above. */ + readonly #allocated = new Map(); #next = 2; roles(): Map { @@ -157,7 +166,8 @@ class EndpointModel { } private add(spec: EndpointSpec): number { - const endpointNumber = this.#next++; + const endpointNumber = this.#allocated.get(spec.indigoDeviceId) ?? this.#next++; + this.#allocated.set(spec.indigoDeviceId, endpointNumber); this.#endpoints.set(spec.indigoDeviceId, { role: spec.role, endpointNumber }); return endpointNumber; } @@ -189,7 +199,19 @@ export class StubBridge implements BridgeFacade { #windowClosed?: (reason: WindowClosedReason) => void; #command?: (data: CommandEventData) => void; + /** + * Makes `get_status` answer something `JSON.stringify` cannot encode, so + * the §1 "exactly one response" guarantee can be tested against a result + * that fails on the way *out* rather than on the way in. + */ + poisonStatus = false; + getStatus(): StatusReport { + if (this.poisonStatus) { + const circular: Record = {}; + circular.self = circular; + return circular as unknown as StatusReport; + } const endpoints = this.model.summaries(); return { commissioned: this.statusCommissioned, @@ -197,6 +219,7 @@ export class StubBridge implements BridgeFacade { endpointCount: endpoints.length, endpoints, drift: [], + driftChecked: false, }; } diff --git a/docs/BRIDGE_PROTOCOL.md b/docs/BRIDGE_PROTOCOL.md index 59c7f0c..0e17235 100644 --- a/docs/BRIDGE_PROTOCOL.md +++ b/docs/BRIDGE_PROTOCOL.md @@ -249,6 +249,10 @@ confirms via a warning dialog. Result: ``. endpoint is **rejected** (`error_code: "role_change"`); the plugin must remove and re-add, because ecosystems cache device types per endpoint. - `label` — Bridged Device Basic Information `NodeLabel`. +- `reachable` — Bridged Device Basic Information `Reachable`. **Omitting it + means `true`.** An absent flag says nothing about availability, and the other + reading — an accessory that greys itself out in every ecosystem because the + plugin left a field off — is the worse default by far. - `options` — role-specific extras (e.g. window-covering polarity). ### 4.2 Roles: state keys and commands (v1) @@ -294,7 +298,8 @@ ecosystem acts. Both are enumerated here in full; there is no other source. "fabrics": [{"fabricIndex": 1, "label": "Apple Home", "vendorId": 4937}], "endpointCount": 12, "endpoints": [{"indigoDeviceId": 123456789, "endpointNumber": 2, "role": "onOffLight"}], - "drift": []} + "drift": [], + "driftChecked": false} ``` `drift` lists any `UniqueID → endpointNumber` mappings that changed since last @@ -311,6 +316,12 @@ error, never auto-repaired. Each entry is a `DriftEntry`: - `expected` — the endpoint number the persisted map says this `uniqueId` has. - `actual` — the endpoint number it currently has. +- `driftChecked` — whether `drift` is an answer or an absence. `drift: []` on + its own is ambiguous, and the two readings are opposites: "checked, nothing + moved" versus "there is nothing to check against yet". The node sends `false` + until it persists the endpoint-number map, so a client must not treat an empty + `drift` as an all-clear unless `driftChecked` is `true`. + `endpoints[].role` is one of the §4.2 enum; `endpointCount` is `endpoints.length` (it is sent explicitly so a client can log the size without walking the list). diff --git a/tests/fixtures/bridge_protocol/frames.json b/tests/fixtures/bridge_protocol/frames.json index dc59dd6..895ac10 100644 --- a/tests/fixtures/bridge_protocol/frames.json +++ b/tests/fixtures/bridge_protocol/frames.json @@ -54,7 +54,8 @@ "fabrics": [], "endpointCount": 0, "endpoints": [], - "drift": [] + "drift": [], + "driftChecked": false } } }, @@ -128,7 +129,8 @@ "role": "dimmableLight" } ], - "drift": [] + "drift": [], + "driftChecked": false } } }, @@ -313,7 +315,8 @@ "role": "dimmableLight" } ], - "drift": [] + "drift": [], + "driftChecked": false } } }, @@ -341,7 +344,8 @@ ], "endpointCount": 0, "endpoints": [], - "drift": [] + "drift": [], + "driftChecked": false } } }, @@ -409,6 +413,27 @@ "details": "endpoint 123456789 is onOffLight; remove and re-add to change role" } }, + "upsert_endpoint_unknown_role": { + "request": { + "message_id": "m29", + "command": "upsert_endpoint", + "args": { + "endpoint": { + "indigoDeviceId": 900099, + "role": "airPurifier", + "label": "Bedroom Purifier", + "reachable": true, + "states": {}, + "options": {} + } + } + }, + "response": { + "message_id": "m29", + "error_code": "unknown_role", + "details": "role airPurifier is not in the v1 role enum (§4.2)" + } + }, "remove_endpoint": { "request": { "message_id": "m15", @@ -472,6 +497,23 @@ "details": "no live endpoint for indigoDeviceId 123456791" } }, + "set_state_bad_keys": { + "request": { + "message_id": "m44", + "command": "set_state", + "args": { + "indigoDeviceId": 123456789, + "states": { + "level": 55 + } + } + }, + "response": { + "message_id": "m44", + "error_code": "malformed_args", + "details": "role onOffLight consumed none of the states given; rejected key(s): level (§4.2)" + } + }, "set_reachable": { "request": { "message_id": "m19", @@ -738,7 +780,8 @@ "role": "dimmableLight" } ], - "drift": [] + "drift": [], + "driftChecked": false } } }, @@ -1025,29 +1068,9 @@ "role": "thermostat" } ], - "drift": [] - } - } - }, - "upsert_endpoint_unknown_role": { - "request": { - "message_id": "m29", - "command": "upsert_endpoint", - "args": { - "endpoint": { - "indigoDeviceId": 900099, - "role": "airPurifier", - "label": "Bedroom Purifier", - "reachable": true, - "states": {}, - "options": {} - } + "drift": [], + "driftChecked": false } - }, - "response": { - "message_id": "m29", - "error_code": "unknown_role", - "details": "role airPurifier is not in the v1 role enum (§4.2)" } }, "set_state_on_off_plug_in_unit": { diff --git a/tests/test_bridge_protocol_frames.py b/tests/test_bridge_protocol_frames.py index b990f61..1bc25bc 100644 --- a/tests/test_bridge_protocol_frames.py +++ b/tests/test_bridge_protocol_frames.py @@ -56,7 +56,9 @@ def _events() -> dict: #: Endpoint specs that deliberately carry a role outside the §4.2 enum, to pin #: the node's ``unknown_role`` refusal. Everything else must be a v1 role. -UNLAWFUL_ROLE_FIXTURES = {"pending:upsert_endpoint_unknown_role"} +#: (No ``pending:`` prefix any more — the node's ``upsert_endpoint`` handler +#: rejects this frame today, so it graduated to a live exchange.) +UNLAWFUL_ROLE_FIXTURES = {"upsert_endpoint_unknown_role"} def _build(proto: BridgeProtocol, request: dict):