diff --git a/bridge-node/src/endpoint-map.ts b/bridge-node/src/endpoint-map.ts index 570ebd7..dbed82f 100644 --- a/bridge-node/src/endpoint-map.ts +++ b/bridge-node/src/endpoint-map.ts @@ -71,7 +71,7 @@ import { copyFileSync, readFileSync, unlinkSync } from "node:fs"; import { join } from "node:path"; -import { describeError, type DriftEntry, type OrphanRecord, parsePublishedId, RefuseReason } from "./protocol.js"; +import { describeError, type DriftEntry, parsePublishedId, RefuseReason } from "./protocol.js"; import { writeJsonAtomic } from "./storage.js"; /** What the PRD §7 refuse-to-start decision is taken from. */ @@ -186,17 +186,16 @@ export interface EndpointRecord { */ numberVoid?: true; /** - * Set by {@link EndpointMapStore.forget} (issue #219): the device was - * un-exported, - * so the entry must not be rebuilt by {@link EndpointMapStore.restorable} - * — but unlike the pre-#219 behaviour, `role`/`label` are KEPT rather - * than deleted, because they are the only evidence a future re-adopt UI - * could match a recreated device against. Cleared the moment - * {@link EndpointMapStore.check} sees the same `UniqueID` live again - * (`noteRestorable`) — a live device is by - * definition not orphaned, and re-exporting the same accessory must not - * leave it permanently excluded from the next boot's pre-seeding. Absent - * on every ordinary entry, same convention as `numberVoid`. + * Set by {@link EndpointMapStore.forget}: the device was un-exported, so + * the entry must not be rebuilt by {@link EndpointMapStore.restorable} — + * the actual fix for issue #141's appear-then-vanish churn. + * `role`/`label` are KEPT rather than deleted alongside it (they cost two + * strings, and nothing here has a reason to clear them). Cleared the + * moment {@link EndpointMapStore.check} sees the same `UniqueID` live + * again (`noteRestorable`) — a live device is by definition not + * orphaned, and re-exporting the same accessory must not leave it + * permanently excluded from the next boot's pre-seeding. Absent on every + * ordinary entry, same convention as `numberVoid`. */ orphaned?: true; /** @@ -211,27 +210,19 @@ export interface EndpointRecord { */ battery?: true; /** - * Set by {@link recordFor}/{@link noteRestorable} (issue #219 re-adopt): - * the Indigo device CURRENTLY driving this published identity. Absent - * means "the key's own derivation" — every pre-PR5 record, and every - * un-re-adopted one, for which `parsePublishedId(uniqueId)?.deviceId` is - * still the right answer (see {@link RestorableEndpoint.indigoDeviceId}). + * Set by {@link recordFor}/{@link noteRestorable}: the Indigo device + * CURRENTLY driving this published identity. Absent means "the key's own + * derivation" — every record whose identity has never been retargeted + * onto a different device, for which `parsePublishedId(uniqueId)?.deviceId` + * is still the right answer (see {@link RestorableEndpoint.indigoDeviceId}). * * **Replace-on-change, unlike {@link numberVoid}/{@link orphaned}/ - * {@link battery}'s add-only convention.** A re-adopt IS a change of - * driving device — pinning the FIRST (now-deleted) device id forever - * would make restore-on-start rebuild the accessory bound to a device - * that no longer exists, which is the whole failure #219 exists to fix. + * {@link battery}'s add-only convention.** Migrating an accessory (issue + * #246) IS a change of driving device for an identity that stays LIVE the + * whole time — pinning the first device id forever would make + * restore-on-start rebuild the accessory bound to the wrong one. */ deviceId?: number; - /** - * Set by {@link EndpointMapStore.forget} (issue #219) the moment an entry - * is FIRST orphaned — an ISO-8601 stamp from the store's injected clock, - * purely for the re-adopt picker's "un-exported on …" column. Absent on - * every pre-PR5 orphan; the picker renders that as "date unknown" rather - * than treating it as a fault (§4.2). - */ - orphanedAt?: string; /** * Set by {@link EndpointMapStore.forget} when the caller has decided this * identity was RETIRED, not merely un-exported (issue #240): the published @@ -239,19 +230,21 @@ export interface EndpointRecord { * * **The narrow rule, and it is narrow on purpose.** Only a LATER * GENERATION of the same identity — `indigo-7` → `indigo-7~2`, which is - * what `protocol.ts`'s `supersedes()` tests — counts. A **re-adopt** is - * the same shape at a glance (one removal plus one create, for one device) - * and is deliberately NOT one: the identity a re-adopt leaves behind is an - * ordinary orphan the picker must go on offering, PR5 design E5 in as many - * words. Both node-side writers apply `supersedes()` before setting this, - * whether the pair lands in one mutation (`node.ts`'s `forgetRemoved`) or - * as two commands (`noteSupersessionIfPaired`). + * what `protocol.ts`'s `supersedes()` tests — counts. Migrating an + * accessory onto an already-exported device (issue #246, the ● case) is + * the same shape at a glance (one removal plus one create, for one + * device) and is deliberately NOT one: the target's own old identity and + * the newly-inherited one are unrelated strings, so it is left an + * ORDINARY orphan instead. Both node-side writers apply `supersedes()` + * before setting this, whether the pair lands in one mutation + * (`node.ts`'s `forgetRemoved`) or as two commands + * (`noteSupersessionIfPaired`). * - * A superseded entry is {@link orphaned} too (it is not live), but it is - * NOT a re-adopt candidate — {@link EndpointMapStore.restorable} and - * {@link EndpointMapStore.orphans} both exclude it, because re-adopting it - * would resurrect the OLD-role accessory under a number every paired - * ecosystem has already processed a removal for. + * A superseded entry is {@link orphaned} too (it is not live), but it + * must never be REBUILT at the next restart — + * {@link EndpointMapStore.restorable} excludes it, because that would + * resurrect the OLD-role accessory under a number every paired ecosystem + * has already processed a removal for. */ supersededBy?: string; } @@ -309,13 +302,13 @@ export interface RestorableEndpoint { /** Issue #220 — `true` only when the persisted entry has ever seen one. */ battery?: true; /** - * Issue #219 — the device `node.ts`'s restore-on-start builds the spec - * for. Resolved by {@link EndpointMapStore.restorable} from - * {@link EndpointRecord.deviceId} where a re-adopt has set it, falling + * The device `node.ts`'s restore-on-start builds the spec for. Resolved + * by {@link EndpointMapStore.restorable} from + * {@link EndpointRecord.deviceId} where a migrate has set it, falling * back to the published identity's own derivation - * (`parsePublishedId(uniqueId)?.deviceId`) for every pre-PR5 and - * un-re-adopted entry — an entry that resolves to neither is not - * restorable at all, so this is never `undefined` here. + * (`parsePublishedId(uniqueId)?.deviceId`) for every entry that has never + * been retargeted — an entry that resolves to neither is not restorable + * at all, so this is never `undefined` here. */ indigoDeviceId: number; } @@ -329,14 +322,14 @@ export interface EndpointMapLoad { problem?: string; /** * Entry-level oddities tolerated during the read, for the caller to put on - * the §4.3 channel (issues #219/#240). + * the §4.3 channel (issue #240). * * `readRecord` is deliberately forgiving — a bad field costs the field, not * the file (see its own comment) — but "tolerated" is not the same as - * "unremarkable", and a DISCARDED `supersededBy` re-arms a retired identity - * as an ordinary re-adopt candidate. That is a decision about the user's - * accessories, taken from a value we could not read, and it must not be - * taken in silence. + * "unremarkable", and a DISCARDED `supersededBy` re-arms a retired + * identity as an ordinary, rebuildable one. That is a decision about the + * user's accessories, taken from a value we could not read, and it must + * not be taken in silence. */ notes?: string[]; } @@ -395,27 +388,23 @@ function readRecord(value: unknown, uniqueId: string, notes: string[]): Endpoint if (typeof candidate.deviceId === "number" && Number.isInteger(candidate.deviceId)) { record.deviceId = candidate.deviceId; } - // Same tolerance: a stamp that is not a non-empty string is simply - // absent — the picker already renders that as "date unknown" (§4.2). - if (isNonEmptyString(candidate.orphanedAt)) { - record.orphanedAt = candidate.orphanedAt; - } // Same tolerance: anything other than a non-empty string is simply not a // supersession marker — but unlike the others, LOSING this one changes // what the node offers the user. A superseded identity is excluded from - // `restorable()` and `orphans()`; without the marker it becomes an - // ordinary re-adopt candidate again, and re-adopting it resurrects an - // old-role accessory under a number every paired ecosystem has already - // processed a removal for. Present-but-unusable is therefore said out - // loud; absent (every ordinary record) is not. + // `restorable()`; without the marker it becomes eligible to be REBUILT + // again at the next restart, resurrecting an old-role accessory under a + // number every paired ecosystem has already processed a removal for. + // Present-but-unusable is therefore said out loud; absent (every + // ordinary record) is not. if (isNonEmptyString(candidate.supersededBy)) { record.supersededBy = candidate.supersededBy; } else if (candidate.supersededBy !== undefined) { notes.push( `Endpoint map entry ${uniqueId} has an unusable supersededBy ` + `(${JSON.stringify(candidate.supersededBy)}); it was discarded, so this retired ` + - "accessory identity is a re-adopt candidate again. Re-adopting it would resurrect " + - "an old-role accessory under a retired number — check it before you do.", + "accessory identity is eligible to be rebuilt again at the next restart — that " + + "would resurrect an old-role accessory under a retired number — check it before " + + "the bridge next restarts.", ); } return record; @@ -541,11 +530,11 @@ function recordFor(entry: LiveEndpointNumber): EndpointRecord { * un-exported. * * **Also adopts a CHANGED `deviceId`, replacing rather than adding (issue - * #219 re-adopt).** A re-adopt IS a change of driving device — pinning the - * first device id forever, `battery`'s convention, would make the next - * restore-on-start rebuild the accessory bound to a device that no longer - * exists. {@link EndpointMapStore.check} reads the record's OLD device id - * before calling this, so it can still tell a re-adopt from a first sighting + * #246).** Migrating an accessory IS a change of driving device — pinning + * the first device id forever, `battery`'s convention, would make the next + * restore-on-start rebuild the accessory bound to the wrong one. + * {@link EndpointMapStore.check} reads the record's OLD device id before + * calling this, so it can still tell the change apart from a first sighting * once this has overwritten it — see that method's own doc comment. */ function noteRestorable(record: EndpointRecord, entry: LiveEndpointNumber): boolean { @@ -573,24 +562,17 @@ function noteRestorable(record: EndpointRecord, entry: LiveEndpointNumber): bool } if (record.orphaned) { delete record.orphaned; - // `orphanedAt` goes with it (issue #219): a live device is not - // orphaned, so a stamp saying when it WAS would be stale evidence - // sitting next to a marker that no longer agrees with it. Harmless - // either way — {@link forget} only ever reads it when re-orphaning, - // and stamps a fresh one then — but a raw read of the file should - // never see the two disagree. - delete record.orphanedAt; changed = true; } if (record.supersededBy !== undefined) { - // And so does `supersededBy`: this identity is LIVE again, so the + // `supersededBy` goes too: this identity is LIVE again, so the // marker saying another one replaced it is now simply false. It gets // here when an identity that was once superseded is published again // — un-export a role-changed export, re-export it, and the plugin // sends the default `indigo-` the supersession retired. // Left in place it would outlive the resurrection and hide the entry - // from `orphans()` (§3.12) the NEXT time it was un-exported, making a - // perfectly ordinary orphan permanently un-re-adoptable. + // from `restorable()` the NEXT time it was un-exported, making a + // perfectly ordinary orphan permanently unrebuildable. delete record.supersededBy; changed = true; } @@ -638,14 +620,13 @@ export class EndpointMapStore { /** Persistence failures worth putting in `StatusReport.warnings` (§4.3). */ #warnings = new Map(); /** - * One-shot NOTICES that ride the same §4.3 channel (issues #219/#240). + * One-shot NOTICES that ride the same §4.3 channel (issue #240). * * `this.log` is stdout, and the node is started by launchd — so a line - * written only there is a line no user will ever see. Three of this file's - * most consequential statements were stdout-only: the re-adopt nudge (the - * whole discoverability moment for `Re-adopt a Matter accessory…`), the - * "this entry cannot be rebuilt by this bridge version" skip, and the - * confirmation that a re-adopt actually landed. {@link warnings} therefore + * written only there is a line no user will ever see. Two of this file's + * most consequential statements were stdout-only: the "this entry cannot + * be rebuilt by this bridge version" skip, and the confirmation that a + * device's driving identity actually changed. {@link warnings} therefore * carries these too, which is the one channel `export_bridge.py` already * mirrors into the Indigo event log. * @@ -785,11 +766,10 @@ export class EndpointMapStore { * whatever number came back. Excluding void entries here would only widen * the online-and-empty window issue #141 already closed, for no benefit. * - * **`orphaned` IS part of this filter (issue #219).** A {@link forget}en - * entry now keeps its `role`/`label` as re-adopt evidence rather than - * losing them, so the orphan marker is what still keeps it out of the - * pre-attach rebuild — exactly the churn {@link forget}'s own doc comment - * exists to prevent, unaffected by role/label now surviving alongside it. + * **`orphaned` IS part of this filter (issue #141).** A {@link forget}en + * entry keeps its `role`/`label` (they cost nothing to keep), so the + * orphan marker is what actually keeps it out of the pre-attach rebuild — + * exactly the churn {@link forget}'s own doc comment exists to prevent. * * **So is `supersededBy` (issue #240), checked in its own right rather * than left to ride on `orphaned`.** The two are written together, but a @@ -800,15 +780,15 @@ export class EndpointMapStore { * four conditions, not three: no `role`, no `label`, `orphaned`, or * `supersededBy`. * - * **Resolves `indigoDeviceId` — and skips what it cannot (issue #219).** - * `record.deviceId` wins where a re-adopt has set it; otherwise it falls + * **Resolves `indigoDeviceId` — and skips what it cannot.** + * `record.deviceId` wins where a migrate has set it; otherwise it falls * back to the published identity's own derivation - * (`parsePublishedId(uniqueId)?.deviceId`), which is every pre-PR5 and - * un-re-adopted entry. An entry with a `role`/`label` but neither source - * of a device id — a map written by a NEWER node, or hand-edited — is - * skipped and named on the §4.3 {@link warnings} channel (so it reaches - * the user's Indigo log, not just stdout), in the same "cannot be rebuilt - * by this bridge version" words `node.ts`'s own loop uses for an + * (`parsePublishedId(uniqueId)?.deviceId`), which is every entry that has + * never been retargeted. An entry with a `role`/`label` but neither + * source of a device id — a map written by a NEWER node, or hand-edited — + * is skipped and named on the §4.3 {@link warnings} channel (so it + * reaches the user's Indigo log, not just stdout), in the same "cannot be + * rebuilt by this bridge version" words `node.ts`'s own loop uses for an * unsupported `role`: this is the only place that ever sees such an entry, * since a skipped one never reaches that loop at all. */ @@ -848,63 +828,6 @@ export class EndpointMapStore { return restorable; } - /** - * §3.12's answer (issue #219) — every left-behind accessory identity the - * re-adopt picker could plausibly offer, in map order (the order numbers - * were first recorded). - * - * **Excludes `supersededBy` records (issue #240).** Those are orphaned - * too, but re-adopting one would resurrect the OLD-role accessory under a - * number every paired ecosystem has already processed a removal for — - * see {@link EndpointRecord.supersededBy}. - * - * **Includes entries with no `role`/`label` (PR5 design E4, §4.2), unlike - * {@link restorable}'s filter.** A pre-2026.16.2 bare `{number}` orphan — - * from a plugin old enough to have deleted them on un-export — carries - * nothing a replacement device could be matched against, so it can never - * be re-adopted, but the ruling (PR5 design §5 E4) is to show it anyway: "the number - * is reserved and the user is entitled to see why", as the picker's - * "no role recorded, cannot be re-adopted" row (§4.2's third row). Callers - * that need to tell the two shapes apart check `role`/`label` for - * `undefined`, which {@link restorable}'s own skip-and-log already treats - * as unrebuildable for the SAME entries. - * - * **Skips a key `parsePublishedId` refuses — PR5 design E11's `restorable()` - * treatment, applied here for a sharper reason.** This list feeds the - * re-adopt picker, and picking a row writes its key into the plugin's - * `ExportStore` as a `publishedAs`, which the node then refuses at the - * next attach with `malformed_args` — and an attach refusal takes EVERY - * export offline, not just that one. So a hand-edited or newer-node key is - * named once on the §4.3 channel and left out, rather than offered as a - * route into a whole-bridge outage. - */ - orphans(): OrphanRecord[] { - const orphans: OrphanRecord[] = []; - for (const [uniqueId, record] of this.#endpoints) { - if (!record.orphaned || record.supersededBy !== undefined) { - continue; - } - if (parsePublishedId(uniqueId) === undefined) { - this.notice( - `unofferable:${uniqueId}`, - `Endpoint map key ${JSON.stringify(uniqueId)} is not a published accessory identity ` + - "this bridge version understands, so it is not offered for re-adoption; a re-adopt " + - "onto it would be refused at the next attach and stop every export.", - ); - continue; - } - orphans.push({ - uniqueId, - number: record.number, - ...(record.role !== undefined ? { role: record.role } : {}), - ...(record.label !== undefined ? { label: record.label } : {}), - ...(record.orphanedAt !== undefined ? { orphanedAt: record.orphanedAt } : {}), - ...(record.deviceId !== undefined ? { deviceId: record.deviceId } : {}), - }); - } - return orphans; - } - /** * True when the map is non-empty and EVERY entry in it is * {@link EndpointRecord.orphaned} — issue #222's distinction for @@ -932,18 +855,15 @@ export class EndpointMapStore { * devices the user had already told us to stop exporting, and it regresses * XAC7's "un-exported accessories are gone". * - * **The number AND the role/label stay (issue #219).** §3.3 already retained - * the number on purpose, so re-adding the same device gets the same endpoint + * **The number AND the role/label stay.** §3.3 already retained the + * number on purpose, so re-adding the same device gets the same endpoint * number back and paired ecosystems see the accessory they already know - * rather than a new one. Deleting `role`/`label` too — the pre-#219 - * behaviour — threw away the only evidence a future re-adopt UI could match - * a *recreated* device (a factory-reset accessory that comes back with a new - * `UniqueID`) against the endpoint it used to occupy. Instead the entry is - * marked {@link EndpointRecord.orphaned}, which is what keeps it out of - * {@link restorable}'s pre-attach rebuild — the actual thing #141 needed — - * without destroying anything. `check`'s `noteRestorable` clears the marker - * the moment the same `UniqueID` is live again, so a plain re-export is - * unaffected. + * rather than a new one; `role`/`label` are simply never cleared alongside + * it. The entry is marked {@link EndpointRecord.orphaned}, which is what + * keeps it out of {@link restorable}'s pre-attach rebuild — the actual + * thing #141 needed — without destroying anything. `check`'s + * `noteRestorable` clears the marker the moment the same `UniqueID` is + * live again, so a plain re-export is unaffected. * * **Only a caller that watched a specific endpoint go may call this.** An * empty live set is not evidence of anything: a node that has never attached @@ -955,22 +875,19 @@ export class EndpointMapStore { * nothing and the plugin's re-attach re-creates the same set. * * **`options.supersededBy` marks a role-change supersession (issue - * #240) instead of an ordinary un-export**, and `orphanedAt` is stamped - * from the store's injected `now()` the moment an entry is FIRST - * orphaned — never re-stamped on a later call, so it always reads the - * time the accessory actually left, not the time a supersession was - * confirmed. + * #240) instead of an ordinary un-export.** * - * **This is also how a TWO-COMMAND supersede gets its `supersededBy` - * (§3 of the design: `replace()` sends `remove_endpoint` then - * `upsert_endpoint` as separate commands).** The first call orphans the - * entry with no `supersededBy` yet — the create has not happened. The - * SECOND call, once the create lands, passes the SAME `uniqueId` again - * with `{supersededBy}` — the guard below therefore does not stop at + * **This is also how a CROSS-COMMAND supersede gets its `supersededBy`** + * (a device un-exported by an `attach` reconcile, then later re-exported + * under a later generation of its identity as a SEPARATE + * `upsert_endpoint` — see `node.ts`'s `forgetRemoved`/ + * `noteSupersessionIfPaired` pair). The first call orphans the entry + * with no `supersededBy` yet — the create has not happened. The SECOND + * call, once the create lands, passes the SAME `uniqueId` again with + * `{supersededBy}` — the guard below therefore does not stop at * "already orphaned" the way it does for a caller with nothing new to * say; a second call that has a `supersededBy` this record does not yet - * carry still counts as a change, and only `orphanedAt` is protected - * from being re-stamped. + * carry still counts as a change. * * Returns how many entries CHANGED — newly orphaned, newly given a * `supersededBy`, or both — so a no-op (no record at all, nothing worth @@ -990,9 +907,6 @@ export class EndpointMapStore { continue; } record.orphaned = true; - if (newlyOrphaned) { - record.orphanedAt = this.now().toISOString(); - } if (newlySuperseded) { record.supersededBy = options.supersededBy; } @@ -1003,8 +917,8 @@ export class EndpointMapStore { options.supersededBy !== undefined ? `marked ${changed} superseded accessory identity(ies) retired, keeping their numbers ` + "so they can never be handed to another accessory" - : `marked ${changed} un-exported endpoint(s) orphaned, keeping their role/label ` + - "as re-adopt evidence and their numbers so a re-export returns the same accessory", + : `marked ${changed} un-exported endpoint(s) orphaned, keeping their numbers so a ` + + "re-export returns the same accessory", ); } return changed; @@ -1022,9 +936,8 @@ export class EndpointMapStore { * call: matter.js's OWN persisted allocation for the retired * `Endpoint.id` — a separate store this file never touches — is what * actually prevents reissue, and it is untouched by deleting our own - * witness of it. What is lost is re-adopt: {@link restorable} and - * {@link orphans} can only offer what is still in `#endpoints`, so a - * destroyed identity is gone from both — no re-adopt picker entry, no + * witness of it. What is lost is {@link restorable}: it can only offer + * what is still in `#endpoints`, so a destroyed identity is gone — no * pre-attach rebuild, ever again. * * **Only a caller certain the device will never come back may call @@ -1033,8 +946,8 @@ export class EndpointMapStore { * an identity this pass) must stay soft: the node cannot tell a * deliberate departure from a device that merely failed classification * this one time (issue #274 constraint 2). `node.ts`'s `removeEndpoint` - * is the only caller, and only when the plugin's `remove_endpoint` - * carried `permanent: true`. + * is the only caller, and it always does — every `remove_endpoint` + * command is the plugin confirming the driving device is gone for good. * * Returns how many entries actually existed and were removed, so a call * over an already-forgotten or never-recorded `uniqueId` costs no disk @@ -1051,7 +964,7 @@ export class EndpointMapStore { if (removed > 0) { this.persist( `destroyed ${removed} endpoint map record(s): the driving device is gone for good, so ` + - "neither the number nor the role/label is retained for re-adopt", + "neither the number nor the role/label is retained", ); } return removed; @@ -1077,17 +990,17 @@ export class EndpointMapStore { * what has to stop being true, not just the mismatch it was guarding * against. * - * **A changed `deviceId` on an EXISTING record is logged as a re-adopt - * (issue #219).** The record's device id is read before - * {@link noteRestorable} overwrites it, so a real change (both sides - * defined, and different) can still be told apart from a first sighting - * (`added`, above — nothing to compare against) and from this build - * simply filling in a field a pre-PR5 record never had (`record.deviceId` - * was `undefined`, not merely different). Reported directly here, the same - * place the `numberVoid` adoption above logs itself, because — like that - * one — only `check` ever sees both the old and the new value at once; and - * on the §4.3 {@link warnings} channel rather than stdout, because it is - * the confirmation that the user's re-adopt actually landed. + * **A changed `deviceId` on an EXISTING record is logged.** The record's + * device id is read before {@link noteRestorable} overwrites it, so a + * real change (both sides defined, and different) can still be told + * apart from a first sighting (`added`, above — nothing to compare + * against) and from this build simply filling in a field the record never + * had (`record.deviceId` was `undefined`, not merely different). Reported + * directly here, the same place the `numberVoid` adoption above logs + * itself, because — like that one — only `check` ever sees both the old + * and the new value at once; and on the §4.3 {@link warnings} channel + * rather than stdout, because it is the confirmation that the change + * actually landed. */ check(live: readonly LiveEndpointNumber[]): DriftEntry[] { const drift: DriftEntry[] = []; @@ -1097,17 +1010,16 @@ export class EndpointMapStore { for (const entry of live) { const record = this.#endpoints.get(entry.uniqueId); if (record === undefined) { - this.noteReadoptableMatch(entry); this.#endpoints.set(entry.uniqueId, recordFor(entry)); added += 1; continue; } const previousDeviceId = record.deviceId; // Read before `noteRestorable` un-orphans the record below — this - // is the only way left to tell a MIGRATE (issue #246: the - // identity stayed live the whole time, a rekey moved it) from a - // RE-ADOPT (issue #219: `forget()` marked it orphaned when it was - // un-exported, and this is the first live sighting since). + // is what tells a MIGRATE (issue #246: the identity stayed live + // the whole time, a rekey moved it) from an identity that was + // orphaned (`forget()` marked it so when it was un-exported) and + // is only now live again under a different device. const wasOrphaned = record.orphaned === true; // The restoration half is refreshed even when the number drifted: // a renamed accessory whose number also moved must still come back @@ -1122,19 +1034,17 @@ export class EndpointMapStore { // only the promise after the dash differs, because the two // paths earn different ones. A migrate's identity NEVER // lapsed (issue #246: this PR's own rekey), so room/name/ - // scenes provably survive. A re-adopt's identity DID lapse - // (it was orphaned first), and this PR corrected that exact - // promise elsewhere as measured-false: an ecosystem that - // already processed the removal has purged room/name context - // within moments, so only identity and number are restored. + // scenes provably survive. An identity that was orphaned + // first DID lapse, so an ecosystem that already processed the + // removal has purged room/name context within moments, + // meaning only identity and number are restored. this.notice( - `readopted:${entry.uniqueId}`, + `redriven:${entry.uniqueId}`, wasOrphaned ? `Accessory ${entry.uniqueId} (number ${record.number}) is now driven by Indigo device ` + `${entry.deviceId}, replacing device ${previousDeviceId}. Its identity and number ` + "are restored, but an ecosystem that already processed its removal may already " + - "have purged its room and name — this identity was orphaned first (issue #219 " + - "re-adopt)." + "have purged its room and name — this identity was orphaned first." : `Accessory ${entry.uniqueId} (number ${record.number}) is now driven by Indigo device ` + `${entry.deviceId}, replacing device ${previousDeviceId} — nothing on the wire ` + "changed, so every paired ecosystem keeps the room, name, scenes and automations " + @@ -1179,50 +1089,6 @@ export class EndpointMapStore { return drift; } - /** - * Owner ruling 4 (issue #219) — a cheap nudge, not a decision. Called - * from {@link check}'s `added` branch, before the brand-new record is - * inserted: when the identity `check` is about to record for the FIRST - * time has a `role` AND `label` that exactly match an existing - * ORPHANED, non-superseded record, say so once — on the §4.3 - * {@link warnings} channel, because a nudge nobody reads is not a nudge — - * naming the `Re-adopt a Matter accessory…` menu action. This is exactly the - * motivating case for #219 — a device deleted, recreated and - * re-exported under a brand-new identity, with the room now empty in - * every ecosystem and no obvious reason why — offered before the user - * has to notice and go looking for it. - * - * Deliberately does nothing else: it changes no state, blocks nothing, - * and never fires for an update to an EXISTING identity (an ordinary - * re-export, or a role-change supersede) — those are not the - * "orphaned accessory nobody re-adopted yet" case this exists to - * surface. An ambiguous match (more than one orphan with the same - * role/label) logs against the first one found, in map order — good - * enough for a nudge that only ever points a user at the menu, never - * acts on their behalf. - */ - private noteReadoptableMatch(entry: LiveEndpointNumber): void { - if (entry.role === undefined || entry.label === undefined) { - return; - } - for (const [uniqueId, record] of this.#endpoints) { - if (!record.orphaned || record.supersededBy !== undefined) { - continue; - } - if (record.role === entry.role && record.label === entry.label) { - this.notice( - `readoptable:${entry.uniqueId}`, - `"${entry.label}" was just exported as a NEW accessory (${entry.uniqueId}), but a ` + - `left-behind accessory with the same role and name (${uniqueId}, number ` + - `${record.number}) is still in the endpoint map. If this is the same device, ` + - "'Re-adopt a Matter accessory…' in the plugin menu keeps its room, name, scenes " + - "and automations instead of leaving this one to start from scratch (issue #219).", - ); - return; - } - } - } - /** * §3.10's preserving reset and the last-fabric self-reset (issue #140) — * mark every entry's number as no longer trustworthy, WITHOUT discarding diff --git a/bridge-node/src/endpoints.ts b/bridge-node/src/endpoints.ts index 5a4b8ff..10e52e7 100644 --- a/bridge-node/src/endpoints.ts +++ b/bridge-node/src/endpoints.ts @@ -130,9 +130,9 @@ export function indigoDeviceIdFrom(uniqueId: string): number | undefined { * or mutated. * * **It is NOT where a live endpoint's id comes from, and reverting it to that - * would be silent.** Since issues #219/#240 an `Endpoint.id` is built from + * would be silent.** Since issue #240 an `Endpoint.id` is built from * `spec.publishedAs` (`createEndpoint`), which merely DEFAULTS to this - * derivation — a re-adopted or role-changed accessory publishes something + * derivation — a migrated or role-changed accessory publishes something * else, and rebuilding the id from `indigoDeviceId` here would hand it a * different `Endpoint.id`, i.e. a different matter.js endpoint number, i.e. a * duplicate accessory in every paired ecosystem. Neither this nor @@ -146,9 +146,9 @@ export const endpointIdFor = uniqueIdFor; /** * `SerialNumber` must differ from `UniqueID` (Matter rejects equal values), so * the `indigo-` prefix is stripped here and kept there. Takes the *published* - * identity, not the driving device id (issues #219/#240): a re-adopted - * accessory keeps publishing the OLD serial number, because §6.3's identity - * flow now runs through `publishedAs`, not `indigoDeviceId`, directly. + * identity, not the driving device id (issue #240): a migrated accessory + * keeps publishing the identity it inherited, because §6.3's identity flow + * runs through `publishedAs`, not `indigoDeviceId`, directly. */ export function serialNumberFrom(publishedAs: string): string { return publishedAs.slice(UNIQUE_ID_PREFIX.length); @@ -587,8 +587,8 @@ interface CommandSink { * roles. This map is that closure, keyed on the one identifier a behaviour can * always reach: `this.endpoint.id`, which IS the endpoint's `spec.publishedAs` * (see {@link createEndpoint}) — the CommandSink value carries `indigoDeviceId` - * separately (issues #219/#240) precisely because, under a re-adopted - * identity, the two are no longer a pure function of one another. + * separately (issue #240) precisely because, under a migrated identity, the + * two are no longer a pure function of one another. * * Registration and teardown both live in {@link watchCommands} so the lifetime * is identical to the observable listeners': one place removes both, and a @@ -2639,11 +2639,10 @@ export async function applyStates( // gets. const { battery, rest } = splitBattery(states); if (battery !== undefined && !hasBattery) { - // Passed in by the caller (issues #219/#240) rather than derived from - // `endpoint.id`: under a re-adopted published identity, `endpoint.id` + // Passed in by the caller (issue #240) rather than derived from + // `endpoint.id`: under a migrated published identity, `endpoint.id` // no longer parses back to the device driving it, and deriving from it - // here would name the WRONG device — the old, deleted one — in the - // refusal. + // here would name the WRONG device in the refusal. refuseBatteryLevelWithoutBattery(indigoDeviceId); } // A consumed `batteryLevel` counts as consumption for refusal purposes @@ -2938,7 +2937,7 @@ export function watchCommands( ): () => void { const teardown: (() => void)[] = []; // The published identity, not `endpointIdFor(spec.indigoDeviceId)`: under a - // re-adopted identity (issue #219) the two differ, and `COMMAND_SINKS` is + // migrated identity (issue #240) the two differ, and `COMMAND_SINKS` is // keyed on `endpoint.id`/`Endpoint.id`, which IS `spec.publishedAs` (see // {@link createEndpoint}) — the sink has to be found under the same key. const endpointId = spec.publishedAs; diff --git a/bridge-node/src/node.ts b/bridge-node/src/node.ts index 4d8f467..6ea7c60 100644 --- a/bridge-node/src/node.ts +++ b/bridge-node/src/node.ts @@ -64,7 +64,6 @@ import { ErrorCode, type FabricInfo, isRole, - type OrphanRecord, type PairingReport, ProtocolError, RefuseReason, @@ -312,20 +311,20 @@ export class BridgeNode implements BridgeFacade { */ readonly #hygieneClosed = { superseded: 0, dead: 0, rotated: 0 }; /** - * Issue #240, §3 steps 3/5 — `indigoDeviceId → the published identity it - * was last removed under`, for the ONE device that removal drove, kept - * only until the next {@link upsertEndpoint} for that same device - * consumes it. This is what lets a role-change supersession be recognised - * even when its remove and its create arrive as two SEPARATE commands - * (`replace()`) rather than one `attach` batch — see - * {@link noteSupersessionIfPaired}. + * Issue #240 — `indigoDeviceId → the published identity it was last + * SOFTLY removed under`, for the ONE device that removal drove, kept only + * until the next {@link upsertEndpoint} for that same device consumes it. + * This is what lets a role-change supersession be recognised even when + * its remove (an `attach` reconcile's ordinary, soft un-export — see + * {@link forgetRemoved}) and its create arrive as two SEPARATE commands + * rather than one `attach` batch — see {@link noteSupersessionIfPaired}. * * **Written only when NO identity for that device appeared in the same * mutation.** A create that already landed alongside the removal has * settled the question either way — {@link forgetRemoved} has already - * marked it superseded, or decided it was not one (a re-adopt, PR5 design - * E2/E5) — and remembering the removal past that is exactly how a much - * later, unrelated create for the same device would mispair with it. + * marked it superseded, or decided it was not one — and remembering the + * removal past that is exactly how a much later, unrelated create for the + * same device would mispair with it. */ readonly #lastRemoved = new Map(); @@ -569,8 +568,8 @@ export class BridgeNode implements BridgeFacade { const specs: EndpointSpec[] = []; for (const entry of restorable) { - // Issues #219/#240 — `entry.indigoDeviceId` is already resolved - // ({@link EndpointMapStore.restorable}), from a re-adopt's + // Issue #240 — `entry.indigoDeviceId` is already resolved + // ({@link EndpointMapStore.restorable}), from a migrate's // `deviceId` where one is recorded, falling back to the published // identity's own derivation otherwise; an entry that resolved to // neither is never returned by `restorable()` at all, and was @@ -591,13 +590,13 @@ export class BridgeNode implements BridgeFacade { } specs.push({ indigoDeviceId: entry.indigoDeviceId, - // Issues #219/#240 — the map's OWN key, not a re-derivation - // from the device id: a re-adopted identity's `Endpoint.id` - // must be the identity a paired ecosystem already knows, not - // whatever `indigo-` would derive to for whichever - // device currently drives it (`entry.indigoDeviceId` and the - // device id embedded in `entry.uniqueId` can now legitimately - // differ — that IS a re-adopt). + // Issue #240 — the map's OWN key, not a re-derivation from the + // device id: a migrated identity's `Endpoint.id` must be the + // identity a paired ecosystem already knows, not whatever + // `indigo-` would derive to for whichever device + // currently drives it (`entry.indigoDeviceId` and the device + // id embedded in `entry.uniqueId` can now legitimately differ + // — that IS a migrate). publishedAs: entry.uniqueId, role: entry.role, label: entry.label, @@ -1398,11 +1397,11 @@ export class BridgeNode implements BridgeFacade { * The live set as `endpoint-map.json` records it: the number, plus the * `role`/`label` that let it be rebuilt at the next start (issue #141). * - * Issues #219/#240 — `uniqueId` is `identity.publishedAs`, not a re- - * derivation from `indigoDeviceId`: a re-adopted or superseded identity is - * no longer a pure function of the device id (§1.1). `deviceId` is carried - * along too, so {@link EndpointMapStore.check} can tell a re-adopt (the - * same identity, a CHANGED driving device) from a first sighting. + * Issue #240 — `uniqueId` is `identity.publishedAs`, not a re-derivation + * from `indigoDeviceId`: a migrated or superseded identity is no longer a + * pure function of the device id (§1.1). `deviceId` is carried along too, + * so {@link EndpointMapStore.check} can tell a migrate (the same + * identity, a CHANGED driving device) from a first sighting. */ private liveIdentities(): LiveEndpointNumber[] { return (this.#registry?.identities() ?? []).map(identity => ({ @@ -1455,9 +1454,12 @@ export class BridgeNode implements BridgeFacade { * `plan.remove`/`plan.create` land in one before/after snapshot here (§3's * full-`attach` path), so the pairing is found directly. {@link supersedes} * is what decides, and it is deliberately narrower than "one removal and - * one create for the same device": a re-adopt onto an already-exported - * device (PR5 design E2/E5) is that shape too, and the identity it leaves behind - * is an ORDINARY orphan the re-adopt picker must go on offering. + * one create for the same device": migrating an accessory onto an + * already-exported device (issue #246, the ● case) is that shape too — + * the target's own OLD identity and the newly-inherited one are unrelated + * strings — and the identity it leaves behind is an ORDINARY orphan: kept + * (its number is never reissued — ADR-0010), but not restorable, and + * offered nowhere. * * A removal with no create for its device at all is remembered in * {@link #lastRemoved}: the plugin's `replace()` sends `remove_endpoint` @@ -1466,17 +1468,18 @@ export class BridgeNode implements BridgeFacade { * finishes — {@link upsertEndpoint} is what closes the gap when it does. * * **`options.hard` (issue #274) destroys rather than orphans the - * `ordinary` bucket.** Only {@link removeEndpoint} ever passes it, and - * only when the plugin has declared the removal PERMANENT — the driving - * device is deleted, or deliberately un-exported. `attach`'s reconcile - * (`reconcile()`) never does: a full reconcile cannot tell "the plugin - * dropped this on purpose" from "this device exists but failed - * classification this pass" (issue #274 constraint 2), so its ordinary - * removals stay soft — orphaned, kept, restorable — exactly as before - * this issue. A hard removal is also never eligible for the supersede - * pairing above (a deleted device gets no later create to pair with) or - * for {@link #lastRemoved} (nothing should retroactively mark a - * DESTROYED entry `supersededBy`). + * `ordinary` bucket.** Only {@link removeEndpoint} ever passes it, and it + * always does — every caller of the `remove_endpoint` command is + * confirming the driving device is gone for good (deleted, or + * deliberately un-exported). `attach`'s reconcile (`reconcile()`) never + * does: a full reconcile cannot tell "the plugin dropped this on + * purpose" from "this device exists but failed classification this + * pass" (issue #274 constraint 2), so its ordinary removals stay soft — + * orphaned, kept, restorable — exactly as before this issue. A hard + * removal is also never eligible for the supersede pairing above (a + * deleted device gets no later create to pair with) or for + * {@link #lastRemoved} (nothing should retroactively mark a DESTROYED + * entry `supersededBy`). */ private forgetRemoved(before: ReadonlyMap, options: { hard?: boolean } = {}): void { const after = this.livePublishedIdentities(); @@ -1502,9 +1505,9 @@ export class BridgeNode implements BridgeFacade { if (!options.hard && created === undefined) { // Only when NO identity for this device appeared in this // mutation: a create that already landed and was NOT a - // supersession (a re-adopt, PR5 design E2/E5) has settled the - // question, and remembering the removal past it is exactly - // how a much later, unrelated create would mispair. + // supersession has settled the question, and remembering the + // removal past it is exactly how a much later, unrelated + // create would mispair. this.#lastRemoved.set(deviceId, uniqueId); } } @@ -1513,8 +1516,7 @@ export class BridgeNode implements BridgeFacade { if (forgotten > 0) { this.log( `${forgotten} endpoint map record(s) destroyed — the driving device is gone for good ` + - "(deleted or un-exported); no re-adopt is offered for them and their numbers are not " + - "retained (issue #274)", + "(deleted or un-exported); their numbers are not retained (issue #274)", ); } return; @@ -1529,25 +1531,25 @@ export class BridgeNode implements BridgeFacade { } /** - * The other half of a two-command role-change supersession (issue #240, - * §3 steps 3/5): `replace()` sends `remove_endpoint` then - * `upsert_endpoint` as two SEPARATE commands, so the pairing - * {@link forgetRemoved} finds within one mutation cannot see this one — - * the create had not happened yet when the remove's `forgetRemoved` ran - * and orphaned the old identity with no `supersededBy`. This is called - * once a create has actually landed: if the device this spec drives was - * last removed under an identity this one {@link supersedes}, that - * removal is retroactively marked superseded by this one. + * The other half of a cross-command role-change supersession (issue + * #240): a device un-exported by an `attach` reconcile — necessarily + * SOFT, since a full reconcile can never declare a removal permanent + * (issue #274 constraint 2, {@link forgetRemoved}) — with no create for + * it in that SAME mutation is remembered in {@link #lastRemoved}. If a + * LATER, separate `upsert_endpoint` then lands for that device under a + * later generation of the identity it left behind, this retroactively + * marks that removal superseded by the new one — the pairing + * {@link forgetRemoved} finds within one mutation cannot see a create + * that had not happened yet. * * **Consumed unconditionally — matched or not.** Anything that is not a - * generation bump on the SAME identity is an ordinary re-export or a - * re-adopt (PR5 design E2/E5), neither of which retires the identity that was - * removed; and either way the bookkeeping must not linger to pair against - * some unrelated, later create for the same device. `#lastRemoved` is - * in-memory only and never persisted: lost on restart is safe, because a - * restart means the plugin's next `attach` reconciles from scratch, and a - * removal that never got its `supersededBy` this way is simply an - * ordinary orphan (PR5 design edge case E7). + * generation bump on the SAME identity is an ordinary re-export, which + * does not retire the identity that was removed; and either way the + * bookkeeping must not linger to pair against some unrelated, later + * create for the same device. `#lastRemoved` is in-memory only and never + * persisted: lost on restart is safe, because a restart means the + * plugin's next `attach` reconciles from scratch, and a removal that + * never got its `supersededBy` this way is simply an ordinary orphan. */ private noteSupersessionIfPaired(spec: EndpointSpec): void { const oldPublishedAs = this.#lastRemoved.get(spec.indigoDeviceId); @@ -1664,6 +1666,10 @@ export class BridgeNode implements BridgeFacade { /** * §3.3 * + * Destroys the endpoint-map record along with the live endpoint (issue + * #274) — every caller of this command is confirming the driving device + * is gone for good. See {@link forgetRemoved}'s ``hard`` option. + * * The drift check runs here too, even though a removal creates nothing. * matter.js re-allocates while endpoints come and go, and a removal is one * of the moments a *surviving* endpoint's number could differ from what the @@ -1671,12 +1677,12 @@ export class BridgeNode implements BridgeFacade { * only one that never looked would have made that invisible until the next * upsert happened to notice. */ - async removeEndpoint(indigoDeviceId: number, permanent = false): Promise { + async removeEndpoint(indigoDeviceId: number): Promise { const before = this.livePublishedIdentities(); try { return await this.registry.remove(indigoDeviceId); } finally { - this.forgetRemoved(before, { hard: permanent }); + this.forgetRemoved(before, { hard: true }); this.checkDrift(); } } @@ -2029,11 +2035,6 @@ export class BridgeNode implements BridgeFacade { return this.getStatus(); } - /** §3.12 (issue #219) — the re-adopt picker's data, straight off the map. */ - listOrphans(): OrphanRecord[] { - return this.#endpointMap.orphans(); - } - getPairing(): PairingReport { const commissioned = this.server.lifecycle.isCommissioned; const window = this.#window.current; diff --git a/bridge-node/src/protocol.ts b/bridge-node/src/protocol.ts index 7fdcb58..06cbb28 100644 --- a/bridge-node/src/protocol.ts +++ b/bridge-node/src/protocol.ts @@ -201,12 +201,12 @@ export function parsePublishedId(value: string): { deviceId: number; generation: * * The narrow question, deliberately, because it is the only removal-plus-create * pair that retires an identity for good. A removal and a create for the same - * `indigoDeviceId` is NOT enough on its own: a re-adopt onto an - * already-exported device (PR5 design E2/E5) is also one removal plus one create for - * one device, and the identity it leaves behind is an ordinary orphan the - * re-adopt picker must go on offering — PR5 design E5 says so in as many words. Only a - * generation bump means "this identity has been replaced and its number is - * retired"; everything else means "this identity is simply not live right now". + * `indigoDeviceId` is NOT enough on its own: migrating an accessory onto an + * already-exported device (issue #246) is also one removal plus one create + * for one device, and the identity it leaves behind is an ordinary orphan, + * not a retired one. Only a generation bump means "this identity has been + * replaced and its number is retired"; everything else means "this identity + * is simply not live right now". */ export function supersedes(oldPublishedAs: string, newPublishedAs: string): boolean { const before = parsePublishedId(oldPublishedAs); @@ -524,26 +524,6 @@ export interface DriftEntry { actual: number; } -/** - * §3.12 `list_orphans` (issue #219) — one left-behind accessory identity the - * re-adopt picker could offer. Defined here rather than in `endpoint-map.ts` - * (which is where {@link EndpointMapStore.orphans} actually builds these) so - * `BridgeFacade.listOrphans()` — landing alongside the `list_orphans` command - * itself — can be declared without `endpoint-map.ts` reaching back into a - * module that already imports FROM it: the existing direction is - * `endpoint-map.ts` → `protocol.ts`, never the reverse. - */ -export interface OrphanRecord { - uniqueId: string; - number: number; - role?: string; - label?: string; - /** ISO-8601, or absent for a pre-PR5 orphan — the picker renders that as "date unknown". */ - orphanedAt?: string; - /** The device that drove this identity before it was un-exported, if recorded. */ - deviceId?: number; -} - /** §3.7 */ export interface PairingReport { commissioned: boolean; @@ -587,17 +567,11 @@ export interface BridgeFacade { /** * §3.3 — idempotent; `{removed: false}` for a device with no live endpoint. * - * `permanent` (issue #274, default `false`) is the confirmed-gone - * declaration: `true` destroys the endpoint-map record along with the - * live endpoint (no orphan, no re-adopt), for a device that has been - * deleted or deliberately un-exported. `false` — the default, and what - * every pre-#274 caller still gets — keeps the pre-existing orphan - * behaviour, which the two-command supersede/re-adopt sequence - * (`export_bridge.replace()`) depends on: its `upsert_endpoint` half - * still needs an orphaned entry to mark `supersededBy`, or to hand to - * the re-adopt picker. + * Destroys the endpoint-map record along with the live endpoint (issue + * #274): the driving device has been deleted or deliberately un-exported, + * and is not coming back. */ - removeEndpoint(indigoDeviceId: number, permanent?: boolean): Promise; + 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`. */ @@ -638,11 +612,6 @@ export interface BridgeFacade { factoryReset(preserveEndpointNumbers: boolean): Promise; /** §3.11 — adopt the live endpoint numbers as the new persisted map. */ rebuildEndpointMap(): Promise; - /** - * §3.12 — every left-behind accessory identity the re-adopt picker (#219) - * could offer. Read-only: nothing about listing orphans changes the map. - */ - listOrphans(): OrphanRecord[]; } /** 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 index 2b65338..9a618f3 100644 --- a/bridge-node/src/reconcile.ts +++ b/bridge-node/src/reconcile.ts @@ -177,31 +177,6 @@ export function parsePreserveEndpointNumbers(value: unknown): boolean { return value; } -/** - * §3.3: whether a `remove_endpoint` is a PERMANENT departure — the driving - * Indigo device is confirmed gone (deleted, or deliberately un-exported) — - * rather than the removal half of a role-change/re-adopt `replace()`, which - * the plugin always follows with an `upsert_endpoint` for the same identity - * (issue #274). - * - * Absent means `false`, matching every `remove_endpoint` call before this - * flag existed: the two-command supersede/re-adopt sequence - * (`export_bridge.replace()`) must keep getting the ORIGINAL soft removal — - * orphaned, not destroyed — so the second command's `upsert_endpoint` still - * has an entry to retroactively mark `supersededBy`, or to hand back to the - * re-adopt picker. Only a caller that KNOWS the device will never come back - * opts in explicitly. - */ -export function parsePermanentRemoval(value: unknown): boolean { - if (value === undefined) { - return false; - } - if (typeof value !== "boolean") { - throw new ProtocolError(ErrorCode.malformedArgs, "permanent must be a boolean"); - } - return value; -} - /** §3.1: the opt-in that makes emptying the endpoint set deliberate. */ export function parseReplaceAll(intent: unknown): boolean { if (intent === undefined) { @@ -368,11 +343,11 @@ export function planReconcile( // // **Deliberately the BROAD rule, unlike `supersedes()`.** The narrow // generation test is right for `supersededBy`, which answers "is this - // identity retired for good?" — a re-adopt's is not. The guard asks a - // different question: "does this device still have an accessory - // afterwards?" A re-adopt and a supersession both answer yes, so both - // belong here, and `stillExportedCount` is named for what it counts - // rather than for one of the two things that produce it. + // identity retired for good?" The guard asks a different question: + // "does this device still have an accessory afterwards?" — true for any + // remove-plus-create pair on the same device, supersession or not, so + // `stillExportedCount` is named for what it counts rather than for the + // one thing that usually produces it. const createdDeviceIds = new Set(plan.create.map(spec => spec.indigoDeviceId)); const stillExportedCount = plan.remove.filter(publishedAs => { const removedDeviceId = live.get(publishedAs)?.indigoDeviceId; diff --git a/bridge-node/src/registry.ts b/bridge-node/src/registry.ts index 0e13e68..a1dd5ba 100644 --- a/bridge-node/src/registry.ts +++ b/bridge-node/src/registry.ts @@ -59,10 +59,10 @@ interface LiveEndpoint { */ battery: boolean; /** - * Issues #219/#240 — the accessory identity this live endpoint was built - * with (`Endpoint.id`/`UniqueID`/`SerialNumber`). Held here rather than - * re-derived because a re-adopted identity is no longer a pure function of - * `indigoDeviceId` — see {@link EndpointSpec.publishedAs}. + * Issue #240 — the accessory identity this live endpoint was built with + * (`Endpoint.id`/`UniqueID`/`SerialNumber`). Held here rather than + * re-derived because a role-changed or migrated identity is no longer a + * pure function of `indigoDeviceId` — see {@link EndpointSpec.publishedAs}. */ publishedAs: string; /** Reassigned when a failed close forces the listeners to be restored. */ @@ -300,7 +300,7 @@ export class EndpointRegistry { const live = this.liveComposition(); const plan = planReconcile(live, desired, replaceAll); - // Issues #219/#240 — a removed identity whose `indigoDeviceId` also + // Issue #240 — a removed identity whose `indigoDeviceId` also // appears in `plan.create` is one device changing which identity it // publishes, planned as removal+create rather than an in-place recreate // (`reconcile.ts`'s `planReconcile`). Logged before anything mutates so @@ -309,12 +309,10 @@ export class EndpointRegistry { // TWO different things have that shape, and they must not share a // sentence. `supersedes()` is the same narrow test `node.ts` uses to // decide whether to write `supersededBy`: only a LATER GENERATION of - // the same identity retires the old one for good. A re-adopt (PR5 - // design E2/E5) is also one removal plus one create for one device, and - // the identity it leaves behind is an ORDINARY orphan the map goes on - // offering to the re-adopt picker — so saying "retired … never reused" - // about it was flatly wrong, and its "role X → X" made the line read - // as a role change that had not happened. + // the same identity retires the old one for good. Anything else with + // this shape leaves an ORDINARY orphan behind — so saying "retired … + // never reused" about it would be flatly wrong, and its "role X → X" + // would make the line read as a role change that had not happened. const createdByDeviceId = new Map(plan.create.map(spec => [spec.indigoDeviceId, spec])); for (const oldPublishedAs of plan.remove) { const old = live.get(oldPublishedAs); @@ -336,8 +334,7 @@ export class EndpointRegistry { `Endpoint ${old.indigoDeviceId} is moving from accessory identity ${oldPublishedAs} ` + `(number ${number}) to ${created.publishedAs}: controllers process a removal and an ` + "addition. This is NOT a supersession — the identity it is leaving is an ordinary " + - "left-behind accessory that stays re-adoptable, and its number is kept for it " + - "(issue #219 re-adopt).", + "left-behind accessory, and its number is kept for it.", ); } } diff --git a/bridge-node/src/ws-server.ts b/bridge-node/src/ws-server.ts index 4b14fde..93040b0 100644 --- a/bridge-node/src/ws-server.ts +++ b/bridge-node/src/ws-server.ts @@ -33,7 +33,6 @@ import { parseEndpointSpec, parseEndpointSpecs, parseFabricIndex, - parsePermanentRemoval, parsePreserveEndpointNumbers, parseReplaceAll, } from "./reconcile.js"; @@ -90,7 +89,7 @@ export class BridgeWsServer { this.options.bridge.upsertEndpoint(parseEndpointSpec(args.endpoint)), ); this.#handlers.set("remove_endpoint", async args => - this.options.bridge.removeEndpoint(parseDeviceId(args.indigoDeviceId), parsePermanentRemoval(args.permanent)), + 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)); @@ -105,7 +104,6 @@ export class BridgeWsServer { return {}; }); this.#handlers.set("rebuild_endpoint_map", async () => this.options.bridge.rebuildEndpointMap()); - this.#handlers.set("list_orphans", async () => this.options.bridge.listOrphans()); options.bridge.onWindowClosed(reason => this.sendEvent(EventName.windowClosed, { reason })); options.bridge.onCommand(data => this.sendEvent(EventName.command, data)); options.bridge.onDriftDetected(drift => this.sendEvent(EventName.driftDetected, { drift })); diff --git a/bridge-node/test/endpoint-map.test.ts b/bridge-node/test/endpoint-map.test.ts index fb9b523..c34242e 100644 --- a/bridge-node/test/endpoint-map.test.ts +++ b/bridge-node/test/endpoint-map.test.ts @@ -652,15 +652,12 @@ describe("EndpointMapStore.forget — un-export without losing the number (issue it("marks the entry orphaned, keeping BOTH the restoration half and the identity half", () => { // ⊗ The other half of `restorable`. Without it a device the user // un-exported stayed restorable for ever and was rebuilt-then-removed on - // every boot. Deleting role/label (the pre-#219 behaviour) worked too but - // threw away the only evidence a future re-adopt UI could match a - // recreated device against — so #219 marks `orphaned` instead of erasing - // anything. Deleting the ENTRY instead would also be wrong: §3.3 retains - // the allocation so a re-export comes back as the same accessory rather - // than a new one in every paired ecosystem. + // every boot. #219 marks `orphaned` rather than erasing role/label — they + // cost nothing to keep. Deleting the ENTRY instead would also be wrong: + // §3.3 retains the allocation so a re-export comes back as the same + // accessory rather than a new one in every paired ecosystem. const dir = storage(); - const now = new Date("2026-08-18T07:21:04.000Z"); - const store = new EndpointMapStore(dir, undefined, () => now); + const store = new EndpointMapStore(dir); store.load(); store.check([ { uniqueId: "indigo-1", endpointNumber: 2, role: "onOffLight", label: "Lamp" }, @@ -679,7 +676,6 @@ describe("EndpointMapStore.forget — un-export without losing the number (issue role: "onOffLight", label: "Lamp", orphaned: true, - orphanedAt: now.toISOString(), }, "indigo-2": { number: 3, role: "dimmableLight", label: "Other" }, }); @@ -697,9 +693,6 @@ describe("EndpointMapStore.forget — un-export without losing the number (issue assert.deepEqual(store.restorable(), [ { uniqueId: "indigo-1", endpointNumber: 2, indigoDeviceId: 1, role: "onOffLight", label: "Lamp" }, ]); - // `orphanedAt` is dropped along with `orphaned` itself: a live device - // is not orphaned, so a stamp saying when it WAS would be stale - // evidence masquerading as current. assert.deepEqual(mapFileIn(dir).endpoints, { "indigo-1": { number: 2, role: "onOffLight", label: "Lamp" }, }); @@ -754,7 +747,7 @@ describe("EndpointMapStore.forget — un-export without losing the number (issue }); describe("EndpointMapStore.destroy — a confirmed-gone device loses its record entirely (issue #274)", () => { - it("deletes the entry outright: no number, no role/label, not restorable, not offered for re-adopt", () => { + it("deletes the entry outright: no number, no role/label, not restorable", () => { const dir = storage(); const store = new EndpointMapStore(dir); store.load(); @@ -769,7 +762,6 @@ describe("EndpointMapStore.destroy — a confirmed-gone device loses its record assert.deepEqual(store.restorable(), [ { uniqueId: "indigo-2", endpointNumber: 3, indigoDeviceId: 2, role: "dimmableLight", label: "Other" }, ]); - assert.deepEqual(store.orphans(), [], "destroyed, not orphaned — nothing left to offer the re-adopt picker"); assert.deepEqual(mapFileIn(dir).endpoints, { "indigo-2": { number: 3, role: "dimmableLight", label: "Other" }, }); @@ -781,11 +773,10 @@ describe("EndpointMapStore.destroy — a confirmed-gone device loses its record store.load(); store.check([{ uniqueId: "indigo-1", endpointNumber: 2, role: "onOffLight", label: "Lamp" }]); store.forget(["indigo-1"]); - assert.deepEqual(store.orphans().map(orphan => orphan.uniqueId), ["indigo-1"]); + assert.equal(mapFileIn(dir).endpoints["indigo-1"]?.orphaned, true); assert.equal(store.destroy(["indigo-1"]), 1); - assert.deepEqual(store.orphans(), []); assert.equal(mapFileIn(dir).endpoints["indigo-1"], undefined); }); @@ -1322,7 +1313,7 @@ describe("battery (issue #220)", () => { }); }); -describe("the driving device, the orphan date and supersession (issues #219/#240)", () => { +describe("the driving device and supersession (issue #240)", () => { it("records deviceId for a live entry and REPLACES it when the driver changes", () => { const dir = storage(); const store = new EndpointMapStore(dir); @@ -1335,9 +1326,8 @@ describe("the driving device, the orphan date and supersession (issues #219/#240 "indigo-1": { number: 2, role: "onOffLight", label: "Lamp", deviceId: 100 }, }); - // A driver change with no orphan step first — a #246 migrate's shape - // (a #219 re-adopt would have seen forget() in between): the SAME - // identity, now driven by a DIFFERENT device. + // A driver change with no orphan step first — a #246 migrate's shape: + // the SAME identity, now driven by a DIFFERENT device. store.check([ { uniqueId: "indigo-1", endpointNumber: 2, role: "onOffLight", label: "Lamp", deviceId: 200 }, ]); @@ -1358,7 +1348,7 @@ describe("the driving device, the orphan date and supersession (issues #219/#240 endpoints: { "indigo-1": { number: 2, role: "onOffLight", label: "Kitchen Lamp" }, // The KEY alone would parse to device id 2 — `deviceId` must win. - "indigo-2": { number: 3, role: "dimmableLight", label: "Re-adopted", deviceId: 999 }, + "indigo-2": { number: 3, role: "dimmableLight", label: "Migrated", deviceId: 999 }, }, }), ); @@ -1372,7 +1362,7 @@ describe("the driving device, the orphan date and supersession (issues #219/#240 endpointNumber: 3, indigoDeviceId: 999, role: "dimmableLight", - label: "Re-adopted", + label: "Migrated", }, ]); }); @@ -1393,24 +1383,6 @@ describe("the driving device, the orphan date and supersession (issues #219/#240 ); }); - it("stamps orphanedAt from the injected clock when forget() marks an entry", () => { - const dir = storage(); - const now = new Date("2026-08-18T07:21:04.000Z"); - const store = new EndpointMapStore(dir, undefined, () => now); - store.load(); - store.check([{ uniqueId: "indigo-1", endpointNumber: 2, role: "onOffLight", label: "Lamp" }]); - - store.forget(["indigo-1"]); - - assert.deepEqual(mapFileIn(dir).endpoints["indigo-1"], { - number: 2, - role: "onOffLight", - label: "Lamp", - orphaned: true, - orphanedAt: now.toISOString(), - }); - }); - it("marks supersededBy only when the caller says the removal was a supersession", () => { const dir = storage(); const store = new EndpointMapStore(dir); @@ -1431,58 +1403,7 @@ describe("the driving device, the orphan date and supersession (issues #219/#240 assert.equal(mapFileIn(dir).endpoints["indigo-2"]?.supersededBy, "indigo-2~2"); }); - it("orphans() omits superseded entries but includes bare no-role/label ones (PR5 design E4)", () => { - const dir = storage(); - writeFileSync( - join(dir, ENDPOINT_MAP_FILE), - JSON.stringify({ - version: 2, - endpoints: { - "indigo-1": { - number: 2, - role: "onOffLight", - label: "Kitchen Lamp", - orphaned: true, - orphanedAt: "2026-08-01T00:00:00.000Z", - }, - "indigo-2": { - number: 3, - role: "dimmableLight", - label: "Lounge Lamp", - orphaned: true, - supersededBy: "indigo-2~2", - }, - // Pre-2026.16.2: an older plugin's forget() deleted role/label. - "indigo-9": { number: 7, orphaned: true }, - // Live — not orphaned at all. - "indigo-4": { number: 8, role: "onOffLight", label: "Live One" }, - }, - }), - ); - const store = new EndpointMapStore(dir); - store.load(); - - // Superseded "indigo-2" is excluded (not re-adoptable); the live - // "indigo-4" is excluded (not orphaned); the bare "indigo-9" IS - // included, per PR5 design E4 — shown so the picker can tell the user its - // number is reserved and why it cannot be re-adopted, rather than - // hiding it. - assert.deepEqual(store.orphans(), [ - { - uniqueId: "indigo-1", - number: 2, - role: "onOffLight", - label: "Kitchen Lamp", - orphanedAt: "2026-08-01T00:00:00.000Z", - }, - { - uniqueId: "indigo-9", - number: 7, - }, - ]); - }); - - it("clears orphaned and adopts the new deviceId the first time a re-adopted identity is live", () => { + it("clears orphaned and adopts the new deviceId the first time a retargeted identity is live", () => { const dir = storage(); const logged: string[] = []; const store = new EndpointMapStore(dir, message => logged.push(message)); @@ -1500,75 +1421,11 @@ describe("the driving device, the orphan date and supersession (issues #219/#240 assert.deepEqual( mapFileIn(dir).endpoints["indigo-1"], { number: 2, role: "onOffLight", label: "Lamp", deviceId: 200 }, - "orphaned AND orphanedAt are gone, and the device id is the NEW driver", + "orphaned is gone, and the device id is the NEW driver", ); assert.ok( logged.some(line => line.includes("is now driven by Indigo device 200, replacing device 100")), - `expected the re-adopt log line, got ${JSON.stringify(logged)}`, - ); - }); - - it("nudges towards Re-adopt when a NEW identity's role+label match an existing orphan (PR5 design owner ruling 4)", () => { - const dir = storage(); - const logged: string[] = []; - const store = new EndpointMapStore(dir, message => logged.push(message)); - store.load(); - store.check([ - { uniqueId: "indigo-1", endpointNumber: 2, role: "onOffLight", label: "Kitchen Lamp", deviceId: 100 }, - ]); - store.forget(["indigo-1"]); - logged.length = 0; - - // A DIFFERENT device, exported under a BRAND-NEW identity, with the - // same role and the same name — the "deleted, recreated, re-exported" - // case #219 exists for. - store.check([ - { uniqueId: "indigo-2", endpointNumber: 5, role: "onOffLight", label: "Kitchen Lamp", deviceId: 200 }, - ]); - - assert.ok( - logged.some(line => line.includes("Re-adopt a Matter accessory…") && line.includes("indigo-1")), - `expected the readopt nudge, got ${JSON.stringify(logged)}`, - ); - }); - - it("does not nudge for an UPDATE to an existing identity", () => { - const dir = storage(); - const logged: string[] = []; - const store = new EndpointMapStore(dir, message => logged.push(message)); - store.load(); - store.check([{ uniqueId: "indigo-1", endpointNumber: 2, role: "onOffLight", label: "Kitchen Lamp" }]); - store.forget(["indigo-1"]); - logged.length = 0; - - // Re-adding the SAME identity — an ordinary re-export, not a new one. - store.check([{ uniqueId: "indigo-1", endpointNumber: 2, role: "onOffLight", label: "Kitchen Lamp" }]); - - assert.ok( - !logged.some(line => line.includes("Re-adopt a Matter accessory…")), - `did not expect a nudge for an existing identity, got ${JSON.stringify(logged)}`, - ); - }); - - it("does not nudge against a SUPERSEDED orphan", () => { - const dir = storage(); - const logged: string[] = []; - const store = new EndpointMapStore(dir, message => logged.push(message)); - store.load(); - store.check([{ uniqueId: "indigo-1", endpointNumber: 2, role: "onOffLight", label: "Lamp" }]); - store.check([{ uniqueId: "indigo-1~2", endpointNumber: 5, role: "dimmableLight", label: "Lamp" }]); - store.forget(["indigo-1"], { supersededBy: "indigo-1~2" }); - logged.length = 0; - - // Same role+label as the SUPERSEDED indigo-1 — must not nudge towards - // resurrecting a retired role under its retired number. - store.check([ - { uniqueId: "indigo-9", endpointNumber: 9, role: "onOffLight", label: "Lamp", deviceId: 900 }, - ]); - - assert.ok( - !logged.some(line => line.includes("Re-adopt a Matter accessory…")), - `did not expect a nudge against a superseded record, got ${JSON.stringify(logged)}`, + `expected the driver-change log line, got ${JSON.stringify(logged)}`, ); }); @@ -1577,8 +1434,8 @@ describe("the driving device, the orphan date and supersession (issues #219/#240 // the default `indigo-` the supersession retired, so the // identity is LIVE again and the marker saying another one replaced // it is now false. Left behind, it would hide this record from - // `orphans()` the NEXT time it was un-exported — an ordinary orphan - // that could never be re-adopted. + // `restorable()` the NEXT time it was un-exported — an ordinary + // orphan that could never be rebuilt. const dir = storage(); const store = new EndpointMapStore(dir); store.load(); @@ -1600,10 +1457,10 @@ describe("the driving device, the orphan date and supersession (issues #219/#240 ); store.forget(["indigo-1"]); - assert.deepEqual( - store.orphans().map(orphan => orphan.uniqueId), - ["indigo-1"], - "un-exported again, it is an ORDINARY orphan the re-adopt picker can offer", + assert.equal( + mapFileIn(dir).endpoints["indigo-1"]?.supersededBy, + undefined, + "un-exported again, it is an ORDINARY orphan, not a superseded one", ); }); @@ -1665,57 +1522,13 @@ describe("the driving device, the orphan date and supersession (issues #219/#240 }); }); -describe("re-adopt notices reach the client, not just stdout (PR5 design §4.5, issues #219/#240)", () => { +describe("endpoint-map notices reach the client, not just stdout (issue #240)", () => { // `this.log` is stdout and the node is launched by launchd, so a line - // written only there is a line no user will ever see. Each of these three - // was stdout-only; `StatusReport.warnings` is the one channel + // written only there is a line no user will ever see. Each of these was + // stdout-only; `StatusReport.warnings` is the one channel // `export_bridge.py` mirrors into the Indigo event log. - it("puts the Re-adopt nudge on the warnings channel", () => { - const store = new EndpointMapStore(storage()); - store.load(); - store.check([ - { uniqueId: "indigo-1", endpointNumber: 2, role: "onOffLight", label: "Kitchen Lamp", deviceId: 100 }, - ]); - store.forget(["indigo-1"]); - - store.check([ - { uniqueId: "indigo-2", endpointNumber: 5, role: "onOffLight", label: "Kitchen Lamp", deviceId: 200 }, - ]); - - assert.ok( - store.warnings.some(line => line.includes("Re-adopt a Matter accessory…")), - `the nudge must ride StatusReport.warnings, got ${JSON.stringify(store.warnings)}`, - ); - }); - - it("says the nudge once, however many attaches follow", () => { - const store = new EndpointMapStore(storage()); - store.load(); - store.check([ - { uniqueId: "indigo-1", endpointNumber: 2, role: "onOffLight", label: "Kitchen Lamp", deviceId: 100 }, - ]); - store.forget(["indigo-1"]); - store.check([ - { uniqueId: "indigo-2", endpointNumber: 5, role: "onOffLight", label: "Kitchen Lamp", deviceId: 200 }, - ]); - store.forget(["indigo-2"]); - // A second brand-new identity matching the SAME orphan: the nudge - // names indigo-1 again, and the channel must not accumulate a copy - // per attach — a warning set that grows re-logs plugin-side every - // time it changes (`_report_node_warnings` latches on the SET). - store.check([ - { uniqueId: "indigo-3", endpointNumber: 6, role: "onOffLight", label: "Kitchen Lamp", deviceId: 300 }, - ]); - - assert.equal( - store.warnings.filter(line => line.includes("Re-adopt a Matter accessory…")).length, - 2, - `one notice per NEW identity, never a repeat: ${JSON.stringify(store.warnings)}`, - ); - }); - - it("puts the landed-re-adopt confirmation on the warnings channel", () => { + it("puts the driven-by-a-different-device confirmation on the warnings channel", () => { const store = new EndpointMapStore(storage()); store.load(); store.check([ @@ -1774,45 +1587,12 @@ describe("re-adopt notices reach the client, not just stdout (PR5 design §4.5, }); describe("a map entry that cannot be trusted is named, never silently used (PR5 design E11/E12)", () => { - it("does not offer an orphan whose key parsePublishedId refuses", () => { - // The picker writes the row's key into the plugin's ExportStore as a - // `publishedAs`, and the node refuses an unlawful one at the NEXT - // attach with `malformed_args` — which stops every export, not just - // this one. `restorable()` already declines such an entry (PR5 design E11); the - // list feeding a user's choice must too. - const dir = storage(); - writeFileSync( - join(dir, ENDPOINT_MAP_FILE), - JSON.stringify({ - version: ENDPOINT_MAP_VERSION, - endpoints: { - "indigo-1": { number: 2, role: "onOffLight", label: "Lamp", orphaned: true }, - // `Number("indigo-1e3")` would coerce; `parsePublishedId` refuses. - "indigo-1e3": { number: 3, role: "onOffLight", label: "Not Ours", orphaned: true }, - "made-up-key": { number: 4, role: "onOffLight", label: "Mystery", orphaned: true }, - }, - }), - ); - const store = new EndpointMapStore(dir); - store.load(); - - assert.deepEqual( - store.orphans().map(orphan => orphan.uniqueId), - ["indigo-1"], - ); - for (const key of ["indigo-1e3", "made-up-key"]) { - assert.ok( - store.warnings.some(line => line.includes(JSON.stringify(key))), - `${key} must be named, not merely dropped: ${JSON.stringify(store.warnings)}`, - ); - } - }); - it("says so when a present-but-unusable supersededBy is discarded", () => { - // The marker's LOSS re-arms a retired identity as an ordinary re-adopt - // candidate, and re-adopting it resurrects an old-role accessory under - // a number every paired ecosystem has already processed a removal for. - // Tolerating the field is right; losing it in silence is not. + // The marker's LOSS re-arms a retired identity as an ordinary, + // rebuildable one, and rebuilding it resurrects an old-role accessory + // under a number every paired ecosystem has already processed a + // removal for. Tolerating the field is right; losing it in silence + // is not. const dir = storage(); writeFileSync( join(dir, ENDPOINT_MAP_FILE), @@ -1832,7 +1612,6 @@ describe("a map entry that cannot be trusted is named, never silently used (PR5 const store = new EndpointMapStore(dir); store.load(); - assert.equal(store.orphans().length, 1, "it IS re-adoptable again — that is the point"); assert.ok( store.warnings.some( line => line.includes("unusable supersededBy") && line.includes("indigo-1"), @@ -1851,7 +1630,7 @@ describe("a map entry that cannot be trusted is named, never silently used (PR5 const reloaded = new EndpointMapStore(dir); reloaded.load(); - assert.equal(reloaded.orphans().length, 1); + assert.equal(mapFileIn(dir).endpoints["indigo-1"]?.orphaned, true); assert.deepEqual(reloaded.warnings, [], "an ordinary orphan is not an oddity"); }); }); diff --git a/bridge-node/test/fixture-shapes.ts b/bridge-node/test/fixture-shapes.ts index 0d925c9..766fe7b 100644 --- a/bridge-node/test/fixture-shapes.ts +++ b/bridge-node/test/fixture-shapes.ts @@ -20,7 +20,6 @@ import type { EventFrame, FabricInfo, HandshakeFrame, - OrphanRecord, PairingReport, RemoveResult, SessionHygiene, @@ -378,25 +377,3 @@ export const endpointMapInvalid = { error_code: "endpoint_map_invalid", details: "endpoint map is unreadable; only get_status, get_pairing and rebuild_endpoint_map are accepted", } satisfies ErrorFrame; - -/** - * §3.12's answer (issue #219) — three left-behind identities: one with a full - * date, one "date unknown" (`orphanedAt` absent, a pre-PR5 orphan), and one - * bare `{uniqueId, number}` entry (PR5 design E4) — a pre-2026.16.2 orphan with no - * role/label, listed but unmatchable. - */ -export const orphanList = [ - { - uniqueId: "indigo-223456791", - number: 7, - role: "dimmableLight", - label: "Kitchen Lamp", - orphanedAt: "2026-08-12T09:15:00Z", - deviceId: 223456791, - }, - { uniqueId: "indigo-223456792", number: 4, role: "onOffLight", label: "Porch Light" }, - { uniqueId: "indigo-223456793", number: 9 }, -] satisfies OrphanRecord[]; - -/** The same command over a map with nothing orphaned. */ -export const orphanListEmpty = [] satisfies OrphanRecord[]; diff --git a/bridge-node/test/fixtures.test.ts b/bridge-node/test/fixtures.test.ts index 014383b..bbb91b3 100644 --- a/bridge-node/test/fixtures.test.ts +++ b/bridge-node/test/fixtures.test.ts @@ -84,8 +84,6 @@ describe("golden fixtures match their typed mirror", () => { shapes.factoryResetResult], ["rebuild_endpoint_map result", golden.rebuild_endpoint_map.response.result, shapes.rebuiltStatus], ["endpoint_map_invalid", golden.endpoint_map_invalid.response, shapes.endpointMapInvalid], - ["list_orphans result", golden.list_orphans.response.result, shapes.orphanList], - ["list_orphans_empty result", golden.list_orphans_empty.response.result, shapes.orphanListEmpty], ]; for (const [name, actual, expected] of cases) { diff --git a/bridge-node/test/protocol.test.ts b/bridge-node/test/protocol.test.ts index 8f59aea..025af5c 100644 --- a/bridge-node/test/protocol.test.ts +++ b/bridge-node/test/protocol.test.ts @@ -6,7 +6,7 @@ import assert from "node:assert/strict"; import { after, before, describe, it } from "node:test"; -import { ErrorCode, type OrphanRecord, type PairingReport, PROTOCOL_VERSION, ProtocolError } from "../src/protocol.js"; +import { ErrorCode, type PairingReport, PROTOCOL_VERSION, ProtocolError } from "../src/protocol.js"; import { BridgeWsServer } from "../src/ws-server.js"; import { TestClient } from "./client.js"; import { golden, StubBridge } from "./stub-bridge.js"; @@ -361,9 +361,6 @@ describe("gating (§1.1)", () => { // Exempt from this gate only while the node is REFUSING (§1.1); with // a healthy node it is an ordinary command and needs an attach. ["rebuild_endpoint_map", {}], - // §3.12: read-only, but NOT a recovery command — an ordinary attach - // is still required, like every other row above. - ["list_orphans", {}], ]; const client = await connect(); for (const [command, args] of gated) { @@ -894,51 +891,6 @@ describe("fabric and reset commands (§3.9-§3.11)", () => { }); }); -describe("list_orphans (§3.12, issue #219)", () => { - it("answers the golden populated orphan list", async () => { - await withColdBridge(async (cold, connectCold) => { - const client = await connectCold(); - await client.request(golden.attach_with_endpoints.request); - cold.orphans = structuredClone(golden.list_orphans.response.result) as OrphanRecord[]; - const exchange = golden.list_orphans; - assert.deepEqual(await client.request(exchange.request), exchange.response); - client.close(); - }); - }); - - it("answers an empty array when nothing is orphaned", async () => { - await withColdBridge(async (_cold, connectCold) => { - const client = await connectCold(); - await client.request(golden.attach_with_endpoints.request); - const exchange = golden.list_orphans_empty; - assert.deepEqual(await client.request(exchange.request), exchange.response); - client.close(); - }); - }); - - it("requires attach first, like an ordinary command", async () => { - await withColdBridge(async (_cold, connectCold) => { - const client = await connectCold(); - const response = await client.request({ message_id: "lo-gate", command: "list_orphans", args: {} }); - assert.equal(response.error_code, ErrorCode.notAttached); - client.close(); - }); - }); - - it("is NOT a recovery command: refused during endpoint_map_invalid", async () => { - // §1.1's RECOVERY_COMMANDS is deliberately just get_status/get_pairing/ - // rebuild_endpoint_map: a node refusing over an unreadable map has no - // orphans it can vouch for. - await withColdBridge(async (cold, connectCold) => { - cold.refusal = "endpoint map is unreadable"; - const client = await connectCold(); - const response = await client.request({ message_id: "lo-refuse", command: "list_orphans", args: {} }); - assert.equal(response.error_code, ErrorCode.endpointMapInvalid); - client.close(); - }); - }); -}); - describe("the endpoint_map_invalid refuse-to-start state (§1.1, PRD §7)", () => { it("refuses everything outside the recovery trio, attach included", async () => { await withColdBridge(async (cold, connectCold) => { @@ -1025,7 +977,6 @@ describe("the endpoint_map_invalid refuse-to-start state (§1.1, PRD §7)", () = { command: "upsert_endpoint", args: golden.upsert_endpoint.request.args }, { command: "set_reachable", args: { indigoDeviceId: 123456789, reachable: false } }, { command: "open_commissioning_window", args: { durationSeconds: 900 } }, - { command: "list_orphans", args: {} }, ]; await withColdBridge(async (cold, connectCold) => { cold.refusal = "endpoint map is unreadable"; diff --git a/bridge-node/test/published-identity.test.ts b/bridge-node/test/published-identity.test.ts index 182ef41..a668765 100644 --- a/bridge-node/test/published-identity.test.ts +++ b/bridge-node/test/published-identity.test.ts @@ -106,10 +106,12 @@ describe("supersedes() — only a later generation retires an identity (issue #2 assert.equal(supersedes("indigo-7~2", "indigo-7~3"), true); }); - it("is FALSE for the same generation — the re-adopt shape", () => { - // PR5 design E2/E5: one device, one removal, one create, and the - // identity left behind is an ordinary orphan the picker must go on - // offering. Getting this wrong hides a re-adoptable accessory for ever. + it("is FALSE for the same generation, or a different device's identity", () => { + // Migrating an accessory onto an already-exported device (issue + // #246) is also one removal plus one create for one device, and the + // identity left behind must be an ordinary orphan, not a retired + // one. Getting this wrong would resurrect an old-role accessory at + // the next restart. assert.equal(supersedes("indigo-7", "indigo-7"), false); assert.equal(supersedes("indigo-7", "indigo-99"), false); assert.equal(supersedes("indigo-7~2", "indigo-7~2"), false); diff --git a/bridge-node/test/reconcile.test.ts b/bridge-node/test/reconcile.test.ts index 66e1547..b6fca72 100644 --- a/bridge-node/test/reconcile.test.ts +++ b/bridge-node/test/reconcile.test.ts @@ -12,7 +12,6 @@ import { parseDeviceId, parseEndpointSpec, parseEndpointSpecs, - parsePermanentRemoval, parseReplaceAll, planReconcile, } from "../src/reconcile.js"; @@ -34,7 +33,7 @@ function spec( /** * Build a `live` map keyed on `publishedAs`, defaulting each entry to today's * derivation (`indigo-`) the same way {@link spec} does. A test that cares - * about a re-adopted/superseded identity passes one explicitly as the 5th + * about a migrated/superseded identity passes one explicitly as the 5th * tuple element. */ function live( @@ -150,22 +149,6 @@ describe("parseEndpointSpecs / parseReplaceAll (§3.1)", () => { }); }); -describe("parsePermanentRemoval (§3.3, issue #274)", () => { - it("defaults to false — every remove_endpoint call before this flag existed keeps the soft (orphan) behaviour", () => { - assert.equal(parsePermanentRemoval(undefined), false); - }); - - it("honours an explicit true or false", () => { - assert.equal(parsePermanentRemoval(true), true); - assert.equal(parsePermanentRemoval(false), false); - }); - - it("rejects a non-boolean", () => { - assert.equal(refusal(() => parsePermanentRemoval("true")).code, ErrorCode.malformedArgs); - assert.equal(refusal(() => parsePermanentRemoval(1)).code, ErrorCode.malformedArgs); - }); -}); - describe("publishedAs (§4.1, issues #219/#240)", () => { it("defaults publishedAs to indigo- when the wire omits it", () => { assert.equal( diff --git a/bridge-node/test/registry.test.ts b/bridge-node/test/registry.test.ts index 0b7b730..27fc25c 100644 --- a/bridge-node/test/registry.test.ts +++ b/bridge-node/test/registry.test.ts @@ -1266,20 +1266,20 @@ describe("endpoint-number stability (PRD §4.3 / XAC5)", () => { }); }); -/** A re-adopt's replacement device: drives identity `indigo-1`, is not 1. */ +/** A migrated identity's new driving device: drives identity `indigo-1`, is not 1. */ const REPLACEMENT_DEVICE = 700_000_001; -describe("supersede & published identity (issues #219/#240)", () => { - it("publishes the OLD UniqueID and SerialNumber for a re-adopted identity", async () => { - // The PR5 design F7/F8 pin, and the whole of #219 in one assertion: the only - // identity a controller can see is the endpoint number plus the - // bridged Basic Information attributes, and matter.js does NOT - // persist `uniqueId`/`serialNumber` — whatever is declared at - // construction is what is published. A re-adopted accessory is - // therefore only "the same accessory" to Apple Home if BOTH are - // derived from `publishedAs` rather than from the device now driving - // it; derive either from `indigoDeviceId` and every paired ecosystem - // sees a brand-new accessory, room and scenes gone. +describe("supersede & published identity (issue #240)", () => { + it("publishes the OLD UniqueID and SerialNumber for a migrated identity", async () => { + // The PR5 design F7/F8 pin: the only identity a controller can see is + // the endpoint number plus the bridged Basic Information attributes, + // and matter.js does NOT persist `uniqueId`/`serialNumber` — + // whatever is declared at construction is what is published. A + // migrated accessory is therefore only "the same accessory" to Apple + // Home if BOTH are derived from `publishedAs` rather than from the + // device now driving it; derive either from `indigoDeviceId` and + // every paired ecosystem sees a brand-new accessory, room and scenes + // gone. const h = await harness(); try { await h.registry.reconcile( @@ -1297,11 +1297,11 @@ describe("supersede & published identity (issues #219/#240)", () => { } }); - it("routes a command from a re-adopted accessory to the NEW Indigo device", async () => { + it("routes a command from a migrated accessory to the NEW Indigo device", async () => { // The COMMAND_SINKS indirection (owner decision 2). The sink is keyed // on `Endpoint.id` — which IS `publishedAs` — while the value carries // the driving `indigoDeviceId`. Key it on `indigo-` - // instead and an ecosystem-originated command on a re-adopted + // instead and an ecosystem-originated command on a migrated // accessory finds no sink at all: Apple Home says the accessory did // not respond, for ever, and nothing in Indigo moves. const h = await harness(); @@ -1359,38 +1359,38 @@ describe("supersede & published identity (issues #219/#240)", () => { } }); - it("does not call a re-adopt a supersession, or print role X → X (comment-analyzer C2)", async () => { - // The PR5 design E2/E5 shape: ONE device, ONE removal, ONE create, and the - // identity left behind is an ORDINARY orphan `orphans()`/§3.12 must - // go on offering. The broad "removal + create for the same device" - // rule logged "identity … is being retired … never reused" about it — - // the opposite of what the map does — and printed "role onOffLight → - // onOffLight", which reads as a role change nobody asked for. + it("does not call this shape a supersession, or print role X → X (comment-analyzer C2)", async () => { + // ONE device, ONE removal, ONE create, and the identity left behind + // is an ORDINARY orphan. The broad "removal + create for the same + // device" rule logged "identity … is being retired … never reused" + // about it — the opposite of what the map does — and printed "role + // onOffLight → onOffLight", which reads as a role change nobody + // asked for. const h = await harness(); try { await h.registry.reconcile([spec(1, Role.onOffLight)], false); h.logs.length = 0; // Device 1 stops publishing indigo-1 and starts publishing - // indigo-99 — a re-adopt onto an already-exported device. + // indigo-99 — not a generation bump of its own identity. await h.registry.reconcile( [spec(1, Role.onOffLight, { publishedAs: uniqueIdFor(99) })], false, ); assert.ok( !h.logs.some(line => line.startsWith("Superseding endpoint")), - `a re-adopt is not a supersession: ${h.logs.join("\n")}`, + `not a supersession: ${h.logs.join("\n")}`, ); assert.ok( !h.logs.some(line => line.includes("The retired number is never reused.")), - `the old identity stays re-adoptable and keeps its number: ${h.logs.join("\n")}`, + `the old identity stays an ordinary orphan and keeps its number: ${h.logs.join("\n")}`, ); assert.ok( h.logs.some( line => line.includes(`moving from accessory identity ${uniqueIdFor(1)}`) && line.includes(`to ${uniqueIdFor(99)}`) && - line.includes("stays re-adoptable"), + line.includes("ordinary left-behind accessory"), ), `expected the identity-move line, got ${h.logs.join("\n")}`, ); diff --git a/bridge-node/test/restore.test.ts b/bridge-node/test/restore.test.ts index eb56377..4b9c136 100644 --- a/bridge-node/test/restore.test.ts +++ b/bridge-node/test/restore.test.ts @@ -172,17 +172,6 @@ function readMap(storagePath: string): EndpointMapFile { return JSON.parse(readFileSync(join(storagePath, ENDPOINT_MAP_FILE), "utf8")) as EndpointMapFile; } -/** - * Strip `forget()`'s non-deterministic `orphanedAt` stamp (issue #219) so the - * rest of a persisted record can still be `deepEqual`ed exactly; its own - * stamping is pinned in `endpoint-map.test.ts`, so these integration tests - * only need to know it is THERE, not what it says. - */ -function withoutOrphanedAt(record: T): Omit { - const { orphanedAt, ...rest } = record; - return rest; -} - /** `upsert_endpoint` for one freshly-specced onOffLight accessory, awaited. */ async function upsertOne(client: TestClient, messageId: string, indigoDeviceId: number): Promise { client.send({ @@ -510,9 +499,8 @@ describe("issue #141: an un-exported device stops being restored", () => { // `forgetRemoved` call in `node.reconcile` and the third boot below // restores 2 and the attach removes one of them again. // - // Since issue #219, `forget` keeps role/label (re-adopt evidence for a - // future recreated-device UI) rather than deleting them — `orphaned` is - // what now keeps the entry out of `restorable`. + // `forget` keeps role/label rather than deleting them — `orphaned` is + // what keeps the entry out of `restorable`. const storagePath = storage(); const numbers = await seedTwoAccessories(storagePath); @@ -526,11 +514,10 @@ describe("issue #141: an un-exported device stops being restored", () => { const afterRemoval = readMap(storagePath); assert.deepEqual( - withoutOrphanedAt(afterRemoval.endpoints[uniqueIdFor(LOUNGE)]!), + afterRemoval.endpoints[uniqueIdFor(LOUNGE)]!, { number: numbers[LOUNGE], role: "dimmableLight", label: "Lounge Lamp", orphaned: true, deviceId: LOUNGE }, - "the number AND role/label survive (§3.3, #219) so a re-export returns the SAME " + - "accessory and the entry remains re-adopt evidence, but `orphaned` keeps it " + - "out of the pre-attach rebuild", + "the number AND role/label survive (§3.3) so a re-export returns the SAME accessory, " + + "but `orphaned` keeps it out of the pre-attach rebuild", ); assert.deepEqual( afterRemoval.endpoints[uniqueIdFor(KITCHEN)], @@ -614,11 +601,13 @@ describe("issue #141: an un-exported device stops being restored", () => { } }); - it("forgets one removed through remove_endpoint too, not just through attach", async () => { + it("destroys one removed through remove_endpoint too, not just through attach", async () => { // ⊗ §3.3's own removal path is the other way a device leaves the live - // set, and a fix wired only into `attach` would leave it making ghosts. + // set. Every `remove_endpoint` command is the plugin confirming the + // driving device is gone for good (issue #274), so this is a destroy, + // not an orphan. const storagePath = storage(); - const numbers = await seedTwoAccessories(storagePath); + await seedTwoAccessories(storagePath); const session = await boot(storagePath); try { @@ -639,13 +628,7 @@ describe("issue #141: an un-exported device stops being restored", () => { await session.close(); } - assert.deepEqual(withoutOrphanedAt(readMap(storagePath).endpoints[uniqueIdFor(LOUNGE)]!), { - number: numbers[LOUNGE], - role: "dimmableLight", - label: "Lounge Lamp", - orphaned: true, - deviceId: LOUNGE, - }); + assert.equal(readMap(storagePath).endpoints[uniqueIdFor(LOUNGE)], undefined); }); it("forgets only what it watched go, never merely what is absent", async () => { @@ -709,17 +692,16 @@ describe("issue #141: an un-exported device stops being restored", () => { }); }); -/** `remove_endpoint`, optionally with the issue #274 `permanent` flag, awaited. */ +/** `remove_endpoint`, awaited. */ async function removeOne( client: TestClient, messageId: string, indigoDeviceId: number, - permanent?: boolean, ): Promise<{ removed: boolean }> { client.send({ message_id: messageId, command: "remove_endpoint", - args: { indigoDeviceId, ...(permanent === undefined ? {} : { permanent }) }, + args: { indigoDeviceId }, }); for (;;) { const frame = await client.next(10_000); @@ -730,8 +712,8 @@ async function removeOne( } } -describe("issue #274: remove_endpoint's permanent flag destroys rather than orphans", () => { - it("drops the map entry entirely — no number, not restorable, not offered for re-adopt", async () => { +describe("issue #274: remove_endpoint destroys rather than orphans", () => { + it("drops the map entry entirely — no number, not restorable", async () => { const storagePath = storage(); const numbers = await seedTwoAccessories(storagePath); @@ -741,13 +723,11 @@ describe("issue #274: remove_endpoint's permanent flag destroys rather than orph // with the SAME set already live, so this is a pure update and // does not itself touch LOUNGE. await attach(session.client, "a1", BOTH); - const result = await removeOne(session.client, "r1", LOUNGE, true); + const result = await removeOne(session.client, "r1", LOUNGE); assert.equal(result.removed, true); assert.deepEqual(readMap(storagePath).endpoints[uniqueIdFor(LOUNGE)], undefined, "the entry is gone, not orphaned"); - assert.deepEqual(session.bridge.listOrphans(), [], - "nothing is left to offer the re-adopt picker"); assert.equal( childOf(session.bridge.server, LOUNGE), undefined, "the live accessory is gone from the aggregator too", @@ -771,55 +751,17 @@ describe("issue #274: remove_endpoint's permanent flag destroys rather than orph } }); - it("without the flag, keeps today's soft (orphan) behaviour — the default is unchanged", async () => { - const storagePath = storage(); - await seedTwoAccessories(storagePath); - - const session = await boot(storagePath); - try { - await attach(session.client, "a1", BOTH); - await removeOne(session.client, "r1", LOUNGE); - - const record = readMap(storagePath).endpoints[uniqueIdFor(LOUNGE)]; - assert.equal(record?.orphaned, true, "the pre-#274 orphan behaviour is untouched by default"); - assert.deepEqual( - session.bridge.listOrphans().map(orphan => orphan.uniqueId), - [uniqueIdFor(LOUNGE)], - "still offered for re-adopt, exactly as before this issue", - ); - } finally { - await session.close(); - } - }); - - it("is idempotent on an already-absent endpoint, exactly like a non-permanent remove", async () => { + it("is idempotent on an already-absent endpoint", async () => { const storagePath = storage(); const session = await boot(storagePath); try { await attach(session.client, "a1", []); - const result = await removeOne(session.client, "r1", 999999999, true); + const result = await removeOne(session.client, "r1", 999999999); assert.deepEqual(result, { removed: false }); } finally { await session.close(); } }); - - it("rejects a non-boolean permanent with malformed_args", async () => { - const storagePath = storage(); - const session = await boot(storagePath); - try { - await attach(session.client, "a1", []); - session.client.send({ - message_id: "r1", - command: "remove_endpoint", - args: { indigoDeviceId: KITCHEN, permanent: "yes" }, - }); - const frame = await session.client.next(10_000); - assert.equal(frame.error_code, ErrorCode.malformedArgs); - } finally { - await session.close(); - } - }); }); describe("issue #141: what a restored endpoint actually publishes", () => { @@ -1039,9 +981,9 @@ describe("issue #141: one unusable map entry costs only itself", () => { // key — so that collision can no longer happen BY CONSTRUCTION: every // restored spec gets a distinct `Endpoint.id` for free. What can still // fail on its way into the Matter tree is an entry whose KEY is not a - // legal one: `deviceId` makes it resolvable (issue #219 — a re-adopted - // or hand-edited entry need not have a key `parsePublishedId` accepts) - // while the key itself violates PR5 design F9's "no `.` in an `Endpoint.id`" — + // legal one: `deviceId` makes it resolvable (a migrated or hand-edited + // entry need not have a key `parsePublishedId` accepts) while the key + // itself violates PR5 design F9's "no `.` in an `Endpoint.id`" — // exactly the kind of corrupted-but-resolvable entry `restore()`'s // per-spec try/catch exists to survive. const storagePath = storage(); @@ -1450,12 +1392,17 @@ describe("issue #220: a battery survives a restart", () => { }); }); -describe("issues #219/#240 at node level: re-adopt and the two-command supersede", () => { - it("logs the re-adopt line and rebinds deviceId when a different device claims the same identity", async () => { - // #219: a device is deleted, its export is un-exported (`remove_endpoint`), - // and a REPLACEMENT device claims the same published identity by name — - // exactly what the (future) Re-adopt menu action sends. Nothing on the - // wire should move: same `Endpoint.id`, same number (PR5 design F1), same UniqueID. +describe("issue #274 at node level: remove_endpoint destroys, so a later upsert is a fresh add", () => { + it("a device confirmed gone and later re-created gets the SAME matter.js number back, with no memory of the old owner", async () => { + // Once `remove_endpoint` destroys rather than orphans, a plugin-side + // deletion followed by a later re-export under the same identity (a + // different device claiming the same `publishedAs` by name) is + // simply a fresh ADD to this file's own witness — there is no + // previous record left to compare against, so no "driven by device + // X, replacing device Y" notice fires. What still holds is the + // number itself: matter.js's OWN persisted allocation for that + // `Endpoint.id` is untouched by us destroying our own witness of it + // (see `EndpointMapStore.destroy`'s own docstring). const storagePath = storage(); const session = await boot(storagePath); try { @@ -1474,6 +1421,8 @@ describe("issues #219/#240 at node level: re-adopt and the two-command supersede break; } } + assert.equal(readMap(storagePath).endpoints[uniqueIdFor(KITCHEN)], undefined, + "destroyed outright — nothing left in the witness map"); session.logged.length = 0; const REPLACEMENT_DEVICE_ID = 700_000_001; @@ -1504,7 +1453,7 @@ describe("issues #219/#240 at node level: re-adopt and the two-command supersede assert.equal( (upsertFrame?.result as { endpointNumber: number } | undefined)?.endpointNumber, numberBefore, - "PR5 design F1: the SAME Endpoint.id gets the SAME number back", + "the SAME Endpoint.id gets the SAME matter.js number back regardless of our own witness", ); assert.deepEqual( @@ -1513,245 +1462,18 @@ describe("issues #219/#240 at node level: re-adopt and the two-command supersede ); const map = readMap(storagePath); assert.equal(map.endpoints[uniqueIdFor(KITCHEN)]?.deviceId, REPLACEMENT_DEVICE_ID); - assert.equal(map.endpoints[uniqueIdFor(KITCHEN)]?.orphaned, undefined, "live again, so un-orphaned"); - - assert.ok( - session.logged.some( - line => - line.includes(`Accessory ${uniqueIdFor(KITCHEN)}`) && - line.includes(`is now driven by Indigo device ${REPLACEMENT_DEVICE_ID}`) && - line.includes(`replacing device ${KITCHEN}`), - ), - `expected the re-adopt log line, got ${session.logged.join(" | ")}`, - ); - } finally { - await session.close(); - } - }); - - it("nudges towards Re-adopt when an attach creates a NEW identity matching an orphan's role+label (PR5 design owner ruling 4)", async () => { - // Distinct from the re-adopt test above: here the REPLACEMENT device is - // exported under its OWN brand-new identity (not the orphan's), the way - // an ordinary export naturally would be if the user never knew the old - // accessory was still sitting in the map. The node cannot act on this — - // only the plugin owns publishedAs — so the best it can do is say so. - const storagePath = storage(); - const session = await boot(storagePath); - try { - await attach(session.client, "a0", [KITCHEN_SPEC]); - - session.client.send({ - message_id: "r0", - command: "remove_endpoint", - args: { indigoDeviceId: KITCHEN }, - }); - for (;;) { - const frame = await session.client.next(10_000); - if (frame.message_id === "r0") { - assert.equal(frame.error_code, undefined, JSON.stringify(frame)); - break; - } - } - session.logged.length = 0; - - const REPLACEMENT_DEVICE_ID = 700_000_002; - session.client.send({ - message_id: "u0", - command: "upsert_endpoint", - args: { - endpoint: { - indigoDeviceId: REPLACEMENT_DEVICE_ID, - // No publishedAs — the default derivation, a BRAND-NEW - // identity distinct from the orphaned KITCHEN one. - role: KITCHEN_SPEC.role, - label: KITCHEN_SPEC.label, - reachable: true, - states: { onOff: false }, - options: {}, - }, - }, - }); - for (;;) { - const frame = await session.client.next(10_000); - if (frame.message_id === "u0") { - assert.equal(frame.error_code, undefined, JSON.stringify(frame)); - break; - } - } - - assert.ok( - session.logged.some( - line => - line.includes("Re-adopt a Matter accessory…") && - line.includes(uniqueIdFor(KITCHEN)) && - line.includes(uniqueIdFor(REPLACEMENT_DEVICE_ID)), - ), - `expected the readopt nudge naming both identities, got ${session.logged.join(" | ")}`, - ); - // ...and it has to leave stdout. The node is launched by launchd, - // so `session.logged` is a terminal nobody is watching; §4.3 - // `warnings` is what `export_bridge.py` mirrors into the Indigo - // event log, and this nudge is the whole discoverability moment - // for `Re-adopt a Matter accessory…`. - assert.ok( - session.bridge - .getStatus() - .warnings.some(line => line.includes("Re-adopt a Matter accessory…")), - `the nudge must ride StatusReport.warnings, got ${JSON.stringify( - session.bridge.getStatus().warnings, - )}`, - ); - } finally { - await session.close(); - } - }); - - it("marks the old identity supersededBy the new one when remove and create arrive as two separate commands", async () => { - // #240 §3 steps 3/5: the plugin's future `replace()` sends - // `remove_endpoint` then `upsert_endpoint` as two SEPARATE commands, not - // one `attach` batch — this is the two-command half of a role change, - // and it is the ordinary path a role change actually takes. - const storagePath = storage(); - const session = await boot(storagePath); - try { - await attach(session.client, "a0", [KITCHEN_SPEC]); - - session.client.send({ - message_id: "r0", - command: "remove_endpoint", - args: { indigoDeviceId: KITCHEN }, - }); - for (;;) { - const frame = await session.client.next(10_000); - if (frame.message_id === "r0") break; - } - - // No supersededBy yet — the create has not happened (§3 step 3). - assert.equal(readMap(storagePath).endpoints[uniqueIdFor(KITCHEN)]?.supersededBy, undefined); - - const newIdentity = `${uniqueIdFor(KITCHEN)}~2`; - session.client.send({ - message_id: "u0", - command: "upsert_endpoint", - args: { - endpoint: { - indigoDeviceId: KITCHEN, - publishedAs: newIdentity, - role: "dimmableLight", - label: "Kitchen Lamp", - reachable: true, - states: { onOff: false, level: 50 }, - options: {}, - }, - }, - }); - for (;;) { - const frame = await session.client.next(10_000); - if (frame.message_id === "u0") { - assert.equal(frame.error_code, undefined, JSON.stringify(frame)); - break; - } - } - - const map = readMap(storagePath); - assert.equal(map.endpoints[uniqueIdFor(KITCHEN)]?.orphaned, true); - assert.equal( - map.endpoints[uniqueIdFor(KITCHEN)]?.supersededBy, - newIdentity, - "the create half landed as a SEPARATE command, and still paired with the removal", - ); - assert.equal(map.endpoints[newIdentity]?.deviceId, KITCHEN); - } finally { - await session.close(); - } - }); - - it("leaves the interim identity an ORDINARY orphan when a re-adopt replaces it (PR5 design E5)", async () => { - // PR5 design E2/E5: the device was recreated and re-exported under its OWN - // identity before the user noticed the empty room, so re-adopt has to - // remove THAT accessory and publish the orphaned one in its place. - // That is one removal plus one create for one device — the same shape - // as a supersede — but it is NOT one: the identity left behind was - // never replaced by a later generation of itself, its number is not - // retired, and PR5 design E5 rules it "itself re-adoptable later, which is - // harmless". Marking it superseded would hide it from the picker for - // good. - const storagePath = storage(); - const session = await boot(storagePath); - const answered = async (messageId: string): Promise => { - for (;;) { - const frame = await session.client.next(10_000); - if (frame.message_id === messageId) { - // `error_code`, not `error` — the refusal frame's own key - // (`ws-server.ts`'s `sendError`). - assert.equal(frame.error_code, undefined, JSON.stringify(frame)); - return; - } - } - }; - try { - await attach(session.client, "a0", [KITCHEN_SPEC]); - - // The original accessory is un-exported: its identity is now an - // orphan, and its number is held for it (§3.3). - session.client.send({ - message_id: "r0", - command: "remove_endpoint", - args: { indigoDeviceId: KITCHEN }, - }); - await answered("r0"); - - // The user recreates the device and exports it normally — a - // SECOND, brand-new accessory under its own default identity. - const REPLACEMENT_DEVICE_ID = 700_000_004; - await upsertOne(session.client, "u0", REPLACEMENT_DEVICE_ID); - - // Then notices the empty room and re-adopts, which - // `ExportBridge.replace()` sends as remove-then-upsert. - session.client.send({ - message_id: "r1", - command: "remove_endpoint", - args: { indigoDeviceId: REPLACEMENT_DEVICE_ID }, - }); - await answered("r1"); - session.client.send({ - message_id: "u1", - command: "upsert_endpoint", - args: { - endpoint: { - indigoDeviceId: REPLACEMENT_DEVICE_ID, - publishedAs: uniqueIdFor(KITCHEN), - role: "onOffLight", - label: "Kitchen Lamp (new)", - reachable: true, - states: { onOff: false }, - options: {}, - }, - }, - }); - await answered("u1"); - - const interim = uniqueIdFor(REPLACEMENT_DEVICE_ID); - const record = readMap(storagePath).endpoints[interim]; - assert.equal(record?.orphaned, true, "the interim accessory did leave the ecosystems"); - assert.equal( - record?.supersededBy, - undefined, - "but nothing SUPERSEDED it — no later generation of it was ever published", - ); - assert.ok( - session.bridge.listOrphans().some(orphan => orphan.uniqueId === interim), - `§3.12 must go on offering it (PR5 design E5), got ${JSON.stringify(session.bridge.listOrphans())}`, - ); + assert.equal(map.endpoints[uniqueIdFor(KITCHEN)]?.orphaned, undefined, "never orphaned — a fresh add"); } finally { await session.close(); } }); it("does not pair an unrelated upsert against a stale removal of a DIFFERENT device", async () => { - // The `#lastRemoved` bookkeeping is keyed per device id — removing - // KITCHEN must never taint a plain, unrelated create for some other, - // brand-new device. + // `#lastRemoved` is only ever populated by a SOFT (reconcile) removal + // — `remove_endpoint`'s hard destroy never touches it — but the + // guard is keyed per device id regardless: removing KITCHEN must + // never taint a plain, unrelated create for some other, brand-new + // device. const storagePath = storage(); const session = await boot(storagePath); try { @@ -1772,9 +1494,9 @@ describe("issues #219/#240 at node level: re-adopt and the two-command supersede assert.equal(readMap(storagePath).endpoints[uniqueIdFor(UNRELATED_DEVICE_ID)]?.supersededBy, undefined); assert.equal( - readMap(storagePath).endpoints[uniqueIdFor(KITCHEN)]?.supersededBy, + readMap(storagePath).endpoints[uniqueIdFor(KITCHEN)], undefined, - "KITCHEN's own removal must also stay an ordinary orphan — nothing paired with it", + "KITCHEN's own removal destroyed the entry outright — nothing left to pair with", ); } finally { await session.close(); @@ -1782,13 +1504,13 @@ describe("issues #219/#240 at node level: re-adopt and the two-command supersede }); }); -describe("restore binds to the DRIVING device, and never to a retired identity (issues #219/#240)", () => { +describe("restore binds to the DRIVING device, and never to a retired identity (issue #240)", () => { it("restores an accessory bound to the map's deviceId, not to its key", async () => { - // The whole of #219 in the one place nothing else covers: `restore()` - // runs BEFORE any attach, from the file alone, so a bug here is a - // re-adopted accessory that comes back bound to the DELETED device — - // commands routed nowhere and state pushed by nobody, silently, until - // the plugin's next attach happens to correct it. + // The one place nothing else covers: `restore()` runs BEFORE any + // attach, from the file alone, so a bug here is a migrated accessory + // that comes back bound to the OLD device — commands routed nowhere + // and state pushed by nobody, silently, until the plugin's next + // attach happens to correct it. const storagePath = storage(); const numbers = await seedTwoAccessories(storagePath); const REPLACEMENT_DEVICE_ID = 700_000_010; @@ -1826,7 +1548,7 @@ describe("restore binds to the DRIVING device, and never to a retired identity ( // The rest needs an attached client (`set_state` and the §5 // command event both require one), so attach with exactly what a - // re-adopted export sends. It must be an UPDATE of the restored + // migrated export sends. It must be an UPDATE of the restored // endpoint, not a recreate — the number proves which. await attach(session.client, "a0", [{ ...KITCHEN_SPEC, @@ -1913,111 +1635,4 @@ describe("restore binds to the DRIVING device, and never to a retired identity ( } }); - it("still offers an ordinary orphan after a restart when no create ever paired with it (PR5 design E7)", async () => { - // E7: the bridge or plugin dies between a supersede's remove and its - // add. `#lastRemoved` is in-memory only, so the pairing is lost — and - // the ruling is that the survivor must be an ORDINARY orphan, offered - // by §3.12 like any other, rather than a speculatively-retired one. - const storagePath = storage(); - const first = await boot(storagePath); - try { - await attach(first.client, "a0", [KITCHEN_SPEC]); - first.client.send({ - message_id: "r0", - command: "remove_endpoint", - args: { indigoDeviceId: KITCHEN }, - }); - for (;;) { - const frame = await first.client.next(10_000); - if (frame.message_id === "r0") { - assert.equal(frame.error_code, undefined, JSON.stringify(frame)); - break; - } - } - } finally { - await first.close(); - } - // The create never happens — the process ends here. - - const second = await boot(storagePath); - try { - assert.equal( - readMap(storagePath).endpoints[uniqueIdFor(KITCHEN)]?.supersededBy, - undefined, - "a supersession marker must never be written speculatively", - ); - assert.deepEqual( - second.bridge.listOrphans().map(orphan => orphan.uniqueId), - [uniqueIdFor(KITCHEN)], - `§3.12 must go on offering it, got ${JSON.stringify(second.bridge.listOrphans())}`, - ); - assert.equal( - childOf(second.bridge.server, KITCHEN), - undefined, - "offered for re-adoption, but NOT rebuilt — it is orphaned", - ); - } finally { - await second.close(); - } - }); - - it("supersedes a SECOND time: indigo-N~2 retires when indigo-N~3 arrives", async () => { - // The gap a "does the new identity have a suffix?" test would leave: - // a role change on an already-role-changed export. Driven through the - // two-command path, which is the one a real `replace()` takes. - const storagePath = storage(); - const session = await boot(storagePath); - const GEN2 = `${uniqueIdFor(KITCHEN)}~2`; - const GEN3 = `${uniqueIdFor(KITCHEN)}~3`; - try { - await attach(session.client, "a0", [{ ...KITCHEN_SPEC, publishedAs: GEN2 }]); - - session.client.send({ - message_id: "r0", - command: "remove_endpoint", - args: { indigoDeviceId: KITCHEN }, - }); - for (;;) { - const frame = await session.client.next(10_000); - if (frame.message_id === "r0") { - assert.equal(frame.error_code, undefined, JSON.stringify(frame)); - break; - } - } - - session.client.send({ - message_id: "u0", - command: "upsert_endpoint", - args: { - endpoint: { - indigoDeviceId: KITCHEN, - publishedAs: GEN3, - role: "dimmableLight", - label: "Kitchen Lamp", - reachable: true, - states: { onOff: false, level: 40 }, - options: {}, - }, - }, - }); - for (;;) { - const frame = await session.client.next(10_000); - if (frame.message_id === "u0") { - assert.equal(frame.error_code, undefined, JSON.stringify(frame)); - break; - } - } - - const map = readMap(storagePath); - assert.equal(map.endpoints[GEN2]?.supersededBy, GEN3); - assert.equal(map.endpoints[GEN2]?.orphaned, true); - assert.deepEqual( - session.bridge.listOrphans().map(orphan => orphan.uniqueId), - [], - "a retired identity is never offered for re-adoption", - ); - } finally { - await session.close(); - } - }); }); diff --git a/bridge-node/test/stub-bridge.ts b/bridge-node/test/stub-bridge.ts index 2b91b39..0998048 100644 --- a/bridge-node/test/stub-bridge.ts +++ b/bridge-node/test/stub-bridge.ts @@ -17,7 +17,6 @@ import { type EndpointSpec, ErrorCode, type FabricInfo, - type OrphanRecord, type PairingReport, ProtocolError, type RemoveFabricResult, @@ -116,10 +115,6 @@ export interface GoldenFrames { factory_reset_discard_map: GoldenExchange; rebuild_endpoint_map: GoldenExchange; endpoint_map_invalid: GoldenExchange; - /** §3.12 (issue #219) — the re-adopt picker's data. */ - list_orphans: GoldenExchange; - /** The same command over a map with nothing orphaned. */ - list_orphans_empty: GoldenExchange; /** * The holding pen for commands the node does not implement yet — EMPTY * since E5, and asserted empty. Kept because it is the mechanism the next @@ -308,8 +303,6 @@ export class StubBridge implements BridgeFacade { readonly removedFabrics: number[] = []; readonly factoryResets: boolean[] = []; rebuilds = 0; - /** §3.12 — what `listOrphans` answers; the re-adopt tests set this. */ - orphans: OrphanRecord[] = []; getStatus(): StatusReport { if (this.poisonStatus) { @@ -350,10 +343,6 @@ export class StubBridge implements BridgeFacade { return structuredClone(golden.rebuild_endpoint_map.response.result) as StatusReport; } - listOrphans(): OrphanRecord[] { - return structuredClone(this.orphans); - } - onDriftDetected(listener: (drift: DriftEntry[]) => void): void { this.#drift = listener; } diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 0cb9cd8..0c977d3 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -29,7 +29,7 @@ loop→Indigo writes go straight through `device_sync.apply_states` (thread-safe | `matter_handlers/writable_attributes.py` | **GENERATED, checked in** — `{cluster: {attribute: name}}` for all 1319 model attributes, the 141 the spec calls writable, and the 5 of those that are also a field of a command on their own cluster. That collision set is no longer a filter, only a cross-checked DETECTOR (ADR-0006, superseding ADR-0005): a spec review found the command-field rule unsound as a decision authority — right about its founding members through three unrelated mechanisms, and (once a parser bug hiding 13 attributes was fixed) wrongly demoting the real `DoorLock.OperatingMode` setting. Regenerate with `tools/enumerate_settings.py --emit-table` when `bridge-node/package.json`'s `@matter/*` pin moves. A build artefact rather than a runtime lookup because only the bridge node may import `@matter/model` (workspace ADR-0006) | | `pairing_menu_mixin.py` | **Pair Matter Bridge…** (§3.8, codes to the event log because Indigo dialogs have no dynamic labels), **Unpair an Ecosystem…** (§3.9, two confirm gates + a dynamic fabric picker), and fabric backup/restore (issues #26, #136) | | `matter_server_menu_mixin.py` | matter-server install/restart menus, manual commission/decommission-device menus (+ folder and node pickers), and node-device recovery ("Recreate Matter node devices…", issue #204 — `device_sync`'s synthetic `matterNode` devices, not the export side, despite having sat physically among the export-recovery code before this split). **Since issue #210 also owns "Share a Matter device with another ecosystem…"** — the reverse of the share model: Indigo already holds admin 1 on a node it commissioned, and `_share_node` opens an Enhanced commissioning window (`matter_client.open_commissioning_window`) so a second ecosystem can join without displacing it. Lives here rather than in `pairing_menu_mixin.py` because that mixin's self-attribute contract declares only export-side attributes; this one already has `runtime`/`matter`/`device_sync`. Reuses `PairingMenuMixin._window_duration` and `DiagnosticsMenuMixin._fetch_node` across mixins via MRO (`self.()` + `# pylint: disable=no-member`, issue #146's established pattern) rather than importing either mixin directly, which would be the back-import that pattern exists to avoid — `_fetch_node`'s own `MatterUnavailable` is caught via a bare `except Exception`, since that method never raises anything else. `_share_node` is the shared core the device action (`Plugin.actionShareMatterNode`, `plugin.py`) also calls. Was one third of `server_menu_mixin.py` until issue #146's follow-up split (one file → three): that file's own row used to justify keeping matter-server install/restart, export recovery, and the bridge LaunchAgent together because "they share one construction rule (`ServerProcess` only ever from `Plugin._server_prefs()`) and one install thread" — both still true, but neither is a reason to share a FILE: `self._install_thread`/`self._server_prefs()` are `Plugin` instance state, reached identically from a mixin no matter which module defines it. Issue #146 requires only that every callback resolve as a flat attribute on `Plugin`; it does not require the mixins that provide them to be few | -| `export_recovery_menu_mixin.py` | Export-bridge recovery menus (BRIDGE_PROTOCOL §3.10/§3.11): rebuild the endpoint map, migrate an exported accessory onto a different Indigo device (#246), re-adopt a left-behind accessory (issue #219), and reset the bridge's ecosystem pairings. The other third of the pre-split `server_menu_mixin.py` — see `matter_server_menu_mixin.py`'s row for why the old one-file "shared construction rule and install thread" justification does not survive the split | +| `export_recovery_menu_mixin.py` | Export-bridge recovery menus (BRIDGE_PROTOCOL §3.10/§3.11): rebuild the endpoint map, migrate an exported accessory onto a different Indigo device (#246), and reset the bridge's ecosystem pairings. The other third of the pre-split `server_menu_mixin.py` — see `matter_server_menu_mixin.py`'s row for why the old one-file "shared construction rule and install thread" justification does not survive the split. **Re-adopt a left-behind accessory (issue #219) was removed in issue #274b**: a deleted or un-exported device now destroys its Matter accessory outright (#274, ADR-0015) rather than leaving an orphan to hand to a replacement device — see workspace ADR-0015 | | `bridge_agent_menu_mixin.py` | The Matter bridge node's LaunchAgent (E7 — PRD §4.2, XG5, XAC1): install/reinstall/start/stop the bridge process and the `agent_diagnose` seam that reads its error log. The last third of the pre-split `server_menu_mixin.py` — process lifecycle, as distinct from `export_recovery_menu_mixin.py`'s reconciliation of what the running bridge serves | | `async_runtime.py` | The event loop + thread + bridge primitives | | `mired_units.py` | Reciprocal mireds ↔ Kelvin, and nothing else — the only arithmetic the plugin's two otherwise-disjoint directions genuinely share. Extracted from `matter_handlers/color_control.py` (issue #293 refactor round): four OUTBOUND modules imported the pair from an INBOUND cluster handler, so renaming or restructuring that handler would have broken the export side with nothing in the import graph to warn of it. Deliberately NOT folded into `ct_bounds.py`, which is Indigo-free but also owns the export-side bound PRECEDENCE and the `ctMinMireds`/`ctLearnedMinMireds` vocabulary — moving them there would have reversed the direction of the violation rather than removing it | @@ -64,7 +64,7 @@ matter.js is imported (workspace ADR-0006). Its wire contract is `docs/BRIDGE_PR |---|---| | `main.ts` | Entry point: arg parsing, identity load, ordered SIGTERM shutdown. Exits **0** on a clean stop (launchd's `KeepAlive SuccessfulExit:false` reads the number) — `ServerNode.erase()` leaves a ref'd timer `close()` never clears, so a clean stop after a factory reset used to exit 1. An unusable `identity.json` is moved aside to `identity.json.unreadable-` and a replacement is minted **in memory only** — never written over the `SerialNumber`/`UniqueID` every paired ecosystem knows | | `node.ts` | The `ServerNode` + aggregator, the PRD §7 refuse-to-start decision, §3.9–§3.11, and the §5 event sinks | -| `endpoint-map.ts` | `endpoint-map.json` — the persisted `UniqueID → {number, role, label}` map (schema v2 since issue #141; v1 numbers-only files are migrated in place, never discarded) and its drift detector (PRD §4.3). Since #141 it is also what `node.ts` **restores the endpoint set from before `server.start()`**, so the bridge is never online with an empty aggregator — an empty `PartsList` made Apple re-create every accessory in the bridge's own room on every restart. **It does not allocate anything: matter.js owns the numbers**, keyed on `Endpoint.id` in its own store; this file is the independent *witness*, so a lost/reset matter.js storage becomes a log line instead of silently duplicating every accessory. Lives OUTSIDE matter.js's storage context on purpose (§3.10's reset wipes that). Report-only — drift is never repaired, or the next pass would call the same fault clean. A **commissioned bridge with no map at all bootstraps** a baseline from matter.js's own persisted numbers and serves (every pre-E5 install is in that state); only a *present-but-unreadable* map refuses. `refuseReasonFor` is a pure function with no matter.js import, because the case that matters most cannot be reached in a test without real hardware. **`battery`** (issue #220) joins `numberVoid`/`orphaned` as a third present-`true`-or-absent-never-`false` marker: set add-only the first time a live entry is seen with PowerSource, never cleared — the witness has to be as monotonic as the wire rule it mirrors (§4.1), or restore-on-start would rebuild an accessory that has a battery without the cluster it actually carries. **`orphaned` no longer means "kept forever" (issue #274, ADR-0015).** `forget()` — the ordinary `attach`-reconcile removal path — is unchanged: it still marks an entry `orphaned` and keeps its number/role/label, because a full reconcile cannot tell a deliberate departure from a device that merely failed classification this one pass. But a device the PLUGIN has confirmed is gone for good — deleted, or deliberately un-exported — now reaches the new `destroy()` instead, via `node.ts`'s `removeEndpoint(id, permanent: true)`: the record is deleted outright, not orphaned, so a destroyed identity is invisible to both `restorable()` and `orphans()`/`list_orphans` from that moment on. `destroy()` is the one mutation here that discards a `number`; that is safe because this file was never what stopped a retired number being reissued — matter.js's own persisted allocation for the `Endpoint.id`, a store this file never touches, is what actually does that | +| `endpoint-map.ts` | `endpoint-map.json` — the persisted `UniqueID → {number, role, label}` map (schema v2 since issue #141; v1 numbers-only files are migrated in place, never discarded) and its drift detector (PRD §4.3). Since #141 it is also what `node.ts` **restores the endpoint set from before `server.start()`**, so the bridge is never online with an empty aggregator — an empty `PartsList` made Apple re-create every accessory in the bridge's own room on every restart. **It does not allocate anything: matter.js owns the numbers**, keyed on `Endpoint.id` in its own store; this file is the independent *witness*, so a lost/reset matter.js storage becomes a log line instead of silently duplicating every accessory. Lives OUTSIDE matter.js's storage context on purpose (§3.10's reset wipes that). Report-only — drift is never repaired, or the next pass would call the same fault clean. A **commissioned bridge with no map at all bootstraps** a baseline from matter.js's own persisted numbers and serves (every pre-E5 install is in that state); only a *present-but-unreadable* map refuses. `refuseReasonFor` is a pure function with no matter.js import, because the case that matters most cannot be reached in a test without real hardware. **`battery`** (issue #220) joins `numberVoid`/`orphaned` as a third present-`true`-or-absent-never-`false` marker: set add-only the first time a live entry is seen with PowerSource, never cleared — the witness has to be as monotonic as the wire rule it mirrors (§4.1), or restore-on-start would rebuild an accessory that has a battery without the cluster it actually carries. **`orphaned` no longer means "kept forever" (issue #274, ADR-0015).** `forget()` — the ordinary `attach`-reconcile removal path — is unchanged: it still marks an entry `orphaned` and keeps its number/role/label, because a full reconcile cannot tell a deliberate departure from a device that merely failed classification this one pass. But a device the PLUGIN has confirmed is gone for good — deleted, or deliberately un-exported — now reaches the new `destroy()` instead: EVERY `node.ts` `removeEndpoint(id)` call goes through it unconditionally (issue #274b removed the `permanent` opt-in once nothing else called `remove_endpoint` without meaning it), so the record is deleted outright, not orphaned, and a destroyed identity is invisible to `restorable()` from that moment on. `destroy()` is the one mutation here that discards a `number`; that is safe because this file was never what stopped a retired number being reissued — matter.js's own persisted allocation for the `Endpoint.id`, a store this file never touches, is what actually does that. **The re-adopt picker's own surface (`orphans()`, the `list_orphans` command, `OrphanRecord`, and the `noteReadoptableMatch()` discoverability nudge) was removed in the same issue** — an orphan is still retained by an ordinary reconcile removal (§141's churn fix still needs it), but nothing hands it to a user any more; `orphaned`/`deviceId`/`supersededBy` all stay because `restorable()`, migrate's device-id rekey (`check()`), and the role-change supersession accounting still read them | | `storage.ts` | `identity.json`: install id, passcode, discriminator, and the `commissionedAt` witness for §7's "storage missing but previously commissioned". Atomic temp-plus-`rename` writes; witness writes report whether they landed | | `registry.ts` / `endpoints.ts` | The live endpoint set and one Matter device-type factory per §4.2 role. Every bridged child publishes the **full** Bridged Device Basic Information identity (`BridgedIdentity`: vendor name/id, product name, hardware + software versions, plus the per-accessory label/serial/uniqueId/reachable) — all optional in the cluster at 0.17.8, all populated, and the same values the root node's `BasicInformation` carries so an ecosystem is never shown two answers. **PowerSource (issue #220)** is composed onto any role via `deviceTypeFor`'s second `.with(PowerSourceServer.with("Battery"))` when `EndpointSpec.battery` is set — role-independent by construction (`splitBattery` peels `batteryLevel` off `states` before any role's own `statePatch` sees it, so the key never needed fifteen per-role additions). `BATTERY_INITIAL` seeds `batPercentRemaining: null` unconditionally: an endpoint built without that key never gets the attribute into `attributeList` at all (measured against 0.17.8), so a later write would succeed into an attribute no controller could ever read. A battery **gain** on an existing endpoint recreates (PowerSource can only be declared at construction); a **loss** is an ordinary update — the live cluster set is monotonic (§4.1) | | `ws-server.ts` | The loopback protocol server (§1–§3). Holds an un-attached socket OPEN while the node is refusing — it is the client's only route to the §3.11 rebuild | diff --git a/docs/BRIDGE_PROTOCOL.md b/docs/BRIDGE_PROTOCOL.md index 62cb62c..6079f6e 100644 --- a/docs/BRIDGE_PROTOCOL.md +++ b/docs/BRIDGE_PROTOCOL.md @@ -302,13 +302,13 @@ with `error_code: "mass_removal_refused"` unless the args carry ecosystem must be deliberate (the §5.1 allow-list being emptied), never the side effect of a stale or buggy client attaching with a default state. -**The guard counts DEPARTURES, not removals** (issues #219/#240). A removed +**The guard counts DEPARTURES, not removals** (issue #240). A removed `publishedAs` whose `indigoDeviceId` also appears among the identities being created is one device changing which accessory identity it publishes — a -role-change supersession, or a re-adopt — and the device stays exported -throughout, so it is not a departure and does not count towards emptying the -live set. Without that carve-out, changing the role of the only exported -device on a bridge would be refused as a mass removal. Note this is +role-change supersession, or an ordinary retarget — and the device stays +exported throughout, so it is not a departure and does not count towards +emptying the live set. Without that carve-out, changing the role of the only +exported device on a bridge would be refused as a mass removal. Note this is deliberately the BROAD "one removal plus one create for the same device" rule, not the narrow generation test `supersededBy` uses: both shapes keep the device exported, which is the only question the guard is asking. @@ -343,12 +343,13 @@ ecosystem, not every change the rekey carries. This is the mechanism behind the plugin's "Migrate an exported accessory…" menu action — a live device replaced by another one, or a module moved to new hardware, while both Indigo devices exist at once — and it is why -migrate can honestly promise room/name/scene survival where the `list_orphans` -re-adopt path (whose identity DID lapse) cannot. A retarget combined with a -role change or a battery gain cannot be represented this way — a live cluster -set cannot be reshaped without being rebuilt — and falls back to the ordinary -`recreate` outcome above, wire activity and all; the plugin refuses cross-role -migrate at the dialog before it ever reaches here (#219/#240 reasoning). +migrate can honestly promise room/name/scene survival: the identity never +lapses, so no ecosystem ever processes a removal of it. A retarget combined +with a role change or a battery gain cannot be represented this way — a live +cluster set cannot be reshaped without being rebuilt — and falls back to the +ordinary `recreate` outcome above, wire activity and all; the plugin refuses +cross-role migrate at the dialog before it ever reaches here (issue #240 +reasoning). ### 3.2 `upsert_endpoint` @@ -364,9 +365,9 @@ neither can be done to a live endpoint in place: - a `role` different from the live endpoint's (§4.1: ecosystems cache the Matter device type per endpoint); -- a `publishedAs` different from the live endpoint's (issues #219/#240). - That is a supersession or a re-adopt, and the only path allowed to do it is - the plugin's remove-then-add, which sends the two mutations as two separate +- a `publishedAs` different from the live endpoint's (issue #240). That is a + role-change supersession, and the only path allowed to do it is the + plugin's remove-then-add, which sends the two mutations as two separate commands rather than asking this one to move a live endpoint out from under itself. @@ -392,41 +393,28 @@ Removes the child endpoint (`endpoint.close()`). Idempotent — removing an absent endpoint succeeds with `{"removed": false}`; a live removal returns `{"removed": true}`. Bulk removals are paced ~100ms apart by the node. -**`permanent` (issue #274, default `false`) decides what happens to the -endpoint-map record, not just the live endpoint.** Omitted or `false` keeps -the behaviour every caller had before this flag existed: the persisted -endpoint-number allocation is **retained** and the entry is marked -`orphaned` (§4.3's `endpoint-map.json` subsection) — re-adding the same -device, or re-adopting the identity onto a different one, restores the same -number. `true` is the confirmed-gone declaration: the Indigo device has been -deleted, or the user has deliberately un-exported it. The node then -**destroys** the endpoint-map record outright — no number kept, no -`orphaned` entry, nothing offered to `list_orphans` — because ADR-0015 -retired the premise that a departed accessory is worth holding open for a -re-adopt that will never come, once the driving device itself is gone. - -Only a caller certain the device is not coming back may pass `true`. The -plugin passes it from exactly three places: `deviceDeleted` (a live +**Destroys the endpoint-map record along with the live endpoint (issue +#274).** No number kept, no `orphaned` entry — because ADR-0015 retired the +premise that a departed accessory is worth holding open for a re-adopt that +will never come, once the driving device itself is gone. Every caller of +this command is confirming the driving device is not coming back: the +plugin sends it from exactly three places — `deviceDeleted` (a live deletion), the export dialog's "Remove" (a deliberate un-export), and a sweep in `_on_attached` that closes the one gap those two cannot — a device deleted from Indigo while the plugin was not running to hear about it (§3.1 above already re-classifies every allow-list entry on each attach; a device -missing there is exactly this case, `export_bridge.py`'s `_spec_for`). The -plugin's own two-command role-change/re-adopt sequence (`replace()`, which -sends this `remove_endpoint` and a paired `upsert_endpoint` for the same or -a related identity) never passes `true` — that removal is deliberately soft, -because the very next command may retroactively mark it `supersededBy` -(§3.1's supersede accounting) or the map may still offer it as an ordinary -re-adopt candidate. +missing there is exactly this case, `export_bridge.py`'s `_spec_for`) — +plus the plugin's two-command role-change sequence (`replace()`, which +sends this `remove_endpoint` and a paired `upsert_endpoint` for a new +identity on the same device). **`attach`'s own reconcile never destroys.** A full reconcile (§3.1) cannot tell "the plugin dropped this on purpose" from "this device exists but failed classification this one pass" — a dependent plugin not yet started, a role this build does not know — so any identity that merely falls out of an `attach`'s desired set keeps the pre-#274 soft/orphan treatment -regardless of `permanent`. Destroying is only ever a `remove_endpoint` -decision, made with the device's existence already confirmed on the plugin -side. +regardless. Destroying is only ever a `remove_endpoint` decision, made with +the device's existence already confirmed on the plugin side. ### 3.4 `set_state` @@ -605,50 +593,6 @@ The name is timestamped for the same reason `identity.json.unreadable-` is: a fixed name means the second quarantine silently destroys the first, and the first is the one nearest the original truth. -### 3.12 `list_orphans` - -```json -{"command": "list_orphans", "args": {}} -``` - -Read-only, no arguments: every left-behind accessory identity the endpoint map -still remembers — issue #219's re-adopt picker is built from this. Result is a -bare array, `[, …]`, in map order (the order numbers were first -recorded, not necessarily newest-orphan-first): - -```json -[{"uniqueId": "indigo-123456789", "number": 7, "role": "dimmableLight", - "label": "Kitchen Lamp", "orphanedAt": "2026-08-12T09:15:00Z", "deviceId": 123456789}, - {"uniqueId": "indigo-123456790", "number": 4}] -``` - -- `role`/`label` are absent for a pre-2026.16.2 orphan (that plugin version - deleted them on un-export instead of recording them) — the picker lists it - anyway, with nothing to match a replacement device against, and refuses to - re-adopt it. -- `orphanedAt` is absent for an orphan recorded before this field existed — the - picker renders that as "date unknown". -- `deviceId` is the Indigo device that drove the identity before it was - un-exported, when recorded. -- A **superseded** identity (issue #240 — a role change moved its device to a - new identity) is never in this list: re-adopting one would resurrect an - old-role accessory under a number every paired ecosystem has already - processed a removal for. - -**Since issue #274 (ADR-0015), an ordinary departure no longer lands here at -all.** A device deletion or a deliberate un-export now goes through -`remove_endpoint`'s `permanent: true` (§3.3), which destroys the record -instead of orphaning it — see the `endpoint-map.json` subsection above. Only -a device that fell out of an `attach`'s desired set without the plugin ever -confirming why (§3.1's own reconcile never destroys) still produces an -ordinary orphan, so a healthy bridge's `list_orphans` is expected to answer -empty far more often than before this issue. - -**Not a recovery command.** `list_orphans` requires an ordinary `attach` first, -same as any other §3 command — it is deliberately absent from §1.1's recovery -trio (`get_status`, `get_pairing`, `rebuild_endpoint_map`): a node refusing -over an unreadable map has no orphans it can vouch for. - ## 4. Shapes ### 4.1 `EndpointSpec` @@ -665,10 +609,10 @@ over an unreadable map has no orphans it can vouch for. - `indigoDeviceId` — the immutable Indigo device ID: the device this accessory is **driven by** — every §5 command is addressed to it and every §3.4 state push arrives keyed on it. It is no longer the accessory's - identity (ADR-0010, issues #219/#240): `publishedAs` below is what + identity (ADR-0010, issue #240): `publishedAs` below is what `Endpoint.id`/`UniqueID`/`SerialNumber` are derived from, and it defaults to `indigo-` — which is why an ordinary export looks exactly as - it always has, and why a re-adopted one has the two deliberately disagree. + it always has, and why a migrated one has the two deliberately disagree. - `role` — one of the v1 role enum (§4.2). A role change for an existing endpoint is **rejected** (`error_code: "role_change"`); the plugin must remove and re-add, because ecosystems cache device types per endpoint. @@ -702,13 +646,13 @@ over an unreadable map has no orphans it can vouch for. them. The plugin never sends its own SEED/LEARNED bookkeeping keys on the wire, only this effective pair, and only when it differs from the generic 153/500 domain — see `ct_bounds.wire_options` in the plugin source. -- `publishedAs` — issues #219/#240. The accessory identity this device +- `publishedAs` — issue #240. The accessory identity this device publishes as (`Endpoint.id`/`UniqueID`/`SerialNumber`) — **the plugin owns and persists it; the node owns only the number that identity gets** (§0 (e)-(j), invariant 3). **Omitted (or equal to `indigo-`) means "use today's default derivation"** — every export that has never changed role sends no key at all, so this field is invisible on the wire - until a role change or a re-adopt moves an accessory off its default + until a role change or a migrate moves an accessory off its default identity. **Validation:** must match `indigo-[~]` and be ≤32 characters (§0 (j)); anything else is refused with `malformed_args`. **Duplicate refusal:** `attach` rejects, also with @@ -903,11 +847,12 @@ and is still never repaired. Each entry is a `DriftEntry`: - `warnings` — faults the node has hit and cannot fix on its own, in prose. Most are persistence failures: the endpoint map could not be written or deleted, the commissioning witness could not be cleared, `identity.json` could not be - saved. Since bridge-node 0.15.0 the channel also carries two things that are - not writes at all — the issues #219/#240 re-adopt notices, and the issue #286 - subscription-churn notice described below — because they are the same kind of - thing: a fault a human has to act on that appears nowhere else. Empty is the - normal state, and entries are **current, not historical** — a warning + saved. Since bridge-node 0.15.0 the channel also carries things that are not + writes at all — a driving-device-change confirmation (issue #240), an + unrebuildable-entry skip, and the issue #286 subscription-churn notice + described below — because they are the same kind of thing: a fault or + notable event a human has to know about that appears nowhere else. Empty is + the normal state, and entries are **current, not historical** — a warning disappears the moment the condition it describes ends. This exists because the node's only other channel is stdout, and while the @@ -1121,32 +1066,23 @@ version 2** since bridge-node 0.6.0: start — a visible cluster-set change in every paired ecosystem, which self-heals the moment a newer bridge (or the plugin's next `attach`) recreates the endpoint with `battery: true` again. -- `deviceId` — issues #219/#240, the Indigo device *currently driving* this +- `deviceId` — issue #240, the Indigo device *currently driving* this published identity. Absent means "the key's own derivation" — every - pre-`publishedAs` record, and every entry that has never been re-adopted. - **Unlike `battery`, this is replace-on-change, not add-only** — a re-adopt - is precisely a change of driving device, and add-only-forever would pin - the deleted device's id in the record permanently. `restorable()` resolves - an entry's device id from this field first, falling back to parsing it out - of the `publishedAs` key itself (§4.1) — without it, a restore-on-start - after a re-adopt would rebuild the accessory bound to the OLD, deleted - device rather than the new one. Same inert-extra-key parse tolerance as - the keys above. **Rollback caveat:** an older bridge drops the key on its - next write and falls back to deriving the device id from the key itself — - which is the *previous* driving device, not the re-adopted one — so a - re-adopted accessory restored by a rolled-back bridge rebuilds bound to - whichever device the key's own `indigo-` names, self-healing the next - time a `publishedAs`-aware plugin attaches and records the real driver. -- `orphanedAt` — issues #219/#240, an ISO-8601 stamp written by an un-export - alongside `orphaned`, from the node's own injected clock. Purely - informational — the re-adopt picker's "un-exported on …" column; absent on - every orphan recorded before this field existed, which the picker renders - as "date unknown" rather than guessing. Same inert-extra-key parse - tolerance. **Rollback caveat:** an older bridge drops the key on its next - write; the orphan is unaffected (still re-adoptable, still keeps its - number), it simply reads as "date unknown" in the picker from then on — - there is no self-heal because there is nothing to heal, the date was never - load-bearing. + pre-`publishedAs` record, and every entry that has never been retargeted + by a migrate. **Unlike `battery`, this is replace-on-change, not add-only** + — migrating an accessory (issue #246) is precisely a change of driving + device for an identity that stays live throughout, and add-only-forever + would pin the OLD device's id in the record permanently. `restorable()` + resolves an entry's device id from this field first, falling back to + parsing it out of the `publishedAs` key itself (§4.1) — without it, a + restore-on-start after a migrate would rebuild the accessory bound to the + wrong device. Same inert-extra-key parse tolerance as the keys above. + **Rollback caveat:** an older bridge drops the key on its next write and + falls back to deriving the device id from the key itself — which is the + *previous* driving device, not the migrated one — so a migrated accessory + restored by a rolled-back bridge rebuilds bound to whichever device the + key's own `indigo-` names, self-healing the next time a + `publishedAs`-aware plugin attaches and records the real driver. - `supersededBy` — issue #240, the published identity that replaced this one on a role change. Set when the node observes a removal paired with a create of a **later generation of that same identity** @@ -1154,25 +1090,23 @@ version 2** since bridge-node 0.6.0: (`forgetRemoved`'s pairing logic) or arrive as separate commands from the plugin's `replace()` (`noteSupersessionIfPaired`'s, called by `upsertEndpoint` once a create has landed). The generation test is the whole - test, deliberately: a **re-adopt** onto an already-exported device is also - one removal plus one create for one device, and the identity it leaves - behind is an ORDINARY orphan that stays re-adoptable — only a generation - bump retires an identity. A superseded record is `orphaned` too (it - is not live) but is **excluded from both `restorable()` and - `orphans()`/§3.12** — restoring it would resurrect an old-role accessory - under a number every paired ecosystem has already processed a removal for, - and re-adopting it would do the same thing by hand. Same inert-extra-key - parse tolerance. **Rollback caveat:** an older bridge build drops the key - on its next write, and the superseded record becomes an ordinary orphan — - visible in `list_orphans` and re-adoptable, which would resurrect the - retired role under its retired number if acted on. Reaching this state at - all needs an out-of-band swap (a hand-installed older bridge-node build - pointed at storage a newer one already wrote to) rather than an ordinary - reconnect: invariant 5's fail-closed already refuses any *live* pairing - whose `protocolVersion`s disagree, and the ordinary install/reinstall menus - always fetch the pinned version. Documented here rather than defended - against in code, for the same reason `numberVoid`'s and `orphaned`'s own - rollback paths are not. + test, deliberately: migrating an accessory onto an already-exported device + (issue #246) is also one removal plus one create for one device, and the + identity it leaves behind is an ORDINARY orphan, not a retired one — only a + generation bump retires an identity. A superseded record is `orphaned` too + (it is not live) but is **excluded from `restorable()`** — restoring it + would resurrect an old-role accessory under a number every paired ecosystem + has already processed a removal for. Same inert-extra-key parse tolerance. + **Rollback caveat:** an older bridge build drops the key on its next write, + and the superseded record becomes an ordinary, rebuildable orphan again — + which would resurrect the retired role under its retired number at the next + restart if left that way. Reaching this state at all needs an out-of-band + swap (a hand-installed older bridge-node build pointed at storage a newer + one already wrote to) rather than an ordinary reconnect: invariant 5's + fail-closed already refuses any *live* pairing whose `protocolVersion`s + disagree, and the ordinary install/reinstall menus always fetch the pinned + version. Documented here rather than defended against in code, for the same + reason `numberVoid`'s and `orphaned`'s own rollback paths are not. **Version 1 files are read, migrated in place, and never treated as corrupt.** A v1 entry is a bare number; it keeps that number, is simply not restorable @@ -1211,43 +1145,41 @@ subscribed to see any of it, and the alternative (deferring the restore until after the refusal decision) reopens the online-and-empty window for every healthy start, which is the bug itself. -**Un-exporting marks an entry orphaned, and (since issue #219) keeps its -restoration half.** When a reconcile or a `remove_endpoint` actually takes an -endpoint away, its entry is marked `orphaned` (see above) while `role`, -`label` **and** `number` are all kept. Before #219, `role`/`label` were -dropped instead — which also excluded the entry from restore-on-start, but -threw away the only evidence a future re-adopt UI could match a *recreated* -device (a factory-reset accessory that comes back with a new `UniqueID`) -against the endpoint it used to occupy. Without either mechanism, `check`'s -add-and-refresh-only behaviour would leave an un-exported device restorable -for ever: it would be rebuilt before every `server.start()` and removed again -by the next attach — the same appear-then-vanish churn, aimed at devices the -user had already removed — and each ghost would also cost a removal-pacing -slot on every attach. Keeping the number is §3.3's retained allocation, so a -re-export returns the *same* accessory rather than a new one; keeping -`role`/`label` costs nothing extra, since `orphaned` alone already keeps the -entry out of restore-on-start. `orphaned` is cleared the moment the same -`UniqueID` is live again, so a plain re-export is unaffected. Only a measured -removal marks an entry: absence from the live set proves nothing (a node that -never attached has an empty one), so a factory reset, a seed and an entry this -build cannot rebuild all leave the map alone. - -**Destroying an entry (issue #274, ADR-0015) is the alternative to -orphaning it — number, role, label and every other field, gone.** A -`remove_endpoint` carrying `permanent: true` (§3.3) — the driving device is -confirmed deleted, or deliberately un-exported — deletes the record from -`endpoints` outright rather than marking it `orphaned`. Unlike every mutation -above, this is the one place a record's `number` is not kept: the guarantee -that a retired number is never reissued (ADR-0010, `numberVoid`'s and -`orphaned`'s own comments above) still holds, because it was never this file -that enforced it — matter.js's own persisted allocation for the retired -`Endpoint.id`, a separate store this file never touches, is what actually -prevents reuse, and it is untouched by deleting this witness of it. What is -lost is re-adopt: a destroyed identity is invisible to both `restorable()` -and `orphans()`/§3.12 from the moment it is destroyed — there is nothing left -to rebuild or to offer the picker. `attach`'s own reconcile never destroys -(see §3.1 and §3.3 above); only an explicit `remove_endpoint` with -`permanent: true` does. +**An `attach` reconcile's un-export marks an entry orphaned, keeping its +restoration half.** When a reconcile takes an endpoint away because it +simply falls out of the desired set, its entry is marked `orphaned` (see +above) while `role`, `label` **and** `number` are all kept — a full reconcile +can never tell "the plugin dropped this on purpose" from "this device exists +but failed classification this one pass" (issue #274 constraint), so it must +stay reversible. Without this, `check`'s add-and-refresh-only behaviour would +leave an un-exported device restorable for ever: it would be rebuilt before +every `server.start()` and removed again by the next attach — the same +appear-then-vanish churn, aimed at devices the user had already removed — +and each ghost would also cost a removal-pacing slot on every attach. Keeping +the number is §3.3's retained allocation, so a re-export returns the *same* +accessory rather than a new one; keeping `role`/`label` costs nothing extra, +since `orphaned` alone already keeps the entry out of restore-on-start. +`orphaned` is cleared the moment the same `UniqueID` is live again, so a +plain re-export is unaffected. Only a measured removal marks an entry: +absence from the live set proves nothing (a node that never attached has an +empty one), so a factory reset, a seed and an entry this build cannot +rebuild all leave the map alone. + +**An explicit `remove_endpoint` destroys instead (issue #274, ADR-0015) — +number, role, label and every other field, gone.** Every `remove_endpoint` +command (§3.3) is the plugin confirming the driving device is gone for good +— deleted, or deliberately un-exported — so the node deletes the record from +`endpoints` outright rather than marking it `orphaned`. Unlike every +reconcile-driven mutation above, this is the one place a record's `number` +is not kept: the guarantee that a retired number is never reissued +(ADR-0010, `numberVoid`'s and `orphaned`'s own comments above) still holds, +because it was never this file that enforced it — matter.js's own persisted +allocation for the retired `Endpoint.id`, a separate store this file never +touches, is what actually prevents reuse, and it is untouched by deleting +this witness of it. What is lost is `restorable()`: a destroyed identity is +gone from it the moment it is destroyed — there is nothing left to rebuild. +`attach`'s own reconcile never destroys (see §3.1 and §3.3 above); only an +explicit `remove_endpoint` does. `drift` is populated by every operation that can change the live endpoint set — `attach`/`reconcile`, `upsert_endpoint` and `remove_endpoint` — and **on the @@ -1288,7 +1220,7 @@ is already gone). (issues #219/#240) — a narrowing of the earlier `indigoDeviceId` → `Endpoint.id` → number statement, not a reversal: nothing is still ever keyed on list position or label, and every export - that has never role-changed or been re-adopted publishes under exactly + that has never role-changed or been migrated publishes under exactly today's derivation. 4. **State pushes are echo-guarded** in the node (`ctx.offline`); the plugin never receives a `command` event for a change it pushed. diff --git a/docs/MATTER.md b/docs/MATTER.md index 88b55c6..34600b9 100644 --- a/docs/MATTER.md +++ b/docs/MATTER.md @@ -766,9 +766,9 @@ If the device you migrate onto already has its own exported accessory, THAT accessory is removed from every ecosystem and replaced by the migrated one — the dialog marks it and warns you before you confirm. -It only works onto a device that can take the accessory's role, for the same -reason re-adopt below has that limit — export the device normally instead if -it cannot, and accept that it becomes a new accessory. +It only works onto a device that can take the accessory's role — export the +device normally instead if it cannot, and accept that it becomes a new +accessory. The old Indigo device is not locked out afterwards — you can export it again later — but re-exporting it makes a NEW accessory under a fresh identity; it @@ -776,40 +776,21 @@ does not steal back the identity you just migrated away. ### If you delete and recreate an exported device -Deleting an Indigo device that is exported does not touch your ecosystems — -its accessory keeps sitting in its room, still remembered, just no longer -driven by anything. Re-creating the device (or restoring it a different way) -gets you a NEW Indigo device with a NEW id, and Indigo has no way to know it -is "the same" device you deleted, so exporting it normally builds a SECOND, -brand-new accessory — the old one is still there, in its old room, doing -nothing. - -**`Plugin ▸ Re-adopt a Matter accessory…`** is the way back — for when the -old device is already gone, so migrating (above) is no longer an option. -Pick the left-behind accessory (it lists what role it was, and when it was -un-exported), pick the Indigo device that should drive it now, and confirm. -The accessory returns under its original identity and number, and no -duplicate is created — but by the time an accessory is left behind at all, -most ecosystems have already processed the removal: Apple Home purges an -accessory's room, name and scene associations within moments of noticing it -is gone. So expect the accessory to come back in the Default Room under a -default name, with any scene or automation that referenced it needing to be -rebuilt. Room and name survive intact only when no ecosystem ever gets the -chance to notice the removal — which is exactly what migrating is for, while -the old device still exists. - -It only works when the replacement device can take the SAME role the -accessory already has — a plug can be re-adopted onto another plug-like -device, but not onto a light. If it cannot, export the device normally -instead and accept that it becomes a new accessory. - -**One thing re-adopt cannot fix:** an accessory left behind by a plugin -version **older than 2026.16.2** has no role or name recorded against it — -that version deleted the record on un-export instead of keeping it. The -menu still lists it, so you can see its accessory number is spoken for, but -there is nothing left to match a replacement device against, so it cannot be -re-adopted; export the device normally and accept a new accessory. Nothing -un-exported by 2026.16.2 or later has this problem. +Deleting an Indigo device that is exported removes its accessory from every +paired ecosystem — this happens as soon as the plugin notices the deletion +(immediately if it is running, at the next reconnect if it was not). There +is nothing left behind to manage: the accessory is gone, not merely +un-driven. + +Re-creating the device (or restoring it a different way) gets you a NEW +Indigo device with a NEW id, and Indigo has no way to know it is "the same" +device you deleted — exporting it normally builds a brand-new accessory, +in the Default Room under a default name, with any scene or automation that +referenced the old one needing to be rebuilt. If the old device still +exists at the time you want to move its accessory onto a replacement, use +**Migrate** (above) instead — it is the only way to carry room, name, scenes +and automations across, and only works while the old device has not yet +been deleted. ### Which ecosystems this actually works with diff --git a/docs/adr/0016-destruction-is-unconditional-there-is-no-soft-removal.md b/docs/adr/0016-destruction-is-unconditional-there-is-no-soft-removal.md new file mode 100644 index 0000000..6b30bdb --- /dev/null +++ b/docs/adr/0016-destruction-is-unconditional-there-is-no-soft-removal.md @@ -0,0 +1,110 @@ +--- +parent: Decisions +nav_order: 16 +title: "ADR-0016: Endpoint removal is unconditionally destructive — there is no soft removal to opt out of" + +status: "accepted" +date: 2026-08-26 +decision-makers: solo (Simon) +consulted: none +informed: none +--- +# ADR-0016: Endpoint removal is unconditionally destructive — there is no soft removal to opt out of + +## Context and Problem Statement + +ADR-0015 (accepted one day earlier, 2026-08-26) established the decision this +one does not revisit: a **confirmed** device deletion destroys the accessory, +with no orphan retention and no re-adopt. That ruling stands in full. + +What ADR-0015 also recorded, and what this supersedes, is the **mechanism** it +was reached by. It shipped in two halves. The first added destruction as an +opt-in — the node went on orphaning by default and a new `permanent: true` +wire flag asked it not to — and ADR-0015 was written against that half, while +the second half, deleting the re-adopt UI, was still pending. + +Two of its consequences are therefore now false: + +> * Good, because the destructive path is opt-in per call (`permanent`, …) + +> * …supersede/re-adopt sequence, are structurally unable to trigger it. + +Once the re-adopt UI was deleted, nothing in the plugin ever wanted an orphan. +A default that no caller selects is not a safety property; it is an unused +branch, a second wire vocabulary, and a second set of node-side bookkeeping to +keep correct. The question this ADR answers: **once nothing retains an orphan, +should the opt-out remain?** + +## Decision Drivers + +* An opt-in whose every caller opts in is dead weight that reads like a choice. +* A wire field with one lawful value is a compatibility liability, not a hedge. +* The `orphaned`/`orphanedAt` markers only had meaning for the re-adopt picker. +* Whatever guards a wrongly-issued removal must guard it in one place, not two. + +## Considered Options + +* **Remove the flag; every removal destroys** (chosen) +* Keep `permanent` as a documented no-op default-true, for wire compatibility +* Keep the soft path for `replace()`'s two-command supersede sequence + +## Decision Outcome + +Chosen: **remove the flag; every removal destroys.** `remove_endpoint` has one +meaning again — the endpoint is gone, and its `endpoint-map.json` record with +it. `permanent`/`ARG_PERMANENT`, `parsePermanentRemoval`, `list_orphans`, +`OrphanRecord`, `EndpointMapStore.orphans()`, `noteReadoptableMatch()` and the +`orphanedAt` field are deleted on both sides of the wire. + +We own both ends of this protocol and `protocolVersion` is what protects us +(`bridge_protocol.py` is explicitly **not** a rename firewall), so a field with +one lawful value earns nothing by surviving. + +### Consequences + +* Good, because there is one removal path to reason about rather than two, and + no default anybody has to know is safe. +* Good, because `endpoint-map.json` now records only live endpoints. It was + already the independent witness rather than the allocator; it is now a + witness to a smaller, truer thing. +* Bad, because `replace()`'s literal two-command wire sequence now destroys and + re-adds rather than superseding in place. **Verified not to break migrate:** + accessory numbers come from matter.js's own persisted allocation, not from + this witness file, so the number and the identity move survive. What is lost + is only the retroactive `supersededBy` marking for that literal sequence; it + remains for the reconcile-based case. Three bridge-node tests that drove the + old sequence and asserted the soft bookkeeping were rewritten to assert the + destroy-then-fresh-add reality. +* Bad, because destruction is now unconditional, which makes + `mass_removal_refused` (`reconcile.ts`) the single remaining guard between a + plugin bug that sends a short list and a wiped fabric. It is untouched and + deliberately so; it should be the first thing anyone re-reads before changing + how the export list is computed. +* Neutral: `supersededBy`, `orphaned` and `deviceId` survive on + `endpoint-map.json` records. They read as re-adopt leftovers and are not — + they serve `restorable()`'s startup rebuild (issue #141) and migrate's + driving-device rekey detection in `check()`. Do not delete them on the + strength of the name. + +### Confirmation + +`grep -rn "orphan"` over the plugin and the bridge node returns only historical +citations and the unrelated launchd orphan-process reaping in +`launch_agent.py`/`server_process.py` — a different word for a different thing. + +Suites after the deletion: **3804** Python (from 3891) on 3.11 and 3.13, and +**673** bridge-node (from 697). Both fell because a feature and its tests were +removed, which is the intended direction; migrate's own 56 tests still pass. +Net diff **+700/-3355** across 38 files. + +## More Information + +Supersedes the mechanism recorded in +[ADR-0015](0015-a-confirmed-deletion-destroys-the-accessory-no-retention.md); +its ruling — that a confirmed deletion destroys — is unchanged and remains the +governing decision. ADR-0015 is not edited, per this repo's immutability rule. + +The staging that produced the stale text was a sequencing error worth naming: +the ADR was written against a half-finished migration whose second half was +already planned and already known to remove the flag. An ADR should be written +against the end state, or explicitly marked as describing an interim one. diff --git a/docs/adr/INDEX.md b/docs/adr/INDEX.md index 8b06243..005e6e9 100644 --- a/docs/adr/INDEX.md +++ b/docs/adr/INDEX.md @@ -31,6 +31,7 @@ chosen), see [`../ARCHITECTURE.md`](../ARCHITECTURE.md). * [ADR-0012](0012-export-eligibility-can-be-a-user-declaration.md) - ADR-0012: For a device Indigo does not type, export eligibility is a user declaration (accepted; narrows ADR-0003) * [ADR-0013](0013-a-confirmed-commanded-colour-temperature-is-pushed-as-state.md) - ADR-0013: A confirmed, commanded colour-temperature is pushed as state (accepted) * [ADR-0014](0014-ct-physical-bounds-are-learned-declarations-only-seed-them.md) - ADR-0014: Colour-temperature physical bounds are learned from clamped echoes; declarations only seed them (accepted) -* [ADR-0015](0015-a-confirmed-deletion-destroys-the-accessory-no-retention.md) - ADR-0015: A confirmed device deletion destroys the accessory — no orphan retention, no re-adopt (accepted; supersedes in part ADR-0010) +* [ADR-0015](0015-a-confirmed-deletion-destroys-the-accessory-no-retention.md) - ADR-0015: A confirmed device deletion destroys the accessory — no orphan retention, no re-adopt (accepted; supersedes in part ADR-0010; its **mechanism** — the opt-in `permanent` flag — is **superseded by ADR-0016**, its ruling stands) +* [ADR-0016](0016-destruction-is-unconditional-there-is-no-soft-removal.md) - ADR-0016: Endpoint removal is unconditionally destructive — there is no soft removal to opt out of (accepted; supersedes ADR-0015's mechanism) diff --git a/indigo-matter.indigoPlugin/Contents/Info.plist b/indigo-matter.indigoPlugin/Contents/Info.plist index ca0b8c8..021e9b4 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.29.0 + 2026.29.1 ServerApiVersion 3.6 diff --git a/indigo-matter.indigoPlugin/Contents/Server Plugin/MenuItems.xml b/indigo-matter.indigoPlugin/Contents/Server Plugin/MenuItems.xml index 4a5f489..8a09efa 100644 --- a/indigo-matter.indigoPlugin/Contents/Server Plugin/MenuItems.xml +++ b/indigo-matter.indigoPlugin/Contents/Server Plugin/MenuItems.xml @@ -594,61 +594,6 @@ A ROLE CHANGE is not what this is for — export the replacement device normally - - - Re-adopt a Matter accessory… - menuReadoptExport - - - - - - - - readoptOrphanChanged - - - - - - - - Only devices that can take the accessory's role are offered. A ● marks a device that is already exported. - - - - - - - - -