06-sync-coordinator-desktop.md

16.5 KB · raw

rendered · source

1# Change 6 — Include `"desktop"` peers in sync sessions23> **DRAFT.** Conforms to `_GROUND-TRUTH.md`. Target file (apply-ready patch):4> `/Users/june/projects/peach-browser/src/background/sync-trigger.ts`.5> Nothing here is applied — it is an apply-ready draft.67## VERDICT: NOT a pure no-op. One small, additive host-filter widening is required.89The brief assumed the sync runner/coordinator might branch on device type and10exclude `"desktop"`. The **runner and coordinator are in fact generic** — they11never look at `deviceType` — BUT the **dispatcher's host filter12(`sync-trigger.ts:104-108`) hard-codes `"android" | "ios"` and silently drops a13`"desktop"` peer.** So a desktop-paired device would be persisted correctly and14would sync correctly *if reached*, but `triggerSyncIfReachable` never hands it to15the runner. **Change 6 = add `"desktop"` to that one filter.** Everything else is16verification-only.1718### Deciding citation1920```ts21// src/background/sync-trigger.ts:103-10822const devices = await listPairedDevices();23const hosts = devices.filter(24  (d) =>25    (d.deviceType === "android" || d.deviceType === "ios") &&   // ← desktop excluded26    d.trustSource === "direct",27);28```2930`"desktop"` is not in the disjunction, so a desktop peer is filtered out before31`runSyncRounds` (`sync-trigger.ts:128`) ever iterates. This is the **only**32device-type gate on the sync path.3334---3536## What is already generic (verification-only — no change)3738### 1. The runner iterates whatever hosts it is given, with no type gate3940`runSyncRounds` simply loops the filtered hosts and calls the runner per host41(`sync-trigger.ts:128-144`). It reads only `host.deviceId` and passes the whole42`PairedDevice` to `state.runner(host)`. No `deviceType` reference.4344### 2. `productionSyncRunner` / `runWithDeps` are type-agnostic4546`runWithDeps` (`chirp-sync-runner.ts:80-127`) needs only:47- `device.longTermPublicKeyHex` (`:85,88`) — the desktop helper supplies this in48  its `PAIR_IDENTITY` (`_GROUND-TRUTH.md` §4), persisted by Change 3.49- `device.wsPort ?? DEFAULT_PORT` where `DEFAULT_PORT = 8765`50  (`chirp-sync-runner.ts:40,89`). The helper announces `ws_port: 8765`51  (`peach-sync/src/pairing.zig:179`); Change 3 persists that, defaulting to52  `8765` if absent (`03-desktop-pairing-handler.md` hunk 8). **The runner's53  default and the helper's announced port are the same 8765.**5455There is **no** `deviceType` branch anywhere in `chirp-sync-runner.ts`.5657### 3. The coordinator never inspects device type5859`runChirpSyncInitiator` (`chirp-sync-coordinator.ts:88-244`) is driven entirely60by the `ChirpSyncVaultAdapter` and the wire protocol. It contains zero references61to `deviceType` or to any peer identity beyond the `vaultId`/`deviceId` exchanged62in `SYNC_HELLO`.6364---6566## Exact connection path a desktop peer takes (post-pairing, for SYNC)6768This is **phase 2 / sync** — distinct from pairing. The handshake used here is69**Noise XX over the persistent sync port**, NOT the Noise NK / port-49142 path70used during pairing.71721. `triggerSyncIfReachable` → filter includes the desktop host (after this73   change) → `runSyncRounds` → `state.runner(host)` → `productionSyncRunner`74   → `runWithDeps` (`chirp-sync-runner.ts:73-127`).752. `port = device.wsPort ?? 8765` (`chirp-sync-runner.ts:89`).763. `connect()` (`chirp-sync-runner.ts:129-173`):77   - **fast path:** `getLastKnownIp(longTermPubkeyHex)` → if cached, open a78     session straight to `ws://<cachedIp>:8765` (`:136-145`). Change 3 already79     primes this cache via `setLastKnownIp` on a successful pairing80     (`03-desktop-pairing-handler.md` `cacheLastKnownIp`), so the first sync can81     skip the probe.82   - **fallback:** `probeSubnetForPeer(subnet, 8765, …)` over the /2483     (`:156-162`) — this is the **Noise-XX** probe (`chirp-sync-transport.ts`),84     authenticating each host against `expectedRemoteHex =85     device.longTermPublicKeyHex`. (Note: this is the sync-transport probe, NOT86     the handshake-free pairing probe from Change 5 — different phase, different87     port, different handshake.)88   - on success, `setLastKnownIp(...)` caches the IP (`:171`).894. `openChirpSyncSession(endpoint, ourPriv, ourPub, expectedRemoteHex)` runs90   **Noise XX** and verifies the responder's static key equals91   `expectedRemoteHex` (`chirp-sync-runner.ts:64-66,139-144,165-170`).925. `runChirpSyncInitiator(session, adapter)` drives `SYNC_HELLO → … → SYNC_DONE`.9394### Which handshake for which phase (pinned)9596| Phase | Port | Handshake | Browser role | Citation |97| --- | --- | --- | --- | --- |98| **Pairing** | 49142 | Noise **NK** | initiator | `_GROUND-TRUTH.md` §3; `pairing.zig:153-154` |99| **Sync** | 8765 | Noise **XX** | initiator (WS client) | `chirp-sync-runner.ts:5,64-66`; `chirp-sync-coordinator.ts:88-90` |100101Verified: SYNC uses **Noise XX** (mutual static-key auth, both ends know each102other's long-term key from pairing), NOT NK. The runner header documents "Noise103XX session" (`chirp-sync-runner.ts:3`).104105---106107## `vault_id` flows correctly into SYNC_HELLO validation108109End-to-end chain, verified across both repos:1101111. **Pairing (Change 1):** the browser sends `PAIR_IDENTITY.vault_id =112   computeVaultId(masterKey)` — 64-char lowercase hex113   (`sync-identity.ts:75-82`; `01-vault-id-in-pair-identity.md`).1142. **Helper persists it:** `peach-sync/src/pairing.zig:212` stores the115   browser-sent `vault_id`; `saveVaultId` writes `{ "vaultId": vault_id }` to116   disk (`pairing.zig:241-244`). On the helper's next start, `main.zig:63`117   reads it back and passes it as `opts.vault_id` into the sync coordinator118   (`main.zig:80`).1193. **Sync HELLO compare:** the browser sends120   `SYNC_HELLO { vaultId: await vault.computeVaultId(), … }`121   (`chirp-sync-coordinator.ts:102,107-113`). The helper compares it122   byte-for-byte against its stored value:123   ```zig124   // peach-sync/src/sync_coordinator.zig:141125   if (!std.mem.eql(u8, parsed.value.vaultId, opts.vault_id)) {126       sendError(... "vault_id_mismatch") ...;127   }128   ```129   The helper's `HelloMsg.vaultId` field is **camelCase** (`sync_coordinator.zig:59`),130   matching the browser's `SYNC_HELLO` JSON key `vaultId`131   (`chirp-sync-coordinator.ts:108`).1324. **Browser-side check too:** the browser independently rejects a mismatched133   peer HELLO (`chirp-sync-coordinator.ts:118-122`).134135**Both sides compute the SAME `vaultId` for the same vault** because both call136`computeVaultId(masterKey)` over the identical master key material — the browser137sent the value at pairing, the helper stored exactly those bytes, and the138browser recomputes the identical value at sync time. **They match by139construction.** No transformation, no case folding, no truncation differs140between the pairing value and the SYNC_HELLO value (both are the full 64-char141lowercase-hex BLAKE3 digest).142143> Subtlety worth noting for the integrator: the browser's `SYNC_HELLO.vaultId`144> is `computeVaultId(masterKey)` recomputed at sync time, NOT a value read back145> from the paired-device record. The paired-device record stores the *peer's*146> identity, not our vault id. So the match relies on the same master key being147> present at both pairing time and sync time — which holds for the local vault.148> The desktop helper, conversely, replays the value we GAVE it. The equality is149> therefore "our recomputed id == the id we previously sent the helper" — stable150> as long as `computeVaultId` is deterministic over the root key (it is —151> `sync-identity.ts:75-82`, SEV-2-B uses the stable root key).152153---154155## The patch (apply to `src/background/sync-trigger.ts`)156157### Hunk — widen the host filter to include `"desktop"`158Context: lines 103-108.159```diff160   const devices = await listPairedDevices();161   const hosts = devices.filter(162     (d) =>163-      (d.deviceType === "android" || d.deviceType === "ios") &&164+      (d.deviceType === "android" ||165+        d.deviceType === "ios" ||166+        d.deviceType === "desktop") &&167       d.trustSource === "direct",168   );169```170171That is the entire functional change for Change 6.172173### Why `trustSource === "direct"` still holds for desktop174Change 3 persists the desktop device with `trustSource: "direct"`175(`03-desktop-pairing-handler.md`, the `PairedDevice` built in step 8 sets176`trustSource: "direct"`). So the desktop peer passes the second predicate177unchanged — no further edit needed.178179### Why `"extension"` stays excluded (and SHOULD)180The filter still omits `"extension"`: two browser extensions do not run a sync181*server* (no `wsPort`, never a host). This change adds only `"desktop"`, the182third device type that runs a listening sync server (port 8765), alongside the183two mobile types. This matches the existing intent: the filter selects184*reachable sync hosts*, and desktop is one.185186---187188## Does any EXISTING sync-trigger test break? — NO.189190`src/background/sync-trigger.test.ts` uses three fixtures: `ANDROID`, `IOS`191(`:44-58`) and `EXT` (extension, `:61-67`). **There is no `desktop` fixture.**192The two assertions that pin the filter are:193194- `:75-81` — "does not call the runner when no mobile hosts are paired": pairs195  only `EXT`, asserts the runner is NOT called. Adding `"desktop"` to the filter196  does not change this (EXT is still excluded; no desktop device present).197- `:83-94` — "calls the runner once per mobile host": pairs `ANDROID`, `IOS`,198  `EXT` and asserts `seen.sort()).toEqual([ANDROID.deviceId, IOS.deviceId])`.199  Adding `"desktop"` to the filter does not change this either — EXT remains200  excluded and no desktop device is present, so the seen set is unchanged.201202**Conclusion: the filter widening is additive and breaks no existing test.** The203new behaviour is covered by the new spec below, which adds a `desktop` fixture.204205---206207## Draft test file — `tests/06-sync-coordinator-desktop.test.ts`208209Two specs, matching the repo's `sync-trigger.test.ts` idiom (the injected-runner210seam via `setSyncRunner`, `upsertPairedDevice`, the storage stub). On apply this211relocates to `src/background/sync-trigger.desktop.test.ts` (or is merged into212`sync-trigger.test.ts`); the `../../` import prefixes become `../` then.213214```ts215// Tests for Change 6 — desktop peers participate in sync sessions.216//217// DRAFT / apply-ready. On apply, relocate to218//   src/background/sync-trigger.desktop.test.ts219// and change the "../../background/..." / "../../shared/..." prefixes to220// "./..." / "../shared/...". The fixtures and idiom mirror the existing221// sync-trigger.test.ts (injected runner via setSyncRunner, storage stub).222//223// Spec 1 asserts the host filter (sync-trigger.ts:104) includes "desktop".224// Spec 2 asserts the browser's SYNC_HELLO carries a vaultId equal to the225// desktop helper's stored vault_id (== computeVaultId of the same master key),226// which is what peach-sync compares byte-for-byte (sync_coordinator.zig:141).227228import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";229230import {231  triggerSyncIfReachable,232  setSyncRunner,233  _resetSyncTriggerForTest,234} from "../../background/sync-trigger";235import {236  upsertPairedDevice,237  _resetPairedDevicesForTest,238} from "../../background/paired-devices";239import { computeVaultId } from "../../shared/sync-identity";240import { runChirpSyncInitiator, type ChirpSyncVaultAdapter } from "../../background/chirp-sync-coordinator";241import type { ChirpSyncSession } from "../../shared/chirp-sync-transport";242import type { ChirpSyncMessage } from "../../shared/sync-types";243import type { PairedDevice } from "../../shared/sync-types";244245// NOTE (integrator): sync-trigger.test.ts already exports a makeStorageStub()246// helper inline. Reuse it (or its chrome.storage.local fake) here rather than247// re-declaring; reproduced minimally below so this draft is self-contained.248function makeStorageStub(): void {249  const store: Record<string, unknown> = {};250  (globalThis as unknown as { chrome: unknown }).chrome = {251    storage: {252      local: {253        get: async (keys?: string | string[]) => {254          if (!keys) return { ...store };255          const list = Array.isArray(keys) ? keys : [keys];256          const out: Record<string, unknown> = {};257          for (const k of list) if (k in store) out[k] = store[k];258          return out;259        },260        set: async (obj: Record<string, unknown>) => { Object.assign(store, obj); },261        remove: async (keys: string | string[]) => {262          for (const k of Array.isArray(keys) ? keys : [keys]) delete store[k];263        },264      },265    },266    runtime: { lastError: undefined },267  };268}269270const MASTER = Uint8Array.from(Array.from({ length: 32 }, (_, i) => (i * 7 + 1) & 0xff));271272const DESKTOP: PairedDevice = {273  deviceId: "desktop-001",274  deviceName: "June's iMac",275  deviceType: "desktop",276  longTermPublicKeyHex: "a".repeat(64),277  trustSource: "direct",278  wsPort: 8765,279  pairedAt: 1700000000000,280};281282beforeEach(() => {283  makeStorageStub();284  _resetSyncTriggerForTest();285  _resetPairedDevicesForTest?.();286});287288afterEach(() => {289  vi.restoreAllMocks();290});291292describe("Change 6 — desktop peers in sync", () => {293  it("desktop peer is included in sync session attempts", async () => {294    await upsertPairedDevice(DESKTOP);295    const seen: string[] = [];296    setSyncRunner(async (device) => {297      seen.push(device.deviceId);298      return { ok: true, entriesReceived: 0, entriesSent: 0 };299    });300301    await triggerSyncIfReachable({ reason: "manual" });302303    // Before Change 6, the filter (sync-trigger.ts:104) only matched304    // android|ios, so the runner was never called for a desktop host.305    expect(seen).toEqual([DESKTOP.deviceId]);306  });307308  it("SYNC_HELLO vaultId matches the desktop helper's stored vault_id", async () => {309    // The helper stores exactly the vault_id the browser sent at pairing310    // (peach-sync pairing.zig:212), which is computeVaultId(masterKey). At311    // sync time the browser sends SYNC_HELLO { vaultId: computeVaultId(...) }.312    // peach-sync compares the two byte-for-byte (sync_coordinator.zig:141).313    // Here we capture the browser's outbound SYNC_HELLO and assert its vaultId314    // equals the canonical computeVaultId(MASTER) — i.e. the value the helper315    // holds.316    const ourVaultId = await computeVaultId(MASTER);317318    // Capture frames the coordinator sends; reply just enough to reach the319    // equal-version SYNC_DONE shortcut so the round completes cleanly.320    const sent: ChirpSyncMessage[] = [];321    const inbound: ChirpSyncMessage[] = [322      { type: "SYNC_HELLO", vaultId: ourVaultId, deviceId: "desktop-001", version: 7, deviceTime: Date.now() },323      { type: "SYNC_DONE", entriesReceived: 0, entriesSent: 0 },324    ];325    let rx = 0;326    const session: ChirpSyncSession = {327      send: (bytes: Uint8Array) => { sent.push(JSON.parse(new TextDecoder().decode(bytes))); },328      receive: async () => new TextEncoder().encode(JSON.stringify(inbound[rx++])),329      close: async () => {},330    } as unknown as ChirpSyncSession;331332    const adapter: ChirpSyncVaultAdapter = {333      computeVaultId: async () => ourVaultId,334      getOurDeviceId: async () => "ext-local",335      getLocalVersion: async () => 7, // equal version → SYNC_DONE shortcut336      buildEntryManifest: async () => ({}),337      buildTombstoneManifest: async () => ({}),338      getEncryptedEntries: async () => ({}),339      applyEntries: async () => {},340      applyTombstones: async () => {},341    };342343    const outcome = await runChirpSyncInitiator(session, adapter);344345    const hello = sent.find((m) => m.type === "SYNC_HELLO") as346      | { type: "SYNC_HELLO"; vaultId: string }347      | undefined;348    expect(hello).toBeTruthy();349    // This is the exact value peach-sync compares against opts.vault_id.350    expect(hello!.vaultId).toBe(ourVaultId);351    expect(hello!.vaultId).toMatch(/^[0-9a-f]{64}$/);352    expect(outcome.status).toBe("completed");353  });354});355```356357> **INTEGRATOR TODO (test wiring):**358> 1. `paired-devices.ts` may not export `_resetPairedDevicesForTest`; the359>    existing `sync-trigger.test.ts` resets via `makeStorageStub()` re-init each360>    `beforeEach`. Drop the optional reset call if the symbol does not exist.361> 2. Reuse the real `makeStorageStub` from `sync-trigger.test.ts` instead of the362>    inline copy when merging into that file.363> 3. `ChirpSyncSession` is a class/interface from `chirp-sync-transport.ts`; the364>    cast above stubs only the three methods the coordinator uses365>    (`send`/`receive`/`close`). Confirm those are the only members touched366>    (verified against `chirp-sync-coordinator.ts:269-289`).367368---369370## Open integration TODOs (Change 6)3713721. The one-line filter widening above — the only functional change.3732. Confirm `paired-devices` test-reset export name when wiring the spec.3743. (Optional) a CONTINUITY note that desktop now participates in the same375   30s-timeout sync round (`chirp-sync-coordinator.ts:25`) as mobile — no376   per-type timeout tuning is introduced.