feat(triki-controller): pluggable transport + Node NobleTransport (receive outside the browser)

[Fl0p]

What

Make TrikiController transport-agnostic so the token can be received outside the browser, not just via Web Bluetooth in Chrome — with an explicit, tested transport contract and a production-grade Node transport.

Transport layer (TrikiTransport v2)

• Small, explicitly-owned interface: the controller claims a transport with attach(handlers) (one owner at a time; detach() releases) and hands over onFrame / onDisconnect / onBattery callbacks — a transport only calls what it has data for, so minimal plain-JS transports can't crash the controller.

• Honest contract, enforced by both built-ins and the test suite: onDisconnect fires exactly once per successful connect and never when no connect succeeded; disconnect() is idempotent and cancels a pending connect() (which then rejects); optional preflight(op) runs sync precondition checks before any state change, so failures like "no Web Bluetooth" or "reconnect before connect" reject with zero spurious connectionchange events.

• All Triki protocol knowledge (START/LED commands, frame parsing, fusion) stays in the controller; transports are dumb NUS pipes.

• WebBluetoothTransport — the existing GATT code, still the default in the browser. Public TrikiController API and behaviour are unchanged.

• NobleTransport — production Node transport backed by @abandonware/noble (optional peer dep, lazily imported, externalized from the build), exposed from triki-controller/node:

  • cancellable, time-bounded connect() — one scanTimeoutMs budget (default 15 s, Infinity to disable) covers adapter power-on + scan + link setup; named errors ScanTimeoutError / ConnectAbortedError;
  • terminal adapter states (unsupported / unauthorized) rejected synchronously (noble never replays stateChange, so waiting on it hung forever);
  • every scan exit path removes the discover listener and stops scanning — no listener stacking on the module-level noble emitter, no zombie session resurrecting into streaming after an explicit disconnect;
  • true reconnect(): re-links to the cached peripheral of the previous successful connect — no scan, so it can never latch onto a different token and mis-apply per-device calibration;
  • NUS + Battery discovered in one combined GATT round trip; battery read/subscribe is genuinely fire-and-forget with per-session staleness guards;
  • fire-and-forget noble calls carry .catch() so a rejection can't crash the process.

Controller hardening

• connect()/reconnect() share one flow; a failure after the transport connected (e.g. the START write rejects) now also closes the transport — previously the live link kept feeding frame/orientation events into a "disconnected" controller, and disconnect() was a permanent no-op.
• #ingest ignores frames outside streaming; late battery readings after cleanup are dropped; connectionchange transitions are deduped.
• A malformed custom transport fails fast: one TypeError naming every missing member.

Testing

• 123 tests (was 73): the original 19 controller tests pass unchanged; new suites cover the controller↔transport seam through a scripted FakeTransport (failure cleanup, cancellation, state guards, ownership) and the full NobleTransport lifecycle through an injected mock noble (timeouts, cancellation, terminal states, reconnect caching, battery non-blocking, exactly-once disconnect, bounded writes, listener hygiene). CI needs no native module — the mock is injected via the public noble option.
• npm ci + typecheck + build clean; the browser bundle has zero noble imports; dist/node.js keeps noble as a runtime import().
• node example/node.mjs without noble installed prints the install hint and exits 1 (no crash, no hang); Ctrl+C exits through the BLE teardown instead of killing Node mid-disconnect.

Docs

• docs/guide/library.md: full TrikiTransport contract (the old list omitted onBattery — a transport written from the docs crashed the constructor), Node options (scanTimeoutMs, scanServiceUuids), error semantics, event guarantees, a minimal replay-transport example.
• Package README: was never updated for transports — now documents the transport option and a Node section.
• protocol.ts carries canonical 128-bit battery UUIDs + DEVICE_NAME_PREFIX (noble derives its short IDs via the exported nobleUuid() instead of hardcoding "180f"/"2a19").

Post-review hardening

