fix(export): restore endpoints at startup so the bridge is never online-and-empty (#141) [no-release] - #142
Conversation
…ne-and-empty (#141) The node called `server.start()` with a childless aggregator and stayed empty until the plugin attached — 23 seconds on jarvis after a reboot. Apple reconnects inside that window, reads an empty PartsList, concludes every accessory has gone, and re-adds them as NEW accessories in the bridge's own room with metadata the user can no longer edit. Every restart destroyed the user's room assignments for every exported device. `endpoint-map.json` gains schema version 2: each entry now carries `role` and `label` alongside `number` — exactly what `createEndpoint` needs. `BridgeNode. start()` rebuilds every restorable entry through the ordinary create path (same `Endpoint.id`, same persisted number, `reachable: false`, role-default state) BEFORE `server.start()`. `attach` stays authoritative and reconciles against the restored set as before. Version 1 files are migrated in place, never discarded: their bare numbers are the one thing that cannot be re-derived and the thing every paired ecosystem is keyed on. They are simply not restorable until the next attach records a role and label. A node in a refuse-to-start state restores nothing: `identityUnreadable` is known before the stack starts and skips the restore; `fabricStorageLost` can only be decided after `server.start()`, so the restored set is withdrawn there instead — safe because that state means no fabrics. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxqGhP3DcENf68AZK21U4S
|
Warning Review limit reached
Next review available in: 47 seconds You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe bridge now persists endpoint numbers with role and label metadata, migrates legacy maps, restores endpoints before Matter startup, and reconciles them after plugin attachment. The plugin also handles fully unbridgeable allow-lists with explicit replacement intent. ChangesEndpoint restoration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant EndpointMapStore
participant BridgeNode
participant EndpointRegistry
participant MatterServer
EndpointMapStore->>BridgeNode: provide restorable endpoint records
BridgeNode->>EndpointRegistry: restore valid endpoint specifications
EndpointRegistry-->>BridgeNode: return restored endpoint count
BridgeNode->>MatterServer: start with restored endpoints
BridgeNode->>EndpointRegistry: withdraw restored endpoints if startup refuses
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@bridge-node/src/endpoints.ts`:
- Around line 98-103: Update the UniqueID parsing logic around the visible
digits validation and indigoDeviceId conversion to accept only canonical
representations: after parsing, require uniqueIdFor(indigoDeviceId) to exactly
equal the original uniqueId, otherwise return undefined. Add regression coverage
for indigo-01 and indigo--0, while preserving valid canonical IDs and
safe-integer validation.
In `@docs/HANDOVER.md`:
- Around line 1573-1575: Update the later “The room mystery — SOLVED” section in
docs/HANDOVER.md to remove the superseded claim that stale Apple records were
not a bridge fault. Mark the section obsolete or replace it with the
restart-time empty-aggregator diagnosis consistent with issue `#141`, including
the required upgrade guidance.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 10fb5c5b-32db-46bd-876b-30f82d8edcad
📒 Files selected for processing (15)
CLAUDE.mdbridge-node/package.jsonbridge-node/src/endpoint-map.tsbridge-node/src/endpoints.tsbridge-node/src/node.tsbridge-node/src/registry.tsbridge-node/test/endpoint-map.test.tsbridge-node/test/persistence.test.tsbridge-node/test/restore.test.tsbridge-node/test/units.test.tsdocs/BRIDGE_PROTOCOL.mddocs/HANDOVER.mddocs/INSTALL.mdindigo-matter.indigoPlugin/Contents/Info.plistindigo-matter.indigoPlugin/Contents/Server Plugin/bridge_agent.py
| const digits = uniqueId.slice(UNIQUE_ID_PREFIX.length); | ||
| if (!/^-?\d+$/.test(digits)) { | ||
| return undefined; | ||
| } | ||
| const indigoDeviceId = Number(digits); | ||
| return Number.isSafeInteger(indigoDeviceId) ? indigoDeviceId : undefined; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject non-canonical UniqueID values.
indigo-01 passes the regular expression and becomes device ID 1, but uniqueIdFor(1) produces indigo-1. A hand-edited map can then restore one identity while persisting another identity key.
Require the parsed value to round-trip through uniqueIdFor before restoring it. Add regression cases for indigo-01 and indigo--0.
Proposed fix
const indigoDeviceId = Number(digits);
- return Number.isSafeInteger(indigoDeviceId) ? indigoDeviceId : undefined;
+ return Number.isSafeInteger(indigoDeviceId) && uniqueIdFor(indigoDeviceId) === uniqueId
+ ? indigoDeviceId
+ : undefined;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const digits = uniqueId.slice(UNIQUE_ID_PREFIX.length); | |
| if (!/^-?\d+$/.test(digits)) { | |
| return undefined; | |
| } | |
| const indigoDeviceId = Number(digits); | |
| return Number.isSafeInteger(indigoDeviceId) ? indigoDeviceId : undefined; | |
| const digits = uniqueId.slice(UNIQUE_ID_PREFIX.length); | |
| if (!/^-?\d+$/.test(digits)) { | |
| return undefined; | |
| } | |
| const indigoDeviceId = Number(digits); | |
| return Number.isSafeInteger(indigoDeviceId) && uniqueIdFor(indigoDeviceId) === uniqueId | |
| ? indigoDeviceId | |
| : undefined; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@bridge-node/src/endpoints.ts` around lines 98 - 103, Update the UniqueID
parsing logic around the visible digits validation and indigoDeviceId conversion
to accept only canonical representations: after parsing, require
uniqueIdFor(indigoDeviceId) to exactly equal the original uniqueId, otherwise
return undefined. Add regression coverage for indigo-01 and indigo--0, while
preserving valid canonical IDs and safe-integer validation.
| is 2026.8.3 with the bridge running under launchd. (Superseded later the same | ||
| day by issue #141 — see the top of this file: plugin `2026.8.4`, bridge `0.6.0`, | ||
| which still needs publishing.) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove the superseded room-diagnosis conclusion.
The later “The room mystery — SOLVED” section still says that stale Apple records were not a bridge fault. That conflicts with the issue #141 diagnosis at Lines 539-548.
Mark that later section as obsolete or replace it with the restart-time empty-aggregator diagnosis. Otherwise, an operator can follow the obsolete conclusion and omit the required upgrade.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/HANDOVER.md` around lines 1573 - 1575, Update the later “The room
mystery — SOLVED” section in docs/HANDOVER.md to remove the superseded claim
that stale Apple records were not a bridge fault. Mark the section obsolete or
replace it with the restart-time empty-aggregator diagnosis consistent with
issue `#141`, including the required upgrade guidance.
Two defects the restore itself introduced, plus the mutation survivors that let them through. **Ghost accessories.** `check()` only ever added and refreshed, so once a v2 entry carried `role`/`label` it carried them for ever — including for a device the user deliberately UN-EXPORTED. It stayed restorable, was rebuilt as a child endpoint before every `server.start()`, and was removed again by the plugin's next attach seconds later: the exact appear-then-vanish churn #141 exists to eliminate, aimed at the devices the user had already removed, and a regression of XAC7. Each ghost also spent a `REMOVAL_PACING_MS` slot on every attach while counting towards neither the desired set nor the un-export debt, so an accumulation of them ate the 8s attach floor for accessories nobody wanted. On a removal the entry now loses `role`/`label` and keeps `number`: `restorable()` filters on role AND label, so it goes non-restorable while §3.3's allocation survives, and one re-export refills it at the same number — the same accessory in every paired ecosystem, not a new one. The removals are MEASURED, by diffing the live set across one mutation, never inferred from absence: an empty live set proves nothing (a node that never attached has one, `seed([])` knows nothing by design), so a factory reset, a seed, and an entry this build cannot rebuild all leave the map alone. **A permanent halt this PR made reachable — and the PR body has the before-state backwards.** When the classifier empties a non-empty allow-list (device deleted, no longer exportable, role lost, `states_for` raising) the attach goes out as `[]` with no intent; with a restored live set §3.1's guard now fires, and `mass_removal_refused` is in `HALTING_ATTACH_ERRORS`, so the client halts permanently and does it again on every reload. The PR body says this "previously slipped through as a silent no-op and halted on the next one". It did not halt on the next one, or ever: with nothing created before the plugin attached, the node's live set stayed empty on every attach, the guard was never armed, and the state was UNREACHABLE. The restore is what reaches it — on a routine restart, from a plausible user state (a one-device allow-list whose device was deleted). `bridge_client._replace_all` now carries `intent: replace_all` there too, gated on a new `export_count_provider` so a genuinely empty allow-list still carries nothing and §3.1 keeps its teeth against the stale client it was written for; a provider that raises fails towards the guard, not past it. Announced at WARNING on both sides naming every skipped device, latched per cause because the provider runs per reconnect. The declared count joins the attach-deadline `max()`, being the only proxy for how much the node is about to remove. Chosen over "do not attach at all": that leaves the node serving accessories the plugin can neither drive nor push state to, with nothing to clear it, and §3.3's retained numbers make the removal recoverable. **Mutation survivors closed**, each verified fail-before: the restored accessory's `label` (it really did come up as "Restored Accessory" with the suite green), `registry.restore`'s per-spec try/catch (one corrupt entry aborted the whole restore and the bridge booted empty), the `indigoDeviceIdFrom`/`isRole`/`isSupportedRole` skip, the `label` half of the `restorable()` predicate, `check`'s refresh-when-the-number-drifted branch, and `states: {}`. Also: `bridge_agent.py` no longer claims the package is unpublished (0.5.0 and 0.6.0 are both on the registry); BRIDGE_PROTOCOL §4.3 now says that only two of the three refusal reasons restore literally nothing, because `fabricStorageLost` restores then withdraws and that is wire-observable; `seed()` asserts it is a full replace; `main.test.ts` pins its children to loopback and documents the residual UDP 5353 contention it cannot fix. The live-validated behaviour is untouched: both changes fire only on a removal or an empty desired set, so a restart that restores N and attaches the same N still reports `0 created, N updated`. Suites 380 TS (from 366) and 2252 Python (from 2243), pylint 9.42 unchanged. One review reported 353 TS; not reproducible — `node --test` without a prior build skips `main.test.ts`'s 8 and yields 358, and `npm test` always builds. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxqGhP3DcENf68AZK21U4S
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
indigo-matter.indigoPlugin/Contents/Server Plugin/export_bridge.py (1)
763-763: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueUse
len(self._store)instead oflen(self._store.all()).
ExportStore.all()builds a sorted tuple of every entry under the lock. This method needs only the count, and the rest of this file already useslen(self._store)for that (Lines 202, 311, 1293). The client calls this provider on every attach and again when sizing the attach deadline.♻️ Proposed change
- return len(self._store.all()) + return len(self._store)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@indigo-matter.indigoPlugin/Contents/Server` Plugin/export_bridge.py at line 763, Update the affected count-returning method to use len(self._store) instead of len(self._store.all()), avoiding materialization and sorting while preserving the returned entry count.docs/HANDOVER.md (1)
153-157: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winPublishing 0.6.0 twice with different content is a silent-failure risk.
Line 68 records that
0.6.0is on the registry. Lines 153-157 record that this branch changes node source after that publish and keepspackage.jsonat0.6.0. Two different builds then answer to the same version string.
bridge_agent.DEFAULT_INSTALL_SPECpins0.6.0andtest_bridge_agent.pyasserts that the two agree, so the test stays green while a registry install delivers a bridge without the ghost-endpoint fix. The failure surfaces only in the field, as ghost accessories on a machine whose versions all report correct.Bump to
0.6.1and publish before merge, or add an explicit blocker line naming0.6.1as required so nobody reads the current state as releasable.[operational]
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/HANDOVER.md` around lines 153 - 157, The publishing handover must not describe the unreleased post-0.6.0 changes as releasable with package.json still at 0.6.0. Update the Publishing section in docs/HANDOVER.md to explicitly block release until version 0.6.1 is required, or reflect that 0.6.1 has been bumped and published before merge; keep bridge_agent.DEFAULT_INSTALL_SPEC and its matching test aligned with the released version.bridge-node/test/restore.test.ts (1)
629-654: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert a non-default state so the test distinguishes "wrote nothing" from "wrote false".
The test sets
onOff: falseand then assertsfalseafter the restart.falseis also the OnOff cluster's own default. A restore that explicitly wroteonOff: falsewould pass this test unchanged. Setting the lamp ON before the restart makes the assertion depend on the restore writing nothing.♻️ Proposed change
- const off = { ...KITCHEN_SPEC, states: { onOff: false } }; + const on = { ...KITCHEN_SPEC, states: { onOff: true } }; const first = await boot(storagePath); try { - await attach(first.client, "s0", [off]); - assert.equal(onOffOf(first.bridge.server, KITCHEN), false); + await attach(first.client, "s0", [on]); + assert.equal(onOffOf(first.bridge.server, KITCHEN), true); } finally { await first.close(); } const second = await boot(storagePath); try { assert.equal( onOffOf(second.bridge.server, KITCHEN), - false, + true, "the restore wrote nothing, so nothing changed", );This assumes matter.js persists the cluster value across the restart, which the current test already relies on.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@bridge-node/test/restore.test.ts` around lines 629 - 654, Update the restore test around the off fixture and restart assertion so it uses a non-default on/off state before shutdown, such as turning the lamp on, then verifies the restored value remains the cluster default after restart. Keep the test’s purpose and existing boot/close flow unchanged, ensuring a restore that writes the persisted value would fail the assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@bridge-node/test/main.test.ts`:
- Around line 17-18: Update the parallel-safety documentation near the ports
reference to avoid claiming PID-derived ports cannot collide; replace that
wording with “usually reduce collisions.” Do not change the port allocation or
process-spawning behavior.
- Around line 117-118: Update the test bridge setup around the LOOPBACK-derived
mDNS arguments to fail closed when LOOPBACK is undefined: throw or skip before
spawning the child rather than omitting --mdns-interface. Preserve
constrained-interface behavior for valid LOOPBACK values and ensure the child is
never started without that restriction.
- Around line 21-30: Make mDNS ownership deterministic for all tests creating
real ServerNode instances, rather than documenting the UDP 5353 race in
main.test.ts. Serialize the relevant test files or implement a cross-process
lock acquired before ServerNode startup and released during teardown, including
the registry, restore, persistence, integration, and main test paths.
In `@docs/BRIDGE_PROTOCOL.md`:
- Around line 136-146: In the documentation text around the restore guard
behavior, replace the unclear phrase “un-export debt” with the repository’s
established spelling, preferably “unexported endpoint debt,” while preserving
the existing meaning and surrounding explanation.
---
Nitpick comments:
In `@bridge-node/test/restore.test.ts`:
- Around line 629-654: Update the restore test around the off fixture and
restart assertion so it uses a non-default on/off state before shutdown, such as
turning the lamp on, then verifies the restored value remains the cluster
default after restart. Keep the test’s purpose and existing boot/close flow
unchanged, ensuring a restore that writes the persisted value would fail the
assertion.
In `@docs/HANDOVER.md`:
- Around line 153-157: The publishing handover must not describe the unreleased
post-0.6.0 changes as releasable with package.json still at 0.6.0. Update the
Publishing section in docs/HANDOVER.md to explicitly block release until version
0.6.1 is required, or reflect that 0.6.1 has been bumped and published before
merge; keep bridge_agent.DEFAULT_INSTALL_SPEC and its matching test aligned with
the released version.
In `@indigo-matter.indigoPlugin/Contents/Server` Plugin/export_bridge.py:
- Line 763: Update the affected count-returning method to use len(self._store)
instead of len(self._store.all()), avoiding materialization and sorting while
preserving the returned entry count.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7834f64c-af16-40cb-b588-1d65677c80e7
📒 Files selected for processing (12)
bridge-node/src/endpoint-map.tsbridge-node/src/node.tsbridge-node/test/endpoint-map.test.tsbridge-node/test/main.test.tsbridge-node/test/restore.test.tsdocs/BRIDGE_PROTOCOL.mddocs/HANDOVER.mdindigo-matter.indigoPlugin/Contents/Server Plugin/bridge_agent.pyindigo-matter.indigoPlugin/Contents/Server Plugin/bridge_client.pyindigo-matter.indigoPlugin/Contents/Server Plugin/export_bridge.pytests/test_bridge_client.pytests/test_export_bridge.py
🚧 Files skipped from review as they are similar to previous changes (2)
- indigo-matter.indigoPlugin/Contents/Server Plugin/bridge_agent.py
- bridge-node/src/node.ts
| * **Parallel safety, and its limit.** The Matter and protocol ports are derived | ||
| * from the PID ({@link ports}) so concurrent runs cannot collide on them, and |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Do not claim that PID-derived ports cannot collide.
ports reduces process.pid modulo 4,000 at Line 131. Two concurrent processes whose PIDs differ by 4,000 can produce the same ports. Replace “cannot collide” with “usually reduce collisions”, or reserve the ports before spawning.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@bridge-node/test/main.test.ts` around lines 17 - 18, Update the
parallel-safety documentation near the ports reference to avoid claiming
PID-derived ports cannot collide; replace that wording with “usually reduce
collisions.” Do not change the port allocation or process-spawning behavior.
| * hosts for it. What that does NOT make ephemeral is **mDNS itself**: the | ||
| * responder binds UDP 5353, the port number is fixed by the protocol, and | ||
| * matter.js offers no knob to move it. Every other file that stands up a real | ||
| * `ServerNode` (`registry`, `restore`, `persistence`, `integration`) binds it | ||
| * too, and node's runner forks test files concurrently — so a run that | ||
| * interleaves them can still see a bind conflict on 5353. This test file is the | ||
| * one that surfaces it, because its nodes are separate PROCESSES that fail to | ||
| * start rather than in-process nodes that share a responder. If it flakes with | ||
| * an address-in-use or a start timeout, that is what happened; re-run it on its | ||
| * own (`node --test .test-build/test/main.test.js`) to confirm. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Make mDNS ownership deterministic instead of documenting a known race.
The comments confirm that concurrent tests can bind UDP 5353 and fail to start. Re-running one file after failure does not make npm test deterministic. Serialize tests that create real ServerNode instances, or add a cross-process lock around mDNS ownership.
The changed documentation identifies UDP 5353 as a shared resource across the test suite.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@bridge-node/test/main.test.ts` around lines 21 - 30, Make mDNS ownership
deterministic for all tests creating real ServerNode instances, rather than
documenting the UDP 5353 race in main.test.ts. Serialize the relevant test files
or implement a cross-process lock acquired before ServerNode startup and
released during teardown, including the registry, restore, persistence,
integration, and main test paths.
| * outright. `undefined` if there is somehow no internal interface, in which case | ||
| * the flag is simply omitted and the child behaves exactly as it did before. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Fail closed when no internal interface is available.
When LOOPBACK is undefined, Line 135 omits --mdns-interface. The child then uses unconstrained mDNS behavior and can advertise the test bridge on a real interface. Throw or skip before spawning instead of silently restoring the old behavior.
The PR objective requires the test bridge to remain constrained to an internal interface.
Proposed fail-closed change
function ports(offset: number): string[] {
+ if (LOOPBACK === undefined) {
+ throw new Error("No internal network interface available for isolated Matter tests");
+ }
const base = 41_000 + ((process.pid + offset * 37) % 4_000);
return [
"--matter-port", String(base),
"--ws-port", String(base + 1),
- ...(LOOPBACK === undefined ? [] : ["--mdns-interface", LOOPBACK]),
+ "--mdns-interface", LOOPBACK,
];
}Also applies to: 130-136
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@bridge-node/test/main.test.ts` around lines 117 - 118, Update the test bridge
setup around the LOOPBACK-derived mDNS arguments to fail closed when LOOPBACK is
undefined: throw or skip before spawning the child rather than omitting
--mdns-interface. Preserve constrained-interface behavior for valid LOOPBACK
values and ensure the child is never started without that restriction.
| Because restore arms that guard from the first attach, the plugin now carries | ||
| the intent in **two** cases, not one: the un-export debt it already tracked | ||
| (XAC7), and an allow-list that is non-empty but whose every entry the classifier | ||
| skipped — a deleted Indigo device, a device that stopped being exportable, a | ||
| role this build cannot bridge, a `states_for` that raised. That second case sends | ||
| `[]` and is genuinely asking for an empty set; without the intent it would meet | ||
| an armed guard on an ordinary restart and be refused, and `mass_removal_refused` | ||
| halts the client permanently. It is announced in the Indigo log with every | ||
| skipped device named. A *genuinely* empty allow-list still carries no intent — | ||
| the guard must keep its teeth against exactly the stale client it was written | ||
| for. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a clear spelling for “unexported”.
Replace un-export debt with unexported endpoint debt or the repository’s established term. The current phrase is unclear.
🧰 Tools
🪛 LanguageTool
[grammar] ~136-~136: Ensure spelling is correct
Context: ... restore arms that guard from the first attach, the plugin now carries the intent in *...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/BRIDGE_PROTOCOL.md` around lines 136 - 146, In the documentation text
around the restore guard behavior, replace the unclear phrase “un-export debt”
with the repository’s established spelling, preferably “unexported endpoint
debt,” while preserving the existing meaning and surrounding explanation.
Source: Linters/SAST tools
The startup-restore hardening (ghost accessories, the newly-reachable halt, six mutation-survivor test gaps) landed after 0.6.0 was published, so a registry install does not have it. package.json and DEFAULT_INSTALL_SPEC move together because a test pins them to each other. An older exact pin resolves fine against a registry carrying newer versions — the pin is what decides which version users actually get, which is why it has to move for the fix to ship. Publish 0.7.0 to npm BEFORE this merges, or the install menu 404s. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxqGhP3DcENf68AZK21U4S
Three characterisation tests for the create path, written while diagnosing issue #143 (exported accessories showing the wrong on/off state in Alexa and Apple Home). All three pass, and that is the finding: the node is not the fault. They exist because the create path had no coverage of the case that matters — an `Endpoint.id` matter.js has ALREADY persisted attribute values for. `update()` has always followed up with `applyStates`; `create()` relies on `createEndpoint` baking the spec's states into `initialState`, and whether a persisted value outranks that was untested and load-bearing. It does not: * in-process, after an un-export and re-export; * across a node RESTART, so matter.js loads the value from disk rather than carrying a closed endpoint in memory — the exact field sequence from jarvis, and the one the in-process test cannot reach; * through `upsert_endpoint` rather than `attach`, because that is the route the export dialog actually takes and it parses its spec separately. Also worth recording for the #143 follow-up: matter.js does NOT flush a constructor-supplied `initialState` to its store, so the on-disk `…parts.<id>.onOff.onOff` files are not a reading of the live attribute. Two of them said `false` on jarvis for accessories that were on, which sent the diagnosis down a blind alley for a while. Refs #143
1 participant
Fixes #141 — the root cause of Apple Home resetting exported accessories' rooms on every bridge-node restart.
The defect
The node called
server.start()with an aggregator holding zero children and stayed empty until the plugin attached — 23 seconds on the reference server after a reboot. Apple reconnects inside that window, reads an emptyPartsList, concludes every accessory is gone, and treats them as new arrivals when they reappear: dumped in the bridge's room with metadata the user can no longer edit.The fix
endpoint-map.json→ schema v2: entries carryroleandlabelbeside the number. v1 files load and migrate in place; a bare number keeps its number and is simply not restorable until the next attach fills in the rest. A malformed role/label never costs the entry its number.restoreEndpoints()runs beforeserver.start(), rebuilding each restorable entry through the ordinary registry path withreachable: falseand role defaults — sameEndpoint.id, same persisted number.attachstays authoritative and reconciles against the restored set.⊗ Evidence
New
restore.test.ts(11 tests, real ServerNodes). The headline patchesServerNode.prototype.startand snapshots the aggregator's children at the instant start is invoked — not afterBridgeNode.start()returns, which would pass with the restore in the wrong place.server.start()→ exactly 1 failure, the headline. That mutation is the before-state of the bug.Also covered: persisted numbers not fresh allocations; unreachable until attach;
0 created, N updatedon reconcile; v1 migration across three boots; the §3.1 mass-removal guard still armed against a restored set; no spurious drift; both refusal routes restore nothing.Verification
TS 366 (from 347), Python 2243, pylint 9.42. bridge-node 0.6.0, PluginVersion 2026.8.4.
Needs Simon
An npm publish of
indigo-matter-bridge@0.6.0—DEFAULT_INSTALL_SPECnow pins it and only 0.5.0 is on the registry.Behaviour change worth a decision
With the guard armed at first attach, one pre-existing scenario fires a step earlier: a non-empty allow-list whose entries are all skipped by the classifier sends
[]without intent →mass_removal_refused→ halt. Previously the first attach after a restart slipped through as a silent no-op and halted on the next one. Fix, if wanted, is plugin-side.🤖 Generated with Claude Code
https://claude.ai/code/session_01JxqGhP3DcENf68AZK21U4S
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores