diff --git a/plans/009-dns-sd-service-discovery.md b/plans/009-dns-sd-service-discovery.md new file mode 100644 index 0000000..9fa7a02 --- /dev/null +++ b/plans/009-dns-sd-service-discovery.md @@ -0,0 +1,372 @@ +# Plan 009: Advertise DNS-SD (`_devicesdk._tcp`) and browse for servers in the CLI + +> **Executor instructions**: Follow this plan step by step. Run every +> verification command and confirm the expected result before moving to the +> next step. If anything in the "STOP conditions" section occurs, stop and +> report - do not improvise. When done, update the status row for this plan +> in `plans/README.md` - unless a reviewer dispatched you and told you they +> maintain the index. +> +> **Drift check (run first)**: +> `git diff --stat bbd724d..HEAD -- apps/server/src/foundation/mdns packages/cli/src/api apps/server/src/server.ts apps/server/src/config.ts` +> If any in-scope file changed since this plan was written, compare the +> "Current state" excerpts against the live code before proceeding; on a +> mismatch, treat it as a STOP condition. + +## Status + +- **Priority**: P2 (execute BEFORE plan 001 if possible - see Maintenance notes) +- **Effort**: S-M +- **Risk**: LOW (additive protocol surface; existing A-record path untouched) +- **Depends on**: none +- **Category**: direction +- **Planned at**: commit `bbd724d`, 2026-07-06 + +## Why this matters + +The server's mDNS responder only answers A-record queries for one fixed name +(`.local`), so the CLI's "auto-discovery" is really "resolve +`devicesdk.local` and hope": a custom `MDNS_HOSTNAME`, a non-8080 `PORT`, or +two servers on one LAN all break discovery unless the user already knows the +answer. DNS-SD (RFC 6763) service advertisement (`_devicesdk._tcp.local`) lets +clients *browse* for every DeviceSDK server on the network and learn each one's +real hostname AND port from the SRV record. It also unlocks zeroconf discovery +in the planned Home Assistant integration (plan 001) - HA shows a "DeviceSDK +discovered" flow automatically for any integration that advertises a service +type. This is a ROADMAP.md item ("full DNS-SD service-type advertisement +(`_devicesdk._tcp`) so the CLI and the HA integration can browse for servers"). + +## Current state + +Relevant files: + +- `apps/server/src/foundation/mdns/dnsPacket.ts` (193 lines) - pure DNS wire + codec. Exports `TYPE_A = 1`, `TYPE_ANY = 255`, `CLASS_IN`, `FLAG_TOP_BIT`, + `parseQuery()`, `encodeAResponse()`, `encodeName()`, `parseIpv4()`. Encodes + ONLY A-record responses today. No name compression is emitted (fine - keep it + that way; packets stay small). +- `apps/server/src/foundation/mdns/responder.ts` (143 lines) - socket wrapper. + `buildResponseForQuery(packet, fqdn, addresses)` is the pure decision core; + `startMdnsResponder({hostname, port?, getAddresses?})` binds UDP 5353, joins + `224.0.0.251`, replies **unicast to the querier**, and multicasts a gratuitous + announcement on start and a TTL-0 goodbye on stop. +- `apps/server/src/foundation/mdns/dnsPacket.test.ts`, + `apps/server/src/foundation/mdns/responder.test.ts` - bun tests; the exemplar + patterns to extend (`describe`/`it`, pure-function tests plus socket tests + with an injectable port). +- `apps/server/src/server.ts` lines 76-90 - wiring: + + ```ts + const mdns = config.mdnsEnabled + ? startMdnsResponder({ hostname: config.mdnsHostname }) + : null; + ``` + +- `apps/server/src/config.ts` lines 76, 83-84 - `port` (from `PORT`, default + 8080), `mdnsEnabled` (`MDNS_ENABLED`, default true), `mdnsHostname` + (`MDNS_HOSTNAME`, default `"devicesdk"`). +- `packages/cli/src/api/mdnsDiscovery.ts` (235 lines) - the CLI's own zero-dep + mDNS client. `discoverMdnsHost()` sends one A query for + `.local` and resolves + `http://:` or `null` on timeout (1500 ms). + It duplicates `encodeName`/`readName` from the server codec (intentional - + the CLI must not depend on server code). +- `packages/cli/src/api/shared.ts` lines 45-73 - `getApiUrl()` calls + `discoverMdnsHost()` as the last-resort host resolution and prints + `✓ Discovered DeviceSDK server at via mDNS.` +- `packages/cli/src/api/mdnsDiscovery.test.ts` - vitest suite; exemplar for the + new browse tests (it drives `parseAResponses` with hand-built packets and + `discoverMdnsHost` against a fake socket). + +Key wire facts already in the codebase you must stay consistent with: + +- Answers set the cache-flush bit (`FLAG_TOP_BIT` on rrclass) - correct for + *unique* records (A, SRV, TXT) but **must NOT be set on PTR records**, which + are shared (RFC 6762 §10.2). +- The responder replies unicast to `rinfo.address:rinfo.port`. That is correct + for legacy/one-shot resolvers (source port ≠ 5353) but standard mDNS + responses to queries from source port 5353 should go to the multicast group + `224.0.0.251:5353` so all caches (including Home Assistant's zeroconf) see + them. This plan adds that distinction. + +Repo conventions: strict TS, no `any` (use `unknown` + narrowing), zero new +dependencies in both packages (both codecs are deliberately hand-rolled), files +under ~700 LOC, server tests are `bun test`, CLI tests are vitest. + +## Commands you will need + +| Purpose | Command | Expected on success | +|---------|---------|---------------------| +| Install | `pnpm install` | exit 0 | +| Server tests | `pnpm test --filter @devicesdk/server` | all pass | +| CLI tests | `pnpm test --filter @devicesdk/cli` | all pass | +| Server types | `pnpm check-types --filter @devicesdk/server` | exit 0 | +| Server lint | `pnpm lint --filter @devicesdk/server` | exit 0 | +| Repo lint (pre-commit) | `pnpm lint` | exit 0 | +| Changeset | `pnpm changeset` | interactive; see Step 7 | + +## Scope + +**In scope** (the only files you should modify or create): + +- `apps/server/src/foundation/mdns/dnsPacket.ts` +- `apps/server/src/foundation/mdns/dnsPacket.test.ts` +- `apps/server/src/foundation/mdns/responder.ts` +- `apps/server/src/foundation/mdns/responder.test.ts` +- `apps/server/src/server.ts` (one-line wiring change only) +- `packages/cli/src/api/mdnsDiscovery.ts` +- `packages/cli/src/api/mdnsDiscovery.test.ts` +- `packages/cli/src/api/shared.ts` (discovery call site only) +- `README.md` (mDNS paragraph), `ROADMAP.md` (mark DNS-SD shipped) +- `.changeset/*.md` (new changeset) +- `plans/README.md` (status row) + +**Out of scope** (do NOT touch, even though they look related): + +- Firmware mDNS resolution (`firmware/*`) - devices resolve the A record via + lwIP; nothing changes for them. +- `apps/server/src/config.ts` - no new env vars are needed; the service uses + the existing `mdnsHostname` and `port`. +- Interactive server-picker UI in `devicesdk login` when several servers are + found - deferred (see Maintenance notes). Print the list and use the first. +- The Home Assistant integration (plan 001) - only a maintenance note links + them. +- Any change to `encodeAResponse`'s existing behavior consumed by tests. + +## Git workflow + +- `git worktree add .worktrees/dns-sd-discovery -b dns-sd-discovery` and work + only inside that worktree (repo rule: never work in the main checkout). +- Conventional commits, e.g. `feat(server): advertise _devicesdk._tcp over + DNS-SD` / `feat(cli): browse for servers via DNS-SD`. +- Run `pnpm lint` before every commit. +- Open a PR into `main` only if `origin` points at + `github.com/device-sdk/devicesdk`; otherwise report and ask. + +## Steps + +### Step 1: Extend the server DNS codec with PTR/SRV/TXT encoding + +In `apps/server/src/foundation/mdns/dnsPacket.ts`: + +1. Add constants: `TYPE_PTR = 12`, `TYPE_TXT = 16`, `TYPE_SRV = 33`. +2. Add a generic record encoder alongside (not replacing) `encodeAResponse`: + + ```ts + export interface DnsRecord { + name: string; // dotted name + type: number; // TYPE_A | TYPE_PTR | TYPE_TXT | TYPE_SRV + ttl: number; + cacheFlush: boolean; // FLAG_TOP_BIT on rrclass when true + rdata: Uint8Array; + } + export function encodeResponse(records: DnsRecord[], id?: number): Uint8Array + ``` + + Header: `id ?? 0`, flags `0x8400`, QDCOUNT 0, ANCOUNT `records.length`, + NSCOUNT 0, ARCOUNT 0. (Putting SRV/TXT/A in the answer section instead of + additionals is valid and simpler; browsers accept it.) +3. Add rdata builders (all using the existing `encodeName`, no compression): + - `encodePtrRdata(target: string)` → encoded name. + - `encodeSrvRdata(port: number, target: string)` → `u16 priority=0, + u16 weight=0, u16 port, name target`. + - `encodeTxtRdata(pairs: string[])` → each string length-prefixed (1 byte) + and concatenated; for an empty list emit a single zero byte (RFC 6763 + §6.1 requires at least one byte). +4. Reimplement `encodeAResponse` on top of `encodeResponse` + + `encodeARdata(addr)` (via existing `parseIpv4`) so there is one encode path. + Its output bytes must stay identical - the existing tests pin it. + +**Verify**: `pnpm test --filter @devicesdk/server` → all existing dnsPacket +tests still pass (byte-identical A responses), plus your new codec tests +(Step 6) once written. + +### Step 2: Answer DNS-SD queries in the responder + +In `apps/server/src/foundation/mdns/responder.ts`: + +1. Add a required `servicePort: number` to `StartMdnsOptions` (the HTTP port + the SRV record advertises). +2. Define names (all lowercase comparisons, as today): + - host: `.local` (existing `fqdn`) + - service type: `_devicesdk._tcp.local` + - instance: `._devicesdk._tcp.local` + - meta-service: `_services._dns-sd._udp.local` +3. Extend the pure core. Replace `buildResponseForQuery(packet, fqdn, + addresses)` with `buildResponseForQuery(packet, fqdn, addresses, + servicePort)` returning `Uint8Array | null` as before. Answer matrix + (union all matching records into ONE response packet, dedupe): + - Q `.local` type A/ANY → A record per address + (`cacheFlush: true`, TTL 120). [existing behavior] + - Q `_devicesdk._tcp.local` type PTR/ANY → PTR `_devicesdk._tcp.local` → + instance (`cacheFlush: false`, TTL 4500) **plus** SRV + TXT for the + instance and A records (all `cacheFlush: true`; SRV/TXT TTL 120), so + browsers resolve in one round trip. + - Q `._devicesdk._tcp.local` type SRV/TXT/ANY → SRV + TXT + (+ A records for the SRV target). + - Q `_services._dns-sd._udp.local` type PTR/ANY → PTR to + `_devicesdk._tcp.local` (`cacheFlush: false`, TTL 4500). + - SRV: port = `servicePort`, target = `.local`. + - TXT: exactly `["txtvers=1"]` for now. +4. Response routing in the socket handler: if `rinfo.port === MDNS_PORT` + (5353), send the response to `MDNS_ADDRESS:MDNS_PORT` (multicast) with + id 0; otherwise keep today's unicast reply to the querier echoing the query + id. Implement by having `buildResponseForQuery` keep echoing the parsed id + and the caller override destination; the pure core does not need to know. +5. Include PTR (service + meta) + SRV + TXT + A in the gratuitous `announce()` + on start, and in the TTL-0 goodbye on `stop()` (set every record's TTL to + 0, including the PTR ones). + +Do NOT implement probing, conflict resolution, or known-answer suppression - +the module deliberately skips them for the A record already; keep parity and +note it in the module header comment. + +**Verify**: `pnpm test --filter @devicesdk/server` → responder tests pass. + +### Step 3: Wire the port in server boot + +`apps/server/src/server.ts`: pass `servicePort: config.port` to +`startMdnsResponder`. + +**Verify**: `pnpm check-types --filter @devicesdk/server` → exit 0. + +### Step 4: Add DNS-SD browsing to the CLI + +In `packages/cli/src/api/mdnsDiscovery.ts` (keep it dependency-free; extend the +local codec, do not import server code): + +1. Add parsing for answer records beyond A: walk answers + additionals reading + (name, type, class, ttl, rdlength, rdata); decode PTR rdata (name), SRV + rdata (skip 4 bytes, u16 port, name target - note SRV targets may use + compression pointers, and `readName` already follows pointers), and A rdata. +2. Add: + + ```ts + export interface DiscoveredServer { + instance: string; // e.g. "devicesdk._devicesdk._tcp.local" + host: string; // SRV target, e.g. "devicesdk.local" + port: number; // SRV port + addresses: string[];// A records seen for the target (may be empty) + url: string; // http://: + } + export async function browseMdnsServers(options?: { + timeoutMs?: number; // full collection window, default 1500 + }): Promise + ``` + + Send one PTR query for `_devicesdk._tcp.local`, then collect responses for + the whole window (do NOT resolve on first answer - multiple servers answer + independently). Dedupe by instance name. Build `url` from the first A + address if present, else the SRV target name. Prefer the address: not all + stub resolvers handle `.local`. +3. Update `discoverMdnsHost()`: browse first; if the browse returns ≥1 server, + return the first server's `url` (and when >1, the caller prints all - see + next point). If the browse returns none (older server without DNS-SD), fall + back to the existing A-query path unchanged. +4. In `packages/cli/src/api/shared.ts` `getApiUrl()`: when discovery was used + and more than one server was found, print each discovered `instance` + + `url` to stderr and note `Use devicesdk login --host to pin one.` + Keep the existing single-server message otherwise. To surface the list, + have `discoverMdnsHost` accept an optional `onMultiple` callback or export + the browse and call it from `shared.ts` - pick one, keep `discoverMdnsHost`'s + public signature backward compatible (it has tests). + +**Verify**: `pnpm test --filter @devicesdk/cli` → all pass. + +### Step 5: Server tests + +Extend `dnsPacket.test.ts` (model after the existing `describe` blocks): + +- `encodeSrvRdata` / `encodeTxtRdata` / `encodePtrRdata` byte layouts + (hand-computed expected arrays, as the existing tests do). +- `encodeResponse` header fields and a multi-record packet. +- `encodeAResponse` output is byte-identical to the pre-change fixture. + +Extend `responder.test.ts` (`buildResponseForQuery` is pure - no sockets +needed): + +- PTR query for `_devicesdk._tcp.local` → response contains PTR (no + cache-flush bit) + SRV with the configured port + TXT `txtvers=1` + A. +- SRV query for the instance name → SRV + TXT + A. +- Meta-query `_services._dns-sd._udp.local` → PTR to the service type. +- Case-insensitivity (query in mixed case). +- Unrelated service type query → `null`. + +### Step 6: CLI tests + +Extend `packages/cli/src/api/mdnsDiscovery.test.ts` (vitest, model after the +existing packet-fixture style): + +- Parsing a hand-built DNS-SD response (PTR + SRV + TXT + A, including a + compression pointer in the SRV target) into a `DiscoveredServer`. +- Two servers answering → two deduped entries, stable order. +- Browse timeout with no answers → `[]`, and `discoverMdnsHost` falls back to + the legacy A query. + +### Step 7: Docs + changeset + +- `README.md`: extend the mDNS paragraph (currently "The server also + advertises itself over **mDNS** as `devicesdk.local`...") to mention DNS-SD + browsing and that the CLI now finds servers with custom names/ports. +- `ROADMAP.md`: under "Discovery & onboarding", mark the DNS-SD item shipped + (follow the existing "_shipped:_" phrasing used for mDNS advertisement). +- `pnpm changeset`: minor for `@devicesdk/cli`, patch (or minor) for + `@devicesdk/server`. Message: server advertises `_devicesdk._tcp` via + DNS-SD; CLI browses for servers (custom `MDNS_HOSTNAME`/`PORT` setups are + now discoverable). + +**Verify**: `pnpm lint` → exit 0. + +## Test plan + +Covered by Steps 5-6. Full gate: + +- `pnpm test --filter @devicesdk/server` → all pass, including new dnsPacket + and responder cases. +- `pnpm test --filter @devicesdk/cli` → all pass, including browse cases. + +Optional live check (nice, not required): `pnpm dev --filter +@devicesdk/server` on a machine with avahi, then +`avahi-browse -r _devicesdk._tcp` → one entry with port 8080 and TXT +`txtvers=1`. + +## Done criteria + +- [ ] `pnpm check-types --filter @devicesdk/server` exits 0 +- [ ] `pnpm test --filter @devicesdk/server` exits 0 with new DNS-SD tests +- [ ] `pnpm test --filter @devicesdk/cli` exits 0 with new browse tests +- [ ] `pnpm lint` exits 0 +- [ ] `grep -n "TYPE_SRV" apps/server/src/foundation/mdns/dnsPacket.ts` matches +- [ ] `grep -n "browseMdnsServers" packages/cli/src/api/mdnsDiscovery.ts` matches +- [ ] A changeset exists referencing `@devicesdk/server` and `@devicesdk/cli` +- [ ] No files outside the in-scope list modified (`git status`) +- [ ] `plans/README.md` status row updated + +## STOP conditions + +Stop and report back (do not improvise) if: + +- The mdns module files differ from the excerpts above (drift). +- Making `encodeAResponse` delegate to `encodeResponse` changes its output + bytes and existing tests fail - do not "fix" the old tests; report. +- The CLI browse requires binding port 5353 exclusively on some platform to + receive multicast responses and the fallback unicast path can't be made to + work in tests - report the platform specifics instead of shipping a flaky + discovery. +- You find yourself wanting to add an npm dependency (e.g. `bonjour`, + `multicast-dns`) - forbidden; both codecs are deliberately zero-dep. + +## Maintenance notes + +- **Plan 001 (Home Assistant)**: once this lands, 001's config flow can add + `zeroconf: ["_devicesdk._tcp.local."]` to `manifest.json` for automatic + server discovery. Add that note to plan 001 when updating the index. +- **Plan 008 (TLS)**: after TLS-by-default lands, discovered URLs should be + `https://` when the server is in TLS mode. A TXT key (e.g. `tls=1`) is the + natural carrier - whoever lands second must reconcile (add the TXT key and + have the CLI honor it). +- Deferred: interactive server picker in `devicesdk login` for multi-server + LANs; IPv6 (AAAA) records; known-answer suppression. +- Reviewer attention: cache-flush bit must be absent on PTR records; multicast + vs unicast reply routing (source port 5353 test); goodbye packet must + include the PTR records with TTL 0. diff --git a/plans/010-ota-firmware-updates-esp32.md b/plans/010-ota-firmware-updates-esp32.md new file mode 100644 index 0000000..c4f8e2f --- /dev/null +++ b/plans/010-ota-firmware-updates-esp32.md @@ -0,0 +1,441 @@ +# Plan 010: OTA firmware updates for ESP32 (server push, A/B partitions, rollback-safe) + +> **Executor instructions**: Follow this plan step by step. Run every +> verification command and confirm the expected result before moving to the +> next step. If anything in the "STOP conditions" section occurs, stop and +> report - do not improvise. When done, update the status row for this plan +> in `plans/README.md` - unless a reviewer dispatched you and told you they +> maintain the index. +> +> **Drift check (run first)**: +> `git diff --stat bbd724d..HEAD -- apps/server/src/endpoints/devices apps/server/src/foundation apps/server/migrations apps/server/src/janitor.ts firmware/esp32 packages/cli/src` +> If any in-scope file changed since this plan was written, compare the +> "Current state" excerpts against the live code before proceeding; on a +> mismatch, treat it as a STOP condition. In particular, if plan 007 (MQTT) or +> plan 008 (TLS) has already landed, the firmware connection path and the +> download-host semantics have changed - STOP and get the plan re-baselined. + +## Status + +- **Priority**: P1 +- **Effort**: L +- **Risk**: MED-HIGH (firmware update path; mitigated by ESP-IDF rollback) +- **Depends on**: none hard. **Coordinate with plan 008 (TLS)**: both force a + one-time USB reflash of every device; landing them in the same firmware + release means users reflash once, not twice. If 008 landed first, the OTA + HTTP fetch in Phase 2 must use TLS with the pinned cert (see STOP + conditions). +- **Category**: direction +- **Planned at**: commit `bbd724d`, 2026-07-06 + +## Why this matters + +Today every firmware change requires physical USB access to every device +(ROADMAP.md: "OTA updates: let the server push firmware updates to connected +devices (today: re-flash over USB)"). Two pending plans (007 MQTT transport, +008 TLS) both ship firmware changes, so the fleet-reflash pain is about to be +paid twice. After this plan, the server can push a new patched firmware image +to a connected ESP32 over the LAN: the device downloads it into the inactive +OTA slot, verifies it, reboots into it, and automatically rolls back if the +new image fails to reach a healthy state. Pico is explicitly deferred (no +first-party A/B story in pico-sdk). + +## Current state + +### Server + +- `apps/server/src/endpoints/devices/downloadFirmware.ts` - the existing + flash-time image builder. Reads the prebuilt binary from `c.env.FIRMWARES` + (an `FsBlobStore`, keys like `esp32-client.bin`), patches six fixed-length + ASCII placeholder regions (Wi-Fi SSID/pass, API token, host, project id, + device id - placeholder constants `OLD_TOKEN`, `OLD_SSID`, `OLD_PASS`, + `OLD_HOST`, `OLD_PROJECT_ID`, `OLD_DEVICE_ID` at lines 15-21), recalculates + the ESP-IDF image checksum via + `recalculateEsp32Checksum` (`apps/server/src/foundation/esp32ImageChecksum.ts`), + and streams the result. **Critical fact**: it deletes the device's managed + token and creates a fresh one on EVERY download (lines 113-148) because only + the token's hash is stored - the old raw token can never be re-embedded. +- `apps/server/src/foundation/deviceReboot.ts` - exemplar for server-initiated + device commands: `getDeviceStub(env, projectId, deviceId)` (from + `foundation/deviceHandle.ts`) then `stub.triggerRebootForDeploy()`, a method + on the in-process `DeviceSession` + (`apps/server/src/runtime/deviceSession.ts`, 815 lines). +- `apps/server/src/janitor.ts` - hourly cleanup (expired sessions/CLI codes, + old logs/usage). New OTA-artifact expiry hooks in here. +- `apps/server/migrations/` - sequential SQL files; add the next number. +- Auth: `apps/server/src/foundation/auth.ts` resolves Bearer/API tokens; + device WebSocket routes live in + `apps/server/src/endpoints/devices/wsRoutes.ts`. +- Endpoint pattern: Hono + Chanfana + Zod, `BaseRoute` subclass, registered in + `apps/server/src/endpoints/devices/router.ts`. Responses + `{ success: true, result }` / `{ success: false, error }`. + +### Firmware (ESP32) + +- `firmware/esp32/partitions.csv` - current table is factory-only and exactly + fills 2 MB: + + ``` + nvs, data, nvs, 0x9000, 0x6000, + phy_init, data, phy, 0xf000, 0x1000, + factory, app, factory, 0x10000, 0x1F0000, + ``` + +- `firmware/esp32/sdkconfig.defaults` - no explicit flash-size setting (IDF + default applies); `CONFIG_WS_BUFFER_SIZE=2048` (WS frames >2 KB are dropped, + which is why the OTA image must NOT travel over the WebSocket). +- Device→server protocol: JSON frames `{type, id, payload}` parsed in + `firmware/esp32/main/websocket_handler.c` (`handle_websocket_message`, + line 68; e.g. `if (strcmp(type, "reboot") == 0)` at line 102 maps to + `CMD_REBOOT`). Long-running work must NOT run on the worker task's + command/response queue - the reboot pattern (respond first, act in the main + task, `firmware/esp32/main/devicesdk_main.c:265-292`) is the model. +- Targets: `esp32`, `esp32c3` (`sdkconfig.defaults.esp32c3`), `esp32c61` + (`sdkconfig.defaults.esp32c61`). Binaries ship via GitHub Releases and are + bundled into the Docker image; a firmware version bump ONLY happens when a + changeset for `@devicesdk/firmware-esp32` exists (repo rule: no changeset = + won't ship). + +### CLI + +- `packages/cli/src/flash/esp32.ts` + `packages/cli/src/commands/` - the flash + flow: fetches the patched image from the server + (`POST .../firmware/download` with `ssid`, `pass`, `host`, `device_type`) + and writes it over USB. The OTA command reuses the same request surface + minus the USB step. + +### Conventions + +Strict TS, no `any`; Zod at boundaries; migrations are plain sequential SQL +(never run migration SQL through workers-qb - see TROUBLESHOOT.md); IDs +`crypto.randomUUID()`; timestamps `Date.now()`; files under ~700 LOC; +firmware changesets mandatory. + +## Commands you will need + +| Purpose | Command | Expected on success | +|---------|---------|---------------------| +| Install | `pnpm install` | exit 0 | +| Server tests | `pnpm test --filter @devicesdk/server` | all pass | +| Server types | `pnpm check-types --filter @devicesdk/server` | exit 0 | +| CLI tests | `pnpm test --filter @devicesdk/cli` | all pass | +| Lint (pre-commit) | `pnpm lint` | exit 0 | +| ESP32 build (if `idf.py` installed) | `cd firmware/esp32 && idf.py build` | build succeeds; skip gracefully if toolchain absent (repo convention) | +| Regen OpenAPI | `cd apps/server && bun run scripts/generate-openapi.ts` | openapi.json updated | + +Firmware CI builds run in GitHub Actions (`firmware-esp32.yml`) on +`[self-hosted, linux, proxmox-ephemeral]` runners - if the local toolchain is +missing, CI is the build gate. + +## Scope + +**In scope**: + +- `apps/server/src/endpoints/devices/` - new `pushOtaUpdate.ts` (POST) and + `downloadOtaImage.ts` (device GET), registered in `router.ts` +- `apps/server/migrations/_ota_updates.sql` +- `apps/server/src/foundation/deviceOta.ts` (new; mirrors `deviceReboot.ts`) +- `apps/server/src/runtime/deviceSession.ts` - one new stub method to emit the + `ota_update` frame + the "finalize on reconnect" hook +- `apps/server/src/endpoints/devices/wsRoutes.ts` + + `apps/server/src/foundation/auth.ts` - ONLY to thread the authenticated + token id to the device-connect path (see Step 4) +- `apps/server/src/janitor.ts` - expire stale OTA artifacts +- `firmware/esp32/`: `partitions.csv` (or a new `partitions_ota.csv` + + sdkconfig pointer), `sdkconfig.defaults*`, new `main/ota_handler.c/.h`, + `main/websocket_handler.c` (new frame type), `main/devicesdk_main.c` + (mark-valid hook + OTA task kick), `main/CMakeLists.txt` +- `packages/cli/src/commands/` - new `ota` command wired in + `packages/cli/src/index.ts` +- `apps/server/openapi.json` (regenerated), `ROADMAP.md`, `README.md` + (firmware section), `.changeset/*` (server + cli + firmware-esp32), + `plans/README.md` + +**Out of scope** (do NOT touch): + +- `firmware/pico/` - Pico OTA is explicitly deferred; do not "fix it too". +- `downloadFirmware.ts` behavior for USB flashing - it keeps rotating tokens + on every download; do not change its semantics. +- Dashboard UI for OTA - CLI-only in v1. +- Any firmware-version negotiation / auto-update policy - v1 is + user-initiated push of the server's currently bundled firmware. +- `packages/core` - no user-script-visible API changes. + +## Git workflow + +- `git worktree add .worktrees/ota-esp32 -b ota-esp32`; work only there. +- Conventional commits (`feat(server): ...`, `feat(firmware-esp32): ...`, + `feat(cli): ...`); `pnpm lint` before every commit. +- Changesets: `@devicesdk/server` (minor), `@devicesdk/cli` (minor), + `@devicesdk/firmware-esp32` (minor - REQUIRED or firmware won't ship). +- PR into `main` only if `origin` is `github.com/device-sdk/devicesdk`. +- **The PR must not be merged before the owner hardware-validates OTA on a + real ESP32** (same convention as plan 006). Say so in the PR description. + +## Design (decided - do not re-litigate) + +1. **Transport**: the image travels over **HTTP GET**, never the WebSocket + (ESP32 WS buffer is 2 KB). The server only sends a small WS control frame: + + ```json + { "type": "ota_update", "id": "", + "payload": { "path": "/v1/ota/", "size": 123456, + "sha256": "" } } + ``` + + The device builds the full URL from its own configured host (the same + host:port it uses for the WS) + `path` - this avoids host-mismatch bugs + when the CLI reaches the server by a different address than the device. +2. **Artifact**: `POST /v1/projects/:projectId/devices/:deviceId/ota` builds + the patched image immediately (same placeholder patching + checksum + recalculation as `downloadFirmware.ts`), stores the bytes via + `c.env.FIRMWARES.put("ota/.bin", ...)`, records a row in a new + `ota_updates` table, then pushes the WS frame. Request body: `ssid`, + `pass`, `host?`, `device_type` (same Zod shapes as `downloadFirmware.ts`) - + Wi-Fi credentials are compile-time-patched into the image, and the server + never stores them, so the caller must supply them again. +3. **Token lifecycle (the bricking hazard)**: `downloadFirmware.ts` deletes + the old managed token when creating the new one. OTA MUST NOT do that - if + the device fails to apply the image, its old token must keep working. + Instead: + - The OTA endpoint creates the new managed token with description + `" authentication token (ota-pending)"` and stores + `new_token_id` + `old_token_id` on the `ota_updates` row. + - When a device WS connection authenticates with the pending token + (Step 4), the server "finalizes": deletes the old token row, renames the + new token's description to the standard + `" authentication token"`, deletes the artifact blob, marks + the row `completed`. + - The janitor expires rows older than 24 h that never finalized: delete + the artifact blob AND the pending new token (the device evidently still + runs the old image, whose token was never touched). +4. **Device apply path** (ESP32): on `ota_update`, ack the frame, then spawn a + dedicated FreeRTOS task that runs `esp_https_ota` against the URL with the + device's API token in the `Authorization: Bearer` header, verifies the + sha256 while streaming, sets the boot partition, and reboots. + `CONFIG_BOOTLOADER_APP_ROLLBACK_ENABLE=y`: the new image boots in + pending-verify state and calls `esp_ota_mark_app_valid_cancel_rollback()` + only AFTER the WebSocket connects and authenticates; any earlier crash or + auth failure causes the bootloader to fall back to the previous slot. +5. **Partition table**: dual OTA slots require 4 MB flash: + + ``` + nvs, data, nvs, 0x9000, 0x6000, + otadata, data, ota, 0xf000, 0x2000, + phy_init, data, phy, 0x11000, 0x1000, + ota_0, app, ota_0, 0x20000, 0x1E0000, + ota_1, app, ota_1, 0x200000, 0x1E0000, + ``` + + plus `CONFIG_ESPTOOLPY_FLASHSIZE_4MB=y`. This is a breaking layout change: + already-flashed devices need one final USB reflash (coordinate with plan + 008, which breaks flash compatibility anyway - owner accepted that class of + break; see memory of scope decision 008). + +## Steps + +### Phase 0 - Spike (verify feasibility before touching anything) + +1. Determine the current ESP32 app size: download the latest + `firmware-esp32@v*` release asset `esp32-client.bin` (or build locally) and + check `size <= 0x1E0000` (1,966,080 bytes). Record the number in the PR. +2. Confirm `esp_https_ota` + plain-HTTP is available for the pinned ESP-IDF + version used by `firmware/esp32` (check `idf_component.yml` / + `dependencies.lock`): the config knobs are `CONFIG_ESP_HTTPS_OTA_ALLOW_HTTP=y` + (component `esp_https_ota`). If the project's IDF version gates plain HTTP + differently, record how. +3. Confirm all three targets (esp32, esp32c3, esp32c61) are built for modules + with ≥4 MB flash in CI (`.github/workflows/firmware-esp32.yml` and the + sdkconfig files). If any target must stay on 2 MB → STOP (see below). + +**Verify**: a short `plans/notes` comment in the PR description (or plan +update) recording: bin size per target, IDF version, HTTP-OTA knob. No code +changes in this phase. + +### Phase 1 - Server + +1. Migration `_ota_updates.sql`: + + ```sql + CREATE TABLE ota_updates ( + id TEXT PRIMARY KEY, + project_id TEXT NOT NULL, + device_id TEXT NOT NULL, + user_id TEXT NOT NULL, + old_token_id TEXT, + new_token_id TEXT NOT NULL, + sha256 TEXT NOT NULL, + size INTEGER NOT NULL, + status TEXT NOT NULL DEFAULT 'pending', + created_at INTEGER NOT NULL + ); + CREATE INDEX idx_ota_updates_device ON ota_updates(device_id, status); + ``` + + Add the table type to `apps/server/src/types.d.ts` (match existing + `table*` types). +2. `pushOtaUpdate.ts` (POST `.../devices/:deviceId/ota`): validate project + + device ownership exactly like `downloadFirmware.ts` (copy those two + queries); build the patched image by extracting the patching core of + `downloadFirmware.ts` into a shared helper + (`apps/server/src/foundation/firmwareImage.ts`) so the two endpoints share + `padAsciiToLength`/`replacePossiblySplitAscii`/checksum logic instead of + duplicating it (anti-redundancy rule). Token handling per Design #3. + Compute sha256 (`crypto.subtle.digest`), store blob at + `ota/.bin`, insert the row, then call + `triggerDeviceOtaUpdate(env, projectId, deviceId, payload)` in new + `foundation/deviceOta.ts` (mirror `deviceReboot.ts`; the `DeviceSession` + stub method sends the `ota_update` frame and resolves on the device ack, + 5 s timeout like other commands). If the device is offline or does not + ack: mark the row `failed`, delete the pending token + blob, return + `{ success: false, error: "Device offline or did not acknowledge" }` 409. +3. `downloadOtaImage.ts` (GET `/v1/ota/:artifactId`): authenticate via the + standard auth middleware (device API token); verify the artifact row is + `pending`, verify the authenticated user owns it, stream the blob with + `Content-Length`. Do NOT delete on download (finalize happens on + reconnect). +4. Finalize hook: thread the authenticated token id into the device WS + connect path (`wsRoutes.ts` / `auth.ts` - the auth layer already resolves + the token row to hash-match; expose its id on the context). In the session + connect path, look up a `pending` `ota_updates` row with + `new_token_id = `; if found, finalize per Design #3. +5. Janitor: in `janitor.ts`, expire `pending` rows older than 24 h (delete + blob + pending token, set status `expired`). +6. Regenerate `apps/server/openapi.json`. + +**Verify**: `pnpm test --filter @devicesdk/server` and +`pnpm check-types --filter @devicesdk/server` → pass, plus new tests (Test +plan below). + +### Phase 2 - Firmware (ESP32) + +1. Partition table + `CONFIG_ESPTOOLPY_FLASHSIZE_4MB=y` + + `CONFIG_BOOTLOADER_APP_ROLLBACK_ENABLE=y` + + `CONFIG_ESP_HTTPS_OTA_ALLOW_HTTP=y` in `sdkconfig.defaults` (and the + esp32c3/esp32c61 variants as applicable per the Phase 0 findings). +2. `main/ota_handler.c`: task that takes `{path, size, sha256}`, builds the + URL from the configured host (`DEVICESDK_API_HOST` region - see how + `websocket_handler.c`/`devicesdk_main.c` compose the WS URL and reuse + that host parsing), runs `esp_https_ota` with an `Authorization: Bearer + ` header, verifies sha256 incrementally (mbedtls + sha256, already linked), sets boot partition, `esp_restart()`. Emit + progress logs via the standard `ESP_LOGI` tag conventions used in + neighboring files. +3. `websocket_handler.c`: parse `"ota_update"` frames (model after the + `"reboot"` branch at line 102), ack success immediately, hand the payload + to the OTA task. Reject (error response) if an OTA is already running. +4. `devicesdk_main.c`: after the WebSocket is connected AND authenticated + (the point where the device is known-good), call + `esp_ota_mark_app_valid_cancel_rollback()` when the running app state is + `ESP_OTA_IMG_PENDING_VERIFY`. +5. Update `firmware/esp32/README.md` build/partition notes. Changeset for + `@devicesdk/firmware-esp32`. + +**Verify**: `cd firmware/esp32 && idf.py build` for each target if the +toolchain exists locally; otherwise the firmware CI workflow on the PR is the +gate (all targets green). + +### Phase 3 - CLI + +1. New command `devicesdk ota --device-type + --ssid --pass

[--host ]` in `packages/cli/src/commands/` + (register in `index.ts`, follow the structure of the existing flash + command for flag parsing and prompts): POST to the OTA endpoint, print the + ack, then poll the existing device-status endpoint (see + `packages/cli/src/commands/` status command for the call) every 3 s for up + to 120 s and report when the device reconnects ("update applied") or time + out with a "check `devicesdk logs`" hint. +2. CLI tests for arg validation + a mocked happy path (vitest, model after + existing command tests). +3. Changeset for `@devicesdk/cli`. + +**Verify**: `pnpm test --filter @devicesdk/cli` → pass. + +### Phase 4 - Docs + handoff + +- `ROADMAP.md`: mark OTA shipped for ESP32, note Pico deferred. +- `README.md` firmware section: one paragraph on `devicesdk ota` and the + one-time USB reflash needed to adopt the OTA partition table. +- PR description: Phase 0 numbers, the token-lifecycle design, and a bold + "owner must hardware-validate before merge" note. + +## Test plan + +Server (`bun test`, colocated `*.test.ts`, model after existing endpoint +tests): + +- `firmwareImage.ts` helper: patching + checksum parity with the old + `downloadFirmware` behavior (byte-for-byte on a fixture binary containing + the placeholder strings). +- `pushOtaUpdate`: happy path creates row + pending token WITHOUT deleting the + old token (assert both token rows exist); device-offline path cleans up the + pending token and blob. +- `downloadOtaImage`: auth required; wrong user 404; streams exact bytes + + `Content-Length`. +- Finalize: simulated reconnect with the new token id deletes the old token, + renames the new one, marks `completed`. +- Janitor: pending row older than 24 h → blob + pending token gone, status + `expired`; the OLD token still present. + +CLI (vitest): command arg validation, happy-path POST + status polling with +mocked API. + +Firmware: CI build of all targets is the automated gate; on-hardware +validation (flash via USB with new table → `devicesdk ota` → device comes +back on new image → `esp_ota_mark_app_valid` confirmed via logs → pull power +mid-download → device still boots old image) is the OWNER's manual step, +listed in the PR checklist. + +## Done criteria + +- [ ] `pnpm test --filter @devicesdk/server` exits 0 incl. new OTA tests +- [ ] `pnpm check-types --filter @devicesdk/server` exits 0 +- [ ] `pnpm test --filter @devicesdk/cli` exits 0 incl. new `ota` tests +- [ ] `pnpm lint` exits 0 +- [ ] Firmware CI (esp32 + esp32c3 + esp32c61) green on the PR +- [ ] `grep -n "ota_update" firmware/esp32/main/websocket_handler.c` matches +- [ ] `grep -rn "DELETE" apps/server/src/endpoints/devices/pushOtaUpdate.ts` + shows NO deletion of the old token at push time +- [ ] Changesets exist for `@devicesdk/server`, `@devicesdk/cli`, + `@devicesdk/firmware-esp32` +- [ ] `apps/server/openapi.json` regenerated (diff shows the two new routes) +- [ ] PR description contains the hardware-validation checklist; PR NOT merged +- [ ] `plans/README.md` status row updated + +## STOP conditions + +Stop and report back (do not improvise) if: + +- Phase 0 finds any target's app binary > `0x1E0000` bytes, or any supported + module has < 4 MB flash - the partition plan collapses; the owner must + choose slot sizes / target policy. +- Plan 008 (TLS) has landed: the OTA fetch must speak TLS with the pinned + certificate and `CONFIG_ESP_HTTPS_OTA_ALLOW_HTTP` is wrong - the firmware + half of this plan needs re-specification against the TLS code. +- Threading the authenticated token id to the WS connect path requires + restructuring `auth.ts` beyond exposing an id that is already resolved + internally (i.e. more than ~30 changed lines) - report; the fallback design + (keep old tokens forever, surface them for manual cleanup) is an owner + decision, not yours. +- The device ack for `ota_update` cannot reuse the existing 5 s + pendingCommands path in `deviceSession.ts` without protocol changes visible + to user scripts. +- Any change to `packages/core` seems required. + +## Maintenance notes + +- **Plan 008 interplay**: whichever of 008/this lands second must revisit the + OTA fetch transport (plain HTTP vs pinned TLS) and SHOULD share the single + breaking reflash. Update both plans' index rows with the chosen order. +- **Plan 007 interplay**: MQTT-transport devices (007) won't have a WS session + to receive the `ota_update` frame; OTA-over-MQTT is a follow-up, out of + scope here. +- Deferred: Pico OTA (needs a bootloader strategy), dashboard OTA button, + firmware-version reporting + "update available" detection, resumable + downloads. +- Reviewer scrutiny points: the old token must survive every failure path + (grep the push endpoint for token deletion); sha256 verified on-device + before `esp_ota_set_boot_partition`; janitor never deletes a token that has + been finalized as the device's active token. +- Wi-Fi credentials pass through the OTA endpoint request body and into the + patched image but are never persisted server-side - keep it that way. diff --git a/plans/011-device-engine-convergence-spike.md b/plans/011-device-engine-convergence-spike.md new file mode 100644 index 0000000..5a270bf --- /dev/null +++ b/plans/011-device-engine-convergence-spike.md @@ -0,0 +1,245 @@ +# Plan 011: Design spike - extract the device runtime into `packages/device-engine` (kill workerd) + +> **Executor instructions**: This is a DESIGN/SPIKE plan, not a build plan. +> The deliverable is a written report plus disposable spike code - NOT a +> merged refactor. Follow the steps in order, honor the STOP conditions, and +> when done update the status row for this plan in `plans/README.md` - unless +> a reviewer dispatched you and told you they maintain the index. +> +> **Drift check (run first)**: +> `git diff --stat bbd724d..HEAD -- apps/server/src/runtime packages/cli/src/simulator packages/cli/src/commands/dev.ts` +> If these paths changed since this plan was written, re-verify the "Current +> state" facts before proceeding; on a material mismatch, STOP. + +## Status + +- **Priority**: P3 +- **Effort**: M (as a spike; the eventual extraction it specifies is L) +- **Risk**: LOW (no production code is modified) +- **Depends on**: none. NOTE: plan 007 (MQTT transport) touches + `apps/server/src/runtime/` - if 007 is IN PROGRESS, coordinate timing so the + inventory doesn't go stale immediately. +- **Category**: direction +- **Planned at**: commit `bbd724d`, 2026-07-06 + +## Why this matters + +`devicesdk dev` still runs the cloud-era **workerd** simulator: `workerd +^1.20250831.0` is a dependency of `@devicesdk/cli` (`packages/cli/package.json` +line 65) - a large per-platform native binary every npm user downloads - and +`packages/cli/src/simulator/` (~570 lines across `deviceBridge.ts`, +`localDeviceSender.ts`, `worker.ts`, `cloudflare-workers.d.ts`) is a second, +divergent implementation of the device-script semantics that the real server +implements in `apps/server/src/runtime/` (~3,265 lines). Every runtime feature +(crons, RPC, KV, console capture) must be built twice and can drift. The +ROADMAP commits to fixing this: "Extract the server's in-process device +runtime (`apps/server/src/runtime/`) into a shared `packages/device-engine` +and run the simulator on it, so local dev semantics are byte-for-byte the +production server's - and the workerd binary dependency goes away." + +The extraction is blocked by one hard constraint: **Bun-specific APIs must not +appear in `packages/*`** (repo rule - packages run on plain Node for npm +users), and the runtime currently type-imports `bun:sqlite` in four files. +This spike produces the design that resolves that tension, proves it with +running code on Node, and phases the real migration - so the eventual +implementation plan is low-risk instead of an open-ended rewrite. + +## Current state (verified facts to start from) + +- `apps/server/src/runtime/` files and sizes: `consoleCapture.ts` (51), + `cronDispatch.ts` (83), `cronParser.ts` (240), `deviceHub.ts` (70), + `deviceSender.ts` (529), `deviceSession.ts` (815), `devicesBridge.ts` (78), + `logStore.ts` (170), `rpcConstants.ts` (18), `scriptHost.ts` (166), + `types.ts` (58). +- `import type { Database } from "bun:sqlite"` appears in `deviceHub.ts`, + `deviceSession.ts`, `devicesBridge.ts`, `logStore.ts` (all type-only - the + value comes from `src/server.ts` boot). +- Other runtime imports from server land: `../foundation/logger` + (`ServerLogger`), `../foundation/usageMetrics` (`recordDeviceUsage`), + `../foundation/consts`, `../storage/fsBlobStore` (`FsBlobStore`), plus + `@devicesdk/core` and `zod` (both Node-safe). +- `consoleCapture.ts` uses `AsyncLocalStorage` (Node-safe); + `scriptHost.ts` loads user bundles via dynamic `import()` of files + (Node-safe in principle - verify in the spike). +- CLI side: `packages/cli/src/commands/dev.ts` (418 lines) orchestrates the + workerd simulator; `packages/cli/src/simulator/worker.ts` (110) is the + workerd entry; `deviceBridge.ts` (150) and `localDeviceSender.ts` (271) + replicate DEVICE/DEVICES semantics; `cloudflare-workers.d.ts` (38) types the + workerd runtime. `apps/simulation` is the Vue UI the dev command serves. +- The device script contract (MUST NOT change - user-facing): class with + optional `onDeviceConnect/onDeviceDisconnect/onMessage/onCron` + `crons` + map; env exposes `DEVICE` (command sender + `kv`), `DEVICES` (same-project + RPC), `VARS`; public methods RPC-callable, lifecycle methods blocked; + connection-gated crons (missed slots skipped, never caught up). + +## Commands you will need + +| Purpose | Command | Expected on success | +|---------|---------|---------------------| +| Install | `pnpm install` | exit 0 | +| Server tests (behavior baseline) | `pnpm test --filter @devicesdk/server` | all pass | +| CLI tests | `pnpm test --filter @devicesdk/cli` | all pass | +| Node version check | `node --version` | v22+ (`node:sqlite` availability) | +| Run spike | `node --experimental-strip-types ` or `npx tsx ` | see Step 3 | + +## Scope + +**In scope** (all you may create/modify): + +- `plans/011-report-device-engine-spike.md` (create - the main deliverable) +- `plans/spikes/device-engine/**` (create - disposable spike code; NOT a + workspace package, NOT imported by anything, excluded from `pnpm build`) +- `plans/README.md` (status row) + +**Out of scope** (do NOT touch): + +- ANY file under `apps/` or `packages/` - this spike changes zero production + code. The spike may COPY runtime files into `plans/spikes/device-engine/` + and modify the copies freely. +- `pnpm-workspace.yaml`, `turbo.json`, any `package.json` - the spike dir must + not become a workspace member. +- Deleting workerd or the simulator - that happens in the future + implementation plan this spike specifies. + +## Git workflow + +- `git worktree add .worktrees/device-engine-spike -b device-engine-spike`. +- Commit the report + spike code; conventional message + `docs(plans): device-engine convergence spike report`. +- `pnpm lint` before committing (Biome may ignore `plans/`; if it complains + about spike code style, fixing style is fine, fighting the linter is not - + exclude concerns in the report instead). +- No changeset needed (no workspace package touched); if CI's changeset check + objects, add an empty one (`pnpm changeset --empty`). + +## Steps + +### Step 1: Dependency inventory (the coupling map) + +Read every file in `apps/server/src/runtime/` and produce a table in the +report: for each file - (a) every import that is not `@devicesdk/core`, `zod`, +or `node:*`; (b) every *member/method* actually used on `Database` (e.g. +`.query().all()`, `.run()`, transactions), on `FsBlobStore`, on +`ServerLogger`, and from `usageMetrics`/`consts`; (c) every place the Bun WS +socket shape (`types.ts` `RuntimeSocket`) leaks in. Method-level granularity +matters: the storage-port design in Step 2 stands on exactly this list. + +Also inventory the CLI simulator: which runtime semantics +`deviceBridge.ts`/`localDeviceSender.ts`/`worker.ts` re-implement, and which +`dev.ts` features are simulator-UI-specific (those survive the engine swap). + +**Verify**: the report contains both tables; every `bun:sqlite` usage from +`grep -rn "bun:sqlite" apps/server/src/runtime/` appears in them. + +### Step 2: Design the `packages/device-engine` seam + +Write the proposed design in the report: + +- **Ports** (interfaces the engine consumes, defined in the new package): + suggested starting set - `EngineDb` (the minimal query surface from Step 1's + method list; decide between a workers-qb-shaped facade vs a narrow + hand-rolled interface and justify), `EngineBlobReader` (subset of + `FsBlobStore` the runtime reads), `EngineLogger` (subset of `ServerLogger`), + `EngineSocket` (replacing the Bun WS type), `Clock` (if `Date.now` needs + faking for cron tests). +- **Adapters**: `apps/server` implements the ports with `bun:sqlite` + + existing modules (near-zero behavior change); `packages/cli` implements + them with `node:sqlite` (Node 22+; state the minimum-Node consequence for + the CLI's `engines` field) or in-memory equivalents for `dev`. +- **What moves, what stays**: proposed file-by-file disposition + (`cronParser`/`cronDispatch`/`consoleCapture`/`scriptHost`/`deviceSession`/ + `deviceSender`/`devicesBridge`/`logStore`/`deviceHub`/`types` → engine; + what remains as server glue). +- **Contract tests**: propose a shared behavior test suite that runs the SAME + test file against the engine on Bun (server adapters) and on Node (CLI + adapters) - this is the drift-prevention payoff; name where those tests + would live and which existing server tests seed them. +- **Migration phasing** for the future implementation plan: e.g. (1) carve + ports inside `apps/server` in place, (2) move code to + `packages/device-engine` with server as first consumer, (3) rebuild + `devicesdk dev` on the engine behind a flag, (4) delete workerd + simulator + bridge. Each phase independently shippable. + +### Step 3: Spike - run a device session on Node + +In `plans/spikes/device-engine/` (copies only, never imports into production +code): copy the minimal runtime file set, stub the ports from Step 2 +(in-memory KV/logs, console logger, fake socket object), and drive one real +user-script bundle (build `examples/basic` with the CLI build command, or +hand-write a tiny bundle matching the script contract) through: + +1. session connect → `onDeviceConnect` fires; +2. an inbound message → `onMessage` fires and a `DEVICE` command round-trips + through the fake socket; +3. a cron fire via the real `cronParser`/`cronDispatch`; +4. `console.log` inside the handler lands in the captured log store, not the + host console; +5. `device_kv` get/put through the stub storage. + +Run it on **Node 22** (not Bun - that is the whole point): +`npx tsx plans/spikes/device-engine/run.ts` (or +`node --experimental-strip-types`). Record every place a copied file needed +editing to run on Node - that list IS the true coupling cost and goes in the +report verbatim. + +**Verify**: the spike script exits 0 on Node and prints evidence of all five +behaviors; the report includes the command, Node version, and the +edits-required list. + +### Step 4: Write the report and recommendation + +`plans/011-report-device-engine-spike.md` containing: the Step 1 tables, the +Step 2 design, the Step 3 results, open questions for the owner (e.g. minimum +Node version bump for the CLI; whether `devicesdk dev` keeps the simulation +UI protocol unchanged), a go/no-go recommendation, and a drafted skeleton for +the follow-up implementation plan (phases from Step 2 with effort estimates). + +**Verify**: report file exists; a reader who has seen neither this plan nor +the code can follow the recommendation section. + +## Test plan + +No production tests change. The spike run (Step 3) is the executable +verification; `pnpm test --filter @devicesdk/server` and +`--filter @devicesdk/cli` must still pass untouched (proves you changed no +production code). + +## Done criteria + +- [ ] `plans/011-report-device-engine-spike.md` exists with inventory tables, + port design, spike results, phased migration proposal, go/no-go +- [ ] Spike runs on Node 22 (`npx tsx plans/spikes/device-engine/run.ts` + exits 0, or the report documents precisely why not - which is itself a + valid spike outcome) +- [ ] `git status` shows changes ONLY under `plans/` +- [ ] `pnpm test --filter @devicesdk/server` and + `pnpm test --filter @devicesdk/cli` still pass (nothing touched) +- [ ] `plans/README.md` status row updated + +## STOP conditions + +Stop and report back (do not improvise) if: + +- You are tempted to modify anything under `apps/` or `packages/` - the fix is + to copy into the spike dir, never to edit in place. +- `scriptHost.ts`'s dynamic `import()` of version-keyed bundle files does not + work on Node for a reason that can't be stubbed (e.g. loader semantics) - + document the exact error in the report and stop; that finding changes the + design materially. +- The spike needs more than ~2 days of effort - the design doc with a partial + spike and an honest "unproven" section beats a heroic spike. +- Plan 007's executor is concurrently modifying `apps/server/src/runtime/` - + coordinate or pause; an inventory of moving code is worthless. + +## Maintenance notes + +- The follow-up implementation plan (to be written from the report) is where + workerd actually dies; do not delete it here. +- Plans 007 (MQTT transport) and the runtime interact: the engine seam should + anticipate a transport abstraction (WS today, MQTT session tomorrow) - + Step 2's `EngineSocket` port should be checked against plan 007's + `deviceSession` touches before the real extraction starts. +- If the owner rejects a CLI minimum-Node bump to 22, the `node:sqlite` + adapter choice must be revisited (better-sqlite3 violates the zero-native- + deps preference; an in-memory/file-JSON store for dev may suffice - `dev` + sessions are ephemeral). diff --git a/plans/012-device-kv-inspector.md b/plans/012-device-kv-inspector.md new file mode 100644 index 0000000..c8475e2 --- /dev/null +++ b/plans/012-device-kv-inspector.md @@ -0,0 +1,465 @@ +# Plan 012: Device KV inspector - REST list/delete endpoints + dashboard Storage tab + +> **Executor instructions**: Follow this plan step by step. Run every +> verification command and confirm the expected result before moving to the +> next step. If anything in the "STOP conditions" section occurs, stop and +> report - do not improvise. When done, update the status row for this plan +> in `plans/README.md` - unless a reviewer dispatched you and told you they +> maintain the index. +> +> **Drift check (run first)**: +> `git diff --stat bbd724d..HEAD -- apps/server/src/endpoints/devices apps/server/src/runtime/deviceSession.ts apps/server/src/types.d.ts apps/dashboard/src/pages/DeviceDetailsPage.vue apps/dashboard/src/services/api.service.ts` +> If any of these changed since this plan was written, compare the "Current +> state" excerpts against the live code before proceeding; on a mismatch, +> treat it as a STOP condition. + +## Status + +- **Priority**: P2 +- **Effort**: S +- **Risk**: LOW +- **Depends on**: none +- **Category**: direction +- **Planned at**: commit `bbd724d`, 2026-07-06 + +## Why this matters + +User device scripts persist state through `DEVICE.kv` (get/put/delete), and +the docs actively teach the pattern (`docs/public/recipes/persist-counter-kv.md`). +But nothing in the product can *read* that state back for a human: no REST +endpoint, no dashboard view, no CLI command touches the `device_kv` table. +Today, debugging persisted state means opening `sqlite3` against +`DATA_DIR/devicesdk.sqlite` inside the container. This plan closes the +write-without-read asymmetry: a list endpoint, a delete endpoint, and a +"Storage" tab on the device details page. + +Scope is deliberately read + delete only. Editing values from the dashboard is +deferred (see Maintenance notes) because a hand-edited value can violate type +assumptions in the running script. + +## Current state + +### Server + +- `apps/server/migrations/0024_add_device_kv_table.sql` - the table: + + ```sql + CREATE TABLE IF NOT EXISTS device_kv ( + device_id TEXT NOT NULL, + key TEXT NOT NULL, + value TEXT, + updated_at INTEGER NOT NULL, + PRIMARY KEY (device_id, key) + ); + ``` + + Values are JSON-encoded strings (`JSON.stringify`). `device_id` is the + device's UUID (`devices.id`), not the slug. + +- `apps/server/src/runtime/deviceSession.ts:42-43` - the reserved prefix: + + ```ts + // Prefix reserved for internal keys; blocked from the user-facing kv API. + const INTERNAL_KEY_PREFIX = "__internal:"; + ``` + + The runtime stores cron scheduler state under `__internal:cron_schedules` + (`CRON_STORAGE_KEY`, line 40). **Deleting that key from outside would + corrupt scheduler state**, so both new endpoints must exclude/reject + `__internal:`-prefixed keys. + +- `apps/server/src/runtime/deviceSession.ts:584-636` - `kvGet`/`kvPut`/ + `kvDelete` read and write SQLite directly on every call (no in-memory + cache). Consequence: a REST delete is immediately visible to the running + script's next `kv.get`, and no cache invalidation is needed. There is no + `kvList` in the script-facing API; the endpoints query the table directly. + +- `apps/server/src/endpoints/devices/getDeviceEntities.ts` - the exemplar + endpoint to copy. The ownership-resolution chain (project by + `user_id + project_slug`, then device by `project_id + device_slug`, 404 on + either miss) is at lines 48-82: + + ```ts + const project = await qb + .fetchOne({ + tableName: "projects", + where: { + conditions: ["user_id = ?1", "project_slug = ?2"], + params: [user.id, projectId], + }, + }) + .execute() + .then((p) => p.results); + if (!project) { + return c.json({ success: false, error: "Project not found" }, 404); + } + // ...same shape for device by ["project_id = ?1", "device_slug = ?2"]... + ``` + + Match this file's structure exactly: Chanfana `BaseRoute` subclass, `schema` + with `tags`/`summary`/`operationId`/Zod `request`/`responses`, response body + `{ success: true, result: ... }`. + +- `apps/server/src/endpoints/devices/router.ts` - route registration: + + ```ts + devicesRouter.get("/:deviceId/entities", GetDeviceEntities); + devicesRouter.put("/:deviceId/entities", UpsertDeviceEntities); + ``` + + The router is mounted at `/v1/projects/:projectId/devices` + (`apps/server/src/index.ts:177`). + +- `apps/server/src/types.d.ts` - table row types live here (e.g. + `tableDevices` at line 66, `tableDeviceEntityConfigs` at line 107). There is + no `tableDeviceKv` yet; add one. + +- Test harness exemplar: `apps/server/tests/e2e/device-entities.test.ts` - + `TestServer.start()` + `srv.scaffold({ projectSlug, deviceSlug })` + + `srv.get/put/delete(path, { token })`. Model the new test file on it. + +### Dashboard + +- `apps/dashboard/src/pages/DeviceDetailsPage.vue:57-62` - the tab bar: + + ```html + + + + + + + ``` + + The Versions tab is the exemplar for a lazy-loaded table tab: a `q-table` + panel (lines 200-260), a cached loader + (`versionsCached`/`fetchVersions(force)` around lines 552-561), a tab-change + hook that triggers the load (around line 691), and a confirm dialog pattern + (`showRollbackDialog`, lines 392-417) reusable for delete confirmation. + +- `apps/dashboard/src/services/api.service.ts` - services are plain object + literals (`deviceService` at line 310, `scriptService` at line 407) using + the shared `api.call` wrapper. Exemplar method (lines 436-444): + + ```ts + async getVersions(projectId: string, deviceId: string): Promise { + const data = await api.call>( + `/v1/projects/${projectId}/devices/${deviceId}/script/versions` + ); + if (!data || !data.success) { + throw new Error('Failed to fetch script versions'); + } + return data.result; + }, + ``` + +### Conventions that apply + +- Strict types: no `any`; `unknown` + narrowing. +- Validate at boundaries with Zod (Chanfana does this via `schema.request`). +- Response format `{ "success": true, "result": ... }` / + `{ "success": false, "error": "..." }`. +- IDs `crypto.randomUUID()`, timestamps `Date.now()` (not needed here, listed + for completeness). +- No em-dashes anywhere in code, comments, or docs. +- Bun-specific APIs stay in `apps/server`. + +## Commands you will need + +| Purpose | Command | Expected on success | +|---------|---------|---------------------| +| Install | `pnpm install` | exit 0 | +| Server typecheck | `pnpm check-types --filter @devicesdk/server` | exit 0 | +| Server tests | `pnpm test --filter @devicesdk/server` | all pass | +| Dashboard unit tests | `pnpm test:unit --filter @devicesdk/dashboard` | all pass | +| Lint (run before every commit) | `pnpm lint` | exit 0 | +| Regen OpenAPI | `cd apps/server && bun run scripts/generate-openapi.ts` | `openapi.json` updated, exit 0 | +| Changeset | `pnpm changeset` | interactive; or write the file by hand (see Step 7) | + +## Scope + +**In scope** (the only files you should modify or create): + +- `apps/server/src/endpoints/devices/listDeviceKv.ts` (create) +- `apps/server/src/endpoints/devices/deleteDeviceKv.ts` (create) +- `apps/server/src/endpoints/devices/router.ts` (register the two routes) +- `apps/server/src/types.d.ts` (add `tableDeviceKv`) +- `apps/server/openapi.json` (regenerated, not hand-edited) +- `apps/server/tests/e2e/device-kv.test.ts` (create) +- `apps/dashboard/src/services/api.service.ts` (add kv methods to `deviceService`) +- `apps/dashboard/src/pages/DeviceDetailsPage.vue` (add Storage tab) +- `apps/dashboard/src/lib/formatKvValue.ts` (create - pure helper) +- `apps/dashboard/tests/unit/formatKvValue.spec.ts` (create) +- `.changeset/.md` (create) + +**Out of scope** (do NOT touch, even though they look related): + +- `apps/server/src/runtime/deviceSession.ts` - the script-facing kv API is + untouched; you only read it for reference. +- Any `PUT`/write endpoint for kv values - explicitly deferred. +- `packages/cli` - no CLI command in this plan. +- `docs/public/` - docs can follow in a later pass; the OpenAPI regen already + documents the endpoints. +- Migrations - the `device_kv` table already exists; no schema change. + +## Git workflow + +- Create a dedicated worktree and branch (repo rule - never work in the main + checkout, never commit to `main`): + `git worktree add .worktrees/device-kv-inspector -b device-kv-inspector` +- Conventional commit style, e.g. + `feat(server): add device KV list/delete endpoints` and + `feat(dashboard): add device Storage tab`. +- Run `pnpm lint` before every commit. +- Open a PR into `main` with `gh pr create --base main` **only if** the + `origin` remote is `github.com/device-sdk/devicesdk`; otherwise stop and ask + the operator. + +## Steps + +### Step 1: Add the `tableDeviceKv` row type + +In `apps/server/src/types.d.ts`, next to the other table types (e.g. +`tableDeviceEntityConfigs`), add: + +```ts +export type tableDeviceKv = { + device_id: string; + key: string; + value: string | null; + updated_at: number; +}; +``` + +**Verify**: `pnpm check-types --filter @devicesdk/server` → exit 0. + +### Step 2: Create the list endpoint + +Create `apps/server/src/endpoints/devices/listDeviceKv.ts`, copying the +structure of `getDeviceEntities.ts` (imports, `BaseRoute` subclass, ownership +chain). Specifics: + +- `operationId: "devices-list-kv"`, `summary: "List a device's persisted KV entries"`, + `tags: ["Devices"]`. +- Params schema identical to `getDeviceEntities.ts` + (`projectId`/`deviceId`, `z.string().min(1).max(36)`). +- After resolving the device, query with the query builder: + + ```ts + const { results: rows } = await qb + .fetchAll({ + tableName: "device_kv", + where: { + conditions: ["device_id = ?1", "key NOT LIKE '__internal:%'"], + params: [device.id], + }, + }) + .execute(); + ``` + +- Response `result` shape: + + ```ts + { entries: Array<{ key: string; value: string | null; updated_at: number }> } + ``` + + `value` is the raw stored TEXT (a JSON-encoded string). Do **not** + `JSON.parse` it server-side; the dashboard decides how to render it. Sort + entries by `key` ascending in JS after fetching (workers-qb `orderBy` may be + used instead if the existing endpoints use it; match whatever + `listVersions.ts` does). + +**Verify**: `pnpm check-types --filter @devicesdk/server` → exit 0. + +### Step 3: Create the delete endpoint + +Create `apps/server/src/endpoints/devices/deleteDeviceKv.ts`, same skeleton. +Specifics: + +- `operationId: "devices-delete-kv"`, `summary: "Delete a persisted KV entry from a device"`. +- The key arrives as a **query parameter**, not a path segment (user keys may + contain `/`, `:` or spaces): request schema adds + `query: z.object({ key: z.string().min(1).max(512) })`. +- Reject reserved keys **before** touching the DB: if + `key.startsWith("__internal:")`, return + `c.json({ success: false, error: "Reserved key" }, 400)`. This protects + `__internal:cron_schedules` (scheduler state). +- Delete is idempotent, mirroring `deviceSession.kvDelete` (lines 598-608): + select first to learn whether the row existed, then delete, then return + `{ success: true, result: { deleted: boolean } }` with status 200 either + way. Use `c.env.DB.prepare(...).bind(...).run()` or the qb `delete` API - + match whichever pattern `deleteEnvVar.ts` uses (open it and copy its + delete-statement style). + +**Verify**: `pnpm check-types --filter @devicesdk/server` → exit 0. + +### Step 4: Register routes and regenerate OpenAPI + +In `apps/server/src/endpoints/devices/router.ts` add: + +```ts +devicesRouter.get("/:deviceId/kv", ListDeviceKv); +devicesRouter.delete("/:deviceId/kv", DeleteDeviceKv); +``` + +Then regenerate the OpenAPI document: +`cd apps/server && bun run scripts/generate-openapi.ts` + +**Verify**: `git diff --stat apps/server/openapi.json` shows changes, and +`grep -c "devices-list-kv\|devices-delete-kv" apps/server/openapi.json` → `2` +(or more, if referenced multiple times; must be >= 2... use +`grep -o` and count both operationIds appear at least once each). + +### Step 5: Server e2e tests + +Create `apps/server/tests/e2e/device-kv.test.ts` modeled on +`tests/e2e/device-entities.test.ts` (same `TestServer` + +`srv.scaffold` boilerplate). Seed rows by inserting directly through the test +server's DB handle if the harness exposes one; if it does not, seed via the +runtime path is not available over REST (no write endpoint), so check the +harness (`tests/harness.ts`) for a direct SQLite handle (`srv.db` or similar) +and insert: + +```sql +INSERT INTO device_kv (device_id, key, value, updated_at) VALUES (?, ?, ?, ?) +``` + +(The device UUID for the scaffolded device must be looked up by slug; the +harness scaffold result or a `GET /v1/projects/:p/devices/:d` call gives it.) + +Cases to cover (see Test plan for the full list): list empty, list returns +seeded user keys but never `__internal:` keys, delete existing key +(`deleted: true`), delete missing key (`deleted: false`, 200), delete +`__internal:` key → 400 and the row is still present, both endpoints 404 on a +project/device the auth user does not own, both endpoints 401 without a token. + +**Verify**: `pnpm test --filter @devicesdk/server` → all pass, including the +new file. + +### Step 6: Dashboard - service methods, formatter, Storage tab + +1. In `apps/dashboard/src/services/api.service.ts`, add to `deviceService`: + - `async getKvEntries(projectId, deviceId): Promise` calling + `GET /v1/projects/${projectId}/devices/${deviceId}/kv`; + - `async deleteKvEntry(projectId, deviceId, key): Promise` calling + `DELETE .../kv?key=${encodeURIComponent(key)}` and returning + `result.deleted`. + Define `KvEntry = { key: string; value: string | null; updated_at: number }` + wherever the file keeps its response types (search for `ScriptVersion` and + put it alongside). Follow the exact error-handling shape of `getVersions`. +2. Create `apps/dashboard/src/lib/formatKvValue.ts`: a pure function + `formatKvValue(raw: string | null): string` that returns `"null"` for + null, pretty-printed JSON (`JSON.stringify(JSON.parse(raw), null, 2)`) when + `raw` parses, and the raw string otherwise. Check `apps/dashboard/src/lib/` + first for where helpers live and match the local export style. +3. In `DeviceDetailsPage.vue`: + - Add `` after the + Versions tab. + - Add a `q-tab-panel name="storage"` with a `q-table` (columns: Key, Value, + Updated) copying the Versions panel's structure, including its + empty-state block ("No stored keys yet"). Render values through + `formatKvValue`, truncated in the cell (CSS ellipsis or `.slice`) with + the full value in an expand row or tooltip - match however the Versions + table shows long IDs. + - Per-row delete button opening a confirm dialog copied from the rollback + dialog pattern (lines 392-417); on confirm call `deleteKvEntry`, then + refresh the list. + - Lazy-load with a cached flag exactly like `versionsCached` / + `fetchVersions` and the tab-change hook near line 691. + +**Verify**: `pnpm check-types --filter @devicesdk/dashboard` → exit 0 (if that +turbo task does not exist for the dashboard, `pnpm build --filter +@devicesdk/dashboard` → exit 0), and `pnpm lint` → exit 0. + +### Step 7: Dashboard unit test + changeset + +1. Create `apps/dashboard/tests/unit/formatKvValue.spec.ts` modeled on + `tests/unit/metricsFormat.spec.ts`: null input, valid JSON object, plain + non-JSON string, JSON scalar (`"42"`). +2. Create a changeset file, e.g. `.changeset/device-kv-inspector.md`: + + ```markdown + --- + "@devicesdk/server": minor + "@devicesdk/dashboard": minor + --- + + Device KV inspector: REST endpoints to list and delete a device's + persisted `DEVICE.kv` entries, and a Storage tab on the dashboard device + page. Internal (`__internal:`) keys are hidden and protected. + ``` + +**Verify**: `pnpm test:unit --filter @devicesdk/dashboard` → all pass; +`pnpm lint` → exit 0. + +## Test plan + +New server tests in `apps/server/tests/e2e/device-kv.test.ts` (pattern: +`tests/e2e/device-entities.test.ts`): + +1. `GET .../kv` with no rows → 200, `entries: []`. +2. Seed `counter` + `__internal:cron_schedules` → list returns only + `counter`, with raw JSON string value and numeric `updated_at`. +3. `DELETE .../kv?key=counter` → 200 `{ deleted: true }`; second call → + `{ deleted: false }`. +4. `DELETE .../kv?key=__internal:cron_schedules` → 400; row still present + (re-check via direct DB read). +5. Unauthenticated requests to both endpoints → 401. +6. Requests against a project slug owned by a different user → 404 (scaffold + a second user if the harness supports it; if it does not, a nonexistent + project slug returning 404 is the acceptable fallback - note which you did). + +New dashboard test `apps/dashboard/tests/unit/formatKvValue.spec.ts` (pattern: +`tests/unit/metricsFormat.spec.ts`): 4 cases listed in Step 7. + +Verification: `pnpm test --filter @devicesdk/server` and +`pnpm test:unit --filter @devicesdk/dashboard` → all pass. + +## Done criteria + +ALL must hold: + +- [ ] `pnpm check-types --filter @devicesdk/server` exits 0 +- [ ] `pnpm lint` exits 0 +- [ ] `pnpm test --filter @devicesdk/server` exits 0 and includes the new + `device-kv` tests +- [ ] `pnpm test:unit --filter @devicesdk/dashboard` exits 0 and includes + `formatKvValue.spec.ts` +- [ ] `grep -n "devices-list-kv" apps/server/openapi.json` and + `grep -n "devices-delete-kv" apps/server/openapi.json` both match +- [ ] `grep -rn "__internal" apps/server/src/endpoints/devices/listDeviceKv.ts apps/server/src/endpoints/devices/deleteDeviceKv.ts` shows the prefix is filtered in list and rejected in delete +- [ ] A changeset exists naming `@devicesdk/server` and `@devicesdk/dashboard` +- [ ] `git status` shows no modified files outside the in-scope list +- [ ] `plans/README.md` status row for 012 updated + +## STOP conditions + +Stop and report back (do not improvise) if: + +- The `device_kv` schema differs from the excerpt in "Current state" (a + migration after 0026 may have altered it). +- `deviceSession.ts` no longer reads/writes SQLite directly per kv operation + (i.e. an in-memory cache appeared) - the "REST delete is immediately + coherent" assumption would be false and the delete endpoint needs a + session-invalidation design instead. +- The test harness (`tests/harness.ts`) exposes no way to seed `device_kv` + rows (no DB handle) - do not add a write endpoint to work around it. +- `DeviceDetailsPage.vue` no longer has the tab structure shown above + (plan 001 or another plan may have restructured it). +- A step's verification fails twice after a reasonable fix attempt. + +## Maintenance notes + +- **Deferred: value editing (PUT)**. If added later, it must re-validate that + the running script tolerates type changes, and must keep rejecting the + `__internal:` prefix. The delete endpoint's query-param pattern is the one + to copy. +- **Deferred: CLI surface** (`devicesdk kv list/delete`). The REST endpoints + are deliberately CLI-shaped (query-param key) so this is additive. +- Plan 011 (device-engine extraction) inventories `apps/server/src/runtime/`; + these endpoints read the `device_kv` table directly and are unaffected, but + reviewers of that extraction should keep the `__internal:` prefix contract + in one shared constant if the runtime moves. +- Reviewer focus: the `__internal:` filtering in both endpoints (list SQL + `NOT LIKE` + delete 400), and that `value` is passed through raw rather + than parsed server-side. diff --git a/plans/013-multi-user-sharing-spike.md b/plans/013-multi-user-sharing-spike.md new file mode 100644 index 0000000..f84afd1 --- /dev/null +++ b/plans/013-multi-user-sharing-spike.md @@ -0,0 +1,271 @@ +# Plan 013: Design spike - multi-user project sharing (household model) + +> **Executor instructions**: This is a DESIGN/SPIKE plan, not a build plan. +> The deliverable is a **written design report** - NOT merged production +> code. Do not modify any file outside `plans/`. Follow the steps in order, +> honor the STOP conditions, and when done update the status row for this +> plan in `plans/README.md` - unless a reviewer dispatched you and told you +> they maintain the index. +> +> **Drift check (run first)**: +> `git diff --stat bbd724d..HEAD -- apps/server/src/foundation/auth.ts apps/server/src/endpoints apps/server/migrations apps/server/src/runtime/devicesBridge.ts` +> Drift here does not stop the spike (it is an inventory exercise by nature), +> but note any drifted file in the report and inventory the live code, not +> this plan's excerpts. + +## Status + +- **Priority**: P3 +- **Effort**: M (as a spike; the build it specifies is L) +- **Risk**: LOW (no production code is modified) +- **Depends on**: none. Coordination note: plan 003 (bundled MCP server with + OAuth 2.1) introduces new token issuance; if 003 is DONE or IN PROGRESS, + the report's token-scoping section must cover its tokens too. +- **Category**: direction +- **Planned at**: commit `bbd724d`, 2026-07-06 + +## Why this matters + +DeviceSDK is a self-hosted home-automation platform, and homes contain more +than one person. The server already supports multiple accounts - the first +registered user is always allowed and `ALLOW_REGISTRATION` (default **true**, +`apps/server/src/config.ts:79`) admits more - but every resource is +hard-keyed to a single owner: the `projects` table has +`user_id TEXT NOT NULL` with `UNIQUE(user_id, project_id)` +(`apps/server/migrations/0002_add_projects_table.sql:6-10`), and roughly 24 +endpoint files resolve access with a literal `user_id = ?1` condition. A +second household member who registers gets a permanently empty dashboard: no +way to see, control, or receive logs from any device in the house. + +Retrofitting sharing touches the auth surface of every endpoint, the two +WebSocket paths, token semantics, and the blob-storage layout - too risky to +improvise. This spike produces the design that makes the eventual build plan +mechanical: a full inventory of ownership touchpoints, a chosen membership +model, and a phased migration. + +## Current state (verified facts to start from) + +- **Accounts**: local email/password auth + (`apps/server/src/endpoints/auth/localAuth.ts`); first-run logic at lines + 43-48 (`registration_enabled: c.env.config.allowRegistration || count === 0`). +- **Auth precedence** (`apps/server/src/foundation/auth.ts`, + `authenticateUser` at line 123): Bearer token → session cookie → `dsdk_` + CLI token → API token hash. All four resolve to a single user; there is no + role or membership concept anywhere. +- **Ownership pattern**: endpoints resolve project by + `["user_id = ?1", "project_slug = ?2"]` then device by + `["project_id = ?1", "device_slug = ?2"]` - exemplar + `apps/server/src/endpoints/devices/getDeviceEntities.ts:54-82`. + `grep -rln "user_id = ?1" apps/server/src/endpoints/` matched 24 files at + the planned-at commit. +- **Schema**: `projects.user_id` FK → `user(id)` with + `UNIQUE(user_id, project_id)` (migration 0002; 0007 only added + name/description/updated_at columns). `tokens` (API tokens, migration + 0004) and `cli_tokens` (migration 0008) are user-scoped, not + project-scoped. +- **Blob layout**: script bundles live at + `DATA_DIR/scripts/{userId}/{projectSlug}/{deviceSlug}/{versionId}.js` + (repo guide, `CLAUDE.md` "The server stores ALL state under DATA_DIR") - + the **owner's userId is baked into the path**, which any sharing design + must confront. +- **Runtime trust**: inter-device RPC is same-project + (`apps/server/src/runtime/devicesBridge.ts` - "same-project trust model"). + Sessions are keyed `${projectId}:${deviceId}` with UUIDs + (`apps/server/src/runtime/deviceHub.ts`). +- **WebSockets**: device + watcher WS routes in + `apps/server/src/endpoints/devices/wsRoutes.ts`; how each authenticates is + an inventory item (Step 1), not asserted here. +- **Dashboard**: Vue 3 + Quasar SPA; pages are + `ProjectsPage.vue`, `ProjectDetailsPage.vue`, `DeviceDetailsPage.vue`, + `TokensPage.vue`, `AccountPage.vue` (`apps/dashboard/src/pages/`). +- Repo rules that constrain the design: strict types, Zod at boundaries, + sequential SQL migrations in `apps/server/migrations/`, response format + `{ success, result | error }`, no em-dashes, files under ~700 LOC. + +## Commands you will need + +| Purpose | Command | Expected on success | +|---------|---------|---------------------| +| Install | `pnpm install` | exit 0 | +| Server typecheck (read-only sanity while exploring) | `pnpm check-types --filter @devicesdk/server` | exit 0 | +| Server tests (baseline, before you start) | `pnpm test --filter @devicesdk/server` | all pass | +| Ownership-site inventory | `grep -rn "user_id = ?1" apps/server/src \| sort` | list of sites for Step 1 | + +## Scope + +**In scope** (the only files you may create or modify): + +- `plans/013-report-multi-user-sharing.md` (create - the deliverable) +- `plans/README.md` (status row update at the end) + +**Out of scope** (do NOT touch): + +- ALL production code: `apps/`, `packages/`, `firmware/`, `migrations/`. + This spike writes **zero** application code - no prototype branch, no + draft migration files inside `apps/server/migrations/`. Proposed SQL and + TypeScript appear only as fenced code blocks inside the report. +- No changeset (nothing shippable changes). + +## Git workflow + +- Create a dedicated worktree and branch: + `git worktree add .worktrees/multi-user-sharing-spike -b multi-user-sharing-spike` +- Single commit style: `docs(plans): add multi-user sharing design report (plan 013)`. +- Run `pnpm lint` before committing. +- Open a PR into `main` with `gh pr create --base main` **only if** the + `origin` remote is `github.com/device-sdk/devicesdk`; otherwise stop and + ask the operator. + +## Steps + +### Step 1: Inventory every ownership touchpoint + +Produce the report's Appendix A: a table of every place the current code +binds a resource to a single user. Cover at minimum: + +1. Every `user_id = ?1` site in `apps/server/src/endpoints/` (expect ~24 + files) - for each: endpoint, HTTP method/path, resource, and whether the + check is "resolve project by owner" or something else. +2. Both WebSocket routes in `wsRoutes.ts` - how the device WS and the + watcher WS authenticate and what they check against (read the code; do + not guess). +3. Token issuance and validation: `tokens`, `cli_tokens`, session cookies + (`foundation/auth.ts`), CLI device-code flow (`endpoints/cli-auth/`). +4. The blob path construction (find where + `scripts/{userId}/...` paths are built - start at + `apps/server/src/storage/fsBlobStore.ts` and its callers). +5. The runtime: where `deviceHub`/`deviceSession`/`devicesBridge` assume a + project (and therefore, transitively, one owner) - especially anything + that would break if two users can send commands to the same device. +6. The dashboard: which pages/stores assume "my projects" (search + `apps/dashboard/src/services/api.service.ts` and `stores/`). +7. `deleteUser.ts` (`endpoints/user/`) - what cascades today, and what must + change when a user can own shared projects. + +**Verify**: the inventory table's endpoint count matches +`grep -rln "user_id = ?1" apps/server/src/endpoints/ | wc -l` (± sites found +by reading, e.g. `d1Compat` call sites using different placeholder styles - +also grep `"user_id = ?"` and `user_id IN`). + +### Step 2: Design the membership model + +Write the report's core section. Requirements to satisfy: + +- A `project_members` table (proposed SQL in the report), e.g. + `(project_id, user_id, role, created_at)` with + `PRIMARY KEY (project_id, user_id)`; the existing `projects.user_id` + stays as the owner (simplest migration; evaluate and say so explicitly). +- **Roles**: propose exactly three - `owner` (everything, including member + management and deletion), `operator` (send commands, deploy, edit env + vars, watch), `viewer` (read + watch only). Map every endpoint from the + Step 1 inventory to a minimum role in a table. If a two-role model + (owner/member) is honestly sufficient for the household use case, say so + and recommend it - fewer states is a feature. +- **The access helper**: specify one function, e.g. + `resolveProjectAccess(c, projectSlug, minRole)` in + `apps/server/src/foundation/`, that replaces the per-endpoint + `user_id = ?1` project lookup, returns the project row or a 404/403 + result, and becomes the single choke point. Show its exact signature and + its query (owner match OR membership row with sufficient role). Endpoints + keep returning **404** (not 403) for projects the caller cannot see, to + avoid slug-existence disclosure; justify or amend this in the report. +- **Blob paths**: decide between (a) keep `scripts/{ownerUserId}/...` and + always resolve through the project's owner id (no data migration; sharing + is metadata-only), or (b) re-key to `scripts/{projectId}/...` with a + one-time file move in a migration. Recommend one; (a) is the + low-risk default unless the inventory reveals a blocker. +- **Tokens and attribution**: API tokens and CLI tokens stay user-scoped; + a member's token grants that member's role on shared projects. Decide how + device logs / usage / commands are attributed (do they need a `user_id` + stamp? today they do not have one). Cover plan 003's OAuth tokens if that + plan has landed. +- **Invitation flow**: no email infrastructure exists and none should be + added; propose owner-adds-member-by-email (the account must already + exist) via `POST /v1/projects/:projectId/members`, plus list/remove. UI: + a Members section on `ProjectDetailsPage.vue`. +- **What does NOT change**: device firmware, the device WS protocol, the + script contract, inter-device RPC trust (same-project already implies + shared-project once membership exists - state this explicitly and check + `devicesBridge.ts` for user assumptions). + +### Step 3: Phase the build and size it + +Report section "Phased build plan": 3-5 phases, each independently +shippable and testable, e.g. (1) schema + access helper + read paths, +(2) write paths + role enforcement matrix + tests, (3) members CRUD API + +dashboard Members UI, (4) watcher WS + logs/metrics access, (5) docs + +`deleteUser` cascade rules. For each phase: files touched (from the Step 1 +inventory), test strategy (which existing e2e files to extend - e.g. +`tests/e2e/projects.test.ts`, `tests/e2e/devices.test.ts` - and the new +`tests/e2e/project-members.test.ts`), and an S/M/L estimate. State the +total. + +### Step 4: Open questions for the owner + +End the report with a short list of decisions only the project owner can +make. Expected entries (refine from what you learned): two roles or three; +whether `ALLOW_REGISTRATION=true` should remain the default once sharing +exists (an open server on a LAN now hands new registrants a path to being +invited); whether shared projects appear merged in the projects list or +under a "Shared with me" section; whether attribution stamping (who sent a +command) is v1 or deferred. + +### Step 5: Write the report and update the index + +Assemble `plans/013-report-multi-user-sharing.md` with sections: Summary and +recommendation, Membership model (Step 2), Role/endpoint matrix, Blob-path +decision, Token semantics, Phased build plan (Step 3), Open questions +(Step 4), Appendix A: ownership inventory (Step 1). Then update the plan 013 +row in `plans/README.md` to DONE. + +**Verify**: report file exists; `pnpm lint` → exit 0; +`git status --porcelain` shows only the two in-scope files. + +## Test plan + +Not applicable in the usual sense - no production code changes. The spike's +"tests" are its verification gates: the Step 1 count cross-check and the +requirement that every endpoint in the inventory appears in the Step 2 +role matrix (report is incomplete otherwise). + +## Done criteria + +ALL must hold: + +- [ ] `plans/013-report-multi-user-sharing.md` exists with all eight sections +- [ ] Every endpoint file from the Step 1 grep appears in both Appendix A and + the role matrix +- [ ] The report proposes concrete SQL for `project_members` and a concrete + TypeScript signature for the access helper +- [ ] A blob-path recommendation is stated with its rationale +- [ ] `git status --porcelain` shows changes only under `plans/` +- [ ] `pnpm lint` exits 0 +- [ ] `plans/README.md` status row for 013 updated + +## STOP conditions + +Stop and report back (do not improvise) if: + +- You find an existing membership/role/sharing mechanism anywhere in the + server (this plan asserts there is none; if one exists the premise is + wrong). +- The ownership inventory exceeds ~60 distinct sites - the refactor cost + assumption (M-per-phase) breaks and the owner should re-scope before you + finish the design. +- `wsRoutes.ts` device authentication turns out to be coupled to the + owner's identity in a way that firmware also depends on (would put + firmware in the blast radius, which this plan assumes is untouched). +- You are tempted to write any file outside `plans/` - that is out of + scope by definition. + +## Maintenance notes + +- The report goes stale as endpoints are added; each of plans 001-012 that + adds server endpoints (notably 003 `/mcp`, 007 MQTT, 012 device-kv) adds + rows to the ownership inventory. The eventual build plan must re-run the + Step 1 grep, not trust Appendix A. +- If the owner approves the design, the follow-up is a numbered build plan + (or one per phase) written against the then-current HEAD. +- The rejected-findings ledger in `plans/README.md` records that demand for + sharing is **inferred from the domain**, not user-requested; if real + users ask for it, that changes the priority from P3 upward. diff --git a/plans/README.md b/plans/README.md index a9a0a85..3aca54b 100644 --- a/plans/README.md +++ b/plans/README.md @@ -7,7 +7,10 @@ agent SEO/discoverability` on 2026-07-02 (commit `321ef7e`); plan 003 from plans 004-006 from `/improve plan new sensors and iot protocols` on 2026-07-05 (commit `321ef7e`); plan 007 from `/improve plan mqtt transport option` on 2026-07-05 (commit `bbd724d`); plan 008 from `/improve plan ssl support for -device websockets` on 2026-07-06 (commit `bbd724d`). +device websockets` on 2026-07-06 (commit `bbd724d`); plans 009-011 from +`/improve next` (direction audit) on 2026-07-06 (commit `bbd724d`); plans +012-013 from a second `/improve next` (direction audit) on 2026-07-06 +(commit `bbd724d`). Execute in the order below unless dependencies say otherwise. Each executor: read the plan fully before starting, honor its STOP conditions, and update your row when @@ -25,6 +28,11 @@ done. | 006 | OneWire (DS18B20) + DHT11/DHT22 protocol support (core + server + CLI + both firmwares) | P2 | L | — | TODO | | 007 | MQTT device transport as a lazy-loaded aedes plugin (core contract + server + CLI + both firmwares) | P1 | L | — | TODO | | 008 | TLS by default: TLS-only server listener, pinned self-signed cert in firmware, CLI TOFU pinning | P1 | L | — | TODO | +| 009 | DNS-SD `_devicesdk._tcp` advertisement + CLI browse discovery | P2 | S-M | — (best before 001) | TODO | +| 010 | OTA firmware updates for ESP32 (server push, A/B partitions, rollback-safe) | P1 | L | coordinate with 008 | TODO | +| 011 | Design spike: extract device runtime into packages/device-engine (kill workerd) | P3 | M | — (avoid overlap with 007) | TODO | +| 012 | Device KV inspector: REST list/delete endpoints + dashboard Storage tab | P2 | S | — | TODO | +| 013 | Design spike: multi-user project sharing (household roles, report-only) | P3 | M | — (coordinate with 003 tokens) | TODO | Status values: TODO | IN PROGRESS | DONE | BLOCKED (with one-line reason) | REJECTED (with one-line rationale) @@ -102,6 +110,29 @@ Status values: TODO | IN PROGRESS | DONE | BLOCKED (with one-line reason) | REJE if executed near 006, land them sequentially to avoid firmware merge conflicts (`devicesdk_main.c` / `main.cpp` connection paths). +- 009 has no hard dependency but should execute **before 001** so the Home + Assistant config flow can list `zeroconf: ["_devicesdk._tcp.local."]` from + day one; if 001 lands first, add zeroconf to the HA integration afterward. + Whichever of 008/009 lands second adds a `tls=1` TXT key + CLI handling + (see plan 009 maintenance notes). +- 010 has no hard dependency but **shares a breaking one-time USB reflash + with 008** (new OTA partition table vs TLS pinning): landing both in the + same firmware release means users reflash once. If 008 lands first, 010's + OTA fetch must be re-specified for pinned TLS (STOP condition in the plan). + 010 also notes that 007-transport (MQTT) devices cannot receive the OTA + push frame - OTA-over-MQTT is a recorded follow-up, not in scope. +- 011 modifies nothing in production; its only scheduling concern is that + plan 007 touches `apps/server/src/runtime/` - do not run 011's inventory + while 007 is mid-flight. +- 012 is independent of all other plans. It adds two routes to + `apps/server/src/endpoints/devices/` and a tab to + `DeviceDetailsPage.vue`; nothing else queues changes to those exact + surfaces, so no sequencing constraint. +- 013 is report-only (like 011). Its token-scoping section must cover plan + 003's OAuth tokens if 003 has landed by then, and its ownership inventory + goes stale as 003/007/012 add endpoints - the plan tells the executor to + re-run the inventory grep rather than trust prior counts. + ## Findings considered and rejected - (001 run) _None._ Targeted `plan ` invocation, not an audit. @@ -152,3 +183,42 @@ Status values: TODO | IN PROGRESS | DONE | BLOCKED (with one-line reason) | REJE the Docker image) - rejected in favor of bundled-but-lazy: the enable flow stays one env var, while `DATA_DIR/plugins/` still provides the external extension point. +- (009-011 run) **Backup/restore of DATA_DIR** (D3) - offered (S-M, VACUUM + INTO snapshot + CLI commands), not selected by the owner this round. + Roadmap item; revisit on request. +- (009-011 run) **Captive-portal Wi-Fi provisioning** (D5) - offered (L), not + selected. Best sequenced after OTA (plan 010) exists so provisioning bugs + are field-fixable; revisit then. +- (009-011 run) **Prometheus /metrics endpoint** (D6) - offered (S, exports + existing `device_usage` buckets), not selected. Nearly free; revisit on + request. +- (009-011 run) **Home Assistant OS add-on** - considered, not offered: + pointless before plan 001 (HACS integration) ships. +- (009-011 run) **ESP32 fragmented WS frame reassembly** - considered, not + offered as direction: it is a bug/tech-debt item for a standard audit, not + a roadmap feature. +- (012-013 run) **REST-callable script public methods ("actions API", D7)** - + offered (M; `POST .../devices/:id/call/:method` reusing the RPC + blocked-method guard), not selected by the owner this round. Strongest + grounding of the round: plan 001 defers the `number` HA entity precisely + because no such command exists, and `HaEntityType` already ships + `"number"`. Revisit when plan 001's number entity is wanted. +- (012-013 run) **New board support: ESP32-S3 / Pico 2 (D10)** - offered + (L per family, roadmap-stated), not selected. Best sequenced after the + firmware churn from plans 006/007/008/010 settles. +- (012-013 run) **CLI rollback/versions command** - rejected: the server API + (`deployVersion`) and a full dashboard rollback UI (DeviceDetailsPage + Versions tab) already exist; the CLI gap is a minor DX nicety. +- (012-013 run) **Self-hosting docs pass** (roadmap item) - rejected as + already delivered: FAQ, architecture, and rate-limits docs read fully + self-hosted after the OSS-readiness PRs (#198/#203); no hosted-era setup + content found. +- (012-013 run) **Example automation gallery** (roadmap item) - rejected as a + plan: `docs/public/recipes/` already holds 10 recipes; growing it is + incremental content work, not plan-shaped. +- (012-013 run) **Pico `i2c_batch_write` parity gap** - recorded, not + direction: the command is in the published union + (`packages/core/src/commands.ts:86`) and implemented on ESP32, but the Pico + stubs it with "Batch write not yet implemented" + (`firmware/pico/src/multicore/core1_worker.cpp:663`). Bug-fix candidate for + a standard audit; note plan 004's I2C drivers could hit it on Pico.