A second recall-mode multi-agent review of the finished branch (10 finder angles → 14 adversarial verifiers → gap sweep) confirmed another wave of lifecycle races; all fixed with regression tests (see fix(triki-controller): harden the connect lifecycle):

• Controller: #open is epoch-aware and closes the transport before emitting disconnected — an auto-retry from a connectionchange listener no longer spirals into a self-cancelling loop, and a superseded attempt can't clobber a newer one. New dispose() releases the transport for a replacement controller.
• WebBluetoothTransport: per-attempt cancellation + in-progress guards (a retry could previously un-cancel and resurrect an earlier attempt); session state commits atomically; empty battery payloads no longer throw.
• NobleTransport: same-tick disconnect()→connect() retry works; the abort path no longer leaks a forever-pending disconnectAsync promise (verified against noble 1.9.2-26 sources) nor double-disconnects; teardown removes only its own listener on noble's shared cached peripheral; TX notifications are session-guarded; allowDuplicates=true (name-in-scan-response tokens were permanently unmatchable); the scan is stopped before connectAsync (BlueZ rejects connects mid-scan); writes are bounded (noble never settles GATT callbacks on a dropped link — the START write could hang connect() forever); address also matches the peripheral id (macOS hides MACs).
• Packaging: peer range >=1.9.2-26 <2.0.0 (the ^ prerelease comparator would ERESOLVE on noble's next release); tsup splitting: true (dist/node.js embedded a second copy of the core — instanceof broke across entries); version 0.3.0 (0.2.0 is already on npm).
• Refuted during verification (no change): stale-battery leak across reconnect; gattserverdisconnected async race (spec fires it synchronously).

Notes

• NobleTransport({ noble }) accepts an injected noble-compatible module (e.g. @stoprocent/noble) for Node versions where @abandonware/noble's native build fails.
• Version 0.3.0 (0.2.0 is already published on npm without the ./node subpath).

🤖 Generated with Claude Code

[coderabbitai]

📝 Walkthrough

Walkthrough

Refactors TrikiController to delegate BLE work through a transport interface, adds browser and Node transport implementations, exposes a new Node entry point, and updates package metadata, docs, examples, and tests for the transport split.

Changes

Transport-agnostic controller and Node.js BLE support

Layer / File(s)	Summary
Transport contract and controller option src/transport.ts, src/events.ts	Defines TrikiTransport/TrikiTransportHandlers and adds transport?: TrikiTransport to TrikiControllerOptions.
WebBluetoothTransport implementation src/transports/web-bluetooth.ts	Moves browser Web Bluetooth connection setup, writes, battery handling, disconnect teardown, and support checks into WebBluetoothTransport.
NobleTransport implementation src/transports/noble.ts	Adds NobleTransport for Node.js with lazy @abandonware/noble loading, scan/match, reconnect, battery handling, cancellation, and helper errors/constants.
TrikiController delegated to transport src/controller.ts	Replaces Web Bluetooth-specific controller state with transport callbacks and routes connect, reconnect, disconnect, LED, rate, and cleanup through the transport API.
Public exports, Node entry point, and package wiring src/index.ts, src/node.ts, package.json, tsup.config.ts	Exports the new transport types and values, adds the ./node entry point, marks noble as optional/external, and updates build inputs.
Docs, example, and test coverage docs/guide/library.md, docs/guide/overview.md, README.md, example/node.mjs, src/*.test.ts, src/testkit/*	Updates docs and README for the transport model, adds the Node example, and expands controller/transport/noble/fake transport tests and test doubles.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

• Flopsstuff/triki#2: Modifies controller battery handling paths that this PR reroutes through transport callbacks.
• Flopsstuff/triki#3: Adds controller lifecycle tests around connection/error behavior, which overlap with the refactored controller flow here.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly captures the main change: transport abstraction plus Node NobleTransport support for receiving outside the browser.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/noble-transport

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 7

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/guide/library.md`:
- Around line 197-199: The TrikiTransport contract description is missing
onBattery(handler), so the documented interface is out of sync with the actual
transport API. Update the TrikiTransport section in the library guide to include
onBattery(handler) alongside the existing methods, and make sure the transport
examples and wording around NobleTransport and WebBluetoothTransport reflect the
full required contract.

In `@packages/triki-controller/src/controller.ts`:
- Around line 147-155: The connect/reconnect flow in controller.ts leaves the
BLE transport connected if `#startStreaming`() fails after
`#transport.connect`()/reconnect() succeeds. Update the catch blocks in connect()
and reconnect() to explicitly tear down the transport on START failure, then
reset controller state as today; use the existing `#transport` and `#cleanup`
helpers so the failure path fully disconnects instead of only cleaning up local
state.

In `@packages/triki-controller/src/transports/noble.ts`:
- Around line 164-166: The failed-connect error path in noble.ts only calls
`#teardown`(), which clears local state but does not disconnect the peripheral, so
a partially connected BLE session can remain open. Update the catch block in the
connect flow to disconnect the peripheral before calling `#teardown`(), and ensure
`#teardown`() still handles local cleanup while the disconnect logic is handled in
the failed connection path around `#teardown`() and the connect() catch handling.
- Around line 191-193: The reconnect flow in TrikiTransport.noble is not
preserving the originally selected device, since reconnect() just delegates to
connect() and can pick a different TRIKI* device. Update the TrikiTransport
implementation to cache a stable device identifier or token during the first
successful connect() and have reconnect() reuse that stored value instead of
rescanning generically. Use the existing connect() and reconnect() methods in
NobleTransport/TrikiTransport to locate the change.
- Around line 205-208: The `NobleTransport.disconnect()` method should be a true
no-op when `#peripheral` is null, so remove the fallback call to
`#onDisconnected()` and only disconnect when a peripheral exists. Keep the logic
in `disconnect()` limited to the connected case and preserve `#onDisconnected()`
for actual disconnect events from the transport, not for failed or never-started
sessions.

In `@packages/triki-controller/src/transports/web-bluetooth.ts`:
- Around line 100-102: The Web Bluetooth session setup catch in connect() only
calls this.#teardown(), so if discovery or notification setup fails after the
GATT link is established the peripheral can stay connected. Update the catch in
web-bluetooth.ts to explicitly disconnect the active GATT/server or device
connection before tearing down local state, using the same connection objects
created earlier in connect() and ensuring teardown still runs afterward.
- Around line 72-75: The `WebBluetoothTransport.disconnect()` method is invoking
`#onGattDisconnected()` even when there is no active GATT session, which causes
spurious disconnect callbacks. Update `disconnect()` so it only calls
`#gatt.disconnect()` when `#gatt` exists and is connected, and otherwise returns
without triggering the disconnect handler; use the `disconnect()` method and
`#onGattDisconnected` as the key symbols to locate the fix.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f596fc07-9385-487e-ac97-aa02e0c3eff7

📥 Commits

Reviewing files that changed from the base of the PR and between 3343f26 and 228a26d.

📒 Files selected for processing (11)

• docs/guide/library.md
• packages/triki-controller/example/node.mjs
• packages/triki-controller/package.json
• packages/triki-controller/src/controller.ts
• packages/triki-controller/src/events.ts
• packages/triki-controller/src/index.ts
• packages/triki-controller/src/node.ts
• packages/triki-controller/src/transport.ts
• packages/triki-controller/src/transports/noble.ts
• packages/triki-controller/src/transports/web-bluetooth.ts
• packages/triki-controller/tsup.config.ts

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

packages/triki-controller/src/controller.transport.test.ts (1)

54-82: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated minimal-transport literal across two tests.

The "per the docs" minimal TrikiTransport object literal (attach/detach/connect/writeRx/writeCtrl/hasLed/disconnect) is duplicated verbatim in both describe blocks. Extracting a small factory would prevent the two copies drifting apart as the contract evolves.

♻️ Suggested extraction

+/** The exact minimal member set the Custom Transports docs list — no battery, no reconnect. */
+function makeMinimalTransport(): {
+  transport: TrikiTransport;
+  getHandlers: () => Parameters<TrikiTransport["attach"]>[0] | null;
+} {
+  let handlers: Parameters<TrikiTransport["attach"]>[0] | null = null;
+  const transport: TrikiTransport = {
+    attach: (h) => {
+      handlers = h;
+    },
+    detach: () => {
+      handlers = null;
+    },
+    connect: async () => {},
+    writeRx: async () => {},
+    writeCtrl: async () => {},
+    hasLed: false,
+    disconnect: () => {
+      handlers?.onDisconnect();
+    },
+  };
+  return { transport, getHandlers: () => handlers };
+}

Then both tests become const { transport: minimal, getHandlers } = makeMinimalTransport();.

Also applies to: 211-234

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/triki-controller/src/controller.transport.test.ts` around lines 54 -
82, The minimal TrikiTransport object literal is duplicated in
controller.transport.test.ts across both test cases, so extract it into a small
helper/factory to keep the docs-aligned contract in one place. Create a reusable
helper like makeMinimalTransport that returns the transport and a way to access
the attached handlers, then update both tests to use it instead of inlining the
same attach/detach/connect/writeRx/writeCtrl/hasLed/disconnect shape. Use the
existing TrikiTransport, attach, detach, and handlers references to keep the
refactor easy to locate.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/guide/library.md`:
- Around line 252-260: The replay transport lifecycle is incomplete because
`disconnect()` only calls `#handlers?.onDisconnect()` and leaves `#handlers`
attached, so it is not truly detached after disconnect. Update the replay
transport implementation in the class that defines `detach()`, `disconnect()`,
and `feed()` so `disconnect()` also clears `#handlers` (or delegates to
`detach()`), making repeated disconnects idempotent and preventing `feed()` from
delivering frames after disconnection.

---

Nitpick comments:
In `@packages/triki-controller/src/controller.transport.test.ts`:
- Around line 54-82: The minimal TrikiTransport object literal is duplicated in
controller.transport.test.ts across both test cases, so extract it into a small
helper/factory to keep the docs-aligned contract in one place. Create a reusable
helper like makeMinimalTransport that returns the transport and a way to access
the attached handlers, then update both tests to use it instead of inlining the
same attach/detach/connect/writeRx/writeCtrl/hasLed/disconnect shape. Use the
existing TrikiTransport, attach, detach, and handlers references to keep the
refactor easy to locate.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f1cbc4ed-b1c0-4472-9d67-17da5f7efced

📥 Commits

Reviewing files that changed from the base of the PR and between 228a26d and 19f01fb.

📒 Files selected for processing (17)

• docs/guide/library.md
• docs/guide/overview.md
• packages/triki-controller/README.md
• packages/triki-controller/example/node.mjs
• packages/triki-controller/src/controller.test.ts
• packages/triki-controller/src/controller.transport.test.ts
• packages/triki-controller/src/controller.ts
• packages/triki-controller/src/events.ts
• packages/triki-controller/src/index.ts
• packages/triki-controller/src/node.ts
• packages/triki-controller/src/protocol.ts
• packages/triki-controller/src/testkit/fakeNoble.ts
• packages/triki-controller/src/testkit/fakeTransport.ts
• packages/triki-controller/src/transport.ts
• packages/triki-controller/src/transports/noble.test.ts
• packages/triki-controller/src/transports/noble.ts
• packages/triki-controller/src/transports/web-bluetooth.ts

✅ Files skipped from review due to trivial changes (1)

• docs/guide/overview.md

🚧 Files skipped from review as they are similar to previous changes (5)

• packages/triki-controller/src/node.ts
• packages/triki-controller/src/events.ts
• packages/triki-controller/src/index.ts
• packages/triki-controller/example/node.mjs
• packages/triki-controller/src/controller.ts