diff --git a/CLAUDE.md b/CLAUDE.md index d436900..9096a80 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -53,9 +53,9 @@ A lightweight Electron app that connects to a remote Chromium-based browser via - **Pins (live-tab holders)**: A pin holds a remote tab (`targetId`), hidden from the Tabs list while linked. Click activates the linked tab or opens+links a fresh one; cmd/middle-click opens an unlinked throwaway tab. Created from a live tab only (toolbar star, right-click tab → Pin, or drag a tab into the Pinned section). A linked pin mirrors its tab's live title/favicon (restoring the saved title when the tab closes); the active pin shows an Arc-style URL-drift cue (a `/` separator and a favicon "Back to Pinned URL" button) when its tab navigates off the saved URL. Closing a pin's tab reverts it to unlinked; un-pinning (confirm dialog) returns the tab to the Tabs list. Cmd+1..9 indexes all pins then visible tabs; Ctrl+Tab cycles open pins + tabs. Link resolution is pure (`src/lib/pins.ts`); persistence/effects live in main + `app.tsx`. See `docs/adr/0004-pin-live-tab-model.md`. - **Unread badges by group**: Sidebar unread counts are computed by `aggregateUnread` (`src/lib/unread-aggregator.ts`) and keyed by `groupKey` (from the notification entry) falling back to `groupKeyForUrl(url)` — Slack's per-workspace `slack:{teamId}`, else URL origin. Every tab/pin of the same app shares one count whether or not it captured the notification, and a dormant pin still badges by resolving its saved URL through the same key derivation. - **Local tabs**: Real local web pages rendered as in-DOM Electron ``s on a shared `persist:local` session (`src/components/local-webviews.tsx`) — full device access (OS notifications, speaker/mic, camera, screen-share) that CDP screencast tabs can't have. Because a `` is an in-page OOPIF, React overlays (dialogs, menus, tooltips, the settings sheet) stack **above the live page via CSS z-index** — no native z-order, no freeze. `activeKind: 'cdp' | 'local'` chooses the surface and routes the toolbar/nav hotkeys (`RemotePage` vs the active webview's methods). The renderer holds `LocalTab` metadata and maps webview DOM events to it; only the active webview is shown (others `display:none`, kept alive in the background). All open local tabs persist + restore on launch; pinned ones (a `pinned` flag, distinct from CDP PINNED pins) sort atop the LOCAL TABS section. Unpacked MV3 extensions load into the local session only (`localExtensionPaths`) and their content scripts inject into webview guests; the toolbar shows a Chrome-like action icon per extension (opens its popup in a popover), and popup/options also open as a local tab via the `chrome-extension://` URL. Permissions auto-granted behind the `autoGrantLocalMedia` setting (a `media` request triggers `askForMediaAccess`); packaging ships mic/cam/audio-capture Info.plist keys + entitlements (`build/entitlements.mac.plist`, hardened runtime). See `docs/adr/0005-local-tabs-base-window.md`. -- **Web build (no Electron)**: The same renderer runs as a plain web app via `web/server.mjs` — a Node HTTP proxy that serves the built `dist/` and exposes the whole `window.cdp` surface over **SSE** (`GET /api/events`, server→browser pushes incl. screencast frames) + **POST** (`/api/invoke`, `/api/send`, `/api/cdp-batch`, and REST for tabs/config/ui-state/pins/notifications). An optional **WebSocket** transport (`/api/ws`) supersedes SSE+POST when reachable — the user picks `Auto / Fastest (WS) / Streaming / Basic` in settings (2×2 toggle, web-only, `localStorage`). When WS is ready, frames + events + input all ride the one full-duplex socket. WS needs three lines in the nginx custom config (`proxy_http_version 1.1`, `proxy_set_header Upgrade $http_upgrade`, `proxy_set_header Connection $http_connection`); without them the client silently falls back to SSE+POST. See `docs/adr/0007-web-websocket-transport.md`. The proxy→CDP hop is still WS. The renderer installs a web `window.cdp` (`src/lib/cdp-web-transport.ts`, a thin assembler) when no preload exists, satisfying the same `CdpBridge` contract; the transport is split into named seams — a **Downlink** (`src/lib/downlink-dispatcher.ts`: one live WS-or-SSE source, decoder→filter→fan-out→toast-once dispatcher) and an **Uplink** (`src/lib/uplink-router.ts`: WS/stream/POST adapters + ready-transport router), with E2E sealed/opened once per direction through `src/lib/crypto-context.ts`. Input is coalesced via `src/lib/input-coalesce.ts`; the proxy acks frames itself, **except** for a WS client that announces ack-after-paint support (a plaintext `frame-ack-mode` control) — for that client the proxy **defers** its remote-ack and gates the next Screencast Frame on the client's post-paint `frame-ack`, so at most one frame is in flight on the link and a slow link can't accrue a stale-frame backlog (`core/frame-ack-gate.js`, the pure one-in-flight gate + a watchdog that frees the slot if a paint-ack never lands; the renderer fires the ack from `viewport.tsx` after it paints, via `window.cdp.ackPaintedFrame`; SSE/non-supporting clients keep the eager self-ack — see `docs/tasks/done/056-*`); theme follows `matchMedia`. **Always-on latency metrics** (`src/lib/latency-metrics.ts`, t057) ride the same seams: the WS uplink fires a plaintext `ping` (monotonic stamp) every 20s — a keepalive against proxy idle-reap plus an RTT/jitter EWMA probe — and the server echoes `{ t: "pong", seq, ts }` (RTT is measured only on the client clock); every Screencast Frame envelope carries a server `serverTs` so the client computes frame age (`now − serverTs + rtt/2`), recorded by the dispatcher before fan-out. Collection runs continuously (no `?perf=1`); the HUD is `src/components/latency-hud.tsx` (t059), always-on in the status bar. RTT/jitter report unavailable on the SSE+POST fallback. A `window.webCaps` flag (read through one accessor — `getCaps()` in `src/lib/caps.ts`, never inline) gates Electron-only surfaces. Local tabs are gated **structurally at the data source**: `useLocalTabs()` (`src/hooks/use-local-tabs.ts`) reads `caps.localTabs` once and returns an empty list + no-op handlers on web, so the renderer can't drive local-tab logic there (`LocalWebviews` never mounts, the new-tab kind toggle is hidden, Cmd+T/Cmd+Shift+T resolve to CDP only). Extensions are still gated at render only. `window.local` is a no-op stub (the safety net, not the mechanism). See `docs/conventions/feature-gates.md`. Pure shared logic lives in `core/` CJS modules — `cdp-endpoints.js` (`/json` URL builders), `settings-store.js` (settings/pins/ui-state), `notifications-sidechain.js` (Notification Side-Channel state machine + store, DI), `remote-page-connector.js` (Remote Page connect choreography, DI), `notifications.js` (dedup/cap/toast gating, Slack workspace key: `parseSlackContext`/`slackGroupKey`), `theme-emulation.js`, `crypto-envelope.js` (AES-256-GCM server side), `line-splitter.js` (NDJSON reassembly), `frame-throttle.js`, `frame-ack-gate.js`, `quality-tier.js`, and `notif-mutes.js` (per-device mute key + per-device unread, web push gate) — consumed by both `main.js` and `web/server.mjs`. Run `pnpm web`. See `docs/adr/0006-web-proxy-sse-transport.md`. The web build is an installable **PWA** (`public/manifest.webmanifest` with `APP_TITLE`-injected name + `public/sw.js`); the manifest is phone-and-iPad friendly (`"orientation": "any"` since t081 — iOS ignores the field, Android honors it; `viewport-fit=cover`; `interactive-widget=resizes-content` in the viewport meta so iOS shrinks the layout viewport when the keyboard opens; full height is driven by `--app-h` (set by `initAppHeight` in `src/lib/app-height.ts` to `visualViewport.height` — `100dvh` is only the pre-JS fallback); on iOS the keyboard also shifts the visual viewport up (`visualViewport.offsetTop`), so `app-height.ts` publishes `--vv-top` and toggles `html.kb-open` so `#root` translates to follow the visual viewport and bottom-anchored composers collapse the home-indicator inset the keyboard covers; `font-size: max(16px,1em)` on inputs prevents iOS auto-zoom on focus; safe-area insets are applied per-component — sidebar scroll content uses `pb-[max(0.5rem,env(safe-area-inset-bottom))]`, status bar uses `pb-[env(safe-area-inset-bottom)]`; sidebar defaults to 180px on viewports ≤1100px; an install nudge banner (`install-banner.tsx`) prompts Safari-tab visits to Add to Home Screen). Has a web-only **push-notification** toggle (`webPush` ui-state) that drives real **Web Push** on installed PWAs (iOS 16.4+) — VAPID-signed payloads from the server (`web-push` library) reach a service-worker `push` handler that fires `showNotification` even when the PWA is backgrounded or the screen is locked; clicks post-message back to the page and route through the same `notificationActivate` listeners as in-app clicks — on the Phone Shell that listener deep-routes into the **Conversation Reader** (t080): warm taps carry the payload entry (store entry wins via `resolvePushEntry`), cold taps (no window) ride a one-shot `?notif=` URL the SW sets on `openWindow`, consumed by `src/lib/push-route.ts` helpers once the store loads (gone entry → Inbox). The push payload also stamps the conversation identity (`channelId`/`slackKind`/`slackTs`/`slackThreadTs`) and an `unread` count, which the SW mirrors to the home-screen icon via `setAppBadge` (the page keeps it live as entries are read). **Per-device delivery (t093):** capture is global but *delivery* is per-device — each push subscription carries a `deviceId`, and `sendPushToAll` reads that device's master + mutes from ui-state (`notificationsEnabled_` + `notifMutes_`, written via the same device-keyed remap seam as `webPush_`; `core/settings-store.js` round-trips device-suffixed keys by prefix so they survive a PWA refresh). For each sub it **skips** the push when that device's master is off or it muted the entry's `muteKey` (`core/notif-mutes.js`: slack→`groupKey`, else `adapter`), and otherwise stamps a **per-device `unread`** (`unreadExcluding`, excluding that device's muted sources) so the badge stays honest per device. Defaults are opt-out (no stored master = on, no stored mutes = nothing muted; a sub with no `deviceId` keeps receiving). The global `notificationsEnabled` stays Electron-only (gates `shouldNotifyOs`). Foreground tabs still get the in-page `Notification` API as before. Subscriptions persist in `web-push-subs.json` next to the settings file. **Push delivery hardening (t095, ADR-0014):** the server is the authoritative source of `deviceId`, reconciled by push endpoint (`core/push-subscriptions.js`:`reconcileDeviceId`) — after a storage wipe the same endpoint recovers its prior `deviceId` and per-device prefs; the SW push handler (`src/lib/push-notification.ts`:`buildNotificationContent`) always calls `showNotification` (real payload or generic fallback) to avoid WebKit **userVisibleOnly** revocation; the server fans out with `urgency:"high"`, `TTL:1800` (`core/push-send-options.js`) for timely, non-stale delivery; on app foreground the client re-validates the subscription once (`src/lib/push-revalidate.ts`:`createPushRevalidateGate`) to recover a revoked sub before the next push arrives. See `CONTEXT.md` for **Web Push Subscription** and **userVisibleOnly revocation** glossary entries. The toggle is disabled in Safari-tab mode (Web Push needs standalone display), and lowers input latency with a **streaming input channel** — one long-lived `POST /api/input-stream` (fetch `ReadableStream` body over HTTP/2, NDJSON frames reassembled by `core/line-splitter.js`) that a probe/`stream-ack` confirms before use and that falls back to `/api/cdp-batch` if a proxy buffers it. Streaming needs `proxy_request_buffering off` upstream to activate; when it can't (the default behind nginx/Authentik), mouse input is **event-driven** so it doesn't flood the fallback: a **hover gate** (`createHoverGate`) holds buttons-up moves and emits one resting position only when the cursor stops (drag moves bypass it and track live; clicks carry their own coords), and the `/api/cdp-batch` fallback is **single-flight with move-collapsing** (`createSingleFlight` — one POST in flight, consecutive `mouseMoved` collapse to the latest) so the rate auto-adapts to link RTT instead of backing up fire-and-forget POSTs and starving clicks. See `docs/tasks/done/013-*`. An optional **E2E mode** (set `E2E_PASSPHRASE` on the server) seals every `/api` body + SSE frame in AES-256-GCM (`core/crypto-envelope.js` server / `src/lib/crypto-envelope.ts` browser; the single owner is `src/lib/crypto-context.ts` — the uplink seals once before leaving, the downlink opens once on arrival) so content stays opaque to a TLS-intercepting proxy (Zscaler); a verifier handshake rejects a wrong passphrase, and with E2E off everything is plaintext as before. It defeats network content inspection, not endpoint screen capture. See `docs/tasks/done/012-*`. +- **Web build (no Electron)**: The same renderer runs as a plain web app via `web/server.mjs` — a Node HTTP proxy that serves the built `dist/` and exposes the whole `window.cdp` surface over **SSE** (`GET /api/events`, server→browser pushes incl. screencast frames) + **POST** (`/api/invoke`, `/api/send`, `/api/cdp-batch`, and REST for tabs/config/ui-state/pins/notifications). An optional **WebSocket** transport (`/api/ws`) supersedes SSE+POST when reachable — the user picks `Auto / Fastest (WS) / Streaming / Basic` in settings (2×2 toggle, web-only, `localStorage`). When WS is ready, frames + events + input all ride the one full-duplex socket. WS needs three lines in the nginx custom config (`proxy_http_version 1.1`, `proxy_set_header Upgrade $http_upgrade`, `proxy_set_header Connection $http_connection`); without them the client silently falls back to SSE+POST. See `docs/adr/0007-web-websocket-transport.md`. The proxy→CDP hop is still WS. The renderer installs a web `window.cdp` (`src/lib/cdp-web-transport.ts`, a thin assembler) when no preload exists, satisfying the same `CdpBridge` contract; the transport is split into named seams — a **Downlink** (`src/lib/downlink-dispatcher.ts`: one live WS-or-SSE source, decoder→filter→fan-out→toast-once dispatcher) and an **Uplink** (`src/lib/uplink-router.ts`: WS/stream/POST adapters + ready-transport router), with E2E sealed/opened once per direction through `src/lib/crypto-context.ts`. Input is coalesced via `src/lib/input-coalesce.ts`; the proxy acks frames itself, **except** for a WS client that announces ack-after-paint support (a plaintext `frame-ack-mode` control) — for that client the proxy **defers** its remote-ack and gates the next Screencast Frame on the client's post-paint `frame-ack`, so at most one frame is in flight on the link and a slow link can't accrue a stale-frame backlog (`core/frame-ack-gate.js`, the pure one-in-flight gate + a watchdog that frees the slot if a paint-ack never lands — its window is adaptive to measured paint latency via `core/paint-ack-pacer.js`, t096; the renderer fires the ack from `viewport.tsx` after it paints, via `window.cdp.ackPaintedFrame`; SSE/non-supporting clients keep the eager self-ack — see `docs/tasks/done/056-*`); theme follows `matchMedia`. **Always-on latency metrics** (`src/lib/latency-metrics.ts`, t057) ride the same seams: the WS uplink fires a plaintext `ping` (monotonic stamp) every 20s — a keepalive against proxy idle-reap plus an RTT/jitter EWMA probe — and the server echoes `{ t: "pong", seq, ts }` (RTT is measured only on the client clock); every Screencast Frame envelope carries a server `serverTs` so the client computes frame age (`now − serverTs + rtt/2`), recorded by the dispatcher before fan-out. Collection runs continuously (no `?perf=1`); the HUD is `src/components/latency-hud.tsx` (t059), always-on in the status bar. RTT/jitter report unavailable on the SSE+POST fallback. A `window.webCaps` flag (read through one accessor — `getCaps()` in `src/lib/caps.ts`, never inline) gates Electron-only surfaces. Local tabs are gated **structurally at the data source**: `useLocalTabs()` (`src/hooks/use-local-tabs.ts`) reads `caps.localTabs` once and returns an empty list + no-op handlers on web, so the renderer can't drive local-tab logic there (`LocalWebviews` never mounts, the new-tab kind toggle is hidden, Cmd+T/Cmd+Shift+T resolve to CDP only). Extensions are still gated at render only. `window.local` is a no-op stub (the safety net, not the mechanism). See `docs/conventions/feature-gates.md`. Pure shared logic lives in `core/` CJS modules — `cdp-endpoints.js` (`/json` URL builders), `settings-store.js` (settings/pins/ui-state), `notifications-sidechain.js` (Notification Side-Channel state machine + store, DI), `remote-page-connector.js` (Remote Page connect choreography, DI), `notifications.js` (dedup/cap/toast gating, Slack workspace key: `parseSlackContext`/`slackGroupKey`), `theme-emulation.js`, `crypto-envelope.js` (AES-256-GCM server side), `line-splitter.js` (NDJSON reassembly), `frame-throttle.js`, `frame-ack-gate.js`, `quality-tier.js`, and `notif-mutes.js` (per-device mute key + per-device unread, web push gate) — consumed by both `main.js` and `web/server.mjs`. Run `pnpm web`. See `docs/adr/0006-web-proxy-sse-transport.md`. The web build is an installable **PWA** (`public/manifest.webmanifest` with `APP_TITLE`-injected name + `public/sw.js`); the manifest is phone-and-iPad friendly (`"orientation": "any"` since t081 — iOS ignores the field, Android honors it; `viewport-fit=cover`; `interactive-widget=resizes-content` in the viewport meta so iOS shrinks the layout viewport when the keyboard opens; full height is driven by `--app-h` (set by `initAppHeight` in `src/lib/app-height.ts` to `visualViewport.height` — `100dvh` is only the pre-JS fallback); on iOS the keyboard also shifts the visual viewport up (`visualViewport.offsetTop`), so `app-height.ts` publishes `--vv-top` and toggles `html.kb-open` so `#root` translates to follow the visual viewport and bottom-anchored composers collapse the home-indicator inset the keyboard covers; `font-size: max(16px,1em)` on inputs prevents iOS auto-zoom on focus; safe-area insets are applied per-component — sidebar scroll content uses `pb-[max(0.5rem,env(safe-area-inset-bottom))]`, status bar uses `pb-[env(safe-area-inset-bottom)]`; sidebar defaults to 180px on viewports ≤1100px; an install nudge banner (`install-banner.tsx`) prompts Safari-tab visits to Add to Home Screen). Has a web-only **push-notification** toggle (`webPush` ui-state) that drives real **Web Push** on installed PWAs (iOS 16.4+) — VAPID-signed payloads from the server (`web-push` library) reach a service-worker `push` handler that fires `showNotification` even when the PWA is backgrounded or the screen is locked; clicks post-message back to the page and route through the same `notificationActivate` listeners as in-app clicks — on the Phone Shell that listener deep-routes into the **Conversation Reader** (t080): warm taps carry the payload entry (store entry wins via `resolvePushEntry`), cold taps (no window) ride a one-shot `?notif=` URL the SW sets on `openWindow`, consumed by `src/lib/push-route.ts` helpers once the store loads (gone entry → Inbox). The push payload also stamps the conversation identity (`channelId`/`slackKind`/`slackTs`/`slackThreadTs`) and an `unread` count, which the SW mirrors to the home-screen icon via `setAppBadge` (the page keeps it live as entries are read). **Per-device delivery (t093):** capture is global but *delivery* is per-device — each push subscription carries a `deviceId`, and `sendPushToAll` reads that device's master + mutes from ui-state (`notificationsEnabled_` + `notifMutes_`, written via the same device-keyed remap seam as `webPush_`; `core/settings-store.js` round-trips device-suffixed keys by prefix so they survive a PWA refresh). For each sub it **skips** the push when that device's master is off or it muted the entry's `muteKey` (`core/notif-mutes.js`: slack→`groupKey`, else `adapter`), and otherwise stamps a **per-device `unread`** (`unreadExcluding`, excluding that device's muted sources) so the badge stays honest per device. Defaults are opt-out (no stored master = on, no stored mutes = nothing muted; a sub with no `deviceId` keeps receiving). The global `notificationsEnabled` stays Electron-only (gates `shouldNotifyOs`). Foreground tabs still get the in-page `Notification` API as before. Subscriptions persist in `web-push-subs.json` next to the settings file. **Push delivery hardening (t095, ADR-0014):** the server is the authoritative source of `deviceId`, reconciled by push endpoint (`core/push-subscriptions.js`:`reconcileDeviceId`) — after a storage wipe the same endpoint recovers its prior `deviceId` and per-device prefs; the SW push handler (`src/lib/push-notification.ts`:`buildNotificationContent`) always calls `showNotification` (real payload or generic fallback) to avoid WebKit **userVisibleOnly** revocation; the server fans out with `urgency:"high"`, `TTL:1800` (`core/push-send-options.js`) for timely, non-stale delivery; on app foreground the client re-validates the subscription once (`src/lib/push-revalidate.ts`:`createPushRevalidateGate`) to recover a revoked sub before the next push arrives. See `CONTEXT.md` for **Web Push Subscription** and **userVisibleOnly revocation** glossary entries. The toggle is disabled in Safari-tab mode (Web Push needs standalone display), and lowers input latency with a **streaming input channel** — one long-lived `POST /api/input-stream` (fetch `ReadableStream` body over HTTP/2, NDJSON frames reassembled by `core/line-splitter.js`) that a probe/`stream-ack` confirms before use and that falls back to `/api/cdp-batch` if a proxy buffers it. Streaming needs `proxy_request_buffering off` upstream to activate; when it can't (the default behind nginx/Authentik), mouse input is **event-driven** so it doesn't flood the fallback: a **hover gate** (`createHoverGate`) holds buttons-up moves and emits one resting position only when the cursor stops (drag moves bypass it and track live; clicks carry their own coords), and the `/api/cdp-batch` fallback is **single-flight with move-collapsing** (`createSingleFlight` — one POST in flight, consecutive `mouseMoved` collapse to the latest) so the rate auto-adapts to link RTT instead of backing up fire-and-forget POSTs and starving clicks. See `docs/tasks/done/013-*`. An optional **E2E mode** (set `E2E_PASSPHRASE` on the server) seals every `/api` body + SSE frame in AES-256-GCM (`core/crypto-envelope.js` server / `src/lib/crypto-envelope.ts` browser; the single owner is `src/lib/crypto-context.ts` — the uplink seals once before leaving, the downlink opens once on arrival) so content stays opaque to a TLS-intercepting proxy (Zscaler); a verifier handshake rejects a wrong passphrase, and with E2E off everything is plaintext as before. It defeats network content inspection, not endpoint screen capture. See `docs/tasks/done/012-*`. - **Clipboard paste (t065)**: Two gesture-driven one-way bridges — no ambient background sync (focus/permission wall + privacy). **Local→remote text**: ⌘/Ctrl+V reads the local clipboard (`window.cdp.readClipboard()` via Electron IPC / `navigator.clipboard` on web) and calls `RemotePage.paste(text)` → `Input.insertText` (plain) or pre-seed + forwarded ⌘V (rich). **Local→remote image**: `window.cdp.readClipboardImage()` (Electron IPC, reads `clipboard.readImage()`) or the native browser `paste` event (web — Safari/iPad blocks `navigator.clipboard.readText`/images; instead ⌘V is not `preventDefault`ed so the browser fires a `paste` ClipboardEvent on the document); either path calls `RemotePage.pasteImage(dataUrl)` → `Runtime.evaluate` synthesizes a paste `ClipboardEvent` with a `DataTransfer` carrying the image as a `File`. **Typing surface guard**: bare `?` (and other bare-char shortcuts) forward to the remote page when `activeKind` is `cdp` or `local` (`isTypingSurface` in `src/lib/typing-surface.ts`); the shortcut overlay opens via `⌘/` instead. `core/clipboard.js` owns the pure `Browser.grantPermissions` enum-fallback helpers and `selectPasteRoute`. -- **Notifications side-channel**: A per-target read-only CDP socket (no screencast, no input) stays attached to background tabs that match a Notification Adapter (Teams, Outlook, Slack). Lifecycle and state machine live in `core/notifications-sidechain.js` (`createNotificationCenter`, DI) — consumed by both `main.js` and `web/server.mjs`; the server runs it headless. A capture script (per adapter, in `inject/`) is injected at document-start and ships toasts through a `__cdpNotify` binding. Pure dedup/cap/read-model helpers remain in `core/notifications.js`. Each adapter carries a `name`, hostname `match` regex, capture `script`, `iconUrl`, optional `activate` tagged union (`spa-link` | `thread`) for deep-opens, and an optional `groupKey(url)` hook (URL-derived per-workspace bucketing) — adding an adapter is one config entry in `ADAPTERS`. Capture style varies by site: Teams/Outlook use a `MutationObserver` on the site's own in-app toast DOM; Slack uses a two-modality design (ADR-0011): the **Slack Content Sweep** (web build only) is the authoritative capture path — the server polls Slack's web API using extracted `xoxc`/`d`-cookie creds from a live Slack tab, synthesizes entries keyed `slack:{groupId}:{channel}:{ts}` (`groupId = enterprise_id || teamId`, t092 — an Enterprise Grid registers the org as a pseudo-team alongside its member workspaces and both surface the same shared channels, so keying by the Grid group collapses the org+workspace duplicate via the existing id-dedup; the concrete `teamId` is kept on the entry for the deep-link), and is the sole Slack store writer. The in-page hijack script (`inject/slack-notify.js`) is demoted to a **"sweep now" trigger**: a fired notification immediately schedules a sweep of that workspace so delivery is sub-second without the hijack writing to the store (no cross-path dedup needed). A 15s periodic sweep is the completeness backstop; a per-workspace **parked tab** keeps one Slack tab alive on the remote browser so creds self-refresh. A workspace is persisted in `slack-workspaces.json` (non-secret metadata only — no creds on disk) when first seen as its own tab. Clicking a notification activates the tab, then the renderer's activation registry (`src/lib/notification-activation.ts`) maps the `activate` intent to a Remote Page intention (`navigateSpa` for Outlook + Slack channel deep-links, `openTeamsThread` for Teams chats). Teams has no conversation URL (the URL stays bare `/v2/`), so thread-id clicks drive `openTeamsThread`; Slack reuses `spa-link` to `/client/{team}/{channel}` (best-effort — degrades to tab-only when the notification carries no channel id). See `docs/adr/0003-notifications-side-channel.md` and `docs/adr/0011-slack-content-sweep-guaranteed-delivery.md`. +- **Notifications side-channel**: A per-target read-only CDP socket (no screencast, no input) stays attached to background tabs that match a Notification Adapter (Teams, Outlook, Slack). Lifecycle and state machine live in `core/notifications-sidechain.js` (`createNotificationCenter`, DI) — consumed by both `main.js` and `web/server.mjs`; the server runs it headless. A capture script (per adapter, in `inject/`) is injected at document-start and ships toasts through a `__cdpNotify` binding. Pure dedup/cap/read-model helpers remain in `core/notifications.js`. Each adapter carries a `name`, hostname `match` regex, capture `script`, `iconUrl`, optional `activate` tagged union (`spa-link` | `thread`) for deep-opens, and an optional `groupKey(url)` hook (URL-derived per-workspace bucketing) — adding an adapter is one config entry in `ADAPTERS`. Capture style varies by site: Teams/Outlook use a `MutationObserver` on the site's own in-app toast DOM; Slack uses a two-modality design (ADR-0011): the **Slack Content Sweep** (web build only) is the authoritative capture path — the server polls Slack's web API using extracted `xoxc`/`d`-cookie creds from a live Slack tab, synthesizes entries keyed `slack:{groupId}:{channel}:{ts}` (`groupId = enterprise_id || teamId`, t092 — an Enterprise Grid registers the org as a pseudo-team alongside its member workspaces and both surface the same shared channels, so keying by the Grid group collapses the org+workspace duplicate via the existing id-dedup; the concrete `teamId` is kept on the entry for the deep-link), and is the sole Slack store writer. The in-page hijack script (`inject/slack-notify.js`) is demoted to a **"sweep now" trigger**: a fired notification immediately schedules a sweep of that workspace so delivery is sub-second without the hijack writing to the store (no cross-path dedup needed). A 15s periodic sweep is the completeness backstop; a **parked tab** keeps one Slack tab alive on the remote browser so creds self-refresh — but the keeper **defers to a pin** (t098): a pinned workspace is owned by its pin and is never reopened as a stray duplicate when its tab closes, and because one live Slack tab refreshes *every* workspace's creds (shared `d` cookie + `localConfig_v2` carries all teams' tokens) the sweep is unaffected; a single cred lifeline opens one tab (preferring a pinned URL) only when no Slack tab is live and no unpinned workspace would open one. A workspace is persisted in `slack-workspaces.json` (non-secret metadata only — no creds on disk) when first seen as its own tab. Clicking a notification activates the tab, then the renderer's activation registry (`src/lib/notification-activation.ts`) maps the `activate` intent to a Remote Page intention (`navigateSpa` for Outlook + Slack channel deep-links, `openTeamsThread` for Teams chats). Teams has no conversation URL (the URL stays bare `/v2/`), so thread-id clicks drive `openTeamsThread`; Slack reuses `spa-link` to `/client/{team}/{channel}` (best-effort — degrades to tab-only when the notification carries no channel id). See `docs/adr/0003-notifications-side-channel.md` and `docs/adr/0011-slack-content-sweep-guaranteed-delivery.md`. ## File Structure @@ -65,7 +65,7 @@ cdp-browser/ ├── preload.js # IPC bridge (contextBridge) ├── core/ # Backend-agnostic shared CommonJS modules (consumed by main.js + web/server.mjs) │ ├── notifications.js # Pure notification logic (dedup, cap, OS-toast gating, Slack workspace key: parseSlackContext/slackGroupKey) -│ ├── notifications-sidechain.js # Notification Side-Channel state machine (createNotificationCenter, DI); Slack-only cred extraction (xoxc+d-cookie, carries enterpriseId for Grid grouping t092) via onCreds dep + listCreds/getCreds/markCredsStale/setSelfUserId accessors (t069) +│ ├── notifications-sidechain.js # Notification Side-Channel state machine (createNotificationCenter, DI); Slack-only cred extraction (xoxc+d-cookie, carries enterpriseId for Grid grouping t092) via onCreds dep + listCreds/getCreds/markCredsStale/setSelfUserId accessors (t069). Side-channel cdpCall has a timeout + rejects pending on close, and reconcile reaps a hung non-OPEN socket on a still-live target (t096) │ ├── remote-page-connector.js # Remote Page connect choreography (createRemotePageConnector, DI) + screencast rate ceiling (SCREENCAST_TARGET_FPS/EVERY_NTH_FRAME) │ ├── theme-emulation.js # Pure theme-sync logic (emulatedMediaParams) │ ├── cdp-endpoints.js # Pure /json URL builders @@ -73,12 +73,14 @@ cdp-browser/ │ ├── line-splitter.js # Pure NDJSON frame reassembly for the streaming input channel │ ├── frame-throttle.js # Pure screencast rate throttle (createFrameThrottle, DI clock; fresh-frame-wins) │ ├── frame-ack-gate.js # Pure one-in-flight gate + watchdog for WS paint-ack backpressure (t056) +│ ├── paint-ack-pacer.js # Pure adaptive paint-ack watchdog window: EWMA of observed paint latency sizes the stranded-paint timeout (floor 1s, cap 5s) so a slow device isn't tripped early (t096, P2) │ ├── quality-tier.js # Pure Sharp/Balanced/Snappy screencast preset owner (tierToParams/DEFAULT_TIER/parseTier) │ ├── clipboard.js # Pure clipboard helpers: grantPermissions enum-fallback builders + selectPasteRoute (t065) │ ├── slack-api.js # Cred-injected Slack web API client (clientCounts/conversationsHistory/usersInfo/usersPrefsGet/chatPostMessage); xoxc+d-cookie auth, 429-aware, 401→typed (t067/t078, ADR-0011) │ ├── slack-sweep.js # Pure sweep reducer: planFetches/reduceMessages/applyReadUpdates/isMention/tsCmp; counts+history→entries keyed slack:{groupId}:{channel}:{ts} (groupId merges a Grid org+workspaces, t092), parity+mute+exclude+watermark (t068, ADR-0011) │ ├── slack-creds.js # Pure cred helpers: parseLocalConfig (xoxc tokens, incl. enterpriseId)/pickDCookie/groupId (enterpriseId||teamId, t092)/buildSlackGroups (teamId→groupId map)/markFresh/markStale/redact; effectful extraction lives in notifications-sidechain.js (t069, ADR-0011) -│ ├── slack-workspaces.js # Pure workspace registry + parked-tab planner: upsertWorkspace (persists enterpriseId, t092)/liveTeamIds/planParkedTabs; keeper recreates closed Slack tabs (t070, ADR-0011). No creds on disk — re-extracted from any live tab +│ ├── slack-workspaces.js # Pure workspace registry + parked-tab planner: upsertWorkspace (persists enterpriseId, t092)/liveTeamIds/planParkedTabs; keeper recreates closed Slack tabs (t070, ADR-0011) but DEFERS to a pin — a pinned workspace is owned by its pin, never reopened as a stray; a single cred lifeline keeps one tab alive (preferring a pinned URL) when none is, since one live tab refreshes every workspace's creds (t098). No creds on disk — re-extracted from any live tab +│ ├── sweep-scheduler.js # Pure per-key debounced sweep trigger (createSweepScheduler, DI timers): coalesces onCreds+onSlackSignal same-workspace double-fire into one leading + one trailing sweep (t096). 15s runOnce backstop bypasses it │ ├── slack-sweep-runner.js # Effectful sweep orchestrator (createSlackSweeper, DI): per-workspace clientCounts→plan→history→reduce→ingest+read-sync; first-sweep seeding (no cold-start spam); sole authoritative Slack writer (t071). users.counts fallback for team_is_restricted Grid children — seed-to-now + unread-set read-sync (t075). Threads groupId into the entry id/groupKey while keeping the concrete teamId for deep-link (t092). ADR-0011 │ ├── slack-render.js # Pure content renderer: renderBody (mentions/channel-refs/links resolved, mrkdwn stripped, entities decoded) + composeTitle ("{sender} in #{channel}", DM: sender) (t073) + toReaderMessages (reader history shaping, t077). ADR-0011/0012 │ ├── notification-health.js # Pure capture-health aggregator: buildHealth (ONE row per Grid group keyed groupId — merged status/label/teamIds, t092; healthy/degraded/unsupported) + shouldAlert (one-time degrade gate); /api/notifications/health (t074, ADR-0011) @@ -143,7 +145,10 @@ cdp-browser/ │ ├── find-bar.ts # Pure in-page find reducer: open/close/setQuery/setTotal/next-prev(wrap) + counterLabel (t001) │ ├── push-notification.ts # Pure E1: revocation-proof SW push handler (t095). buildNotificationContent(data) → {title, options}; always renders (fallback on parse-fail) │ ├── push-revalidate.ts # Pure E1b: once-per-foreground subscription re-validation gate (t095, ADR-0014). createPushRevalidateGate() → {shouldRevalidateNow(visible)} - │ ├── cdp-web-transport.ts # Web build: thin assembler wiring Downlink + Uplink + REST bridge into window.cdp + │ ├── cdp-web-transport.ts # Web build: thin assembler wiring Downlink + Uplink + REST bridge into window.cdp (3 DI'd factories extracted to web-*.ts, t096) + │ ├── web-ws-channel.ts # Web build: createWsChannel — the optional full-duplex WS channel (extracted from cdp-web-transport, isolated tests; t096/A5) + │ ├── web-input-channel.ts # Web build: createInputChannel — the streaming NDJSON input POST + probe/fallback (extracted; t096/A5) + │ ├── web-reconnect-driver.ts # Web build: createReconnectDriver — the effectful auto-reconnect backoff loop (extracted; t096/A5) │ ├── downlink-dispatcher.ts # Web build: Downlink seam (one WS/SSE source) + Dispatcher (decode→fan-out→toast-once) │ ├── uplink-router.ts # Web build: Uplink seam (WS/stream/POST adapters) + ready-transport router │ ├── crypto-context.ts # Web build: single E2E owner — sealText/openText, handshake gate, mode flag diff --git a/CONTEXT.md b/CONTEXT.md index 5d0941d..aa18679 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -61,7 +61,7 @@ The server-side, authoritative Slack capture modality (ADR-0011). The web server _Avoid_: scraper, poller, backfill. **Workspace Registry**: -The server-side persistence (`slack-workspaces.json`) mapping each Slack `teamId` → `{ url, name, enterpriseId, lastSeen }`, populated the first time a workspace tab is seen live (ADR-0011). Persisting `enterpriseId` lets a cold start resolve each workspace's **Grid Group** without live creds. It drives two things: re-extraction targets for stale creds, and the **Parked Tab** keep-alive loop, which ensures exactly one tab per registered workspace exists on the remote browser (recreated via `/json/new` if closed or after a browser restart) so creds self-refresh and the hijack stays armed. Distinct from ADR-0010 **Workspaces** (multi-CDP-host UI), though both stamp entries with a workspace key. +The server-side persistence (`slack-workspaces.json`) mapping each Slack `teamId` → `{ url, name, enterpriseId, lastSeen }`, populated the first time a workspace tab is seen live (ADR-0011). Persisting `enterpriseId` lets a cold start resolve each workspace's **Grid Group** without live creds. It drives two things: re-extraction targets for stale creds, and the **Parked Tab** keep-alive loop. The keeper ensures at least one Slack tab is live so creds self-refresh and the hijack stays armed — but it **defers to a Pin** (t098): a registered workspace whose URL is pinned is owned by its Pin and the keeper never spawns an anonymous duplicate for it (closing its tab no longer resurrects a stray). When no Slack tab is live at all, a single cred lifeline opens one tab, preferring a pinned URL, so shared creds remain available to the sweep. Per-workspace anonymous tabs are kept only for unpinned workspaces. Distinct from ADR-0010 **Workspaces** (multi-CDP-host UI), though both stamp entries with a workspace key. _Avoid_: account list, team store, tenant table. **Grid Group**: @@ -108,7 +108,7 @@ _Avoid_: native tab, page view, webview tab. - **Viewport Transform** maps canvas coordinates to **Remote Page** coordinates for both drawing **Screencast Frames** and hit-testing **Input Forwarding**. - **Adaptive Viewport** (when enabled) resizes the **Remote Page** to the canvas so **Screencast Frames** fill it without letterbox bars. - A **Notification Side-Channel** attaches to a background **Tab**'s target and uses a **Notification Adapter** to run **Notification Capture** — independent of the **Active Tab**'s screencast socket. Clicking the result activates the owning Tab and, if the entry carries an `activate` intent, the activation registry maps it to a **Remote Page** deep-open intention. -- For Slack, the **Slack Content Sweep** is the authoritative **Notification Capture** writer; the in-page hijack provides only an instant foreground toast. The sweep reads creds from a live **Tab**, persists workspaces in the **Workspace Registry**, keeps a **Parked Tab** alive per registered workspace, and respects the **Channel Exclude** list. +- For Slack, the **Slack Content Sweep** is the authoritative **Notification Capture** writer; the in-page hijack provides only an instant foreground toast. The sweep reads creds from a live **Tab**, persists workspaces in the **Workspace Registry**, uses the **Parked Tab** keeper (pin-deferred since t098 — a pinned workspace is owned by its Pin), and respects the **Channel Exclude** list. - A **Local Tab** renders a real local web page (in-DOM ``) alongside CDP Tabs — it does not use **Screencast Frames** or **Input Forwarding**; it gets direct device access instead. ## Example dialogue diff --git a/core/notifications-sidechain.js b/core/notifications-sidechain.js index 343ded8..9810744 100644 --- a/core/notifications-sidechain.js +++ b/core/notifications-sidechain.js @@ -32,6 +32,14 @@ const { tsCmp } = require("./slack-sweep") const NOTIFY_BINDING = "__cdpNotify" const DEFAULT_CAP = 200 +// An awaitable side-channel CDP call (cred extraction) gets a timeout so a stalled socket +// frees its promise instead of hanging forever (t096, P4). +const CDP_CALL_TIMEOUT_MS = 10_000 +// A side-channel whose target is still live but never reaches OPEN (hung CONNECTING — no open, +// no close, no error) is reaped after this and re-attached on the next reconcile (t096, P3). +// Reconcile runs every ~5s and a healthy local CDP socket opens in well under a second, so a +// still-non-OPEN socket past this threshold is genuinely stuck. +const SIDECHANNEL_STALE_MS = 15_000 // Notification Adapters identify notification-capable sites by URL hostname. Each // names its capture script (loaded via the injected `readInject`) and the icon to @@ -97,6 +105,8 @@ function createNotificationCenter(deps) { // since the sweep can't cover them. const sweepDisabledTeams = new Set() const log = deps.log || (() => {}) + const now = deps.now || (() => Date.now()) + const WS_OPEN = WebSocketCtor && WebSocketCtor.OPEN != null ? WebSocketCtor.OPEN : 1 function recordCreds(team, cookie) { const prev = credsByTeam.get(team.teamId) || {} @@ -187,18 +197,33 @@ function createNotificationCenter(deps) { const adapter = adapterFor(target.url) if (!adapter || !target.webSocketDebuggerUrl) return const ws = new WebSocketCtor(target.webSocketDebuggerUrl) + ws.__attachedAt = now() sideChannels.set(target.id, ws) let cmdId = 1 // Fire-and-forget CDP send (capture-script injection doesn't need the reply). const cdp = (method, params) => ws.send(JSON.stringify({ id: cmdId++, method, params: params || {} })) // Awaitable CDP call — cred extraction needs the evaluate/getCookies results, so it - // correlates the reply by command id through `pending`. + // correlates the reply by command id through `pending`. A timeout rejects a call whose + // reply never arrives (stalled socket), and `drop` rejects any in-flight call on + // close/error — so a dead socket never leaves a promise hanging (t096, P4). const pending = new Map() const cdpCall = (method, params) => - new Promise((resolve) => { + new Promise((resolve, reject) => { const id = cmdId++ - pending.set(id, resolve) + const timer = setTimeout(() => { + if (pending.delete(id)) reject(new Error(`cdp ${method} timed out`)) + }, CDP_CALL_TIMEOUT_MS) + pending.set(id, { + resolve: (v) => { + clearTimeout(timer) + resolve(v) + }, + reject: (e) => { + clearTimeout(timer) + reject(e) + }, + }) ws.send(JSON.stringify({ id, method, params: params || {} })) }) ws.on("open", () => { @@ -217,7 +242,7 @@ function createNotificationCenter(deps) { try { const msg = JSON.parse(data.toString()) if (msg.id != null && pending.has(msg.id)) { - pending.get(msg.id)(msg) + pending.get(msg.id).resolve(msg) pending.delete(msg.id) return } @@ -228,6 +253,9 @@ function createNotificationCenter(deps) { }) const drop = () => { if (sideChannels.get(target.id) === ws) sideChannels.delete(target.id) + // Free any in-flight awaitable call so a stalled socket can't leak its promise. + for (const p of pending.values()) p.reject(new Error("side-channel closed")) + pending.clear() } ws.on("close", drop) ws.on("error", drop) @@ -274,7 +302,14 @@ function createNotificationCenter(deps) { const matched = list.filter((t) => t.type === "page" && adapterFor(t.url)) const liveIds = new Set(matched.map((t) => t.id)) for (const [id, ws] of sideChannels) { - if (!liveIds.has(id)) { + // Reap a vanished/changed target's socket, OR a socket on a still-live target that is + // stuck below OPEN past the stale threshold (hung CONNECTING — no open/close/error fires, + // so it would otherwise sit unreaped and unre-attached; t096, P3). + const stale = + liveIds.has(id) && + ws.readyState !== WS_OPEN && + now() - (ws.__attachedAt || 0) > SIDECHANNEL_STALE_MS + if (!liveIds.has(id) || stale) { try { ws.close() } catch {} diff --git a/core/notifications-sidechain.test.ts b/core/notifications-sidechain.test.ts index 309de5c..f783640 100644 --- a/core/notifications-sidechain.test.ts +++ b/core/notifications-sidechain.test.ts @@ -672,3 +672,63 @@ describe("load + close", () => { expect(FakeWs.instances.every((w) => w.closed)).toBe(true) }) }) + +// A socket that never reaches OPEN — models a hung CONNECTING side-channel. +class HungWs extends FakeWs { + readyState = 0 +} + +describe("reconcile — reap hung side-channel (t096, P3)", () => { + it("reaps a non-OPEN socket on a still-live target past the stale window and re-attaches", async () => { + const { center, setNow } = makeCenter({ WebSocketCtor: HungWs as any }) + setNow(1_000) + await center.reconcile([teamsTarget()]) + expect(FakeWs.instances).toHaveLength(1) + const hung = FakeWs.instances[0] + + setNow(1_000 + 15_000 + 1) // past SIDECHANNEL_STALE_MS + await center.reconcile([teamsTarget()]) + + expect(hung.closed).toBe(true) + expect(FakeWs.instances).toHaveLength(2) // re-attached + }) + + it("does not reap a freshly-attached non-OPEN socket within the stale window", async () => { + const { center, setNow } = makeCenter({ WebSocketCtor: HungWs as any }) + setNow(1_000) + await center.reconcile([teamsTarget()]) + + setNow(1_000 + 5_000) // before the stale threshold + await center.reconcile([teamsTarget()]) + + expect(FakeWs.instances).toHaveLength(1) + expect(FakeWs.instances[0].closed).toBe(false) + }) + + it("does not reap an OPEN socket on a live target", async () => { + const { center, setNow } = makeCenter() // default FakeWs is OPEN (readyState 1) + setNow(1_000) + await center.reconcile([teamsTarget()]) + setNow(1_000 + 60_000) + await center.reconcile([teamsTarget()]) + expect(FakeWs.instances).toHaveLength(1) + expect(FakeWs.instances[0].closed).toBe(false) + }) +}) + +describe("side-channel cdpCall reject-on-close (t096, P4)", () => { + it("rejects an in-flight cred extraction when the socket closes — no creds, no hang", async () => { + const onCreds = vi.fn() + const { center } = makeCenter({ onCreds }) + await center.reconcile([slackTarget()]) + const ws = FakeWs.instances[0] + ws.open() // fires extractSlackCreds → sends the localConfig read, awaits its reply + + ws.close() // reply never comes — drop() must reject the pending call + await Promise.resolve() + await Promise.resolve() + + expect(center.listCreds()).toEqual([]) + expect(onCreds).not.toHaveBeenCalled() + }) +}) diff --git a/core/paint-ack-pacer.js b/core/paint-ack-pacer.js new file mode 100644 index 0000000..bab89d7 --- /dev/null +++ b/core/paint-ack-pacer.js @@ -0,0 +1,28 @@ +// Adaptive paint-ack watchdog window (t096, P2). +// +// The stranded-paint watchdog (web/server.mjs) frees the one-in-flight slot and re-acks the +// remote if a supporting client never acks a painted frame. A FIXED window trips early on a +// device that legitimately paints slower than it — degrading that device to eager self-ack and +// re-introducing the stale-frame backlog the paint-ack gate exists to prevent. This tracks an +// EWMA of observed paint-ack latency (markSent → client ack) and sizes the window to a multiple +// of it, never below a floor and never above a cap: a fast link keeps the tight floor, a +// genuinely-slow device gets the slack it needs, and a pathological sample can't run away. +// +// Pure: no timers, no clock — the server measures the latency and owns the setTimeout. Tested +// by paint-ack-pacer.test.ts. + +function createPaintAckPacer({ floorMs = 1000, factor = 3, capMs = 5000, alpha = 0.3 } = {}) { + let ewma = null + return { + record(latencyMs) { + if (!(latencyMs >= 0)) return // ignore negative / NaN + ewma = ewma === null ? latencyMs : alpha * latencyMs + (1 - alpha) * ewma + }, + windowMs() { + if (ewma === null) return floorMs + return Math.max(floorMs, Math.min(capMs, Math.round(factor * ewma))) + }, + } +} + +module.exports = { createPaintAckPacer } diff --git a/core/paint-ack-pacer.test.ts b/core/paint-ack-pacer.test.ts new file mode 100644 index 0000000..5fb4cf8 --- /dev/null +++ b/core/paint-ack-pacer.test.ts @@ -0,0 +1,42 @@ +import { describe, expect, it } from "vitest" +// CommonJS shared core (ADR-0008): adaptive paint-ack watchdog window. +import { createPaintAckPacer } from "./paint-ack-pacer" + +describe("createPaintAckPacer", () => { + it("returns the floor before any sample", () => { + expect(createPaintAckPacer().windowMs()).toBe(1000) + }) + + it("stays at the floor for fast paints", () => { + const p = createPaintAckPacer() + p.record(50) + expect(p.windowMs()).toBe(1000) // 3 * 50 < floor + }) + + it("grows to a multiple of the EWMA for slow paints", () => { + const p = createPaintAckPacer({ alpha: 1 }) // alpha 1 → EWMA tracks the last sample + p.record(500) + expect(p.windowMs()).toBe(1500) // 3 * 500 + }) + + it("caps the window for pathologically slow paints", () => { + const p = createPaintAckPacer({ alpha: 1 }) + p.record(10_000) + expect(p.windowMs()).toBe(5000) + }) + + it("smooths samples via the EWMA", () => { + const p = createPaintAckPacer({ alpha: 0.5, floorMs: 0 }) + p.record(1000) // EWMA = 1000 + p.record(2000) // EWMA = 0.5*2000 + 0.5*1000 = 1500 + expect(p.windowMs()).toBe(4500) // 3 * 1500 + }) + + it("ignores negative / NaN latencies", () => { + const p = createPaintAckPacer({ alpha: 1 }) + p.record(600) // window 1800 + p.record(-5) + p.record(Number.NaN) + expect(p.windowMs()).toBe(1800) + }) +}) diff --git a/core/slack-workspaces.js b/core/slack-workspaces.js index c3ba85e..4fd5345 100644 --- a/core/slack-workspaces.js +++ b/core/slack-workspaces.js @@ -44,15 +44,37 @@ function liveTeamIds(targets) { } // Which registered workspaces need a parked tab created: those with no live tab and not -// created within the cooldown. `createdAt` maps teamId → last create timestamp. Pure. -function planParkedTabs(registry, live, createdAt, now) { +// created within the cooldown. `createdAt` maps teamId → last create timestamp. +// +// `pinUrlByTeam` (t098) maps a pinned workspace's teamId → its pin URL. A pinned workspace +// is considered OWNED BY ITS PIN: the keeper never spawns an anonymous duplicate for it +// (closing its tab no longer resurrects a stray). Capture is unaffected because one live +// Slack tab refreshes creds for ALL workspaces and the sweep polls each over the web API +// regardless of which tab is live. So per-workspace tabs aren't needed — only one live tab +// is. When NO Slack tab is live and nothing else would open one, a single cred lifeline +// plan keeps one alive, preferring a pinned URL (so it adopts into the pin on next reload). +// Omitting `pinUrlByTeam` preserves the prior per-workspace behavior. Pure. +function planParkedTabs(registry, live, createdAt, now, pinUrlByTeam = {}) { + const offCooldown = (teamId) => { + const last = createdAt[teamId] + return !(last && now - last < CREATE_COOLDOWN_MS) + } const plans = [] for (const teamId of Object.keys(registry)) { if (live.has(teamId)) continue - const last = createdAt[teamId] - if (last && now - last < CREATE_COOLDOWN_MS) continue + if (pinUrlByTeam[teamId]) continue // pin owns it — don't reopen + if (!offCooldown(teamId)) continue plans.push({ teamId, url: registry[teamId].url }) } + // Cred lifeline: nothing live and nothing else planned → keep exactly one Slack tab alive + // via a pinned workspace, so shared creds keep refreshing. Cooldown-gated; one is enough. + if (live.size === 0 && plans.length === 0) { + for (const teamId of Object.keys(pinUrlByTeam)) { + if (!offCooldown(teamId)) continue + plans.push({ teamId, url: pinUrlByTeam[teamId] }) + break + } + } return plans } diff --git a/core/slack-workspaces.test.ts b/core/slack-workspaces.test.ts index 0f8b79f..a1f1dff 100644 --- a/core/slack-workspaces.test.ts +++ b/core/slack-workspaces.test.ts @@ -83,4 +83,61 @@ describe("planParkedTabs — recreate registered workspaces with no live tab", ( const plans = planParkedTabs(reg, new Set(["T1"]), { T2: 4000 }, 40000) expect(plans).toEqual([{ teamId: "T2", url: CLIENT("T2") }]) }) + + it("omitting pinUrlByTeam is byte-identical to the prior behavior", () => { + expect(planParkedTabs(reg, new Set(["T1"]), {}, 5000)).toEqual([ + { teamId: "T2", url: CLIENT("T2") }, + ]) + }) +}) + +describe("planParkedTabs — defers to a pinned workspace (t098)", () => { + const reg = { + T1: { teamId: "T1", url: CLIENT("T1"), name: "A", lastSeen: 1 }, + T2: { teamId: "T2", url: CLIENT("T2"), name: "B", lastSeen: 1 }, + } + const PIN = (team: string) => `https://app.slack.com/client/${team}/CPIN` + + it("skips a registered workspace that has a pin — the pin owns it", () => { + // T1 live, T2 not live but pinned → no reopen for T2. + const plans = planParkedTabs(reg, new Set(["T1"]), {}, 5000, { T2: PIN("T2") }) + expect(plans).toEqual([]) + }) + + it("still plans an unpinned workspace alongside a pinned one", () => { + // T1 pinned (skip), T2 unpinned + not live → only T2 reopens. + const plans = planParkedTabs(reg, new Set(), {}, 5000, { T1: PIN("T1") }) + expect(plans).toEqual([{ teamId: "T2", url: CLIENT("T2") }]) + }) + + it("cred lifeline: opens one pinned workspace (at the pin URL) when nothing is live and nothing else is planned", () => { + // Both pinned, neither live → no normal plan, but the lifeline opens exactly one at its pin URL. + const plans = planParkedTabs(reg, new Set(), {}, 5000, { T1: PIN("T1"), T2: PIN("T2") }) + expect(plans).toHaveLength(1) + expect(plans[0]).toEqual({ teamId: "T1", url: PIN("T1") }) + }) + + it("no lifeline when a Slack tab is already live", () => { + const plans = planParkedTabs(reg, new Set(["T1"]), {}, 5000, { T1: PIN("T1"), T2: PIN("T2") }) + expect(plans).toEqual([]) + }) + + it("no lifeline when an unpinned plan already keeps a tab alive", () => { + // T1 pinned, T2 unpinned + not live → T2's normal plan covers cred-refresh; no extra lifeline. + const plans = planParkedTabs(reg, new Set(), {}, 5000, { T1: PIN("T1") }) + expect(plans).toEqual([{ teamId: "T2", url: CLIENT("T2") }]) + }) + + it("cred lifeline respects the create cooldown", () => { + // Only T1 registered + pinned, not live, but just created at t=4000 (cooldown) → no lifeline. + const oneReg = { T1: reg.T1 } + const plans = planParkedTabs(oneReg, new Set(), { T1: 4000 }, 5000, { T1: PIN("T1") }) + expect(plans).toEqual([]) + }) + + it("cred lifeline bootstraps from a pin even when the workspace is not yet registered", () => { + // Fresh start: empty registry, no live tab, a pin exists → open it to seed creds. + const plans = planParkedTabs({}, new Set(), {}, 5000, { T9: PIN("T9") }) + expect(plans).toEqual([{ teamId: "T9", url: PIN("T9") }]) + }) }) diff --git a/core/sweep-scheduler.js b/core/sweep-scheduler.js new file mode 100644 index 0000000..8245408 --- /dev/null +++ b/core/sweep-scheduler.js @@ -0,0 +1,53 @@ +// Per-key debounced scheduler for Slack sweep triggers (t096, A6). +// +// `onCreds` and `onSlackSignal` can fire for the SAME workspace within +// milliseconds (a fresh cred-extraction is immediately followed by a hijack +// signal), so both calling `sweepWorkspace(rec)` directly double-sweeps. This +// coalesces rapid same-key triggers into a leading-edge run (preserves +// sub-second delivery) plus at most one trailing run if more arrived during the +// window (catches a message that landed mid-window). Different keys are +// independent. The 15s all-workspaces backstop (`runOnce`) does NOT route here — +// it has no per-workspace key. +// +// Timers are injected so the debounce is unit-testable with a fake clock. + +function createSweepScheduler({ + run, + windowMs = 300, + setTimer = setTimeout, + clearTimer = clearTimeout, +}) { + const timers = new Map() // key → timer handle (window is open while present) + const pending = new Map() // key → latest payload awaiting the trailing edge + + function arm(key) { + const handle = setTimer(() => { + timers.delete(key) + if (pending.has(key)) { + const payload = pending.get(key) + pending.delete(key) + run(payload) + arm(key) // keep the window open so a burst can't slip a second leading run + } + }, windowMs) + timers.set(key, handle) + } + + return { + request(key, payload) { + if (timers.has(key)) { + pending.set(key, payload) // inside the window — coalesce, latest wins + return + } + run(payload) // leading edge + arm(key) + }, + stop() { + for (const h of timers.values()) clearTimer(h) + timers.clear() + pending.clear() + }, + } +} + +module.exports = { createSweepScheduler } diff --git a/core/sweep-scheduler.test.ts b/core/sweep-scheduler.test.ts new file mode 100644 index 0000000..e55b5ec --- /dev/null +++ b/core/sweep-scheduler.test.ts @@ -0,0 +1,138 @@ +import { describe, expect, it, vi } from "vitest" +// CommonJS shared core (ADR-0008): per-key debounced Slack sweep trigger. +import { createSweepScheduler } from "./sweep-scheduler" + +/** Deterministic injectable timer queue (no real time). */ +function fakeTimers() { + let now = 0 + let id = 1 + const queue: { at: number; fn: () => void; id: number }[] = [] + return { + setTimer: (fn: () => void, ms: number) => { + const t = { at: now + ms, fn, id: id++ } + queue.push(t) + return t.id + }, + clearTimer: (h: number) => { + const i = queue.findIndex((t) => t.id === h) + if (i >= 0) queue.splice(i, 1) + }, + advance: (ms: number) => { + now += ms + // Snapshot due timers; callbacks may schedule new ones (not yet due). + const due = queue.filter((t) => t.at <= now).sort((a, b) => a.at - b.at) + for (const t of due) { + const i = queue.indexOf(t) + if (i >= 0) queue.splice(i, 1) + t.fn() + } + }, + } +} + +describe("createSweepScheduler", () => { + it("runs the first request immediately (leading edge)", () => { + const ft = fakeTimers() + const run = vi.fn() + const s = createSweepScheduler({ + run, + windowMs: 300, + setTimer: ft.setTimer, + clearTimer: ft.clearTimer, + }) + + s.request("T1", { teamId: "T1" }) + + expect(run).toHaveBeenCalledTimes(1) + expect(run).toHaveBeenCalledWith({ teamId: "T1" }) + }) + + it("coalesces same-key requests within the window into one trailing run with the latest payload", () => { + const ft = fakeTimers() + const run = vi.fn() + const s = createSweepScheduler({ + run, + windowMs: 300, + setTimer: ft.setTimer, + clearTimer: ft.clearTimer, + }) + + s.request("T1", { n: 1 }) // leading → runs + s.request("T1", { n: 2 }) // within window → coalesced + s.request("T1", { n: 3 }) // within window → coalesced (latest wins) + expect(run).toHaveBeenCalledTimes(1) + + ft.advance(300) // trailing edge + + expect(run).toHaveBeenCalledTimes(2) + expect(run).toHaveBeenLastCalledWith({ n: 3 }) + }) + + it("does not run again at the window edge when nothing was coalesced", () => { + const ft = fakeTimers() + const run = vi.fn() + const s = createSweepScheduler({ + run, + windowMs: 300, + setTimer: ft.setTimer, + clearTimer: ft.clearTimer, + }) + + s.request("T1", {}) + ft.advance(300) + + expect(run).toHaveBeenCalledTimes(1) + }) + + it("treats different keys independently (each gets its own leading run)", () => { + const ft = fakeTimers() + const run = vi.fn() + const s = createSweepScheduler({ + run, + windowMs: 300, + setTimer: ft.setTimer, + clearTimer: ft.clearTimer, + }) + + s.request("T1", { k: "T1" }) + s.request("T2", { k: "T2" }) + + expect(run).toHaveBeenCalledTimes(2) + }) + + it("runs immediately again once the window has closed", () => { + const ft = fakeTimers() + const run = vi.fn() + const s = createSweepScheduler({ + run, + windowMs: 300, + setTimer: ft.setTimer, + clearTimer: ft.clearTimer, + }) + + s.request("T1", { n: 1 }) // leading + ft.advance(300) // window closes, nothing pending + s.request("T1", { n: 2 }) // leading again + + expect(run).toHaveBeenCalledTimes(2) + expect(run).toHaveBeenLastCalledWith({ n: 2 }) + }) + + it("stop() cancels pending timers so no trailing run fires", () => { + const ft = fakeTimers() + const run = vi.fn() + const s = createSweepScheduler({ + run, + windowMs: 300, + setTimer: ft.setTimer, + clearTimer: ft.clearTimer, + }) + + s.request("T1", { n: 1 }) // leading run + s.request("T1", { n: 2 }) // pending trailing + s.stop() + ft.advance(300) + + expect(run).toHaveBeenCalledTimes(1) + }) +}) diff --git a/docs/adr/0012-phone-triage-surface-inbox-rooted-shell-conversati.md b/docs/adr/0012-phone-triage-surface-inbox-rooted-shell-conversati.md index 835cf1a..603c12a 100644 --- a/docs/adr/0012-phone-triage-surface-inbox-rooted-shell-conversati.md +++ b/docs/adr/0012-phone-triage-surface-inbox-rooted-shell-conversati.md @@ -1,6 +1,6 @@ # ADR-0012: Phone triage surface: inbox-rooted shell + conversation reader -- **Status:** Proposed +- **Status:** Accepted (all phases shipped t076–t081) - **Date:** 2026-06-11 ## Context diff --git a/docs/adr/0015-prefer-thin-handlers-over-reducer-indirection.md b/docs/adr/0015-prefer-thin-handlers-over-reducer-indirection.md new file mode 100644 index 0000000..489bbe9 --- /dev/null +++ b/docs/adr/0015-prefer-thin-handlers-over-reducer-indirection.md @@ -0,0 +1,70 @@ +# ADR-0015: Prefer thin handlers + small helpers over reducer/event-bus indirection where orchestration is already concentrated + +- **Status:** Accepted +- **Date:** 2026-06-20 + +## Context + +An architecture investigation (t096) ran finder agents that proposed seven +"deepening" refactors — each suggesting a new reducer, event-bus, or +effect-directive layer to concentrate logic alleged to be scattered across the +codebase. Adversarial verification against the real code **refuted four of them +outright** and down-scoped two more: + +- **A1 Tab Lifecycle Orchestrator** — decisions already concentrated in the pure + `tab-lifecycle.ts` planner (`planClose`/`planSwitch`, 18 tests); the renderer + already applies directives thinly. `main.js`/`server.mjs` do a tabId-keyed CDP + connect, not lifecycle. No scatter to concentrate. +- **A2 Notification Store reducer** — handlers already live in one ~100-line + `app.tsx` block; the read model (`groupByConversation`) is already a shared + pure module; the server `notificationCenter` is already the authoritative + writer. No competing-writers problem. +- **A4 Input Intent Builder** — `forwardInput(InputIntent)` is already a single + tagged-union seam; `toRemoteCoords` is applied exactly once; t084's on-screen + keyboard already proved a new input modality drops into the existing seam. +- **A6 Slack Sweep event stream** — the runner already owns the state machine + (seed-vs-sweep, restricted fallback, stale-vs-permanent error); 429s are + handled in `slack-api.js`; the server only supplies DI deps + trigger sites. + +In each case the **deletion test** showed the orchestration was already deep — +concentrated behind a seam — so wrapping it in a reducer/event-bus would +*relocate* clean code, not concentrate scattered code. The proposals also added +speculative machinery (a settings save-queue, offline handling, an event queue) +for failure modes with no observed evidence. + +## Decision + +Where the deletion test shows orchestration is **already concentrated behind a +seam**, prefer the smallest change that removes the *actual* friction: + +- a thin optimistic handler, +- a small shared helper (e.g. `applyCloseDirective`, an optimistic-mutate + + revert helper, a debounced `scheduleSweep`), +- or lifting an already-DI'd factory into its own tested file. + +Reject "deepening" whose only effect is to rename or relocate code that already +passes the deletion test. A reducer/event-bus/effect-directive layer is +warranted only by **real, observed scatter across N callers** — not by a large +file, a conceptually-separable concern, or a hypothetical future modality. + +## Consequences + +- A future `/improve-codebase-architecture` run that re-surfaces A1, A2, A4, or + A6 should be closed by reference to this ADR — unless new scatter has actually + appeared since (re-run the deletion test to confirm). +- The bar for introducing a reducer/event-bus is explicit: point at the N caller + sites that each re-implement the orchestration. If you can't, it's already + deep. +- This does **not** forbid the small fixes the same investigation kept (t096): + consolidating the genuinely-duplicated settings ui-state load/write owner + (A3), extracting already-DI'd-but-inline transport factories for testability + (A5), and adding revert-on-fail to optimistic handlers (A2's real defect). The + no is specifically to the reducer/event-bus *shape*, not to removing real + duplication. + +## Alternatives + +- **Build the reducers/event-buses as proposed** — rejected. Verification found + no scatter to concentrate; the indirection would add a layer a future reader + must traverse to follow already-local logic, harming the locality it claimed + to improve. diff --git a/docs/conventions/docs-discipline.md b/docs/conventions/docs-discipline.md index 2dc004f..3e4dc01 100644 --- a/docs/conventions/docs-discipline.md +++ b/docs/conventions/docs-discipline.md @@ -87,7 +87,7 @@ Any non-trivial design decision gets an ADR. Examples of "non-trivial": ADRs are **append-only**. When a decision changes, write a new ADR that supersedes the old one. Update the old ADR's *Status* line to reference the superseder, but never edit its body. The history is the documentation. -Current ADRs: `docs/adr/0001-single-remote-page.md`, `0002-adaptive-viewport.md`, `0003-notifications-side-channel.md`, `0004-pin-live-tab-model.md`, `0005-local-tabs-base-window.md`, `0006-web-proxy-sse-transport.md`, `0007-web-websocket-transport.md`, `0008-defer-monorepo-shared-cjs-core.md`, `0009-touch-first-co-primary-input-surface.md`, `0010-multiple-workspaces-deferred-design.md`, `0011-slack-content-sweep-guaranteed-delivery.md`, `0012-phone-triage-surface-inbox-rooted-shell-conversati.md`, `0013-per-device-notification-delivery.md`, `0014-endpoint-reconciled-per-device-push-identity.md`. +Current ADRs: `docs/adr/0001-single-remote-page.md`, `0002-adaptive-viewport.md`, `0003-notifications-side-channel.md`, `0004-pin-live-tab-model.md`, `0005-local-tabs-base-window.md`, `0006-web-proxy-sse-transport.md`, `0007-web-websocket-transport.md`, `0008-defer-monorepo-shared-cjs-core.md`, `0009-touch-first-co-primary-input-surface.md`, `0010-multiple-workspaces-deferred-design.md`, `0011-slack-content-sweep-guaranteed-delivery.md`, `0012-phone-triage-surface-inbox-rooted-shell-conversati.md`, `0013-per-device-notification-delivery.md`, `0014-endpoint-reconciled-per-device-push-identity.md`, `0015-prefer-thin-handlers-over-reducer-indirection.md`. --- diff --git a/docs/tasks/096-investigate-for-arch-and-refactoring-predict-issue.md b/docs/tasks/096-investigate-for-arch-and-refactoring-predict-issue.md new file mode 100644 index 0000000..d5166c9 --- /dev/null +++ b/docs/tasks/096-investigate-for-arch-and-refactoring-predict-issue.md @@ -0,0 +1,308 @@ +# 096 — Arch + refactoring sweep: verified fixes from the predict-issues / improve-architecture investigation + +- **Status:** in-progress +- **Mode:** HITL +- **Estimate:** multi-session (~2–3d; ships as one PR with internal commit boundaries by area) +- **Depends on:** none +- **Blocks:** none + +## Goal + +A predictive-health + architecture-deepening investigation (20 predicted issues ++ 7 deepening candidates) was run and then **adversarially verified against the +real code**. Verification refuted 17 of 20 predicted issues and 4 of 7 deepening +candidates as already-mitigated — the codebase already has the caps, watchdogs, +error handlers, and seams the finders assumed were missing. This task lands the +**genuinely-actionable remainder**: one real consolidation (settings ui-state), +one testability extraction (web-transport factories), one real bug (Electron +`onSwipe` leak), and a batch of small hardening fixes. After it ships, the +ui-state load/write path has a single owner, the WS channel + reconnect driver +have isolated tests, the swipe-listener leak is gone, and every "optional" +robustness gap the verifiers flagged is closed — with the full evidence trail +recorded so the same proposals aren't re-litigated. + +## Why now + +The investigation already did the expensive part (find + verify). The findings +are small, independent, and decay if left unrecorded — the next arch review +would re-run the same analysis and re-propose the same refuted refactors. Batch +the cleanup while the evidence is fresh, and record the "why not a reducer" +reasoning in ADR-0015 so it stops recurring. + +## Acceptance criteria + +Grouped by area. Each is checkable true/false. + +### A — Settings ui-state single owner (was A3, Med) — SCOPED DOWN + +Investigation finding sharpened during implementation: `app.tsx` and +`settings-dialog.tsx` own **mostly-disjoint** settings (app.tsx: sidebar / theme +/ adaptiveViewport / mutes / etc.; settings-dialog: virtualPointer / webPush / +scroll). The only cross-file overlap is `slackExcludes`, and app.tsx reads it +**on-demand** (not cached) while the dialog reloads it on every open — so the +"divergence" self-heals and there is no live cache-staleness bug. A full +`useSettings` hook absorbing all 15 settings would **de-localize** settings from +their UI (the opposite of the locality win) for no correctness gain — exactly the +over-engineering ADR-0015 warns against. The real, bounded defect the verifier +named was **the dual load**. + +- [x] **Dual load removed** — `settings-dialog.tsx` called `getUiState()` **twice** + in its open effect (virtual-pointer/scroll, then web push/excludes); merged + into **one** load, web fields gated inside it. Behavior preserved. +- [x] Writes still partial-merge server-side (unchanged); no save-queue / offline + machinery added. +- [ ] Full single-owner `useSettings` hook + the ~28-prop `app→Toolbar→dialog` + drill-through — **DEFERRED** (see Notes / spillover task). De-localizing + risk + needs Layer-3 visual review unavailable in this AFK run. +- [ ] Visual review of the settings dialog (all cards persist correctly) — + **HITL pending**. + +### B — Web-transport factory extraction + tests (was A5, Med) + +- [x] `createWsChannel`, `createInputChannel`, `createReconnectDriver` are + lifted out of `cdp-web-transport.ts` into their own files, each with + injectable deps preserved. +- [x] Unit tests cover the WS channel (new, 8) and the reconnect driver (existing, + colocated). (Correction: the reconnect driver already had a test — the real + zero-coverage gap was the WS channel.) Input channel +1 (fallback contract). +- [x] `createWebCdp` remains the assembler; the already-pure seams + (`downlink-dispatcher`, `uplink-router`, `crypto-context`, + `transport-selector`, `reconnect-backoff`, `input-coalesce`) are **not** + re-extracted. (Assembler is 970L — the verifier's "~200L" was the rejected + proposal's aspiration; the corrected scope was extract-the-3-factories only.) +- [x] `cdp-web-transport.ts` line count drops materially: 1419 → 970. + +### C — onSwipe listener leak (was P6, S, Electron-only) + +- [x] `preload.js` `onSwipe` returns an unsubscribe + (`removeListener` of the named wrapper); web stub returns a no-op unsubscribe; + `CdpBridge` type updated to `=> () => void`. +- [x] The `app.tsx` swipe effect returns that unsubscribe as cleanup. +- [ ] After repeated reconnects, a single swipe fires `goBack`/`goForward` + exactly once (manual Electron smoke). **HITL pending** — needs the Electron app. + +### D — Notification optimistic-write revert (was A2 real defect, S) + +- [x] The 6 dual-write handlers (`markThreadRead`, `handleMarkAllRead`, + `handleClearNotifications`, `handleClearThread`, `handleMuteChannel`, + `handleToggleRead`) route through one optimistic-mutate helper + (`optimisticNotif`) that applies the local patch and **reverts to the + pre-patch snapshot if the POST rejects**. +- [x] No reducer / event-bus introduced (ADR-0015). + +### E — Hardening nits (all S) + +- [x] **P15** — settings `writeFileSync` is wrapped in try-catch + `console.error` + in both `main.js` and `web/server.mjs` (mirrors the existing `savePushSubs`). + `core/settings-store.js` delegates to the injected persist, so guarding both + injection sites is the fix (no store change). +- [x] **P7** — `sendPushToAll` recomputes `unreadExcluding` from a fresh + `notificationCenter.list()` at each send (not the pre-await snapshot), so an + in-flight push can't stamp a stale per-device badge. +- [x] **P18** — `createClosedStack` caps entries (shift oldest beyond ~50). + Done: `CLOSED_STACK_CAP = 50`, FIFO drop; `closed-tabs.test.ts` covers it. +- [x] **P4** — `cdpCall` (side-channel) races a timeout and rejects pending on + ws close/error, so a stalled socket frees its ~2 promises. Done: + `CDP_CALL_TIMEOUT_MS=10_000`, drop rejects+clears pending; test covers close. +- [x] **A1** — an `applyNextActive` helper dedupes the `nextActive` + switch tail shared by `closeTab` and `closeTabs`. +- [x] **P11** — `page.find()` has a `.catch` (no unhandled rejection on socket drop). +- [x] **P19** — the command-palette `action.run()` is wrapped in try-catch with a + failure toast (`sonner`). +- [x] **P2** — the paint-ack watchdog is adaptive (derived from measured + RTT/paint latency) instead of a fixed 1000 ms, so a legitimately-slow device + can't trip it early. Done: pure `core/paint-ack-pacer.js` (EWMA, floor 1s, + cap 5s, ×3); server records markSent→ack latency and arms the timer from it; + 6 unit tests. +- [x] **P3** — side-channel `reconcile` also closes+reattaches a socket stuck in + CONNECTING/CLOSING (not OPEN), covering the hung-connect edge. Done: + `SIDECHANNEL_STALE_MS=15_000` stale reap; 3 tests (reap/no-reap-fresh/no-reap-open). +- [x] **A6** — the two per-workspace sweep triggers (`onCreds` + `onSlackSignal`, + which both call `sweepWorkspace`) share one debounced `core/sweep-scheduler.js` + (leading + trailing, per workspace key) — not an event-driven orchestrator. + The 15s all-workspaces `runOnce` backstop stays separate (no per-key). The + verifier's "4 trigger sites" overcounted — only these two double-fire. +- [x] **A7** — `readerEntry` is folded into the phone-view union + (`{ view: "reader", entry }`), removing the parallel state. Visual review + of the phone shell remains **HITL pending**. + +### Docs + +- [ ] ADR-0015 committed (already drafted). +- [ ] CLAUDE.md updated for every module whose contract changed + (`use-settings`, the new transport-factory files, `closed-tabs` cap, + `cdp-web-transport` shrink). + +## Test plan + +### Layer 1 — Pure logic (TDD) + +- [x] `closed-tabs.ts` `createClosedStack` — push past the cap drops the oldest; + pop order unchanged (P18). ✓ green +- [x] `core/sweep-scheduler.js` `createSweepScheduler` — leading-edge run, trailing + coalesce of same-key triggers within the window, independent keys, post-window + re-fire, `stop()` cancels (A6). ✓ green +- [ ] the optimistic-mutate helper — applies patch on success; reverts to the + prior list on a rejected POST (A2/D). +- [x] `createReconnectDriver` — backoff schedule + WS re-climb fires the + injected connect at the expected steps (B). ✓ green (pre-existing test + colocated to `web-reconnect-driver.test.ts`). +- [x] `createWsChannel` — frame/ack/input routing over an injected socket; paint- + ack defer path (B). ✓ green (new `web-ws-channel.test.ts`, 8 cases; was 0). +- [x] paint-ack watchdog timeout derivation — given an RTT/paint sample, the + watchdog window is ≥ a floor and tracks the sample (P2). ✓ green + +### Layer 2 — Manual smoke (CDP/IPC) + +Against a live Remote Browser: + +- [ ] **P6** — swipe back/forward in the Electron app; reconnect (switch tabs) + several times; one swipe still navigates exactly once. +- [ ] **P15** — make `settings.json` unwritable; change a setting; app logs the + error and stays up (no crash, no silent data path). +- [ ] **P4** — kill a side-channel socket mid cred-extraction; server keeps + sweeping other workspaces; no hung promise accrues. +- [ ] **P3** — force a side-channel into CONNECTING and leave the target live; + next reconcile re-attaches it. +- [ ] **P7** — fire a push while marking a thread read; the per-device badge + count is correct on the next push. + +### Layer 3 — Visual review + +- [ ] Screenshots via Chrome MCP against `pnpm dev` of the settings dialog + (all cards), driven through the new `useSettings` hook — loading, empty, + error, populated. +- [ ] Phone-shell Conversation Reader still opens/back-navigates after the + `readerEntry`→union fold (A7). +- [ ] Command palette shows the failure toast when an action throws (P19). + +## Design notes + +Behavioral contracts, not file paths. Full file:line evidence is in **Notes**. + +- **Contracts changed:** + - **Settings ui-state** — today two owners (`app.tsx` + `settings-dialog.tsx`) + each load and write ui-state; after, a single `useSettings` hook is the only + loader/writer. Partial-merge write semantics unchanged. No new persistence + machinery. + - **`window.cdp.onSwipe`** — `(cb) => void` → `(cb) => () => void` (returns an + unsubscribe). Web stub stays a no-op but now returns a no-op unsubscribe to + satisfy the same contract. + - **Notification mutation handlers** — fire-and-forget optimistic writes → + optimistic-with-revert (the server's returned list is the source of truth on + success; the prior list is restored on failure). + - **`createClosedStack`** — unbounded → bounded (cap ~50, FIFO drop). + - **side-channel `cdpCall`** — never-times-out → races a timeout; pending + rejected on socket close. + - **paint-ack watchdog** — fixed `PAINT_ACK_WATCHDOG_MS = 1000` → adaptive + window derived from RTT/paint latency (with a floor). +- **New modules:** + - `src/hooks/use-settings.ts` — single ui-state load + write owner (A3). + - `src/lib/web-ws-channel.ts`, `src/lib/web-input-channel.ts`, + `src/lib/web-reconnect-driver.ts` — the three factories lifted from + `cdp-web-transport.ts`, now unit-tested (A5). (Final names TBD; keep + kebab-case.) + - a small `scheduleSweep` debounce helper (A6) and an optimistic-mutate + helper (A2) — likely co-located with their consumers, not standalone seams. +- **New ADR needed?** Yes — **ADR-0015** (drafted): prefer thin handlers + small + helpers over reducer/event-bus indirection where orchestration is already + concentrated. It records *why* A1/A2/A4/A6 were down-scoped, not deepened. + +## Out of scope + +- **The reducer/event-bus refactors themselves** — A1 Tab Lifecycle + Orchestrator, A2 Notification Store reducer, A4 Input Intent Builder, A6 Slack + Sweep event stream as proposed. Verified as already-deep; see ADR-0015. Only + the small real defects they exposed are in scope. +- **The settings save-queue / offline-write machinery** (A3 as originally + proposed) — no observed lost-write or offline failure; would be speculative. +- **The 13 refuted predictions** — verified already-safe, **not touched**: + P1 (200-entry `ingest` cap), P5 (per-socket pending Map, GC'd on close), + P8 (`appliedMetrics`-on-OPEN-socket guard), P9 (per-sweep `groupId` derivation), + P10 (endpoint dedup + 404/410 prune), P12 (`refreshTabs` guards `result.error`), + P13 (once-per-foreground gate; also a TODO stub), P14 (dedup precedes push in + one sync tick), P16 (ResizeObserver native on target), P17 (`clearTimeout` in + cleanup exists), P20 (`innerWidth` fallback exists). Listed here so a future + reader doesn't re-investigate them. + +## Notes + +Free-form scratchpad — the investigation receipts. + +### Implementation summary (AFK session) + +All 15 flagged items implemented across 9 commits on `refactor/t096-arch-refactor-sweep`: + +| Commit | Items | +|---|---| +| `52c0ece` | investigation + ADR-0015 + P18 (closed-tab cap) + A6 (sweep debounce) | +| `efcbe25` | P4 + P3 (side-channel timeout + hung-socket reap) | +| `33094a4` | P15 + P7 (settings persist guard + per-send push badge) | +| `32b31d5` | P2 (adaptive paint-ack watchdog) | +| `8220609` | A5 (extract ws/input/reconnect channels + ws-channel tests) | +| `dab5bae` | P6 + P11 + P19 (swipe unsubscribe + find/palette guards) | +| `f6ad10e` | A1 + A7 + A2 (close-tail dedupe + reader union + notif revert) | +| `46075ef` | A3 (settings dialog dual-load merge; full hook → t097) | + +**Automated gates all green:** `pnpm typecheck`, `pnpm test` (936), `pnpm test:e2e` +(43), `pnpm build`, `node --check` on all 3 backends, `pnpm check:changed` (0 errors; +pre-existing `noConsole` warnings only). New tests added: closed-tabs cap, sweep-scheduler, +paint-ack-pacer, sidechain reap/timeout, ws-channel, input-channel. + +**HITL-pending (could not run AFK — needs the iPad / live Remote Browser / Electron app):** +- Layer-2 smoke: P6 (swipe×reconnect), P15 (unwritable settings), P4 (stalled cred socket), + P3 (hung side-channel reap), P7 (push-while-marking badge). +- Layer-3 visual: A7 (phone shell reader nav), A3 (settings cards persist), P19 (palette toast). +- App-boot check (main.js / preload.js touched by P6, P15). + +This is why the task stays **in-progress** — do the smoke + visual passes, then close. + +### Method + +`/predict-issues` + `/improve-codebase-architecture` ran as two Explore agents +(20 + 7 findings). Every finding was then adversarially verified by a per-finding +agent (skeptical, default-refute) that read the actual code. Verdicts below cite +**real** file:line refs (the finders' original line numbers were often +speculative). The verification refuted the majority — the codebase was in much +better shape than the raw predictions implied. + +### Verdict table — predicted issues + +| ID | Verdict | Real defect (verified) | Where | +|---|---|---|---| +| P1 | refuted | `ingest` slices to `DEFAULT_CAP=200` on every write, all paths | `core/notifications.js:70-74`, `notifications-sidechain.js:34` | +| P2 | **in (P2)** | watchdog already re-acks remote; but window is fixed 1000 ms → slow device trips early | `web/server.mjs:391-440`, `core/frame-ack-gate.js:21-46` | +| P3 | **in (P3)** | `drop` fires on close+error & reconcile reaps; residual hung-CONNECTING socket not reaped | `core/notifications-sidechain.js:229-234,276-285` | +| P4 | **in (P4)** | `cdpCall` has no timeout; fire-and-forget so can't block, but leaks ~2 promises/dead socket | `core/notifications-sidechain.js:198-213,239-260` | +| P5 | refuted | pending Map is per-socket (≤2), freed on close via closure GC | `core/notifications-sidechain.js:197-228` | +| P6 | **confirmed (C)** | `onSwipe` no unsubscribe + effect no cleanup → dup back/fwd per swipe after reconnect; Electron-only | `src/app.tsx:1234-1239`, `preload.js:26`, `cdp-web-transport.ts:1285` | +| P7 | **in (E)** | `sendPushToAll` computes `unreadExcluding` from a pre-await `list` snapshot → one-count-stale badge | `web/server.mjs:173-219` | +| P8 | refuted | `appliedMetrics` stamped only on OPEN-socket send; reconnect rebaselines | `main.js:288-326,382-398`, `adaptive-viewport.ts:86-99` | +| P9 | refuted | `gid` derived per-sweep from cred's `enterpriseId\|\|teamId`; no stale map in keying | `core/slack-creds.js:62-64`, `slack-sweep-runner.js:137` | +| P10 | refuted | subscribe dedups by endpoint; dead pruned on 404/410 | `web/server.mjs:1050-1053,224-225`, `core/push-subscriptions.js:6-15` | +| P11 | **in (E)** | `setQuery` zeroes total before await (no stale total); but `.then` has no `.catch` → unhandled rejection | `src/components/find-bar.tsx:46-56`, `find-bar.ts:39-45` | +| P12 | refuted | `refreshTabs` guards `result.error` before `reconcile`; no tab loss | `src/app.tsx:517-538`, `main.js:209-217` | +| P13 | refuted | once-per-foreground `revalidatedThisForeground` flag; call site is a TODO stub | `src/lib/push-revalidate.ts:4-29`, `app.tsx:256-266` | +| P14 | refuted | dedup precedes push in one sync tick; stable id makes re-sweeps idempotent | `core/notifications-sidechain.js:347-354`, `notifications.js:70-75` | +| P15 | **confirmed (E)** | settings `writeFileSync` unguarded in both backends (inconsistent w/ wrapped `savePushSubs`) | `main.js:76`, `web/server.mjs:123`, `core/settings-store.js:66` | +| P16 | refuted | ResizeObserver native on iOS 16.4+/Electron Chromium; no polyfill needed | `src/components/viewport.tsx:542` | +| P17 | refuted | `clearTimeout(timer)` already in effect cleanup + keyup/blur reset | `src/app.tsx:982-1003` | +| P18 | **confirmed (E)** | `createClosedStack` uncapped (per-session ref, tiny objects) | `src/lib/closed-tabs.ts:18-24`, `app.tsx:954,1045` | +| P19 | **in (E)** | palette closes+clears before `run()` (state safe), but `run()` has no try-catch | `src/components/command-palette.tsx:41-45` | +| P20 | refuted | `shellModeFor(window.innerWidth)` initial + `matchMedia` guard | `src/hooks/use-shell-mode.ts:12-22` | + +### Verdict table — deepening candidates + +| ID | Verdict | Real action (verified) | Where | +|---|---|---|---| +| A1 | refuted→**small (E)** | planner already pure + tested; only a 2-line directive-apply tail duped between close paths | `tab-lifecycle.ts:59-113`, `app.tsx:922-960,1039-1077` | +| A2 | partial→**in (D)** | already one block + shared read model + authoritative server store; real gap = dual-write, no revert | `app.tsx:577-683`, `notifications-view.ts:74` | +| A3 | partial→**in (A)** | real: two ui-state load+write owners; `slackExcludes` ×3 sites; ~28-prop chain. Skip save-queue/offline | `app.tsx:357-456`, `settings-dialog.tsx:373-427`, `toolbar.tsx:443-472` | +| A4 | refuted | `forwardInput`+`InputIntent` already the seam; coords applied once; t084 proved drop-in | `remote-page.ts:82-95,349-404`, `viewport.tsx:519-532` | +| A5 | partial→**in (B)** | hard seams already promoted; 3 DI'd factories left inline + untested (WS channel, reconnect driver = 0 tests) | `cdp-web-transport.ts:408-606,317-399,120-220` | +| A6 | refuted→**small (E)** | runner already owns state machine; 429 in `slack-api.js`; only the 4 trigger sites could share a debounce | `slack-sweep-runner.js:107-318`, `web/server.mjs:544-554,651,773` | +| A7 | refuted→**small (E)** | `phoneView` already a union; ~63 contiguous lines; only `readerEntry` parallel state to fold | `src/app.tsx:188-250` | + +--- + +_When task status flips to `done`, move this file to `done/`._ diff --git a/docs/tasks/097-settings-usesettings-hook-full-consolidation-and-p.md b/docs/tasks/097-settings-usesettings-hook-full-consolidation-and-p.md new file mode 100644 index 0000000..95e2848 --- /dev/null +++ b/docs/tasks/097-settings-usesettings-hook-full-consolidation-and-p.md @@ -0,0 +1,48 @@ +# 097 — settings: useSettings hook full consolidation + prop-drill reduction + +- **Status:** draft +- **Mode:** HITL +- **Estimate:** 1d +- **Depends on:** none +- **Blocks:** none + +## Goal + +Spillover from t096 (A3). The dual `getUiState()` load inside `settings-dialog.tsx` +was already merged to one (t096). This task takes the **larger, visual-review-gated** +half: a single `useSettings` hook (or equivalent) that owns the ui-state load + writes +shared between `app.tsx` and `settings-dialog.tsx`, and reduces the ~28-prop +`app.tsx → Toolbar → SettingsDialog` pass-through. + +## Why now + +Deferred from t096 because it (a) **de-localizes** settings that are currently +appropriately co-located with their UI — so it must be done as a deliberate locality +trade, not a blind sweep (see ADR-0015); and (b) touches the settings dialog across +many cards, so it **requires Layer-3 visual review** (all cards still load + persist +correctly) which the AFK t096 run could not perform. + +## Acceptance criteria + +- [ ] A single ui-state load owner shared by `app.tsx` + `settings-dialog.tsx` + (no component runs an independent `getUiState` for a setting another owns). +- [ ] `slackExcludes` has one owner — the dialog reads it from the shared source + rather than caching its own copy (removes the open-while-muted stale window). +- [ ] The `app → Toolbar → SettingsDialog` prop chain is materially reduced. +- [ ] Partial-merge writes preserved; **no** save-queue / offline machinery + (rejected as over-engineering in t096 — no observed lost-write or offline bug). +- [ ] Layer-3 visual review: every settings card loads its current value and + persists on change (host/port, theme, adaptiveViewport, quality tier, mutes, + excludes, webPush, virtual pointer). + +## Notes + +t096 evidence (verifier A3, corrected): the real defect was the dual load (fixed in +t096) + the duplicate `slackExcludes` owner. `app.tsx` and `settings-dialog.tsx` own +mostly-disjoint settings; the overlap (`slackExcludes`) self-heals on dialog reopen +because app.tsx reads it on-demand. So this is a **locality + ergonomics** refactor, +not a correctness fix — scope it accordingly. + +--- + +_When task status flips to `done`, move this file to `done/`._ diff --git a/docs/tasks/098-slack-keeper-defers-to-pinned-workspace-tab-instea.md b/docs/tasks/098-slack-keeper-defers-to-pinned-workspace-tab-instea.md new file mode 100644 index 0000000..59f3755 --- /dev/null +++ b/docs/tasks/098-slack-keeper-defers-to-pinned-workspace-tab-instea.md @@ -0,0 +1,109 @@ +# 098 — slack keeper defers to pinned workspace tab instead of forcing reopen + +- **Status:** in-progress +- **Mode:** HITL +- **Estimate:** 0.5d +- **Depends on:** none +- **Blocks:** none + +## Goal + +The Slack parked-tab keeper (t070, ADR-0011) reopens an anonymous background tab +for every registered workspace whose tab is not currently live — **even when the +user has that workspace pinned**. Closing the workspace's tab immediately spawns a +stray duplicate in the Tabs list, which is annoying. After this task, the keeper +**defers to a pin**: a workspace that has a pin is considered covered by its pin and +the keeper never spawns a duplicate for it. Capture is unaffected because **one live +Slack tab refreshes creds for all workspaces** (shared `d` cookie + `localConfig_v2` +holds every team's token) and the sweep polls every workspace over the web API +regardless of which tab is live. A cred lifeline keeps exactly one Slack tab alive +(preferring a pinned URL) only when no Slack tab is live and nothing else would open +one. + +## Why now + +Daily-driver annoyance on the priority web/PWA surface: the user pins their Slack +workspaces and the keeper keeps resurrecting closed tabs against their intent. The +fix is small and server-only, and it tightens guaranteed delivery (ADR-0011) rather +than weakening it — capture no longer depends on a per-workspace tab. + +## Acceptance criteria + +- [ ] A registered workspace that has a pin gets **no** anonymous parked tab from + the keeper (it is not re-created on close). +- [ ] A registered workspace with **no** pin keeps today's anonymous parked-tab + behavior (unchanged). +- [ ] Cred lifeline: when **no** Slack tab is live and no unpinned workspace would + open one, the keeper opens exactly **one** tab, preferring a pinned URL, so + creds keep refreshing. +- [ ] The cred lifeline respects the existing create-cooldown (no spam). +- [ ] Omitting the pin map preserves byte-identical prior behavior (back-compat). + +## Test plan + +### Layer 1 — Pure logic (TDD) + +- [ ] `core/slack-workspaces.js` `planParkedTabs(registry, live, createdAt, now, pinUrlByTeam)` + — skips a pinned registered workspace; still plans an unpinned one alongside it. +- [ ] cred lifeline opens one pinned workspace when `live` is empty and no unpinned + plan exists; uses the pin URL; respects cooldown; does nothing when a tab is + live or an unpinned plan already keeps one alive. +- [ ] back-compat: omitting `pinUrlByTeam` equals the prior result. + +### Layer 2 — Manual smoke (CDP/IPC) + +Web build (`pnpm web`) against a live Remote Browser with ≥1 Slack workspace pinned: + +- [ ] Pin a Slack workspace, open it, then close its tab → keeper does **not** reopen + a stray; the workspace still receives notifications (sweep continues). +- [ ] Close **every** Slack tab → keeper opens exactly one (a pinned workspace's URL); + creds recover and sweeps resume. +- [ ] An unpinned workspace closed → still auto-reopens (unchanged). + +### Layer 3 — Visual review + +n/a — no renderer UI is touched (server-side keeper only). + +## Design notes + +- **Contracts changed:** `planParkedTabs` gains an optional 5th arg + `pinUrlByTeam: { [teamId]: url }`. A workspace whose `teamId` is a key is skipped + (pin owns it). When `live` is empty and the normal plan list is empty, the planner + appends **one** lifeline plan from `pinUrlByTeam` (cooldown-gated) so a single + Slack tab stays alive for shared-cred refresh. Absent/empty map → prior behavior. +- The server (`web/server.mjs` `keepSlackTabsAlive`) derives `pinUrlByTeam` from + `settings.getPins()` via `teamIdOf(pin.url)` and passes it in. No renderer change. +- **New modules:** none. +- **New ADR needed?** No — this is a tuning of the t070 keeper within ADR-0011. + Document in CLAUDE.md (keeper bullet + `slack-workspaces.js` note) and the code. + +## Out of scope + +- Renderer pin-adoption of a keeper-opened tab mid-session (ADR-0004 keeps + URL-adoption startup-only; the lifeline tab adopts on next reload). Not needed + for the chosen "don't reopen — pin owns it" behavior. +- Reducing the unpinned per-workspace keeper to one-tab-total (kept as-is per the + decision). + +## Definition of Done + +- [ ] Layer 1 tests written and green +- [ ] Layer 2 smoke checklist completed with a live Remote Browser (web build) +- [ ] `pnpm check:changed` clean +- [ ] `pnpm typecheck` clean +- [ ] `pnpm test` green +- [ ] CLAUDE.md updated (keeper bullet + `slack-workspaces.js` note) +- [ ] No commented-out code, no `console.log` debris, no AI attribution +- [ ] Task closed: status → done, file moved to `docs/tasks/done/`, t098 in commit + +## Notes + +Decided with the user via two grills: (1) "reuse the pin" over "hands-off"; then, +after surfacing the code fact that one live Slack tab refreshes all workspaces' +creds, (2) "don't reopen — pin owns it" with a single-tab cred lifeline. The keeper +was over-aggressive: per-workspace tabs are not needed for the sweep — only one live +Slack tab is. + +--- + +_When task status flips to `done`, move this file to `done/`._ diff --git a/main.js b/main.js index 4bc2a33..3aa83a7 100644 --- a/main.js +++ b/main.js @@ -73,7 +73,13 @@ function readSettingsFile() { } } -const persist = (s) => fs.writeFileSync(settingsPath, JSON.stringify(s, null, 2)) +const persist = (s) => { + try { + fs.writeFileSync(settingsPath, JSON.stringify(s, null, 2)) + } catch (e) { + console.error("[main] settings persist failed:", e.message) + } +} const initialSettings = readSettingsFile() const hadLegacyKeys = diff --git a/preload.js b/preload.js index a1a75d2..275291b 100644 --- a/preload.js +++ b/preload.js @@ -23,7 +23,11 @@ contextBridge.exposeInMainWorld("cdp", { copyToClipboard: (text) => ipcRenderer.invoke("cdp:copy-to-clipboard", text), readClipboard: () => ipcRenderer.invoke("cdp:read-clipboard"), readClipboardImage: () => ipcRenderer.invoke("cdp:read-clipboard-image"), - onSwipe: (cb) => ipcRenderer.on("cdp:swipe", (_, direction) => cb(direction)), + onSwipe: (cb) => { + const handler = (_, direction) => cb(direction) + ipcRenderer.on("cdp:swipe", handler) + return () => ipcRenderer.removeListener("cdp:swipe", handler) + }, // Pins getPins: () => ipcRenderer.invoke("cdp:get-pins"), addPin: (pin) => ipcRenderer.invoke("cdp:add-pin", pin), diff --git a/src/app.tsx b/src/app.tsx index b008fa5..5a28d8c 100644 --- a/src/app.tsx +++ b/src/app.tsx @@ -185,9 +185,14 @@ export default function App() { // view and the browser column is a destination. Width-gated only — never pointer. const shellMode = useShellMode() const isCoarsePointer = usePointerCoarse() - const [phoneView, setPhoneView] = useState<"inbox" | "reader" | "tabs" | "browser">("inbox") - // The entry the Conversation Reader is showing (phone shell only, t077). - const [readerEntry, setReaderEntry] = useState(null) + // Phone shell view (t077). A discriminated union so the reader's entry lives *on* the + // "reader" view instead of a parallel state that could drift out of sync (t096, A7). + type PhoneView = + | { view: "inbox" } + | { view: "tabs" } + | { view: "browser" } + | { view: "reader"; entry: NotifEntry } + const [phoneView, setPhoneView] = useState({ view: "inbox" }) const shellModeRef = useRef(shellMode) useEffect(() => { shellModeRef.current = shellMode @@ -203,8 +208,9 @@ export default function App() { // (notificationsRef is the app-wide ref synced to the live notifications list.) const navPhone = useCallback( (view: "reader" | "tabs" | "browser", entry: NotifEntry | null = null) => { - setReaderEntry(entry) - setPhoneView(view) + setPhoneView( + view === "reader" ? (entry ? { view: "reader", entry } : { view: "inbox" }) : { view }, + ) if (shellModeRef.current !== "phone") return const cur = window.history.state as { phoneView?: string @@ -224,10 +230,7 @@ export default function App() { const inboxPhone = useCallback(() => { const depth = (window.history.state as { depth?: number } | null)?.depth ?? 0 if (depth > 0) window.history.go(-depth) - else { - setReaderEntry(null) - setPhoneView("inbox") - } + else setPhoneView({ view: "inbox" }) }, []) useEffect(() => { const onPop = (e: PopStateEvent) => { @@ -238,12 +241,10 @@ export default function App() { ? (notificationsRef.current.find((n) => n.id === st.entryId) ?? null) : null // The conversation vanished from the store (read + filtered) → fall back to the Inbox. - setReaderEntry(entry) - setPhoneView(entry ? "reader" : "inbox") + setPhoneView(entry ? { view: "reader", entry } : { view: "inbox" }) return } - setReaderEntry(null) - setPhoneView(view) + setPhoneView({ view }) } window.addEventListener("popstate", onPop) return () => window.removeEventListener("popstate", onPop) @@ -571,18 +572,39 @@ export default function App() { switchTabRef.current = switchTab }, [switchTab]) + // Apply an optimistic local patch to the notification list and revert it if the matching + // server write rejects — so a failed POST can't leave the bell/inbox diverged from the store + // (t096, A2). Deliberately a tiny helper, not a reducer/event-bus (ADR-0015): each handler + // keeps its own patch + write together. + const optimisticNotif = useCallback( + (patch: (prev: NotifEntry[]) => NotifEntry[], write: () => Promise | void) => { + const snapshot = notificationsRef.current + setNotifications(patch) + Promise.resolve(write()).catch(() => setNotifications(snapshot)) + }, + [], + ) + // Mark the whole conversation thread read: compute the key, gather unread siblings, // flip them all in state, then flush each id to the server. Shared by the click path // and the in-box `r` key so the logic lives in exactly one place. - const markThreadRead = useCallback((entry: NotifEntry) => { - const key = threadKey(entry) - const siblings = notificationsRef.current.filter( - (n) => n.id !== entry.id && !n.read && threadKey(n) === key, - ) - setNotifications((prev) => prev.map((n) => (threadKey(n) === key ? { ...n, read: true } : n))) - window.cdp.markNotificationRead(entry.id) - for (const n of siblings) window.cdp.markNotificationRead(n.id) - }, []) + const markThreadRead = useCallback( + (entry: NotifEntry) => { + const key = threadKey(entry) + const siblings = notificationsRef.current.filter( + (n) => n.id !== entry.id && !n.read && threadKey(n) === key, + ) + optimisticNotif( + (prev) => prev.map((n) => (threadKey(n) === key ? { ...n, read: true } : n)), + () => + Promise.all([ + window.cdp.markNotificationRead(entry.id), + ...siblings.map((n) => window.cdp.markNotificationRead(n.id)), + ]), + ) + }, + [optimisticNotif], + ) // Phone tap default (t077, ADR-0012): open the Conversation Reader — read without // activating the remote tab. Marks the thread read locally only (the desktop unread @@ -635,14 +657,18 @@ export default function App() { // Opening the popover does NOT mark read — unread clears only via a row click or // the explicit "Mark all read", so the dock/tab badges stay meaningful. const handleMarkAllRead = useCallback(() => { - window.cdp.markNotificationsRead() - setNotifications((prev) => prev.map((n) => ({ ...n, read: true }))) - }, []) + optimisticNotif( + (prev) => prev.map((n) => ({ ...n, read: true })), + () => window.cdp.markNotificationsRead(), + ) + }, [optimisticNotif]) const handleClearNotifications = useCallback(() => { - window.cdp.clearNotifications() - setNotifications([]) - }, []) + optimisticNotif( + () => [], + () => window.cdp.clearNotifications(), + ) + }, [optimisticNotif]) // Mark the selected row's whole thread read (the in-box `r` key) without opening it. const handleMarkThreadRead = markThreadRead @@ -650,37 +676,54 @@ export default function App() { // Clear a whole conversation (t085): remove every entry sharing the group's threadKey — // including the collapsed ones not shown in the capped group — in one tap. The renderer // computes the id set from the live store and posts it. - const handleClearThread = useCallback((entry: NotifEntry) => { - const key = threadKey(entry) - const ids = notificationsRef.current.filter((n) => threadKey(n) === key).map((n) => n.id) - if (!ids.length) return - setNotifications((prev) => prev.filter((n) => !ids.includes(n.id))) - window.cdp.removeNotifications(ids) - }, []) + const handleClearThread = useCallback( + (entry: NotifEntry) => { + const key = threadKey(entry) + const ids = notificationsRef.current.filter((n) => threadKey(n) === key).map((n) => n.id) + if (!ids.length) return + optimisticNotif( + (prev) => prev.filter((n) => !ids.includes(n.id)), + () => window.cdp.removeNotifications(ids), + ) + }, + [optimisticNotif], + ) // "Mute this channel" (t072): add the entry's {team, channelId} to the server-stored // Channel Exclude list so the sweep stops notifying it. Reads the current list from // ui-state, appends (deduped), and writes back. Also marks the entry read for instant feedback. - const handleMuteChannel = useCallback((entry: NotifEntry) => { - const target = excludeTargetFromEntry(entry) - if (!target) return - const label = entry.source || target.channelId - window.cdp.getUiState().then((s) => { - const current = Array.isArray(s.slackExcludes) ? (s.slackExcludes as SlackExclude[]) : [] - const next = addExclude(current, { ...target, label }) - if (next !== current) window.cdp.setUiState({ slackExcludes: next }) - }) - setNotifications((prev) => prev.map((n) => (n.id === entry.id ? { ...n, read: true } : n))) - window.cdp.markNotificationRead(entry.id) - }, []) + const handleMuteChannel = useCallback( + (entry: NotifEntry) => { + const target = excludeTargetFromEntry(entry) + if (!target) return + const label = entry.source || target.channelId + window.cdp.getUiState().then((s) => { + const current = Array.isArray(s.slackExcludes) ? (s.slackExcludes as SlackExclude[]) : [] + const next = addExclude(current, { ...target, label }) + if (next !== current) window.cdp.setUiState({ slackExcludes: next }) + }) + optimisticNotif( + (prev) => prev.map((n) => (n.id === entry.id ? { ...n, read: true } : n)), + () => window.cdp.markNotificationRead(entry.id), + ) + }, + [optimisticNotif], + ) // Toggling the per-row indicator flips read state without opening the notification. - const handleToggleRead = useCallback((entry: NotifEntry) => { - const read = !entry.read - setNotifications((prev) => prev.map((n) => (n.id === entry.id ? { ...n, read } : n))) - if (read) window.cdp.markNotificationRead(entry.id) - else window.cdp.markNotificationUnread(entry.id) - }, []) + const handleToggleRead = useCallback( + (entry: NotifEntry) => { + const read = !entry.read + optimisticNotif( + (prev) => prev.map((n) => (n.id === entry.id ? { ...n, read } : n)), + () => + read + ? window.cdp.markNotificationRead(entry.id) + : window.cdp.markNotificationUnread(entry.id), + ) + }, + [optimisticNotif], + ) // Live tab info for each linked pin (title/favicon/url), so a pin reflects its // tab's current title and detects URL drift. Built from the full tab list since @@ -919,6 +962,16 @@ export default function App() { if (tab) pinTab(tab) }, [pinTab, unpinPin]) + // The one place a close/switch planner directive's chosen next surface is activated — maps a + // nextActive ref onto the right path. Shared by closeTab and closeTabs (t096, A1). + const applyNextActive = useCallback( + async (next: { kind: "cdp" | "local"; id: string } | null | undefined) => { + if (next?.kind === "cdp") await switchTab(next.id) + else if (next?.kind === "local") switchLocalTab(next.id) + }, + [switchTab, switchLocalTab], + ) + const closeTab = useCallback( async (tabId: string) => { const tab = tabsRef.current.find((t) => t.id === tabId) @@ -953,10 +1006,9 @@ export default function App() { }) if (tab?.url) closedTabsRef.current.push(directive.closedEntry) activeOrderRef.current = dropActive(activeOrderRef.current, { kind: "cdp", id: tabId }) - if (directive.nextActive?.kind === "cdp") await switchTab(directive.nextActive.id) - else if (directive.nextActive?.kind === "local") switchLocalTab(directive.nextActive.id) + await applyNextActive(directive.nextActive) }, - [refreshTabs, switchTab, switchLocalTab], + [refreshTabs, applyNextActive], ) // Changing the CDP address invalidates all tab IDs, so reconnect to the @@ -1069,11 +1121,10 @@ export default function App() { locals: localTabsRef.current, pins: pinsRef.current, }) - if (directive.nextActive?.kind === "cdp") await switchTab(directive.nextActive.id) - else if (directive.nextActive?.kind === "local") switchLocalTab(directive.nextActive.id) + await applyNextActive(directive.nextActive) } }, - [refreshTabs, switchTab, switchLocalTab], + [refreshTabs, applyNextActive], ) const goBack = useCallback(() => { @@ -1230,9 +1281,11 @@ export default function App() { } }, [page, refreshTabs, updateNavHistory]) - // Trackpad swipe gestures + // Trackpad swipe gestures. onSwipe returns an unsubscribe — without it, every reconnect + // (goBack/goForward are keyed on `page`) would leak another cdp:swipe listener, so one + // swipe would eventually fire back/forward N times (t096, P6). useEffect(() => { - window.cdp.onSwipe((direction) => { + return window.cdp.onSwipe((direction) => { if (direction === "left") goBack() if (direction === "right") goForward() }) @@ -1792,7 +1845,7 @@ export default function App() { {/* Phone Shell root view (t076): the Inbox. The browser column below stays mounted (hidden) so the canvas, FindBar, and the Toolbar-hosted settings sheet keep working; only the Sidebar is truly absent on phone. */} - {shellMode === "phone" && phoneView === "inbox" && ( + {shellMode === "phone" && phoneView.view === "inbox" && ( )} {/* Flat tab/pin switcher (t081): read-and-go — tap opens the screencast view. */} - {shellMode === "phone" && phoneView === "tabs" && ( + {shellMode === "phone" && phoneView.view === "tabs" && ( { onOpenChange(false) setQuery("") - action.run() + // State is already cleaned up above, so a throwing run-fn can't leave the palette in an + // odd state — surface it as a toast instead of an unhandled error (t096, P19). + try { + action.run() + } catch { + toast.error(`Couldn't run "${action.name}"`) + } } const handleOpenChange = (next: boolean) => { diff --git a/src/components/find-bar.tsx b/src/components/find-bar.tsx index db20496..e79f451 100644 --- a/src/components/find-bar.tsx +++ b/src/components/find-bar.tsx @@ -50,7 +50,12 @@ export function FindBar({ page, ref }: FindBarProps) { page.clearFind() return } - page.find(query).then(({ total }) => dispatch({ type: "setTotal", total })) + // `setQuery` already reset total to 0; a rejected find (socket drop) just stays there + // instead of becoming an unhandled rejection (t096, P11). + page + .find(query) + .then(({ total }) => dispatch({ type: "setTotal", total })) + .catch(() => {}) }, [page], ) diff --git a/src/components/settings-dialog.tsx b/src/components/settings-dialog.tsx index 7e5b85e..e3de60e 100644 --- a/src/components/settings-dialog.tsx +++ b/src/components/settings-dialog.tsx @@ -379,14 +379,27 @@ export function SettingsDialog({ setPort(p) setSaved({ host: config.host, port: p }) }) - // Virtual-pointer mode + scroll offset come from ui-state on every build (not web-gated). - // Restore scroll in a rAF so the content has laid out before we set scrollTop. + // One ui-state load for the whole dialog (t096, A3): virtual-pointer + scroll offset + // unconditionally, plus the web-only push + excludes fields under the same read — was + // two separate getUiState() calls in this effect. window.cdp.getUiState().then((s) => { setVirtualPointer(parseMode(s[VIRTUAL_POINTER_MODE_KEY])) const top = Number(s.settingsScrollTop) || 0 requestAnimationFrame(() => { if (scrollRef.current) scrollRef.current.scrollTop = top }) + if (!caps.web) return + const granted = typeof Notification !== "undefined" && Notification.permission === "granted" + setWebPush(!!s.webPush && granted) + setPushPermBlocked( + typeof Notification !== "undefined" && Notification.permission === "denied", + ) + // Re-validate the push subscription on open when push is enabled. This catches + // subscription rotation/expiry and re-subscribes if needed (idempotent). + if (s.webPush && granted) { + reValidateSubscription() + } + setSlackExcludes(Array.isArray(s.slackExcludes) ? s.slackExcludes : []) }) if (caps.web) { setIsStandalone((navigator as unknown as { standalone?: boolean }).standalone === true) @@ -400,20 +413,6 @@ export function SettingsDialog({ }) .catch(() => {}) } - window.cdp.getUiState().then((s) => { - const granted = - typeof Notification !== "undefined" && Notification.permission === "granted" - setWebPush(!!s.webPush && granted) - setPushPermBlocked( - typeof Notification !== "undefined" && Notification.permission === "denied", - ) - // Re-validate the push subscription on open when push is enabled. This catches - // subscription rotation/expiry and re-subscribes if needed (idempotent). - if (s.webPush && granted) { - reValidateSubscription() - } - setSlackExcludes(Array.isArray(s.slackExcludes) ? s.slackExcludes : []) - }) fetch("/api/notifications/health") .then((r) => r.json()) // t092: the payload is now `{ rows, groups }` (rows merged per Enterprise Grid org; diff --git a/src/lib/CLAUDE.md b/src/lib/CLAUDE.md index 0aed11a..71a1c36 100644 --- a/src/lib/CLAUDE.md +++ b/src/lib/CLAUDE.md @@ -22,7 +22,7 @@ Domain modules that form the renderer's logic layer, plus a React hook that wire **`local-tabs.ts`** — Local tab list logic (a local tab renders as an in-DOM ``; see `docs/adr/0005`). `LocalTab` is the renderer-held metadata shape. `sortPinnedFirst(tabs)` keeps pinned tabs atop the LOCAL TABS section (stable, returns same ref when already ordered). `toPersisted`/`fromPersisted` are the persistence split — all open local tabs are saved (carrying the `pinned` flag; live-only fields like loading/audio dropped) and rehydrated on launch. Pure; the `` elements + their event wiring live in `src/components/local-webviews.tsx`. -**`closed-tabs.ts`** — `createClosedStack()` is the unified close-ordered reopen stack. Entries are `{ kind: 'cdp' | 'local', url }`; `pop()` returns the most recently closed of either kind, so Cmd+Shift+T reopens it in its original kind. Pure. +**`closed-tabs.ts`** — `createClosedStack(cap = CLOSED_STACK_CAP)` is the unified close-ordered reopen stack. Entries are `{ kind: 'cdp' | 'local', url }`; `pop()` returns the most recently closed of either kind, so Cmd+Shift+T reopens it in its original kind. Bounded at `CLOSED_STACK_CAP` (50) — the oldest entry drops when the cap is exceeded, so a long session can't grow it without bound (t096). Pure. **`active-order.ts`** — MRU (most-recently-used) activation order across both CDP and local tabs. `touchActive(order, entry)` moves an `ActiveRef` (`{ kind, id }`) to the tail (most-recent); `dropActive(order, entry)` removes it; `mostRecent(order, isOpen)` returns the newest entry still open — drives "which tab to activate when the current one closes" across kinds. Pure; no side effects. @@ -94,7 +94,13 @@ The transport is split into three named seams assembled by a thin shim: **`crypto-context.ts`** — the single owner of E2E in the web build. `createCryptoContext(init)` wraps `crypto-envelope.ts` seal/open into one object (`sealText`/`openText`/`confirm`/`mode`/`ready`). The uplink router seals every client→server body once before it leaves; the downlink dispatcher opens every server→client payload once on the way in. No transport re-touches crypto. -**`cdp-web-transport.ts`** — thin assembler. Constructs a `Downlink`, `UplinkRouter`, and `CryptoContext`, wires them together, and exposes the REST bridge (tabs/config/ui-state/pins/notifications/theme) plus the `CdpBridge` surface as `window.cdp`. Also holds Web Push methods (`getPushVapidKey`, `subscribePush`, `unsubscribePush` — absent under Electron) and the auto-reconnect driver (`createReconnectDriver`, t040): the Downlink's real-drop signal kicks a bounded-backoff loop (schedule from `reconnect-backoff.ts`) that re-POSTs `/api/connect` for the last tab; the renderer's `onDisconnected` listeners read the driver's phase (`"reconnecting"` → progress, `"lost"` → terminal) instead of the raw dispatch, and a fresh `connect` (tab switch) resets the schedule + cancels any queued retry. The driver also exposes `reconnectNow()` (t042) — the manual force-reconnect verb the status-bar / settings Reconnect tap drives via the bridge's `reconnect()`: it bumps the generation guard, cancels the pending backoff timer, resets the schedule to base, and re-enters the *same* `connect` path immediately for the last tab (one loop, no second counter; rapid taps supersede via the guard). Also holds the visible-tab WS re-climb timer (t041) — orthogonal to the Remote Page driver: while the document is foregrounded and WS is the intended-but-down transport, a `setTimer`-driven loop re-attempts `openWs()` on the `createWsReclimbSchedule` cadence (one attempt per armed timer; a WS `onClose` re-arms the next spaced attempt, `onReady` stops + resets), gated by `shouldReconnect` and armed/disarmed on `visibilitychange` so a backgrounded PWA never hammers the proxy. Routes mouse input: drag (button held) → `batcher.coalesce`; hover (no button) → `hover.move` (bypassed on WS/stream); press/release → `hover.cancel` + `batcher.immediate`; wheel → `batcher.append`. `collapseMoves(items)` — the CDP-specific merge function for `createSingleFlight`. +**`cdp-web-transport.ts`** — thin assembler. Constructs a `Downlink`, `UplinkRouter`, and `CryptoContext`, wires them together, and exposes the REST bridge (tabs/config/ui-state/pins/notifications/theme) plus the `CdpBridge` surface as `window.cdp`. Also holds Web Push methods (`getPushVapidKey`, `subscribePush`, `unsubscribePush` — absent under Electron) and the auto-reconnect driver (`createReconnectDriver`, t040): the Downlink's real-drop signal kicks a bounded-backoff loop (schedule from `reconnect-backoff.ts`) that re-POSTs `/api/connect` for the last tab; the renderer's `onDisconnected` listeners read the driver's phase (`"reconnecting"` → progress, `"lost"` → terminal) instead of the raw dispatch, and a fresh `connect` (tab switch) resets the schedule + cancels any queued retry. The driver also exposes `reconnectNow()` (t042) — the manual force-reconnect verb the status-bar / settings Reconnect tap drives via the bridge's `reconnect()`: it bumps the generation guard, cancels the pending backoff timer, resets the schedule to base, and re-enters the *same* `connect` path immediately for the last tab (one loop, no second counter; rapid taps supersede via the guard). Also holds the visible-tab WS re-climb timer (t041) — orthogonal to the Remote Page driver: while the document is foregrounded and WS is the intended-but-down transport, a `setTimer`-driven loop re-attempts `openWs()` on the `createWsReclimbSchedule` cadence (one attempt per armed timer; a WS `onClose` re-arms the next spaced attempt, `onReady` stops + resets), gated by `shouldReconnect` and armed/disarmed on `visibilitychange` so a backgrounded PWA never hammers the proxy. Routes mouse input: drag (button held) → `batcher.coalesce`; hover (no button) → `hover.move` (bypassed on WS/stream); press/release → `hover.cancel` + `batcher.immediate`; wheel → `batcher.append`. `collapseMoves(items)` — the CDP-specific merge function for `createSingleFlight`. Three already-DI'd effectful factories were lifted out into their own files for an isolated test surface (t096, A5): `web-ws-channel.ts`, `web-input-channel.ts`, `web-reconnect-driver.ts` (below). `cdp-web-transport.ts` still owns the `Cmd`/`WebTransportDeps` types (the factories import them type-only, no runtime cycle) and the assembly. + +**`web-ws-channel.ts`** — `createWsChannel(deps, crypto, opts)` is the optional full-duplex WebSocket channel (t019): one socket carries events in + invokes/sends/batches out, the binary-frame fast path, the ping/RTT keepalive, and the paint-ack control. Returns `isReady/send/invoke/batch/paintAck/close`. Extracted from `cdp-web-transport.ts` (t096); tested by `web-ws-channel.test.ts` (open/ready/send/invoke round-trip/event fan-out/binary-frame pairing/close suppression). + +**`web-input-channel.ts`** — `createInputChannel(deps, postFallback)` is the low-latency streaming input POST (t011): one long-lived `/api/input-stream` body streams NDJSON batches once an SSE `stream-ack` confirms it; until then (and forever if a buffering proxy swallows the probe) it falls back to per-flush POST. Also owns `SUPPORTS_REQUEST_STREAMING`. Extracted (t096); tested by `web-input-channel.test.ts` (fallback-before-ack contract). + +**`web-reconnect-driver.ts`** — `createReconnectDriver(opts)` + `RECONNECT_CONFIG`: the effectful auto-reconnect loop (t040) driving the pure `reconnect-backoff` schedule on a real drop, with `noteConnect`/`onDrop`/`reconnectNow`/`stop` and a generation guard mirroring the server `connectId` race-guard. Extracted (t096); tested by `web-reconnect-driver.test.ts`. **`input-coalesce.ts`** — generic batching + backpressure primitives (no CDP-specific logic): - `createBatcher` — coalesces high-frequency commands onto a scheduler (one POST per rAF instead of one per event). diff --git a/src/lib/cdp-web-transport.characterization.test.ts b/src/lib/cdp-web-transport.characterization.test.ts index 7b32619..6e41563 100644 --- a/src/lib/cdp-web-transport.characterization.test.ts +++ b/src/lib/cdp-web-transport.characterization.test.ts @@ -8,14 +8,10 @@ */ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" -import { - collapseMoves, - createReconnectDriver, - createWebCdp, - type WebTransportDeps, -} from "./cdp-web-transport" +import { collapseMoves, createWebCdp, type WebTransportDeps } from "./cdp-web-transport" import { deriveKey, open as envOpen, seal as envSeal } from "./crypto-envelope" import type { BackoffConfig } from "./reconnect-backoff" +import { createReconnectDriver } from "./web-reconnect-driver" // --- fakes ---------------------------------------------------------------------------- diff --git a/src/lib/cdp-web-transport.ts b/src/lib/cdp-web-transport.ts index dc0821f..eebd0bc 100644 --- a/src/lib/cdp-web-transport.ts +++ b/src/lib/cdp-web-transport.ts @@ -21,15 +21,8 @@ import { type DownlinkSourceHandlers, } from "./downlink-dispatcher" import { type Batch, createBatcher, createHoverGate, createSingleFlight } from "./input-coalesce" -import { noteFrameAge, notePing, notePong, resetLatencyMetrics } from "./latency-metrics" +import { noteFrameAge } from "./latency-metrics" import { isMuted } from "./notif-mutes" -import { perfMark } from "./perf-mark" -import { - type BackoffConfig, - type BackoffState, - initialBackoff, - nextBackoff, -} from "./reconnect-backoff" import { createTransportSelector, createWsReclimbSchedule, @@ -37,12 +30,15 @@ import { shouldReconnect, } from "./transport-selector" import { type AdvisedMode, createUplinkRouter, type Uplink } from "./uplink-router" +import { createInputChannel } from "./web-input-channel" +import { createReconnectDriver, RECONNECT_CONFIG } from "./web-reconnect-driver" +import { createWsChannel } from "./web-ws-channel" // Capabilities moved to ./caps (the pure source of truth). Re-exported so existing // importers of this module keep working — see docs/conventions/feature-gates.md. export { getCaps, type WebCaps } -type Cmd = { method: string; params?: unknown } +export type Cmd = { method: string; params?: unknown } // Generate or retrieve a stable device ID for per-device state (e.g. webPush toggle). // Stored in localStorage so it persists across page reloads; on the iPad PWA it resets @@ -97,128 +93,6 @@ function resolveDeps(): WebTransportDeps { } } -// Bounded-backoff defaults for auto-reconnect on a real drop (t040). 0.5s → 1s → 2s → -// 4s → 8s → 16s (capped at 16s), giving up after 10 tries (~2 min of retries) — long -// enough to ride out a host restart / network blip, bounded so a dead host settles on a -// terminal "Disconnected" instead of retrying forever. -const RECONNECT_CONFIG: BackoffConfig = { - baseMs: 500, - factor: 2, - capMs: 16000, - maxAttempts: 10, -} - -/** - * The effectful reconnect loop (t040) — the pure schedule's caller. On a real Remote Page - * drop it re-invokes `connect(lastTabId)` on the bounded-backoff cadence, surfacing a - * "reconnecting" phase while it retries and a terminal "lost" once the ceiling is hit. A - * fresh `connect` (a tab switch, or a retry that lands) resets the schedule and cancels any - * queued retry; `stop()` (host-initiated teardown) does the same. The server-side - * `connectId` race-guard discards a retry that resolves after a newer connect, so this loop - * never promotes a stale socket — it just drives `connect` through the same guard. - */ -export function createReconnectDriver(opts: { - connect: (tabId: string) => Promise<{ ok?: boolean; error?: string }> - emit: (phase: "reconnecting" | "lost") => void - config?: BackoffConfig - setTimer?: (cb: () => void, ms: number) => ReturnType - clearTimer?: (h: ReturnType) => void -}) { - const cfg = opts.config ?? RECONNECT_CONFIG - const setTimer = opts.setTimer ?? ((cb, ms) => setTimeout(cb, ms)) - const clearTimer = opts.clearTimer ?? ((h) => clearTimeout(h)) - - let state: BackoffState = initialBackoff() - let lastTabId: string | null = null - let pending: ReturnType | null = null - // Bumped on every connect()/stop(); a retry whose timer fires for a stale generation is - // dropped (the renderer-side mirror of the connector's connectId guard). - let generation = 0 - - function cancelPending() { - if (pending !== null) { - clearTimer(pending) - pending = null - } - } - - function scheduleNext() { - const { state: next, step } = nextBackoff(state, "drop", cfg) - state = next - if (step.giveUp) { - opts.emit("lost") - return - } - opts.emit("reconnecting") - const myGen = generation - pending = setTimer(async () => { - pending = null - if (myGen !== generation || lastTabId === null) return - const result = await opts.connect(lastTabId) - if (myGen !== generation) return // a newer connect/stop superseded this retry - if (result?.ok) { - state = nextBackoff(state, "success", cfg).state - return - } - // "cancelled" means a newer connect took the slot — stop quietly (gen already bumped - // in that case). Any other error is the host still being down → climb the next rung. - if (result?.error !== "cancelled") scheduleNext() - }, step.delayMs) - } - - return { - /** A fresh, intentional connect (tab switch or initial). Records the target, resets the - * schedule, and cancels any queued retry so the loop never races a deliberate connect. */ - noteConnect(tabId: string) { - lastTabId = tabId - generation++ - cancelPending() - state = nextBackoff(state, "success", cfg).state - }, - /** A real drop surfaced by the Downlink. Kicks the backoff loop. */ - onDrop() { - if (lastTabId === null) { - // Never connected (or host-disconnected) — surface the terminal loss, don't retry. - opts.emit("lost") - return - } - cancelPending() - scheduleNext() - }, - /** A manual force-reconnect (status-bar / settings tap, later the ⌘K command). Cancels - * any pending backoff timer, resets the schedule to its base delay, and re-enters the - * *same* `connect` path the auto-loop uses — immediately, for the last tab — never a - * second competing loop. Bumping `generation` first supersedes any queued auto-retry - * (the renderer mirror of the server `connectId` guard), so rapid taps don't stack: a - * later tap discards the earlier attempt instead of opening a second socket. */ - reconnectNow() { - if (lastTabId === null) return // nothing to reconnect to (never connected / host gone) - generation++ - cancelPending() - state = initialBackoff() - const tabId = lastTabId - const myGen = generation - opts.emit("reconnecting") - void opts.connect(tabId).then((result) => { - if (myGen !== generation) return // a newer connect/tap/stop superseded this attempt - if (result?.ok) { - state = nextBackoff(state, "success", cfg).state - return - } - // Host still down → fall into the normal bounded-backoff climb (one loop, shared cfg). - if (result?.error !== "cancelled") scheduleNext() - }) - }, - /** Host-initiated teardown — stop retrying and forget the target. */ - stop() { - generation++ - cancelPending() - lastTabId = null - state = initialBackoff() - }, - } -} - const isMouseMove = (c: Cmd) => c.method === "Input.dispatchMouseEvent" && (c.params as { type?: string } | undefined)?.type === "mouseMoved" @@ -282,329 +156,6 @@ function createRestBridge(deps: WebTransportDeps, crypto: CryptoContext) { } } -// Chrome/Edge can stream a request body (ReadableStream + duplex:'half') over HTTP/2. -// Detection per the documented pattern: duplex is read and no Content-Type is auto-set. -const SUPPORTS_REQUEST_STREAMING = (() => { - if (typeof ReadableStream === "undefined" || typeof Request === "undefined") return false - try { - let duplexAccessed = false - const hasContentType = new Request("http://x", { - body: new ReadableStream(), - method: "POST", - get duplex() { - duplexAccessed = true - return "half" - }, - // biome-ignore lint/suspicious/noExplicitAny: duplex not yet in lib.dom RequestInit - } as any).headers.has("Content-Type") - return duplexAccessed && !hasContentType - } catch { - return false - } -})() - -/** - * The low-latency input path: one long-lived POST whose body streams NDJSON frames, - * so input flushes don't each pay a fresh request's TLS/auth/RTT through the proxy - * chain. Pairs with the SSE down-channel — no WebSocket. See t011. - * - * Safety: a buffering proxy (Authentik/openresty without `proxy_request_buffering off`) - * would accept the stream but never deliver the body — input would vanish. So on open we - * send a `probe` frame and only switch real input onto the stream once the server echoes - * a `stream-ack` over SSE. Until confirmed (and forever, if the probe is never acked) we - * use a per-flush POST. `notifyAck()` is called by the SSE `stream-ack` handler. - */ -function createInputChannel(deps: WebTransportDeps, postFallback: (batch: Batch) => void) { - const enc = new TextEncoder() - // Give up after this many establish attempts that never get acked (no HTTP/2, or a - // buffering proxy) and stay on the POST fallback for good — don't loop forever. - const MAX_ATTEMPTS = 2 - let controller: ReadableStreamDefaultController | null = null - let abort: AbortController | null = null - let state: "idle" | "probing" | "streaming" | "blocked" = "idle" - let attempts = 0 - let watchdog: ReturnType | null = null - - function onSettle() { - if (watchdog) { - clearTimeout(watchdog) - watchdog = null - } - controller = null - if (state === "blocked") return - const wasStreaming = state === "streaming" // dropped after working ⇒ transient - state = "idle" - if (wasStreaming) attempts = 0 - else if (attempts >= MAX_ATTEMPTS) { - state = "blocked" - return - } - setTimeout(open, 1000) - } - - function open() { - if (!SUPPORTS_REQUEST_STREAMING || state !== "idle") return - state = "probing" - attempts++ - abort = new AbortController() - const body = new ReadableStream({ - start(c) { - controller = c - c.enqueue(enc.encode(`${JSON.stringify({ probe: 1 })}\n`)) - }, - }) - // Resolves only when the body closes (half-duplex) — we never close it, so settling - // means the channel dropped. Replies (incl. the probe ack) arrive over SSE. - deps - .fetch("/api/input-stream", { - method: "POST", - body, - signal: abort.signal, - duplex: "half", - // biome-ignore lint/suspicious/noExplicitAny: duplex not yet in lib.dom RequestInit - } as any) - .catch(() => {}) - .finally(onSettle) - // No ack in the window ⇒ a buffering proxy swallowed the body (fetch hangs) ⇒ abort - // so onSettle counts the failed attempt and eventually falls back permanently. - watchdog = setTimeout(() => { - if (state === "probing") abort?.abort() - }, 3000) - } - open() - - return { - send(batch: Batch) { - if (state === "streaming" && controller) { - try { - controller.enqueue(enc.encode(`${JSON.stringify(batch)}\n`)) - return - } catch { - state = "idle" - } - } - postFallback(batch) - }, - notifyAck() { - if (state === "probing") { - if (watchdog) { - clearTimeout(watchdog) - watchdog = null - } - attempts = 0 - state = "streaming" - } - }, - } -} - -/** - * Optional WebSocket channel (t019). When connected and ready, all CDP traffic — events - * in, invokes/sends/batches out — rides one full-duplex socket instead of SSE+POST. - * The picker in settings selects between Auto/Fastest(WS)/Streaming/Basic; this channel - * is the "Fastest" path and the head of the Auto fallback chain. Returns a thin handle - * the rest of createWebCdp consults via `ws.ready()` before falling back. - */ -function createWsChannel( - deps: WebTransportDeps, - crypto: CryptoContext, - opts: { - onEvent: (event: string, data: string) => void - /** Fast path for screencast: a text "cdp-frame" envelope pairs with a binary WS frame - * carrying raw JPEG bytes. Skips base64 + the JSON.parse of a 250KB envelope on the - * renderer main thread. The caller forges a CDP event with `dataBlob` populated and - * dispatches through the normal CDP event listeners. */ - onFrameBinary?: (cdpMsg: { - method: string - params: Record & { dataBlob: Blob } - }) => void - onReady: () => void - onClose: () => void - }, -) { - let socket: WebSocket | null = null - let ready = false - // Pending awaited invokes keyed by id; the server echoes the id in invoke-result. - let nextId = 1 - const pending = new Map void>() - // True when the outer asked us to close — suppresses the onClose callback so a stale - // socket's late `close` event doesn't clobber a newly-opened channel's outer state. - let suppressClose = false - - // Keepalive + RTT probe (t057). While the socket is ready, a plaintext `ping` carrying a - // client-monotonic stamp fires on a fixed interval regardless of input/frame traffic, so - // an idle WS isn't reaped by an upstream proxy (nginx default idle ~60s). The server pongs - // it straight back; the onmessage handler folds the round-trip into the RTT estimator. The - // ping is control traffic — never sealed, never routed through the dispatcher's CDP path. - const PING_INTERVAL_MS = 20000 - let pingSeq = 0 - let pingTimer: ReturnType | null = null - function sendPing() { - if (!socket || socket.readyState !== deps.WebSocket.OPEN) return - const seq = ++pingSeq - const sentAt = performance.now() - notePing(seq, sentAt) - try { - // `ts` carries the client's monotonic stamp; the server echoes it unchanged. RTT is - // measured against the local `outstanding` map keyed by `seq`, never the echoed `ts`. - socket.send(JSON.stringify({ t: "ping", seq, ts: sentAt })) - } catch {} - } - function startPingPump() { - if (pingTimer !== null) return - sendPing() // probe immediately so RTT lands without waiting a full interval - pingTimer = setInterval(sendPing, PING_INTERVAL_MS) - } - - // Ack-after-paint control (t056) — plaintext like ping (no user content), so it's E2E- - // agnostic and skips the seal round-trip. `frame-ack-mode` opts this client into the - // server's one-in-flight gate; `frame-ack` is the post-paint ack the viewport fires, - // releasing the slot so the server acks the remote and the next frame flows. - function sendControl(payload: unknown) { - if (!socket || socket.readyState !== deps.WebSocket.OPEN) return - try { - socket.send(JSON.stringify(payload)) - } catch {} - } - function stopPingPump() { - if (pingTimer !== null) { - clearInterval(pingTimer) - pingTimer = null - } - // The estimator degrades cleanly when WS is gone — report unavailable, not a stale RTT. - resetLatencyMetrics() - } - - function open() { - const proto = location.protocol === "https:" ? "wss:" : "ws:" - const url = `${proto}//${location.host}/api/ws` - try { - socket = new deps.WebSocket(url) - } catch { - opts.onClose() - return - } - // The binary frame fast path: server sends a text "cdp-frame" envelope first - // (containing metadata + sessionId) then the raw JPEG bytes as the next WS message. - // Delivering as Blob is faster than ArrayBuffer for createImageBitmap — no extra copy. - socket.binaryType = "blob" - let pendingFrame: { method: string; params: Record } | null = null - socket.onmessage = async (ev) => { - // Binary message → pair with the preceding "cdp-frame" envelope. - if (ev.data instanceof Blob) { - if (!pendingFrame) return // out-of-order binary, drop - const frame = pendingFrame - pendingFrame = null - opts.onFrameBinary?.({ - method: frame.method, - params: { ...frame.params, dataBlob: ev.data }, - }) - return - } - // Envelope is always plaintext JSON (the `t` field is routing metadata, like SSE - // event names). For event messages, `data` is the sealed CDP payload that onSse() - // unwraps. For invoke-result, the server passes the CDP invoke output through plain - // — these are server-generated, not user content. - const tEntry = performance.now() // [DEBUG-perf] - const text = ev.data as string - const tParse = performance.now() // [DEBUG-perf] - let msg: { - t: string - event?: string - data?: unknown - id?: number - result?: unknown - seq?: number - } - try { - msg = JSON.parse(text) - } catch { - return - } - // [DEBUG-perf] Only mark big-payload events (the cdp screencast frames) so we measure - // the actual bottleneck and not all the tiny notification envelopes. - if (msg.t === "event" && msg.event === "cdp" && text.length > 5000) { - perfMark("wsRecv", tParse - tEntry) - perfMark("jsonParse", performance.now() - tParse) - } - if (msg.t === "ready") { - ready = true - startPingPump() - sendControl({ t: "frame-ack-mode" }) // opt into the server's one-in-flight gate (t056) - opts.onReady() - } else if (msg.t === "pong") { - // Control traffic — never fanned out as a CDP event. Fold the round-trip into the - // always-on RTT/jitter estimator against the client clock (t057). - if (typeof msg.seq === "number") notePong(msg.seq, performance.now()) - } else if (msg.t === "event" && msg.event === "cdp-frame") { - // Stash metadata; the next binary message is the JPEG bytes for this frame. - pendingFrame = msg.data as { method: string; params: Record } - } else if (msg.t === "event" && msg.event && msg.data !== undefined) { - opts.onEvent(msg.event, msg.data as string) - } else if (msg.t === "invoke-result" && typeof msg.id === "number") { - const cb = pending.get(msg.id) - pending.delete(msg.id) - // Result is sealed under E2E (the routing envelope around it stays plaintext); the - // open is the CryptoContext's, the one downlink ingress for an invoke reply. - const result = - crypto.mode === "e2e" ? await crypto.openText(msg.result as string) : msg.result - cb?.(result) - } - } - socket.onclose = () => { - ready = false - socket = null - stopPingPump() - for (const cb of pending.values()) cb({ error: "ws closed" }) - pending.clear() - if (suppressClose) return // caller already handled the outer state on explicit close - opts.onClose() - } - socket.onerror = () => { - try { - socket?.close() - } catch {} - } - } - - async function rawSend(payload: unknown) { - if (!socket || socket.readyState !== deps.WebSocket.OPEN) return false - // One seal at the WS uplink egress — the whole `{ t, … }` envelope is sealed (the server - // opens it as one body), via the CryptoContext, not an inline key read. - const text = await crypto.sealText(payload) - try { - socket.send(text) - return true - } catch { - return false - } - } - - open() - - return { - isReady: () => ready, - close: () => { - suppressClose = true - socket?.close() - }, - send: (method: string, params?: unknown) => rawSend({ t: "send", method, params }), - paintAck: (sessionId: number) => sendControl({ t: "frame-ack", sessionId }), - batch: (items: Cmd[]) => rawSend({ t: "batch", items }), - invoke: (method: string, params?: unknown) => - new Promise((resolve) => { - const id = nextId++ - pending.set(id, resolve) - void rawSend({ t: "invoke", id, method, params }).then((ok) => { - if (!ok) { - pending.delete(id) - resolve({ error: "ws send failed" }) - } - }) - }), - } -} - export function createWebCdp(deps: WebTransportDeps = resolveDeps()): CdpBridge { // E2E lives at the seam boundary, not per call (t023). The CryptoContext is the single // owner of the envelope: it is the only thing that calls `envSeal`/`envOpen`. Built once at @@ -1282,7 +833,7 @@ export function createWebCdp(deps: WebTransportDeps = resolveDeps()): CdpBridge // Web reads the clipboard from the native `paste` event (app.tsx), not here — the // async Clipboard API can't reliably read images on Safari/iPad. Stub returns null. readClipboardImage: async () => null, - onSwipe: () => {}, // no trackpad swipe over the web + onSwipe: () => () => {}, // no trackpad swipe over the web — no-op subscribe + unsubscribe getPins: () => rest.getJson("/api/pins"), addPin: (pin) => rest.postJson("/api/pins/add", pin), updatePin: (id, patch) => rest.postJson("/api/pins/update", { id, patch }), diff --git a/src/lib/closed-tabs.test.ts b/src/lib/closed-tabs.test.ts index 62e8e16..7c93ccc 100644 --- a/src/lib/closed-tabs.test.ts +++ b/src/lib/closed-tabs.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest" -import { createClosedStack } from "./closed-tabs" +import { CLOSED_STACK_CAP, createClosedStack } from "./closed-tabs" describe("createClosedStack", () => { it("pops the most recently closed entry", () => { @@ -21,4 +21,27 @@ describe("createClosedStack", () => { it("returns undefined when empty", () => { expect(createClosedStack().pop()).toBeUndefined() }) + + it("drops the oldest entry when the cap is exceeded", () => { + const s = createClosedStack(2) + s.push({ kind: "cdp", url: "https://a.com" }) + s.push({ kind: "cdp", url: "https://b.com" }) + s.push({ kind: "cdp", url: "https://c.com" }) + + expect(s.pop()).toEqual({ kind: "cdp", url: "https://c.com" }) + expect(s.pop()).toEqual({ kind: "cdp", url: "https://b.com" }) + expect(s.pop()).toBeUndefined() + }) + + it("keeps only the most recent entries up to the default cap", () => { + const s = createClosedStack() + for (let i = 0; i < CLOSED_STACK_CAP + 10; i++) { + s.push({ kind: "cdp", url: `https://x${i}.com` }) + } + + let survivors = 0 + while (s.pop()) survivors++ + + expect(survivors).toBe(CLOSED_STACK_CAP) + }) }) diff --git a/src/lib/closed-tabs.ts b/src/lib/closed-tabs.ts index 8029d5d..bc4ef21 100644 --- a/src/lib/closed-tabs.ts +++ b/src/lib/closed-tabs.ts @@ -11,14 +11,22 @@ export interface ClosedStack { pop(): ClosedEntry | undefined } +/** Bound on retained closed entries — drops the oldest beyond this. */ +export const CLOSED_STACK_CAP = 50 + /** * Tracks closed tabs of either kind so Cmd+Shift+T reopens the most recently * closed one in its original kind, preserving close order across CDP and local. + * Bounded at `cap` entries (oldest dropped) so a long session can't grow it + * without limit. */ -export function createClosedStack(): ClosedStack { +export function createClosedStack(cap = CLOSED_STACK_CAP): ClosedStack { const entries: ClosedEntry[] = [] return { - push: (entry) => void entries.push(entry), + push: (entry) => { + entries.push(entry) + if (entries.length > cap) entries.shift() + }, pop: () => entries.pop(), } } diff --git a/src/lib/web-input-channel.test.ts b/src/lib/web-input-channel.test.ts new file mode 100644 index 0000000..d4d3462 --- /dev/null +++ b/src/lib/web-input-channel.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it, vi } from "vitest" +import type { WebTransportDeps } from "./cdp-web-transport" +import type { Batch } from "./input-coalesce" +import { createInputChannel } from "./web-input-channel" + +// A fetch that never settles — models the streaming probe hanging (no SSE stream-ack), so the +// channel never flips to "streaming". Keeps the test deterministic regardless of whether the +// env reports request-streaming support. +function makeDeps(): WebTransportDeps { + return { + fetch: vi.fn(() => new Promise(() => {})), + EventSource: vi.fn() as unknown as typeof EventSource, + WebSocket: vi.fn() as unknown as typeof WebSocket, + } +} + +const batchOf = (method: string): Batch<{ method: string; params?: unknown }> => ({ + items: [{ method }], + seq: 0, +}) + +describe("createInputChannel", () => { + it("routes sends to the POST fallback until the stream is confirmed", () => { + const postFallback = vi.fn() + const ch = createInputChannel(makeDeps(), postFallback) + + const b = batchOf("Input.dispatchMouseEvent") + ch.send(b) + + // No stream-ack (SSE) has confirmed the channel yet, so it is never in "streaming" — + // regardless of whether the env supports request streaming — and the fallback owns the send. + expect(postFallback).toHaveBeenCalledWith(b) + }) +}) diff --git a/src/lib/web-input-channel.ts b/src/lib/web-input-channel.ts new file mode 100644 index 0000000..4dadea3 --- /dev/null +++ b/src/lib/web-input-channel.ts @@ -0,0 +1,125 @@ +/** + * Web build — the low-latency streaming input channel, lifted out of `cdp-web-transport.ts` + * for an isolated test surface (t096, A5). + * + * The low-latency input path: one long-lived POST whose body streams NDJSON frames, + * so input flushes don't each pay a fresh request's TLS/auth/RTT through the proxy + * chain. Pairs with the SSE down-channel — no WebSocket. See t011. + * + * Safety: a buffering proxy (Authentik/openresty without `proxy_request_buffering off`) + * would accept the stream but never deliver the body — input would vanish. So on open we + * send a `probe` frame and only switch real input onto the stream once the server echoes + * a `stream-ack` over SSE. Until confirmed (and forever, if the probe is never acked) we + * use a per-flush POST. `notifyAck()` is called by the SSE `stream-ack` handler. + */ + +import type { Cmd, WebTransportDeps } from "./cdp-web-transport" +import type { Batch } from "./input-coalesce" + +// Chrome/Edge can stream a request body (ReadableStream + duplex:'half') over HTTP/2. +// Detection per the documented pattern: duplex is read and no Content-Type is auto-set. +export const SUPPORTS_REQUEST_STREAMING = (() => { + if (typeof ReadableStream === "undefined" || typeof Request === "undefined") return false + try { + let duplexAccessed = false + const hasContentType = new Request("http://x", { + body: new ReadableStream(), + method: "POST", + get duplex() { + duplexAccessed = true + return "half" + }, + // biome-ignore lint/suspicious/noExplicitAny: duplex not yet in lib.dom RequestInit + } as any).headers.has("Content-Type") + return duplexAccessed && !hasContentType + } catch { + return false + } +})() + +export function createInputChannel( + deps: WebTransportDeps, + postFallback: (batch: Batch) => void, +) { + const enc = new TextEncoder() + // Give up after this many establish attempts that never get acked (no HTTP/2, or a + // buffering proxy) and stay on the POST fallback for good — don't loop forever. + const MAX_ATTEMPTS = 2 + let controller: ReadableStreamDefaultController | null = null + let abort: AbortController | null = null + let state: "idle" | "probing" | "streaming" | "blocked" = "idle" + let attempts = 0 + let watchdog: ReturnType | null = null + + function onSettle() { + if (watchdog) { + clearTimeout(watchdog) + watchdog = null + } + controller = null + if (state === "blocked") return + const wasStreaming = state === "streaming" // dropped after working ⇒ transient + state = "idle" + if (wasStreaming) attempts = 0 + else if (attempts >= MAX_ATTEMPTS) { + state = "blocked" + return + } + setTimeout(open, 1000) + } + + function open() { + if (!SUPPORTS_REQUEST_STREAMING || state !== "idle") return + state = "probing" + attempts++ + abort = new AbortController() + const body = new ReadableStream({ + start(c) { + controller = c + c.enqueue(enc.encode(`${JSON.stringify({ probe: 1 })}\n`)) + }, + }) + // Resolves only when the body closes (half-duplex) — we never close it, so settling + // means the channel dropped. Replies (incl. the probe ack) arrive over SSE. + deps + .fetch("/api/input-stream", { + method: "POST", + body, + signal: abort.signal, + duplex: "half", + // biome-ignore lint/suspicious/noExplicitAny: duplex not yet in lib.dom RequestInit + } as any) + .catch(() => {}) + .finally(onSettle) + // No ack in the window ⇒ a buffering proxy swallowed the body (fetch hangs) ⇒ abort + // so onSettle counts the failed attempt and eventually falls back permanently. + watchdog = setTimeout(() => { + if (state === "probing") abort?.abort() + }, 3000) + } + open() + + return { + send(batch: Batch) { + if (state === "streaming" && controller) { + try { + controller.enqueue(enc.encode(`${JSON.stringify(batch)}\n`)) + return + } catch { + state = "idle" + } + } + postFallback(batch) + }, + notifyAck() { + if (state === "probing") { + if (watchdog) { + clearTimeout(watchdog) + watchdog = null + } + attempts = 0 + state = "streaming" + } + }, + } +} diff --git a/src/lib/reconnect-driver.test.ts b/src/lib/web-reconnect-driver.test.ts similarity index 98% rename from src/lib/reconnect-driver.test.ts rename to src/lib/web-reconnect-driver.test.ts index 361041d..7ed00fd 100644 --- a/src/lib/reconnect-driver.test.ts +++ b/src/lib/web-reconnect-driver.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from "vitest" -import { createReconnectDriver } from "./cdp-web-transport" +import { createReconnectDriver } from "./web-reconnect-driver" // A controllable timer + connect stub so the effectful driver runs without real waits or // sockets. `fireTimers` drains the queue (newest scheduled first matches the loop's single diff --git a/src/lib/web-reconnect-driver.ts b/src/lib/web-reconnect-driver.ts new file mode 100644 index 0000000..424d40f --- /dev/null +++ b/src/lib/web-reconnect-driver.ts @@ -0,0 +1,134 @@ +/** + * Web build — the effectful auto-reconnect loop, lifted out of `cdp-web-transport.ts` so it + * has an isolated test surface (t096, A5). It drives the pure `reconnect-backoff` schedule: + * the assembler hangs this off the Downlink's real-drop signal and the manual Reconnect tap. + */ + +import { + type BackoffConfig, + type BackoffState, + initialBackoff, + nextBackoff, +} from "./reconnect-backoff" + +// Bounded-backoff defaults for auto-reconnect on a real drop (t040). 0.5s → 1s → 2s → +// 4s → 8s → 16s (capped at 16s), giving up after 10 tries (~2 min of retries) — long +// enough to ride out a host restart / network blip, bounded so a dead host settles on a +// terminal "Disconnected" instead of retrying forever. +export const RECONNECT_CONFIG: BackoffConfig = { + baseMs: 500, + factor: 2, + capMs: 16000, + maxAttempts: 10, +} + +/** + * The effectful reconnect loop (t040) — the pure schedule's caller. On a real Remote Page + * drop it re-invokes `connect(lastTabId)` on the bounded-backoff cadence, surfacing a + * "reconnecting" phase while it retries and a terminal "lost" once the ceiling is hit. A + * fresh `connect` (a tab switch, or a retry that lands) resets the schedule and cancels any + * queued retry; `stop()` (host-initiated teardown) does the same. The server-side + * `connectId` race-guard discards a retry that resolves after a newer connect, so this loop + * never promotes a stale socket — it just drives `connect` through the same guard. + */ +export function createReconnectDriver(opts: { + connect: (tabId: string) => Promise<{ ok?: boolean; error?: string }> + emit: (phase: "reconnecting" | "lost") => void + config?: BackoffConfig + setTimer?: (cb: () => void, ms: number) => ReturnType + clearTimer?: (h: ReturnType) => void +}) { + const cfg = opts.config ?? RECONNECT_CONFIG + const setTimer = opts.setTimer ?? ((cb, ms) => setTimeout(cb, ms)) + const clearTimer = opts.clearTimer ?? ((h) => clearTimeout(h)) + + let state: BackoffState = initialBackoff() + let lastTabId: string | null = null + let pending: ReturnType | null = null + // Bumped on every connect()/stop(); a retry whose timer fires for a stale generation is + // dropped (the renderer-side mirror of the connector's connectId guard). + let generation = 0 + + function cancelPending() { + if (pending !== null) { + clearTimer(pending) + pending = null + } + } + + function scheduleNext() { + const { state: next, step } = nextBackoff(state, "drop", cfg) + state = next + if (step.giveUp) { + opts.emit("lost") + return + } + opts.emit("reconnecting") + const myGen = generation + pending = setTimer(async () => { + pending = null + if (myGen !== generation || lastTabId === null) return + const result = await opts.connect(lastTabId) + if (myGen !== generation) return // a newer connect/stop superseded this retry + if (result?.ok) { + state = nextBackoff(state, "success", cfg).state + return + } + // "cancelled" means a newer connect took the slot — stop quietly (gen already bumped + // in that case). Any other error is the host still being down → climb the next rung. + if (result?.error !== "cancelled") scheduleNext() + }, step.delayMs) + } + + return { + /** A fresh, intentional connect (tab switch or initial). Records the target, resets the + * schedule, and cancels any queued retry so the loop never races a deliberate connect. */ + noteConnect(tabId: string) { + lastTabId = tabId + generation++ + cancelPending() + state = nextBackoff(state, "success", cfg).state + }, + /** A real drop surfaced by the Downlink. Kicks the backoff loop. */ + onDrop() { + if (lastTabId === null) { + // Never connected (or host-disconnected) — surface the terminal loss, don't retry. + opts.emit("lost") + return + } + cancelPending() + scheduleNext() + }, + /** A manual force-reconnect (status-bar / settings tap, later the ⌘K command). Cancels + * any pending backoff timer, resets the schedule to its base delay, and re-enters the + * *same* `connect` path the auto-loop uses — immediately, for the last tab — never a + * second competing loop. Bumping `generation` first supersedes any queued auto-retry + * (the renderer mirror of the server `connectId` guard), so rapid taps don't stack: a + * later tap discards the earlier attempt instead of opening a second socket. */ + reconnectNow() { + if (lastTabId === null) return // nothing to reconnect to (never connected / host gone) + generation++ + cancelPending() + state = initialBackoff() + const tabId = lastTabId + const myGen = generation + opts.emit("reconnecting") + void opts.connect(tabId).then((result) => { + if (myGen !== generation) return // a newer connect/tap/stop superseded this attempt + if (result?.ok) { + state = nextBackoff(state, "success", cfg).state + return + } + // Host still down → fall into the normal bounded-backoff climb (one loop, shared cfg). + if (result?.error !== "cancelled") scheduleNext() + }) + }, + /** Host-initiated teardown — stop retrying and forget the target. */ + stop() { + generation++ + cancelPending() + lastTabId = null + state = initialBackoff() + }, + } +} diff --git a/src/lib/web-ws-channel.test.ts b/src/lib/web-ws-channel.test.ts new file mode 100644 index 0000000..1a90942 --- /dev/null +++ b/src/lib/web-ws-channel.test.ts @@ -0,0 +1,171 @@ +import { beforeEach, describe, expect, it, vi } from "vitest" +import type { WebTransportDeps } from "./cdp-web-transport" +import type { CryptoContext } from "./crypto-context" +import { createWsChannel } from "./web-ws-channel" + +/** Fake WebSocket: capture outbound sends, drive open/ready/message/binary/close by hand. */ +class FakeWebSocket { + static OPEN = 1 + static CONNECTING = 0 + static instances: FakeWebSocket[] = [] + readyState = FakeWebSocket.CONNECTING + binaryType = "blob" + sent: string[] = [] + onmessage: ((ev: { data: unknown }) => void) | null = null + onclose: (() => void) | null = null + onerror: (() => void) | null = null + constructor(public url: string) { + FakeWebSocket.instances.push(this) + } + send(text: string) { + this.sent.push(text) + } + close() { + this.readyState = 3 + this.onclose?.() + } + ready() { + this.readyState = FakeWebSocket.OPEN + this.onmessage?.({ data: JSON.stringify({ t: "ready" }) }) + } + message(obj: unknown) { + this.onmessage?.({ data: JSON.stringify(obj) }) + } + binary(blob: Blob) { + this.onmessage?.({ data: blob }) + } + // biome-ignore lint/suspicious/noExplicitAny: parsed test view of wire frames + parsed(): any[] { + return this.sent.map((s) => JSON.parse(s)) + } +} + +// Off-mode crypto: seal/open are pass-through JSON (no envelope), mode "off". +const offCrypto = { + mode: "off", + contentType: "application/json", + sealText: async (o: unknown) => JSON.stringify(o), + openText: async (s: string) => JSON.parse(s), +} as unknown as CryptoContext + +function makeDeps(): WebTransportDeps { + return { + fetch: vi.fn(), + EventSource: vi.fn() as unknown as typeof EventSource, + WebSocket: FakeWebSocket as unknown as typeof WebSocket, + } +} + +function makeChannel() { + const onEvent = vi.fn() + const onReady = vi.fn() + const onClose = vi.fn() + const onFrameBinary = vi.fn() + const ch = createWsChannel(makeDeps(), offCrypto, { + onEvent, + onReady, + onClose, + onFrameBinary, + }) + const ws = FakeWebSocket.instances[FakeWebSocket.instances.length - 1] + return { ch, ws, onEvent, onReady, onClose, onFrameBinary } +} + +beforeEach(() => { + FakeWebSocket.instances = [] + ;(globalThis as { location?: unknown }).location = { protocol: "http:", host: "localhost" } +}) + +describe("createWsChannel", () => { + it("opens a socket to /api/ws on construction", () => { + const { ws } = makeChannel() + expect(ws.url).toContain("/api/ws") + }) + + it("flips ready and opts into the paint-ack gate on the server 'ready'", () => { + const { ch, ws, onReady } = makeChannel() + expect(ch.isReady()).toBe(false) + + ws.ready() + + expect(ch.isReady()).toBe(true) + expect(onReady).toHaveBeenCalled() + expect(ws.parsed().map((m) => m.t)).toContain("frame-ack-mode") + ch.close() + }) + + it("sends a CDP command as a {t:'send'} envelope", async () => { + const { ch, ws } = makeChannel() + ws.ready() + + await ch.send("Input.dispatchMouseEvent", { x: 1, y: 2 }) + + expect(ws.parsed().find((m) => m.t === "send")).toMatchObject({ + method: "Input.dispatchMouseEvent", + params: { x: 1, y: 2 }, + }) + ch.close() + }) + + it("resolves an invoke when its matching invoke-result arrives", async () => { + const { ch, ws } = makeChannel() + ws.ready() + + const p = ch.invoke("Page.getNavigationHistory") + await Promise.resolve() // let the async seal+send settle so the frame is on the wire + const inv = ws.parsed().find((m) => m.t === "invoke") + expect(inv).toMatchObject({ method: "Page.getNavigationHistory" }) + + ws.message({ t: "invoke-result", id: inv.id, result: { currentIndex: 0 } }) + + await expect(p).resolves.toEqual({ currentIndex: 0 }) + ch.close() + }) + + it("fans out a CDP event to onEvent", () => { + const { ch, ws, onEvent } = makeChannel() + ws.ready() + + ws.message({ t: "event", event: "cdp", data: "payload" }) + + expect(onEvent).toHaveBeenCalledWith("cdp", "payload") + ch.close() + }) + + it("pairs a cdp-frame envelope with the following binary message", () => { + const { ch, ws, onFrameBinary } = makeChannel() + ws.ready() + + ws.message({ + t: "event", + event: "cdp-frame", + data: { method: "Page.screencastFrame", params: { sessionId: 7 } }, + }) + const blob = new Blob(["jpeg"]) + ws.binary(blob) + + expect(onFrameBinary).toHaveBeenCalledWith({ + method: "Page.screencastFrame", + params: { sessionId: 7, dataBlob: blob }, + }) + ch.close() + }) + + it("calls onClose on a spontaneous socket close", () => { + const { ws, onClose } = makeChannel() + ws.ready() + + ws.close() + + expect(onClose).toHaveBeenCalled() + }) + + it("suppresses onClose for an explicit channel close()", () => { + const { ch, ws, onClose } = makeChannel() + ws.ready() + + ch.close() + + expect(onClose).not.toHaveBeenCalled() + }) +}) diff --git a/src/lib/web-ws-channel.ts b/src/lib/web-ws-channel.ts new file mode 100644 index 0000000..ff3f1b6 --- /dev/null +++ b/src/lib/web-ws-channel.ts @@ -0,0 +1,215 @@ +/** + * Web build — the optional WebSocket channel, lifted out of `cdp-web-transport.ts` for an + * isolated test surface (t096, A5). + * + * Optional WebSocket channel (t019). When connected and ready, all CDP traffic — events + * in, invokes/sends/batches out — rides one full-duplex socket instead of SSE+POST. + * The picker in settings selects between Auto/Fastest(WS)/Streaming/Basic; this channel + * is the "Fastest" path and the head of the Auto fallback chain. Returns a thin handle + * the rest of createWebCdp consults via `ws.ready()` before falling back. + */ + +import type { Cmd, WebTransportDeps } from "./cdp-web-transport" +import type { CryptoContext } from "./crypto-context" +import { notePing, notePong, resetLatencyMetrics } from "./latency-metrics" +import { perfMark } from "./perf-mark" + +export function createWsChannel( + deps: WebTransportDeps, + crypto: CryptoContext, + opts: { + onEvent: (event: string, data: string) => void + /** Fast path for screencast: a text "cdp-frame" envelope pairs with a binary WS frame + * carrying raw JPEG bytes. Skips base64 + the JSON.parse of a 250KB envelope on the + * renderer main thread. The caller forges a CDP event with `dataBlob` populated and + * dispatches through the normal CDP event listeners. */ + onFrameBinary?: (cdpMsg: { + method: string + params: Record & { dataBlob: Blob } + }) => void + onReady: () => void + onClose: () => void + }, +) { + let socket: WebSocket | null = null + let ready = false + // Pending awaited invokes keyed by id; the server echoes the id in invoke-result. + let nextId = 1 + const pending = new Map void>() + // True when the outer asked us to close — suppresses the onClose callback so a stale + // socket's late `close` event doesn't clobber a newly-opened channel's outer state. + let suppressClose = false + + // Keepalive + RTT probe (t057). While the socket is ready, a plaintext `ping` carrying a + // client-monotonic stamp fires on a fixed interval regardless of input/frame traffic, so + // an idle WS isn't reaped by an upstream proxy (nginx default idle ~60s). The server pongs + // it straight back; the onmessage handler folds the round-trip into the RTT estimator. The + // ping is control traffic — never sealed, never routed through the dispatcher's CDP path. + const PING_INTERVAL_MS = 20000 + let pingSeq = 0 + let pingTimer: ReturnType | null = null + function sendPing() { + if (!socket || socket.readyState !== deps.WebSocket.OPEN) return + const seq = ++pingSeq + const sentAt = performance.now() + notePing(seq, sentAt) + try { + // `ts` carries the client's monotonic stamp; the server echoes it unchanged. RTT is + // measured against the local `outstanding` map keyed by `seq`, never the echoed `ts`. + socket.send(JSON.stringify({ t: "ping", seq, ts: sentAt })) + } catch {} + } + function startPingPump() { + if (pingTimer !== null) return + sendPing() // probe immediately so RTT lands without waiting a full interval + pingTimer = setInterval(sendPing, PING_INTERVAL_MS) + } + + // Ack-after-paint control (t056) — plaintext like ping (no user content), so it's E2E- + // agnostic and skips the seal round-trip. `frame-ack-mode` opts this client into the + // server's one-in-flight gate; `frame-ack` is the post-paint ack the viewport fires, + // releasing the slot so the server acks the remote and the next frame flows. + function sendControl(payload: unknown) { + if (!socket || socket.readyState !== deps.WebSocket.OPEN) return + try { + socket.send(JSON.stringify(payload)) + } catch {} + } + function stopPingPump() { + if (pingTimer !== null) { + clearInterval(pingTimer) + pingTimer = null + } + // The estimator degrades cleanly when WS is gone — report unavailable, not a stale RTT. + resetLatencyMetrics() + } + + function open() { + const proto = location.protocol === "https:" ? "wss:" : "ws:" + const url = `${proto}//${location.host}/api/ws` + try { + socket = new deps.WebSocket(url) + } catch { + opts.onClose() + return + } + // The binary frame fast path: server sends a text "cdp-frame" envelope first + // (containing metadata + sessionId) then the raw JPEG bytes as the next WS message. + // Delivering as Blob is faster than ArrayBuffer for createImageBitmap — no extra copy. + socket.binaryType = "blob" + let pendingFrame: { method: string; params: Record } | null = null + socket.onmessage = async (ev) => { + // Binary message → pair with the preceding "cdp-frame" envelope. + if (ev.data instanceof Blob) { + if (!pendingFrame) return // out-of-order binary, drop + const frame = pendingFrame + pendingFrame = null + opts.onFrameBinary?.({ + method: frame.method, + params: { ...frame.params, dataBlob: ev.data }, + }) + return + } + // Envelope is always plaintext JSON (the `t` field is routing metadata, like SSE + // event names). For event messages, `data` is the sealed CDP payload that onSse() + // unwraps. For invoke-result, the server passes the CDP invoke output through plain + // — these are server-generated, not user content. + const tEntry = performance.now() // [DEBUG-perf] + const text = ev.data as string + const tParse = performance.now() // [DEBUG-perf] + let msg: { + t: string + event?: string + data?: unknown + id?: number + result?: unknown + seq?: number + } + try { + msg = JSON.parse(text) + } catch { + return + } + // [DEBUG-perf] Only mark big-payload events (the cdp screencast frames) so we measure + // the actual bottleneck and not all the tiny notification envelopes. + if (msg.t === "event" && msg.event === "cdp" && text.length > 5000) { + perfMark("wsRecv", tParse - tEntry) + perfMark("jsonParse", performance.now() - tParse) + } + if (msg.t === "ready") { + ready = true + startPingPump() + sendControl({ t: "frame-ack-mode" }) // opt into the server's one-in-flight gate (t056) + opts.onReady() + } else if (msg.t === "pong") { + // Control traffic — never fanned out as a CDP event. Fold the round-trip into the + // always-on RTT/jitter estimator against the client clock (t057). + if (typeof msg.seq === "number") notePong(msg.seq, performance.now()) + } else if (msg.t === "event" && msg.event === "cdp-frame") { + // Stash metadata; the next binary message is the JPEG bytes for this frame. + pendingFrame = msg.data as { method: string; params: Record } + } else if (msg.t === "event" && msg.event && msg.data !== undefined) { + opts.onEvent(msg.event, msg.data as string) + } else if (msg.t === "invoke-result" && typeof msg.id === "number") { + const cb = pending.get(msg.id) + pending.delete(msg.id) + // Result is sealed under E2E (the routing envelope around it stays plaintext); the + // open is the CryptoContext's, the one downlink ingress for an invoke reply. + const result = + crypto.mode === "e2e" ? await crypto.openText(msg.result as string) : msg.result + cb?.(result) + } + } + socket.onclose = () => { + ready = false + socket = null + stopPingPump() + for (const cb of pending.values()) cb({ error: "ws closed" }) + pending.clear() + if (suppressClose) return // caller already handled the outer state on explicit close + opts.onClose() + } + socket.onerror = () => { + try { + socket?.close() + } catch {} + } + } + + async function rawSend(payload: unknown) { + if (!socket || socket.readyState !== deps.WebSocket.OPEN) return false + // One seal at the WS uplink egress — the whole `{ t, … }` envelope is sealed (the server + // opens it as one body), via the CryptoContext, not an inline key read. + const text = await crypto.sealText(payload) + try { + socket.send(text) + return true + } catch { + return false + } + } + + open() + + return { + isReady: () => ready, + close: () => { + suppressClose = true + socket?.close() + }, + send: (method: string, params?: unknown) => rawSend({ t: "send", method, params }), + paintAck: (sessionId: number) => sendControl({ t: "frame-ack", sessionId }), + batch: (items: Cmd[]) => rawSend({ t: "batch", items }), + invoke: (method: string, params?: unknown) => + new Promise((resolve) => { + const id = nextId++ + pending.set(id, resolve) + void rawSend({ t: "invoke", id, method, params }).then((ok) => { + if (!ok) { + pending.delete(id) + resolve({ error: "ws send failed" }) + } + }) + }), + } +} diff --git a/src/vite-env.d.ts b/src/vite-env.d.ts index 56242ce..4de3ee2 100644 --- a/src/vite-env.d.ts +++ b/src/vite-env.d.ts @@ -123,7 +123,8 @@ interface CdpBridge { readClipboard: () => Promise /** Electron-only: the local clipboard's image as a data URL, or null if none. */ readClipboardImage: () => Promise - onSwipe: (cb: (direction: string) => void) => void + /** Subscribe to trackpad swipe gestures; returns an unsubscribe (t096). */ + onSwipe: (cb: (direction: string) => void) => () => void // Pins getPins: () => Promise addPin: (pin: Pin) => Promise diff --git a/web/server.mjs b/web/server.mjs index afa5b98..2da6866 100644 --- a/web/server.mjs +++ b/web/server.mjs @@ -22,6 +22,7 @@ import { createLineSplitter } from "../core/line-splitter.js" import { muteKey, unreadExcluding } from "../core/notif-mutes.js" import { buildHealth, shouldAlert } from "../core/notification-health.js" import sidechain from "../core/notifications-sidechain.js" +import { createPaintAckPacer } from "../core/paint-ack-pacer.js" import { pushSendOptions } from "../core/push-send-options.js" import { reconcileDeviceId } from "../core/push-subscriptions.js" import connector from "../core/remote-page-connector.js" @@ -35,6 +36,7 @@ import { teamIdOf as slackTeamIdOf, upsertWorkspace as slackUpsertWorkspace, } from "../core/slack-workspaces.js" +import { createSweepScheduler } from "../core/sweep-scheduler.js" const HERE = dirname(fileURLToPath(import.meta.url)) const DIST = join(HERE, "..", "dist") @@ -120,7 +122,13 @@ const settings = createSettingsStore({ host: process.env.CDP_HOST || "localhost", port: Number(process.env.CDP_PORT || 9222), }), - persist: (s) => writeFileSync(SETTINGS_PATH, JSON.stringify(s, null, 2)), + persist: (s) => { + try { + writeFileSync(SETTINGS_PATH, JSON.stringify(s, null, 2)) + } catch (e) { + console.error("[web] settings persist failed:", e.message) + } + }, }) // Env wins on first boot so a fresh deploy points at the right host immediately. if (process.env.CDP_HOST) @@ -173,7 +181,6 @@ function devicePrefs(ui, deviceId) { async function sendPushToAll(payload) { if (pushSubs.length === 0) return const key = muteKey(payload) - const list = notificationCenter.list() const ui = settings.getUiState() const dead = [] // Verbose, greppable per-device delivery log (t093) so the mute gating can be verified from @@ -196,7 +203,9 @@ async function sendPushToAll(payload) { console.log(`[push] dev=${dev} skip:muted(${key})`) return // this source muted on this device } - const unread = unreadExcluding(list, mutes, master) + // Recompute from a fresh list at each send (t096, P7) so a mark-read that lands while a + // prior send is in flight can't leave this device's badge stamped from a stale snapshot. + const unread = unreadExcluding(notificationCenter.list(), mutes, master) const data = JSON.stringify(trimPushPayload({ ...payload, unread })) for (let attempt = 0; attempt < 2; attempt++) { try { @@ -389,10 +398,13 @@ const frameThrottle = createFrameThrottle({ targetFps: SCREENCAST_TARGET_FPS }) // arriving while one is outstanding is dropped (never queued) so the freshest always wins. const frameAckGate = createAckGate() // Stranded-paint watchdog: if a supporting client never acks (decode error, tab hidden, a -// connection drop that didn't surface as a close), free the slot and ack the remote anyway -// so a single lost paint can't wedge the stream forever. Generous vs. the frame interval so -// a healthy slow link is never tripped; the ack arriving first cancels it. -const PAINT_ACK_WATCHDOG_MS = 1000 +// connection drop that didn't surface as a close), free the slot and ack the remote anyway so a +// single lost paint can't wedge the stream forever. The window is ADAPTIVE (t096, P2): an EWMA +// of observed paint-ack latency sizes it to a multiple of normal, never below a 1s floor — so a +// device that legitimately paints slower than a fixed 1s isn't tripped early. The ack arriving +// first cancels it. +const paintAckPacer = createPaintAckPacer() +let paintSentAt = 0 let paintAckWatchdog = null function clearPaintAckWatchdog() { if (paintAckWatchdog !== null) { @@ -412,13 +424,14 @@ function admitFrame(sessionId) { } if (!frameAckGate.mayProceed()) return false frameAckGate.markSent(sessionId) + paintSentAt = Date.now() clearPaintAckWatchdog() paintAckWatchdog = setTimeout(() => { const stranded = frameAckGate.outstanding() frameAckGate.reset() paintAckWatchdog = null if (stranded !== null) remotePage.ackFrame(stranded) // release the remote despite no paint - }, PAINT_ACK_WATCHDOG_MS) + }, paintAckPacer.windowMs()) return true } // A supporting client painted + acked the outstanding frame: clear the slot, cancel the @@ -428,6 +441,7 @@ function onClientPaintAck(sessionId) { if (frameAckGate.outstanding() === null) return frameAckGate.ackReceived(sessionId) if (frameAckGate.outstanding() === null) { + paintAckPacer.record(Date.now() - paintSentAt) // feed the adaptive window (t096, P2) clearPaintAckWatchdog() remotePage.ackFrame(sessionId) } @@ -543,14 +557,14 @@ const notificationCenter = createNotificationCenter({ // newly-attached workspace is caught up without waiting for the next interval. onCreds: (rec) => { console.log(`[web] slack creds ready: ${rec.teamId} (${rec.name})`) - if (slackSweeper) slackSweeper.sweepWorkspace(rec).catch(() => {}) + sweepScheduler.request(rec.teamId, rec) }, // The Slack hijack (t064) is demoted to a "sweep now" trigger (ADR-0011): a fired // notification means something happened, so sweep that workspace immediately for the // authoritative, message-anchored entry — sub-second delivery, no double-notify. onSlackSignal: (teamId) => { const rec = notificationCenter.getCreds(teamId) - if (rec && rec.fresh !== false && slackSweeper) slackSweeper.sweepWorkspace(rec).catch(() => {}) + if (rec && rec.fresh !== false) sweepScheduler.request(teamId, rec) }, onEntry: (entry) => { // Verbose prod log (greppable [notif]) — proves the entry's keying: a Grid workspace's @@ -620,9 +634,17 @@ async function keepSlackTabsAlive(targets) { ) if (!before || before.enterpriseId !== enterpriseId) changed = true } - // Recreate a tab for any registered workspace that has no live tab. + // Recreate a tab for any registered workspace that has no live tab — UNLESS the user has + // it pinned (t098): a pinned workspace is owned by its pin, so the keeper never spawns a + // stray duplicate for it. One live Slack tab refreshes every workspace's creds, so capture + // is unaffected; a cred lifeline (inside planParkedTabs) keeps one tab alive when none is. const live = slackLiveTeamIds(targets) - const plans = slackPlanParkedTabs(slackRegistry, live, slackCreatedAt, Date.now()) + const pinUrlByTeam = {} + for (const pin of settings.getPins()) { + const tid = slackTeamIdOf(pin.url || "") + if (tid && !pinUrlByTeam[tid]) pinUrlByTeam[tid] = pin.url + } + const plans = slackPlanParkedTabs(slackRegistry, live, slackCreatedAt, Date.now(), pinUrlByTeam) for (const plan of plans) { slackCreatedAt[plan.teamId] = Date.now() try { @@ -731,6 +753,13 @@ const slackSweeper = createSlackSweeper({ now: Date.now, log: (m) => console.log(m), }) +// Debounced per-workspace sweep trigger (t096, A6): onCreds + onSlackSignal can fire for the +// same workspace within milliseconds — coalesce them into one leading + one trailing sweep so a +// cred-extraction immediately followed by a hijack signal can't double-sweep. The 15s +// all-workspaces backstop (runOnce) has no per-workspace key and stays out of this path. +const sweepScheduler = createSweepScheduler({ + run: (rec) => slackSweeper.sweepWorkspace(rec).catch(() => {}), +}) // Compute the Slack capture health report from cred records + sweep metadata (t074, t092). // Rows are merged per Enterprise Grid group (one row per org); `groups` is the teamId → // groupId map the renderer needs to resolve a Slack Tab/Pin URL (which carries only a