From e73f62f9cbfbc4b073b79af8e22dcac464398322 Mon Sep 17 00:00:00 2001 From: Kyle Kelley Date: Thu, 11 Jun 2026 04:20:46 -0600 Subject: [PATCH 01/16] fix(sift): polish table focus and summaries (#3574) --- packages/sift/src/auto-width.test.ts | 6 +++--- packages/sift/src/auto-width.ts | 2 +- packages/sift/src/library.css | 21 ++++++++++++++++++++- packages/sift/src/table.test.ts | 2 ++ packages/sift/src/table.ts | 1 + src/components/cell/OutputArea.tsx | 4 ++-- 6 files changed, 29 insertions(+), 7 deletions(-) diff --git a/packages/sift/src/auto-width.test.ts b/packages/sift/src/auto-width.test.ts index 1948cef08..2d71dc7ed 100644 --- a/packages/sift/src/auto-width.test.ts +++ b/packages/sift/src/auto-width.test.ts @@ -5,7 +5,7 @@ describe("autoWidth", () => { it("returns minimum width for short column names", () => { expect(autoWidth("x", "numeric")).toBeGreaterThanOrEqual(100); expect(autoWidth("y", "boolean")).toBeGreaterThanOrEqual(90); - expect(autoWidth("z", "timestamp")).toBeGreaterThanOrEqual(130); + expect(autoWidth("z", "timestamp")).toBeGreaterThanOrEqual(170); expect(autoWidth("a", "categorical")).toBeGreaterThanOrEqual(120); }); @@ -35,7 +35,7 @@ describe("autoWidth", () => { it("timestamp columns have highest minimum", () => { const tsW = autoWidth("x", "timestamp"); - expect(tsW).toBeGreaterThanOrEqual(130); + expect(tsW).toBeGreaterThanOrEqual(170); }); it("handles empty string column name", () => { @@ -45,6 +45,6 @@ describe("autoWidth", () => { it("handles unicode column names", () => { const width = autoWidth("日付", "timestamp"); - expect(width).toBeGreaterThanOrEqual(130); + expect(width).toBeGreaterThanOrEqual(170); }); }); diff --git a/packages/sift/src/auto-width.ts b/packages/sift/src/auto-width.ts index c486e785f..92da7b916 100644 --- a/packages/sift/src/auto-width.ts +++ b/packages/sift/src/auto-width.ts @@ -25,7 +25,7 @@ export function autoWidth(name: string, colType: ColumnType): number { case "boolean": return Math.max(90, Math.ceil(labelW)); case "timestamp": - return Math.max(130, Math.ceil(labelW)); + return Math.max(170, Math.ceil(labelW)); case "numeric": return Math.max(100, Math.ceil(labelW)); case "categorical": diff --git a/packages/sift/src/library.css b/packages/sift/src/library.css index 5dd896b95..24430ecc1 100644 --- a/packages/sift/src/library.css +++ b/packages/sift/src/library.css @@ -432,6 +432,9 @@ color: var(--sift-muted); text-transform: none; letter-spacing: normal; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; } .sift-tz-default { @@ -1113,12 +1116,16 @@ align-items: center; gap: 6px; margin-left: 12px; + min-width: 0; + overflow: hidden; } .sift-filter-pill { display: inline-flex; align-items: center; gap: 4px; + flex: 0 1 auto; + min-width: 0; padding: 2px 6px 2px 8px; background: color-mix(in srgb, var(--sift-accent) 10%, transparent); border: 1px solid color-mix(in srgb, var(--sift-accent) 20%, transparent); @@ -1130,13 +1137,25 @@ overflow: hidden; } +.sift-filter-pill-label { + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; +} + .sift-filter-pill-x { + display: inline-flex; + align-items: center; + justify-content: center; + flex: 0 0 14px; + width: 14px; + height: 14px; background: none; border: none; color: var(--sift-accent); font: 14px/1 var(--sift-font); cursor: pointer; - padding: 0 2px; + padding: 0; opacity: 0.6; transition: opacity 100ms ease; } diff --git a/packages/sift/src/table.test.ts b/packages/sift/src/table.test.ts index e85ea6256..170cc030d 100644 --- a/packages/sift/src/table.test.ts +++ b/packages/sift/src/table.test.ts @@ -474,6 +474,8 @@ describe("createTable", () => { const pills = container.querySelectorAll(".sift-filter-pill"); expect(pills.length).toBe(1); expect(pills[0].textContent).toContain("Score"); + expect(pills[0].querySelector(".sift-filter-pill-label")?.textContent).toContain("Score"); + expect(pills[0].querySelector(".sift-filter-pill-x")).not.toBeNull(); }); it("clearFilter removes the pill", async () => { diff --git a/packages/sift/src/table.ts b/packages/sift/src/table.ts index 5b0ee0142..4137ed37e 100644 --- a/packages/sift/src/table.ts +++ b/packages/sift/src/table.ts @@ -1262,6 +1262,7 @@ export function createTable( } const label = document.createElement("span"); + label.className = "sift-filter-pill-label"; label.textContent = text; const closeBtn = document.createElement("button"); diff --git a/src/components/cell/OutputArea.tsx b/src/components/cell/OutputArea.tsx index 49dc01f65..55b915a44 100644 --- a/src/components/cell/OutputArea.tsx +++ b/src/components/cell/OutputArea.tsx @@ -710,9 +710,9 @@ function OutputAreaSingle({ const interactionFrameStyle = hasWheelOwningOutputs ? ({ "--notebook-sift-focus": frameFocusAccent, - "--notebook-sift-focus-hover": `${frameFocusAccent}66`, + "--notebook-sift-focus-hover": `${frameFocusAccent}38`, boxShadow: staticFrameInteractionActive - ? `0 0 0 2px ${frameFocusAccent}cc, 0 12px 30px ${frameFocusAccent}24` + ? `0 0 0 1.5px ${frameFocusAccent}cc, 0 0 0 3px ${frameFocusAccent}14` : undefined, } as React.CSSProperties) : undefined; From f52de9ada80aeb8b6edff013f453557de9d4b850 Mon Sep 17 00:00:00 2001 From: Quill <261289082+quillaid@users.noreply.github.com> Date: Thu, 11 Jun 2026 06:34:57 -0600 Subject: [PATCH 02/16] chore(mcp-app): bump @modelcontextprotocol/sdk to 1.29 for fast-uri advisories (#3576) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves the two high-severity pnpm audit findings — fast-uri path traversal (GHSA-q3j6-qgpj-74h6) and host confusion (GHSA-v39h-62p7-jpjc) — reached via @modelcontextprotocol/sdk -> ajv -> fast-uri. The SDK bump plus a transitive update moves the lockfile to fast-uri 3.1.2. pnpm audit --prod now reports 0 high (was 2 high). --- apps/mcp-app/package.json | 2 +- pnpm-lock.yaml | 96 +++++++++++++++++++++++++++++---------- 2 files changed, 73 insertions(+), 25 deletions(-) diff --git a/apps/mcp-app/package.json b/apps/mcp-app/package.json index 5016a66cf..63f619a8f 100644 --- a/apps/mcp-app/package.json +++ b/apps/mcp-app/package.json @@ -7,7 +7,7 @@ }, "dependencies": { "@modelcontextprotocol/ext-apps": "^1.3.0", - "@modelcontextprotocol/sdk": "^1.28.0", + "@modelcontextprotocol/sdk": "^1.29.0", "react": "^19.1.0", "react-dom": "^19.1.0" }, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index d387bdef0..51dd3309e 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -382,10 +382,10 @@ importers: dependencies: '@modelcontextprotocol/ext-apps': specifier: ^1.3.0 - version: 1.3.2(@modelcontextprotocol/sdk@1.28.0(zod@4.4.3))(react-dom@19.2.6(react@19.2.6))(react@19.2.6)(zod@4.4.3) + version: 1.3.2(@modelcontextprotocol/sdk@1.29.0(zod@4.4.3))(react-dom@19.2.6(react@19.2.6))(react@19.2.6)(zod@4.4.3) '@modelcontextprotocol/sdk': - specifier: ^1.28.0 - version: 1.28.0(zod@4.4.3) + specifier: ^1.29.0 + version: 1.29.0(zod@4.4.3) react: specifier: 19.2.6 version: 19.2.6 @@ -832,10 +832,10 @@ importers: devDependencies: '@earendil-works/pi-ai': specifier: ^0.74.0 - version: 0.74.0(@modelcontextprotocol/sdk@1.28.0(zod@4.4.3))(ws@8.20.1)(zod@4.4.3) + version: 0.74.0(@modelcontextprotocol/sdk@1.29.0(zod@4.4.3))(ws@8.20.1)(zod@4.4.3) '@earendil-works/pi-coding-agent': specifier: ^0.74.0 - version: 0.74.0(@modelcontextprotocol/sdk@1.28.0(zod@4.4.3))(ws@8.20.1)(zod@4.4.3) + version: 0.74.0(@modelcontextprotocol/sdk@1.29.0(zod@4.4.3))(ws@8.20.1)(zod@4.4.3) '@earendil-works/pi-tui': specifier: ^0.74.0 version: 0.74.0 @@ -1170,6 +1170,10 @@ packages: resolution: {integrity: sha512-9NhCeYjq9+3uxgdtp20LSiJXJvN0FeCtNGpJxuMFZ1Kv3cWUNb6DOhJwUvcVCzKGR66cw4njwM6hrJLqgOwbcw==} engines: {node: '>=6.9.0'} + '@babel/code-frame@7.29.7': + resolution: {integrity: sha512-Aup7aUOfpbAUg2ROOJN6Iw5f9DMBlzu0mIkm/malLQFN/YQgO48wCj0Kxa3sEHJvPVFg7siR+qRInwXd2qhQKw==} + engines: {node: '>=6.9.0'} + '@babel/compat-data@7.29.0': resolution: {integrity: sha512-T1NCJqT/j9+cn8fvkt7jtwbLBfLC/1y1c7NtCeXFRgzGTsafi68MRv8yzkYSapBnFA6L3U2VSc02ciDzoAJhJg==} engines: {node: '>=6.9.0'} @@ -1208,6 +1212,10 @@ packages: resolution: {integrity: sha512-qSs4ifwzKJSV39ucNjsvc6WVHs6b7S03sOh2OcHF9UHfVPqWWALUsNUVzhSBiItjRZoLHx7nIarVjqKVusUZ1Q==} engines: {node: '>=6.9.0'} + '@babel/helper-validator-identifier@7.29.7': + resolution: {integrity: sha512-qehxGkRj55h/ff8EMaJ+cYhyaKlHIxqYDn682wQD7RNp9UujOQsHog2uS0r2vzr4pW+sXf90NeeayjcNaX3fFg==} + engines: {node: '>=6.9.0'} + '@babel/helper-validator-option@7.27.1': resolution: {integrity: sha512-YvjJow9FxbhFFKDSuFnVCe2WxXk1zWc22fFePVNEaWJEu8IrZVlda6N0uHwzZrUM1il7NC9Mlp4MaJYbYd9JSg==} engines: {node: '>=6.9.0'} @@ -2502,6 +2510,16 @@ packages: '@cfworker/json-schema': optional: true + '@modelcontextprotocol/sdk@1.29.0': + resolution: {integrity: sha512-zo37mZA9hJWpULgkRpowewez1y6ML5GsXJPY8FI0tBBCd77HEvza4jDqRKOXgHNn867PVGCyTdzqpz0izu5ZjQ==} + engines: {node: '>=18'} + peerDependencies: + '@cfworker/json-schema': ^4.1.1 + zod: ^3.25 || ^4.0 + peerDependenciesMeta: + '@cfworker/json-schema': + optional: true + '@napi-rs/cli@3.6.2': resolution: {integrity: sha512-jy5rABUh9tbE/vPRzw9kGzGuqZiVslyDQUV8LkvjzqVX/oJMN7g0U1uhtr9L3W1H+iRM/urXHXUf+CE4n8FvLA==} engines: {node: '>= 16'} @@ -7250,8 +7268,8 @@ packages: fast-string-width@3.0.2: resolution: {integrity: sha512-gX8LrtNEI5hq8DVUfRQMbr5lpaS4nMIWV+7XEbXk2b8kiQIizgnlr12B4dA3ZEx3308ze0O4Q1R+cHts8kyUJg==} - fast-uri@3.1.0: - resolution: {integrity: sha512-iPeeDKJSWf4IEOasVVrknXpaBV0IApz/gp7S2bb7Z4Lljbl2MGJRqInZiUrQwV16cpzw/D3S5j5Julj/gT52AA==} + fast-uri@3.1.2: + resolution: {integrity: sha512-rVjf7ArG3LTk+FS6Yw81V1DLuZl1bRbNrev6Tmd/9RaroeeRRJhAt7jg/6YFxbvAQXUCavSoZhPPj6oOx+5KjQ==} fast-wrap-ansi@0.2.0: resolution: {integrity: sha512-rLV8JHxTyhVmFYhBJuMujcrHqOT2cnO5Zxj37qROj23CP39GXubJRBUFF0z8KFK77Uc0SukZUf7JZhsVEQ6n8w==} @@ -11445,6 +11463,12 @@ snapshots: js-tokens: 4.0.0 picocolors: 1.1.1 + '@babel/code-frame@7.29.7': + dependencies: + '@babel/helper-validator-identifier': 7.29.7 + js-tokens: 4.0.0 + picocolors: 1.1.1 + '@babel/compat-data@7.29.0': {} '@babel/core@7.29.0': @@ -11505,6 +11529,8 @@ snapshots: '@babel/helper-validator-identifier@7.28.5': {} + '@babel/helper-validator-identifier@7.29.7': {} + '@babel/helper-validator-option@7.27.1': {} '@babel/helpers@7.28.6': @@ -11835,9 +11861,9 @@ snapshots: react: 19.2.6 tslib: 2.8.1 - '@earendil-works/pi-agent-core@0.74.0(@modelcontextprotocol/sdk@1.28.0(zod@4.4.3))(ws@8.20.1)(zod@4.4.3)': + '@earendil-works/pi-agent-core@0.74.0(@modelcontextprotocol/sdk@1.29.0(zod@4.4.3))(ws@8.20.1)(zod@4.4.3)': dependencies: - '@earendil-works/pi-ai': 0.74.0(@modelcontextprotocol/sdk@1.28.0(zod@4.4.3))(ws@8.20.1)(zod@4.4.3) + '@earendil-works/pi-ai': 0.74.0(@modelcontextprotocol/sdk@1.29.0(zod@4.4.3))(ws@8.20.1)(zod@4.4.3) typebox: 1.1.34 transitivePeerDependencies: - '@modelcontextprotocol/sdk' @@ -11848,11 +11874,11 @@ snapshots: - ws - zod - '@earendil-works/pi-ai@0.74.0(@modelcontextprotocol/sdk@1.28.0(zod@4.4.3))(ws@8.20.1)(zod@4.4.3)': + '@earendil-works/pi-ai@0.74.0(@modelcontextprotocol/sdk@1.29.0(zod@4.4.3))(ws@8.20.1)(zod@4.4.3)': dependencies: '@anthropic-ai/sdk': 0.91.1(zod@4.4.3) '@aws-sdk/client-bedrock-runtime': 3.1038.0 - '@google/genai': 1.50.1(@modelcontextprotocol/sdk@1.28.0(zod@4.4.3)) + '@google/genai': 1.50.1(@modelcontextprotocol/sdk@1.29.0(zod@4.4.3)) '@mistralai/mistralai': 2.2.1 chalk: 5.6.2 openai: 6.26.0(ws@8.20.1)(zod@4.4.3) @@ -11870,10 +11896,10 @@ snapshots: - ws - zod - '@earendil-works/pi-coding-agent@0.74.0(@modelcontextprotocol/sdk@1.28.0(zod@4.4.3))(ws@8.20.1)(zod@4.4.3)': + '@earendil-works/pi-coding-agent@0.74.0(@modelcontextprotocol/sdk@1.29.0(zod@4.4.3))(ws@8.20.1)(zod@4.4.3)': dependencies: - '@earendil-works/pi-agent-core': 0.74.0(@modelcontextprotocol/sdk@1.28.0(zod@4.4.3))(ws@8.20.1)(zod@4.4.3) - '@earendil-works/pi-ai': 0.74.0(@modelcontextprotocol/sdk@1.28.0(zod@4.4.3))(ws@8.20.1)(zod@4.4.3) + '@earendil-works/pi-agent-core': 0.74.0(@modelcontextprotocol/sdk@1.29.0(zod@4.4.3))(ws@8.20.1)(zod@4.4.3) + '@earendil-works/pi-ai': 0.74.0(@modelcontextprotocol/sdk@1.29.0(zod@4.4.3))(ws@8.20.1)(zod@4.4.3) '@earendil-works/pi-tui': 0.74.0 '@silvia-odwyer/photon-node': 0.3.4 chalk: 5.6.2 @@ -12193,14 +12219,14 @@ snapshots: '@tailwindcss/oxide': 4.3.0 tailwindcss: 4.3.0 - '@google/genai@1.50.1(@modelcontextprotocol/sdk@1.28.0(zod@4.4.3))': + '@google/genai@1.50.1(@modelcontextprotocol/sdk@1.29.0(zod@4.4.3))': dependencies: google-auth-library: 10.6.2 p-retry: 4.6.2 protobufjs: 7.5.4 ws: 8.20.0 optionalDependencies: - '@modelcontextprotocol/sdk': 1.28.0(zod@4.4.3) + '@modelcontextprotocol/sdk': 1.29.0(zod@4.4.3) transitivePeerDependencies: - bufferutil - supports-color @@ -12590,7 +12616,7 @@ snapshots: '@mcp-ui/client@5.17.3(@preact/signals-core@1.14.1)(preact@10.24.3)(react-dom@19.2.6(react@19.2.6))(react@19.2.6)(zod@4.3.6)': dependencies: - '@modelcontextprotocol/sdk': 1.28.0(zod@4.3.6) + '@modelcontextprotocol/sdk': 1.29.0(zod@4.3.6) '@quilted/threads': 3.3.1(@preact/signals-core@1.14.1) '@r2wc/react-to-web-component': 2.1.1(react-dom@19.2.6(react@19.2.6))(react@19.2.6) '@remote-dom/core': 1.10.1(@preact/signals-core@1.14.1)(preact@10.24.3) @@ -12733,7 +12759,7 @@ snapshots: '@ai-sdk/mistral': 2.0.30(zod@4.3.6) '@ai-sdk/openai': 2.0.102(zod@4.3.6) '@ai-sdk/xai': 2.0.65(zod@4.3.6) - '@modelcontextprotocol/sdk': 1.28.0(zod@4.3.6) + '@modelcontextprotocol/sdk': 1.29.0(zod@4.3.6) '@openrouter/ai-sdk-provider': 2.4.0(ai@6.0.149(zod@4.3.6))(zod@4.3.6) ai: 6.0.149(zod@4.3.6) ollama-ai-provider-v2: 1.5.5(zod@4.3.6) @@ -12797,9 +12823,9 @@ snapshots: react: 19.2.6 react-dom: 19.2.6(react@19.2.6) - '@modelcontextprotocol/ext-apps@1.3.2(@modelcontextprotocol/sdk@1.28.0(zod@4.4.3))(react-dom@19.2.6(react@19.2.6))(react@19.2.6)(zod@4.4.3)': + '@modelcontextprotocol/ext-apps@1.3.2(@modelcontextprotocol/sdk@1.29.0(zod@4.4.3))(react-dom@19.2.6(react@19.2.6))(react@19.2.6)(zod@4.4.3)': dependencies: - '@modelcontextprotocol/sdk': 1.28.0(zod@4.4.3) + '@modelcontextprotocol/sdk': 1.29.0(zod@4.4.3) zod: 4.4.3 optionalDependencies: react: 19.2.6 @@ -12827,7 +12853,29 @@ snapshots: transitivePeerDependencies: - supports-color - '@modelcontextprotocol/sdk@1.28.0(zod@4.4.3)': + '@modelcontextprotocol/sdk@1.29.0(zod@4.3.6)': + dependencies: + '@hono/node-server': 1.19.11(hono@4.12.9) + ajv: 8.18.0 + ajv-formats: 3.0.1(ajv@8.18.0) + content-type: 1.0.5 + cors: 2.8.6 + cross-spawn: 7.0.6 + eventsource: 3.0.7 + eventsource-parser: 3.0.6 + express: 5.2.1 + express-rate-limit: 8.3.1(express@5.2.1) + hono: 4.12.9 + jose: 6.2.2 + json-schema-typed: 8.0.2 + pkce-challenge: 5.0.1 + raw-body: 3.0.2 + zod: 4.3.6 + zod-to-json-schema: 3.25.2(zod@4.3.6) + transitivePeerDependencies: + - supports-color + + '@modelcontextprotocol/sdk@1.29.0(zod@4.4.3)': dependencies: '@hono/node-server': 1.19.11(hono@4.12.9) ajv: 8.18.0 @@ -16386,7 +16434,7 @@ snapshots: '@testing-library/dom@10.4.1': dependencies: - '@babel/code-frame': 7.29.0 + '@babel/code-frame': 7.29.7 '@babel/runtime': 7.28.6 '@types/aria-query': 5.0.4 aria-query: 5.3.0 @@ -17302,7 +17350,7 @@ snapshots: ajv@8.18.0: dependencies: fast-deep-equal: 3.1.3 - fast-uri: 3.1.0 + fast-uri: 3.1.2 json-schema-traverse: 1.0.0 require-from-string: 2.0.2 @@ -18634,7 +18682,7 @@ snapshots: dependencies: fast-string-truncated-width: 3.0.3 - fast-uri@3.1.0: {} + fast-uri@3.1.2: {} fast-wrap-ansi@0.2.0: dependencies: From 1cc0af1ab083d9be2901561c7995dacd2a01704a Mon Sep 17 00:00:00 2001 From: Quill <261289082+quillaid@users.noreply.github.com> Date: Thu, 11 Jun 2026 06:36:38 -0600 Subject: [PATCH 03/16] =?UTF-8?q?fix(notebook-cloud):=20preview-findings?= =?UTF-8?q?=20hardening=20=E2=80=94=20flicker=20gate,=20single=20connect,?= =?UTF-8?q?=20offline=20detection,=20liveness=20probe=20(#3577)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(notebook-cloud): preserve painted projection across live-effect re-runs With IndexedDB seeding, the persisted snapshot paints cells before the live-room effect settles. The effect re-runs for reasons that are not a notebook switch (the mount-time session-status fetch replacing the session object identity, OIDC refreshes, manual retries), and its cleanup cleared the view-store projection unconditionally - a full -> empty -> full flicker on every page load. Reuse the desktop bootstrap-preservation gate: track the painted notebook identity in a ref, and skip the cleanup clear when the same notebook with visible cells is about to re-materialize in place. A real notebook switch still clears, and true unmount still clears via the mount-scoped effect. The helper is re-exported through notebook-surface so the cloud viewer keeps importing desktop code only via the public surface. * fix(notebook-cloud): keep app session identity stable across confirming fetches The mount-time /api/auth/session fetch installed a fresh session object even when it only confirmed the session the page was rendered with. That new object identity rippled through resolveSyncAuth into the live-room effect deps, tearing down and reconnecting the live room once per page load - the second trigger behind the field-reported flicker. Reduce the fetch result through nextCloudAppSessionReadyState: content- equal sessions keep the current session object, and a fully-confirming fetch returns the current state object unchanged so React bails out of the update entirely. * feat(notebook-cloud): detect offline and probe liveness on the cloud transport An OS-level offline window never fires WS close/error: the socket stays OPEN, sends buffer silently, and the only loss signal was the TCP retransmit abort minutes later - connectionStatus$ never flipped before connectivity returned, so the field report saw zero UI change. Two detection paths close that hole: - Browser offline event: a window 'offline' listener mirrors the online short-circuit and proactively recycles the current socket through connectionLost, flipping status to reconnecting immediately and fixing the zombie-socket-on-recovery hazard as well. - App-level liveness probe: after each cloud_room_ready the transport sends a text LIVENESS_PING every 20s and treats a pong missed within 10s as connection loss (covers upstream loss with the interface up, where 'offline' never fires). The room DO answers via CF-native setWebSocketAutoResponse - no DO wake, persists across hibernation - with a manual handleMessage fallback placed before the binary-only rejection so pings never count toward rejected-frame close. Probe cadence is injectable for tests, and the timers are unref'd so a connected transport cannot keep a Node test process alive. * fix(notebook-cloud): preserve runtime/output stores in the flicker gate; make switch-clear reachable Two review findings against the flicker gate: - The gate preserved the cell store but still ran resetRuntimeState() and resetRuntimeStoresProjection() unconditionally. CodeCell renders outputs and execution counts exclusively through the execution/output stores, so output-bearing notebooks kept flickering their dominant visual mass. The gate now keeps or clears every projected store together, exactly matching desktop's preserve path (useAutomergeNotebook beforeBootstrap); resetPoolState stays unconditional as on desktop. Stale-data safety is unchanged: the next materialization replaces cells wholesale and the cloud-owned id sweep still removes stale output/execution entries. - The cleanup's notebook-switch clause was structurally unreachable: the cleanup closes over its own run's config, so previous and next identity could never differ. Switch-clearing now lives where it is reachable — the NEXT effect run's body, before connecting, clears all projected stores iff the painted identity differs (desktop's beforeBootstrap placement) and drops the painted identity so later gates fail closed. The cleanup honestly reduces to "painted with visible cells => preserve", and true unmount clears everything via the mount-scoped effect. The shared gate lives in notebook-view-store-bridge as resetCloudProjectionUnlessPreserved, importing desktop code through a new headless notebook-surface-stores entry (same ./lib implementations as notebook-surface, no component/CSS imports) so the bridge stays loadable under node test runners. Tests paint real cells with outputs through the real projection path and assert cells, outputs, and execution pointers survive a preserved gate and clear on a switch; source pins cover the two session call sites that cannot run under node. * fix(notebook-cloud): count any inbound frame as liveness evidence; skip pong deadline in hidden tabs The probe cleared its pong deadline only on the exact LIVENESS_PONG text frame. The pong is an ordinary frame serialized into the same in-order WS stream as sync frames, so during a large post-reconnect sync over a slow link it queues behind the backlog and the fixed 10s deadline killed sockets that were demonstrably alive and actively delivering frames - recycling exactly the connections least able to afford the resync churn. handleMessage now treats ANY inbound message on the current socket as liveness evidence; the strict pong match remains only the probe's reply channel. A zombie socket delivers nothing, so detection is unchanged. Second false-positive path: background tabs throttle and suspend timers, so a frozen deadline could fire on resume before the queued pong MessageEvent dispatches, declaring a healthy connection dead on every tab foreground. The deadline is no longer armed while document.visibilityState === "hidden" (the simpler shape) - pings still flow, and the next visible-tab ping re-arms enforcement. New coverage: inbound-frame evidence with a true-zombie follow-up, the previously unreachable keep-original-deadline branch (deadline > interval overlap), a pong landing one tick before the deadline, ping send failure -> connectionLost, the livenessPingIntervalMs: 0 disable knob, a stale pong from a superseded socket not satisfying the new probe, hidden-tab deadline suppression with foreground re-arm, and the two uncovered offline windows (parked connectTarget no-op + online supersession, and pre-ready loss keeping status "connecting"). * test(notebook-cloud): pin hibernation re-arm and partial feature detection in the room Two mutants the arm test could not kill: moving the auto-response arming out of the constructor into a once-per-process path (hibernation wake IS a constructor run against the same durable state - re-arming must be an idempotent re-set of the same pair), and dropping the second conjunct of the feature detection (global WebSocketRequestResponsePair present but state.setWebSocketAutoResponse absent would TypeError in the constructor: a total room outage, not a degraded probe). * test(notebook-cloud): pin the session-status updater wiring via a rendered hook The session-identity reducer was thoroughly unit-tested, but its single consumer - the setState functional-updater callsite in useCloudAppSessionStatus - was unpinned: reverting it to a plain object setState kept every suite green while silently reintroducing the once-per-page-load live-room reconnect. Render the real hook with a stubbed readCloudAppSessionStatus and assert the session object reference survives content-equal confirming fetches (mount and manual refresh), renewed sessions are adopted, the no-initial-session path reaches ready, and fetch failures keep the existing session. Cloud viewer hooks need a DOM renderer, so the root vitest config gains an apps/notebook-cloud/viewer/__tests__ include; the rest of the cloud suite stays on node:test. --- apps/notebook-cloud/src/cloudflare-types.ts | 9 + apps/notebook-cloud/src/notebook-room.ts | 37 +- apps/notebook-cloud/src/protocol.ts | 11 + .../test/cloud-projection-flicker.test.ts | 170 +++++++ apps/notebook-cloud/test/live-sync.test.ts | 448 +++++++++++++++++- .../notebook-cloud/test/notebook-room.test.ts | 104 ++++ .../use-cloud-auth-session-identity.test.ts | 101 ++++ .../test/viewer-shared-cell-surface.test.ts | 10 +- .../use-cloud-app-session-status.test.tsx | 93 ++++ .../viewer/cloud-viewer-session.ts | 65 ++- apps/notebook-cloud/viewer/live-sync.ts | 162 ++++++- .../viewer/notebook-view-store-bridge.ts | 41 ++ apps/notebook-cloud/viewer/use-cloud-auth.ts | 36 +- apps/notebook/src/notebook-surface-stores.ts | 18 + apps/notebook/src/notebook-surface.ts | 1 + vitest.config.ts | 3 + 16 files changed, 1292 insertions(+), 17 deletions(-) create mode 100644 apps/notebook-cloud/test/cloud-projection-flicker.test.ts create mode 100644 apps/notebook-cloud/test/use-cloud-auth-session-identity.test.ts create mode 100644 apps/notebook-cloud/viewer/__tests__/use-cloud-app-session-status.test.tsx create mode 100644 apps/notebook/src/notebook-surface-stores.ts diff --git a/apps/notebook-cloud/src/cloudflare-types.ts b/apps/notebook-cloud/src/cloudflare-types.ts index 733ca3657..d0344df07 100644 --- a/apps/notebook-cloud/src/cloudflare-types.ts +++ b/apps/notebook-cloud/src/cloudflare-types.ts @@ -52,6 +52,15 @@ export interface DurableObjectState { waitUntil(promise: Promise): void; acceptWebSocket?(socket: CloudflareWebSocket, tags?: string[]): void; getWebSockets?(tag?: string): CloudflareWebSocket[]; + // Optional like the other hibernation APIs: the runtime answers matching + // text messages without waking the DO; fakes in tests need not implement + // it (the room feature-detects before calling). + setWebSocketAutoResponse?(pair: WebSocketRequestResponsePair): void; +} + +export interface WebSocketRequestResponsePair { + readonly request: string; + readonly response: string; } export interface DurableObjectStorage { diff --git a/apps/notebook-cloud/src/notebook-room.ts b/apps/notebook-cloud/src/notebook-room.ts index 5a16e1d1f..80b8ef134 100644 --- a/apps/notebook-cloud/src/notebook-room.ts +++ b/apps/notebook-cloud/src/notebook-room.ts @@ -1,4 +1,9 @@ -import type { CloudflareWebSocket, DurableObjectState, Env } from "./cloudflare-types.ts"; +import type { + CloudflareWebSocket, + DurableObjectState, + Env, + WebSocketRequestResponsePair, +} from "./cloudflare-types.ts"; import type { WorkstationAttachmentState } from "runtimed"; import { identityDisplayLabel } from "./display-label.ts"; import { @@ -15,6 +20,8 @@ import { frameSizeLimits, frameTypeName, isClientWritableFrame, + LIVENESS_PING, + LIVENESS_PONG, splitTypedFrame, type SessionControlMessage, type TypedFrame, @@ -117,6 +124,23 @@ export class NotebookRoom { private readonly state: DurableObjectState, private readonly env: Env, ) { + // CF-native liveness: the runtime answers the client's text ping for + // hibernatable sockets WITHOUT waking the DO. The pair persists across + // hibernation, so re-setting it per wake is idempotent; matching pings + // never reach webSocketMessage, so frame budgets and the hibernation + // restore path are untouched. Feature-detected like the other + // hibernation APIs (handleMessage answers manually as the fallback). + const pairCtor = ( + globalThis as { + WebSocketRequestResponsePair?: new ( + request: string, + response: string, + ) => WebSocketRequestResponsePair; + } + ).WebSocketRequestResponsePair; + if (pairCtor && this.state.setWebSocketAutoResponse) { + this.state.setWebSocketAutoResponse(new pairCtor(LIVENESS_PING, LIVENESS_PONG)); + } this.restoredPeersReady = this.restoreHibernatedPeers(); this.state.waitUntil(this.restoredPeersReady); } @@ -383,6 +407,17 @@ export class NotebookRoom { message: string | ArrayBuffer | ArrayBufferView, ): Promise { const incomingByteLength = webSocketMessageByteLength(message); + if (message === LIVENESS_PING) { + // Fallback for non-hibernation sockets and runtimes without + // setWebSocketAutoResponse — must answer BEFORE the binary-only + // rejection so pings never count toward consecutiveRejectedFrames. + try { + peer.socket.send(LIVENESS_PONG); + } catch { + // socket is closing; the close handler owns cleanup + } + return; + } if (typeof message === "string") { this.recordFrameBudget(notebookId, peer, "incoming", "text", incomingByteLength); this.rejectFrame( diff --git a/apps/notebook-cloud/src/protocol.ts b/apps/notebook-cloud/src/protocol.ts index 90fe37ff8..25ea5a1f8 100644 --- a/apps/notebook-cloud/src/protocol.ts +++ b/apps/notebook-cloud/src/protocol.ts @@ -11,6 +11,17 @@ export type { FrameSizeLimits, FrameTypeValue }; export const NOTEBOOK_PROTOCOL = "v4"; +/** + * Liveness probe text messages. The client pings on an interval; the room + * DO answers via the runtime's WebSocket auto-response (no DO wake), with + * a manual fallback in the room's message handler for runtimes without + * auto-response support. Text (not typed binary frames) on purpose: the + * auto-response API matches string messages, and the typed-frame channel + * stays binary-only. + */ +export const LIVENESS_PING = "nteract-liveness-ping"; +export const LIVENESS_PONG = "nteract-liveness-pong"; + export interface TypedFrame { type: FrameTypeValue; payload: Uint8Array; diff --git a/apps/notebook-cloud/test/cloud-projection-flicker.test.ts b/apps/notebook-cloud/test/cloud-projection-flicker.test.ts new file mode 100644 index 000000000..8bf5d8a88 --- /dev/null +++ b/apps/notebook-cloud/test/cloud-projection-flicker.test.ts @@ -0,0 +1,170 @@ +import { afterEach, describe, it } from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { getCellIdsSnapshot } from "@/components/notebook/state/cell-store"; +import { + getCellExecutionId, + getExecutionById, + resetNotebookExecutions, +} from "@/components/notebook/state/execution-store"; +import { getOutputById, resetNotebookOutputs } from "@/components/notebook/state/output-store"; +import { + projectCloudCellsIntoNotebookViewStores, + resetCloudProjectionUnlessPreserved, + resetCloudViewStoreProjection, +} from "../viewer/notebook-view-store-bridge.ts"; +import type { ResolvedCell } from "../viewer/render-resolution.ts"; + +// Field-observed flicker: with IndexedDB seeding, the persisted snapshot +// paints cells (and their outputs) well before the live-room effect +// settles. The effect re-runs for reasons that are NOT a notebook switch, +// and clearing ANY of the projected stores on those re-runs blanks the +// notebook into a full→empty→full flash. CodeCell renders outputs and +// execution counts exclusively through the execution/output stores, so the +// gate must keep or clear ALL projected stores together — these tests run +// the real gate against the real stores. +describe("cloud projection flicker gate", () => { + const PAINTED = "id:nb-1"; + + afterEach(() => { + resetCloudViewStoreProjection(); + resetNotebookExecutions(); + resetNotebookOutputs(); + }); + + function paintNotebook(): void { + projectCloudCellsIntoNotebookViewStores([ + { + id: "cell-code", + cellType: "code", + source: "print('painted')", + language: "python", + executionId: "exec-1", + executionCount: 3, + outputs: [ + { + output_type: "stream", + name: "stdout", + text: "painted output\n", + output_id: "out-1", + }, + ], + metadata: {}, + }, + { + id: "cell-md", + cellType: "markdown", + source: "# Painted", + language: null, + executionId: null, + executionCount: null, + outputs: [], + metadata: {}, + }, + ] satisfies ResolvedCell[]); + } + + function assertPainted(): void { + assert.deepEqual(getCellIdsSnapshot(), ["cell-code", "cell-md"]); + assert.equal(getCellExecutionId("cell-code"), "exec-1"); + assert.equal(getExecutionById("exec-1")?.execution_count, 3); + assert.deepEqual(getExecutionById("exec-1")?.output_ids, ["out-1"]); + const output = getOutputById("out-1"); + assert.equal(output?.output_type, "stream"); + } + + it("preserves cells, outputs, and execution pointers across a same-notebook re-run", () => { + paintNotebook(); + assertPainted(); + + const preserved = resetCloudProjectionUnlessPreserved({ + paintedNotebookIdentity: PAINTED, + nextNotebookIdentity: PAINTED, + }); + + assert.equal(preserved, true); + // The whole painted surface survives — not just the cell list. Wiping + // the execution/output stores while keeping cells still flickers every + // output and execution count (the dominant visual mass). + assertPainted(); + }); + + it("clears every projected store on a real notebook switch", () => { + paintNotebook(); + + const preserved = resetCloudProjectionUnlessPreserved({ + paintedNotebookIdentity: PAINTED, + nextNotebookIdentity: "id:nb-2", + }); + + assert.equal(preserved, false); + assert.deepEqual(getCellIdsSnapshot(), [] as string[]); + assert.equal(getCellExecutionId("cell-code"), null); + assert.equal(getExecutionById("exec-1"), undefined); + assert.equal(getOutputById("out-1"), undefined); + }); + + it("fails closed when no painted identity was recorded", () => { + paintNotebook(); + + const preserved = resetCloudProjectionUnlessPreserved({ + paintedNotebookIdentity: null, + nextNotebookIdentity: PAINTED, + }); + + assert.equal(preserved, false); + assert.deepEqual(getCellIdsSnapshot(), [] as string[]); + assert.equal(getOutputById("out-1"), undefined); + }); + + it("does not preserve an empty projection", () => { + const preserved = resetCloudProjectionUnlessPreserved({ + paintedNotebookIdentity: PAINTED, + nextNotebookIdentity: PAINTED, + }); + + assert.equal(preserved, false); + assert.deepEqual(getCellIdsSnapshot(), [] as string[]); + }); + + // Source pins for the session wiring that cannot run under node (the hook + // imports the component-bearing notebook surface): the cleanup and the + // next run's body must both route through the shared gate. + describe("cloud-viewer-session wiring", () => { + const sessionSource = readFileSync( + new URL("../viewer/cloud-viewer-session.ts", import.meta.url), + "utf8", + ); + + it("clears real notebook switches in the next run's body, before connecting", () => { + // The cleanup closes over its own run's config, so switch-clearing + // can only happen here — and a cleared switch also drops the painted + // identity so later gates fail closed. + assert.match( + sessionSource, + /const preservedAcrossRuns = resetCloudProjectionUnlessPreserved\(\{\s*paintedNotebookIdentity: paintedNotebookIdentityRef\.current,\s*nextNotebookIdentity: `id:\$\{config\.notebookId\}`,\s*\}\);\s*if \(!preservedAcrossRuns\) \{\s*paintedNotebookIdentityRef\.current = null;\s*\}/, + ); + }); + + it("gates the cleanup on the shared store gate with only the pool reset unconditional", () => { + assert.match( + sessionSource, + /resetCloudProjectionUnlessPreserved\(\{\s*paintedNotebookIdentity: paintedNotebookIdentityRef\.current,\s*nextNotebookIdentity: `id:\$\{config\.notebookId\}`,\s*\}\);\s*resetPoolState\(\);/, + ); + }); + + it("tracks the painted notebook identity only when cells actually painted", () => { + assert.match( + sessionSource, + /if \(resolvedCells\.length > 0\) \{\s*paintedNotebookIdentityRef\.current = `id:\$\{config\.notebookId\}`;\s*\}/, + ); + }); + + it("keeps an unconditional full clear on true unmount", () => { + assert.match( + sessionSource, + /useEffect\(\s*\(\) => \(\) => \{\s*resetCloudViewStoreProjection\(\);\s*resetRuntimeState\(\);\s*resetRuntimeStoresProjection\(\);\s*\},\s*\[\],\s*\);/, + ); + }); + }); +}); diff --git a/apps/notebook-cloud/test/live-sync.test.ts b/apps/notebook-cloud/test/live-sync.test.ts index 11fc03ced..fb69a6d01 100644 --- a/apps/notebook-cloud/test/live-sync.test.ts +++ b/apps/notebook-cloud/test/live-sync.test.ts @@ -21,9 +21,10 @@ import { syncUrl, syncableCloudHandle, withReadyTimeout, + type CloudConnectTarget, type CloudWebSocketTransportOptions, } from "../viewer/live-sync.ts"; -import { FrameType } from "../src/protocol.ts"; +import { FrameType, LIVENESS_PING, LIVENESS_PONG } from "../src/protocol.ts"; describe("cloud live sync", () => { it("accepts known connection scopes", () => { @@ -981,6 +982,445 @@ describe("cloud transport reconnect loop", () => { } }); + it("recycles the live socket when the browser reports offline and recovers on online", async (t) => { + t.mock.timers.enable({ apis: ["setTimeout"] }); + const fake = installFakeWebSocket(); + const fakeWindow = installFakeWindow(); + try { + const lost: Error[] = []; + const statuses: string[] = []; + const transport = createTransport({ + random: () => 0.5, + onConnectionLost: (reason) => lost.push(reason), + }); + transport.connectionStatus$.subscribe((status) => statuses.push(status)); + const first = await waitForSocket(0); + first.open(); + first.ready("peer-1"); + await transport.ready; + assert.equal(statuses.at(-1), "online"); + + // OS-level offline: the zombie socket would never fire close/error. + // The browser event proactively tears it down and flips status. + fakeWindow.dispatchEvent(new Event("offline")); + assert.equal(lost.length, 1); + assert.match(lost[0].message, /browser reported offline/); + assert.equal(first.readyState, FakeWebSocket.CLOSED); + assert.equal(statuses.at(-1), "reconnecting"); + + // A second offline event with no socket is a no-op (no double loss). + fakeWindow.dispatchEvent(new Event("offline")); + assert.equal(lost.length, 1); + + // Connectivity returns: the online handler short-circuits the backoff. + fakeWindow.dispatchEvent(new Event("online")); + const second = await waitForSocket(1); + second.open(); + second.ready("peer-2"); + await drainMicrotasks(); + assert.equal(statuses.at(-1), "online"); + + transport.disconnect(); + // After manual disconnect the offline listener is unregistered. + fakeWindow.dispatchEvent(new Event("offline")); + assert.equal(lost.length, 1); + assert.equal(statuses.at(-1), "offline"); + } finally { + fakeWindow.restore(); + fake.restore(); + } + }); + + it("sends liveness pings after ready and a timely pong keeps the connection alive", async (t) => { + t.mock.timers.enable({ apis: ["setTimeout", "setInterval"] }); + const fake = installFakeWebSocket(); + try { + const lost: Error[] = []; + const transport = createTransport({ + livenessPingIntervalMs: 20_000, + livenessPongDeadlineMs: 10_000, + onConnectionLost: (reason) => lost.push(reason), + }); + const socket = await waitForSocket(0); + socket.open(); + socket.ready("peer-1"); + await transport.ready; + assert.deepEqual(socket.sentText, [], "no ping before the interval elapses"); + + t.mock.timers.tick(20_000); + assert.deepEqual(socket.sentText, [LIVENESS_PING]); + + // Pong arrives within the deadline: the connection stays healthy + // past the would-be deadline, and the next interval pings again. + socket.message(LIVENESS_PONG); + await drainMicrotasks(); + t.mock.timers.tick(10_000); + await drainMicrotasks(); + assert.equal(lost.length, 0, "answered ping must not count as loss"); + + t.mock.timers.tick(10_000); + assert.deepEqual(socket.sentText, [LIVENESS_PING, LIVENESS_PING]); + + // Manual disconnect stops the probe. + socket.message(LIVENESS_PONG); + await drainMicrotasks(); + transport.disconnect(); + t.mock.timers.tick(60_000); + assert.equal(socket.sentText.length, 2, "no pings after disconnect"); + assert.equal(lost.length, 0); + } finally { + fake.restore(); + } + }); + + it("treats a missed liveness pong as connection loss and restarts the probe on reconnect", async (t) => { + t.mock.timers.enable({ apis: ["setTimeout", "setInterval"] }); + const fake = installFakeWebSocket(); + try { + const lost: Error[] = []; + const statuses: string[] = []; + const transport = createTransport({ + livenessPingIntervalMs: 20_000, + livenessPongDeadlineMs: 10_000, + random: () => 0, + onConnectionLost: (reason) => lost.push(reason), + }); + transport.connectionStatus$.subscribe((status) => statuses.push(status)); + const first = await waitForSocket(0); + first.open(); + first.ready("peer-1"); + await transport.ready; + + t.mock.timers.tick(20_000); + assert.deepEqual(first.sentText, [LIVENESS_PING]); + + // The zombie socket stays OPEN and never answers. One tick before + // the deadline nothing happens; at the deadline the link is declared + // dead even though no close/error event ever fired. + t.mock.timers.tick(9_999); + await drainMicrotasks(); + assert.equal(lost.length, 0); + t.mock.timers.tick(1); + assert.equal(lost.length, 1); + assert.match(lost[0].message, /liveness pong missed/); + assert.equal(statuses.at(-1), "reconnecting"); + + // Retry (random()=0 → 0.5×base = 500ms), then the probe restarts on + // the NEW connection — the old interval is gone. + t.mock.timers.tick(500); + const second = await waitForSocket(1); + second.open(); + second.ready("peer-2"); + await drainMicrotasks(); + assert.equal(statuses.at(-1), "online"); + + t.mock.timers.tick(20_000); + assert.deepEqual(second.sentText, [LIVENESS_PING]); + assert.equal(first.sentText.length, 1, "stale probe must not ping the dead socket"); + + // A late pong surfacing from the DEAD first socket must not clear + // the NEW connection's outstanding deadline: the dead socket's + // listeners are detached, so its messages never reach the transport. + first.message(LIVENESS_PONG); + await drainMicrotasks(); + t.mock.timers.tick(10_000); + assert.equal(lost.length, 2, "the stale pong did not satisfy the new probe"); + assert.match(lost[1].message, /liveness pong missed/); + + transport.disconnect(); + } finally { + fake.restore(); + } + }); + + it("treats any inbound frame as liveness evidence while the pong is queued behind backlog", async (t) => { + t.mock.timers.enable({ apis: ["setTimeout", "setInterval"] }); + const fake = installFakeWebSocket(); + try { + const lost: Error[] = []; + const transport = createTransport({ + livenessPingIntervalMs: 20_000, + livenessPongDeadlineMs: 10_000, + onConnectionLost: (reason) => lost.push(reason), + }); + const socket = await waitForSocket(0); + socket.open(); + socket.ready("peer-1"); + await transport.ready; + + t.mock.timers.tick(20_000); // ping; deadline at +10s + assert.deepEqual(socket.sentText, [LIVENESS_PING]); + + // 5s in, a sync frame arrives. The pong shares the in-order WS + // stream and is queued behind the sync backlog, but a connection + // actively delivering frames is demonstrably alive. + t.mock.timers.tick(5_000); + socket.message(new Uint8Array([FrameType.AUTOMERGE_SYNC, 9]).buffer); + await drainMicrotasks(); + + t.mock.timers.tick(5_000); // the original deadline passes harmlessly + await drainMicrotasks(); + assert.equal(lost.length, 0, "frame delivery cleared the pong deadline"); + + // The probe still catches a TRUE zombie: the next ping re-arms and + // nothing at all arrives this time. + t.mock.timers.tick(10_000); // ping #2 + assert.equal(socket.sentText.length, 2); + t.mock.timers.tick(10_000); // its deadline + assert.equal(lost.length, 1); + assert.match(lost[0].message, /liveness pong missed/); + + transport.disconnect(); + } finally { + fake.restore(); + } + }); + + it("holds the ORIGINAL pong deadline when pings overlap an outstanding deadline", async (t) => { + t.mock.timers.enable({ apis: ["setTimeout", "setInterval"] }); + const fake = installFakeWebSocket(); + try { + const lost: Error[] = []; + // deadline > interval makes pings overlap: per-outstanding-ping + // semantics keep the EARLIEST deadline instead of sliding it. + const transport = createTransport({ + livenessPingIntervalMs: 5_000, + livenessPongDeadlineMs: 12_000, + onConnectionLost: (reason) => lost.push(reason), + }); + const socket = await waitForSocket(0); + socket.open(); + socket.ready("peer-1"); + await transport.ready; + + t.mock.timers.tick(5_000); // ping #1: deadline at t=17s + t.mock.timers.tick(5_000); // ping #2: original deadline kept + t.mock.timers.tick(5_000); // ping #3: original deadline kept + assert.equal(socket.sentText.length, 3); + + t.mock.timers.tick(1_999); // t=16.999s + await drainMicrotasks(); + assert.equal(lost.length, 0); + t.mock.timers.tick(1); // t=17s: ping#1+12s, NOT ping#3+12s + assert.equal(lost.length, 1); + assert.match(lost[0].message, /liveness pong missed/); + + transport.disconnect(); + } finally { + fake.restore(); + } + }); + + it("accepts a pong arriving one tick before the deadline", async (t) => { + t.mock.timers.enable({ apis: ["setTimeout", "setInterval"] }); + const fake = installFakeWebSocket(); + try { + const lost: Error[] = []; + const transport = createTransport({ + livenessPingIntervalMs: 20_000, + livenessPongDeadlineMs: 10_000, + onConnectionLost: (reason) => lost.push(reason), + }); + const socket = await waitForSocket(0); + socket.open(); + socket.ready("peer-1"); + await transport.ready; + + t.mock.timers.tick(20_000); // ping; deadline at t=30s + t.mock.timers.tick(9_999); // t=29.999s: pong arrives just in time + socket.message(LIVENESS_PONG); + await drainMicrotasks(); + t.mock.timers.tick(1); // the would-be deadline tick + await drainMicrotasks(); + assert.equal(lost.length, 0, "an in-deadline pong wins"); + + // Enforcement still works on the next cycle. + t.mock.timers.tick(10_000); // t=40s: ping #2 + assert.equal(socket.sentText.length, 2); + t.mock.timers.tick(10_000); // t=50s: missed + assert.equal(lost.length, 1); + + transport.disconnect(); + } finally { + fake.restore(); + } + }); + + it("recycles the connection when a liveness ping send fails", async (t) => { + t.mock.timers.enable({ apis: ["setTimeout", "setInterval"] }); + const fake = installFakeWebSocket(); + try { + const lost: Error[] = []; + const transport = createTransport({ + livenessPingIntervalMs: 20_000, + livenessPongDeadlineMs: 10_000, + onConnectionLost: (reason) => lost.push(reason), + }); + const socket = await waitForSocket(0); + socket.open(); + socket.ready("peer-1"); + await transport.ready; + + socket.throwOnSend = true; + t.mock.timers.tick(20_000); + assert.equal(lost.length, 1); + assert.match(lost[0].message, /cloud sync liveness ping failed: synthetic send failure/); + assert.equal(socket.readyState, FakeWebSocket.CLOSED); + + transport.disconnect(); + } finally { + fake.restore(); + } + }); + + it("livenessPingIntervalMs: 0 disables the probe entirely", async (t) => { + t.mock.timers.enable({ apis: ["setTimeout", "setInterval"] }); + const fake = installFakeWebSocket(); + try { + const lost: Error[] = []; + const transport = createTransport({ + livenessPingIntervalMs: 0, + onConnectionLost: (reason) => lost.push(reason), + }); + const socket = await waitForSocket(0); + socket.open(); + socket.ready("peer-1"); + await transport.ready; + + t.mock.timers.tick(600_000); + await drainMicrotasks(); + assert.deepEqual(socket.sentText, [] as string[]); + assert.equal(lost.length, 0); + + transport.disconnect(); + } finally { + fake.restore(); + } + }); + + it("does not arm the pong deadline while the document is hidden", async (t) => { + t.mock.timers.enable({ apis: ["setTimeout", "setInterval"] }); + const fake = installFakeWebSocket(); + const visibility = { state: "hidden" }; + const originalDocument = (globalThis as { document?: unknown }).document; + (globalThis as { document?: unknown }).document = { + get visibilityState() { + return visibility.state; + }, + }; + try { + const lost: Error[] = []; + const transport = createTransport({ + livenessPingIntervalMs: 20_000, + livenessPongDeadlineMs: 10_000, + onConnectionLost: (reason) => lost.push(reason), + }); + const socket = await waitForSocket(0); + socket.open(); + socket.ready("peer-1"); + await transport.ready; + + // Hidden tab: background timer throttling could fire a suspended + // deadline on resume before the queued pong dispatches. Pings still + // flow, enforcement does not. + t.mock.timers.tick(20_000); + assert.deepEqual(socket.sentText, [LIVENESS_PING]); + t.mock.timers.tick(15_000); // far past any would-be deadline + await drainMicrotasks(); + assert.equal(lost.length, 0, "no deadline armed while hidden"); + + // Back in the foreground: the next ping re-arms enforcement. + visibility.state = "visible"; + t.mock.timers.tick(5_000); // t=40s: ping #2 arms the deadline + assert.equal(socket.sentText.length, 2); + t.mock.timers.tick(10_000); // t=50s: missed + assert.equal(lost.length, 1); + assert.match(lost[0].message, /liveness pong missed/); + + transport.disconnect(); + } finally { + (globalThis as { document?: unknown }).document = originalDocument; + fake.restore(); + } + }); + + it("offline while parked awaiting connectTarget is a no-op; online supersedes the parked attempt", async (t) => { + t.mock.timers.enable({ apis: ["setTimeout"] }); + const fake = installFakeWebSocket(); + const fakeWindow = installFakeWindow(); + try { + const lost: Error[] = []; + const resolvers: Array<(target: CloudConnectTarget) => void> = []; + const transport = new CloudWebSocketTransport({ + connectTarget: () => + new Promise((resolve) => { + resolvers.push(resolve); + }), + onConnectionLost: (reason) => lost.push(reason), + }); + await drainMicrotasks(); + assert.equal(resolvers.length, 1, "attempt parked awaiting connectTarget()"); + assert.equal(FakeWebSocket.instances.length, 0); + + // Offline with no socket: nothing to recycle, no spurious loss. + fakeWindow.dispatchEvent(new Event("offline")); + assert.equal(lost.length, 0); + + // Connectivity returns: the online handler supersedes the parked + // attempt (connect() bumps the epoch). + fakeWindow.dispatchEvent(new Event("online")); + await drainMicrotasks(); + assert.equal(resolvers.length, 2); + + // The ORIGINAL target settling late is discarded harmlessly. + resolvers[0]({ url: new URL("wss://example.test/n/room/sync"), protocols: [] }); + await drainMicrotasks(); + assert.equal(FakeWebSocket.instances.length, 0, "stale attempt opens no socket"); + + // The superseding attempt proceeds to a healthy session. + resolvers[1]({ url: new URL("wss://example.test/n/room/sync"), protocols: [] }); + const socket = await waitForSocket(0); + socket.open(); + socket.ready("peer-1"); + await transport.ready; + assert.equal(lost.length, 0); + + transport.disconnect(); + } finally { + fakeWindow.restore(); + fake.restore(); + } + }); + + it("offline during the pre-ready handshake window recycles with status connecting", async (t) => { + t.mock.timers.enable({ apis: ["setTimeout"] }); + const fake = installFakeWebSocket(); + const fakeWindow = installFakeWindow(); + try { + const lost: Error[] = []; + const statuses: string[] = []; + const transport = createTransport({ + random: () => 0.5, + onConnectionLost: (reason) => lost.push(reason), + }); + transport.connectionStatus$.subscribe((status) => statuses.push(status)); + const socket = await waitForSocket(0); + socket.open(); // opened, but cloud_room_ready never arrived + + fakeWindow.dispatchEvent(new Event("offline")); + assert.equal(lost.length, 1); + assert.match(lost[0].message, /browser reported offline/); + assert.equal(socket.readyState, FakeWebSocket.CLOSED); + // Never-ready loss keeps the pre-first-handshake status. + assert.equal(statuses.at(-1), "connecting"); + + transport.disconnect(); + } finally { + fakeWindow.restore(); + fake.restore(); + } + }); + it("processes a sync frame arriving immediately after a reconnect ready AFTER the re-establish", async () => { const fake = installFakeWebSocket(); const order: string[] = []; @@ -1640,6 +2080,8 @@ class FakeWebSocket extends EventTarget { readyState = FakeWebSocket.CONNECTING; throwOnSend = false; sent: Uint8Array[] = []; + /** Text sends (liveness pings) recorded separately from binary frames. */ + sentText: string[] = []; readonly url: string; constructor(url?: unknown) { @@ -1652,6 +2094,10 @@ class FakeWebSocket extends EventTarget { if (this.throwOnSend) { throw new Error("synthetic send failure"); } + if (typeof data === "string") { + this.sentText.push(data); + return; + } if (data instanceof Uint8Array) { this.sent.push(data); } diff --git a/apps/notebook-cloud/test/notebook-room.test.ts b/apps/notebook-cloud/test/notebook-room.test.ts index 6126819b9..9c64488bb 100644 --- a/apps/notebook-cloud/test/notebook-room.test.ts +++ b/apps/notebook-cloud/test/notebook-room.test.ts @@ -21,6 +21,8 @@ import { } from "../src/notebook-room.ts"; import { FrameType, + LIVENESS_PING, + LIVENESS_PONG, decodeJsonPayload, encodeTypedFrame, splitTypedFrame, @@ -316,6 +318,108 @@ describe("NotebookRoom peer lifecycle", () => { ); }); + it("arms the CF auto-response liveness pair when the runtime supports it", () => { + const pairs: Array<{ request: string; response: string }> = []; + class PairCtor { + constructor( + readonly request: string, + readonly response: string, + ) {} + } + const globals = globalThis as { WebSocketRequestResponsePair?: unknown }; + const original = globals.WebSocketRequestResponsePair; + globals.WebSocketRequestResponsePair = PairCtor; + try { + const state = fakeState() as DurableObjectState & { + setWebSocketAutoResponse?: (pair: { request: string; response: string }) => void; + }; + state.setWebSocketAutoResponse = (pair) => pairs.push(pair); + new NotebookRoom(state, {} as Env); + assert.equal(pairs.length, 1); + assert.equal(pairs[0].request, LIVENESS_PING); + assert.equal(pairs[0].response, LIVENESS_PONG); + + // Hibernation wake = a fresh constructor run against the SAME durable + // state. Re-arming per wake must be an idempotent re-set of the same + // pair — the load-bearing claim behind arming in the constructor. + new NotebookRoom(state, {} as Env); + assert.equal(pairs.length, 2); + assert.equal(pairs[1].request, pairs[0].request); + assert.equal(pairs[1].response, pairs[0].response); + + // Older runtime shape: the global constructor exists but the state + // lacks setWebSocketAutoResponse. The second conjunct of the feature + // detection must keep the constructor from throwing — a TypeError + // here is a total room outage, not a degraded probe. + assert.doesNotThrow(() => new NotebookRoom(fakeState(), {} as Env)); + } finally { + globals.WebSocketRequestResponsePair = original; + } + // Feature detection: without the global constructor (every other test in + // this file), the constructor must not call setWebSocketAutoResponse. + const uncalled: unknown[] = []; + const bare = fakeState() as DurableObjectState & { + setWebSocketAutoResponse?: (pair: unknown) => void; + }; + bare.setWebSocketAutoResponse = (pair) => uncalled.push(pair); + new NotebookRoom(bare, {} as Env); + assert.equal(uncalled.length, 0); + }); + + it("answers liveness pings without counting them toward rejected-frame close", async () => { + const room = new NotebookRoom(fakeState(), {} as Env); + const identity = authenticateDevRequest( + new Request("https://cloud.test/n/demo/sync?user=alice&operator=desktop:a&scope=editor"), + ); + const socket = new FakeSocket(); + const peer = { + id: "peer-a", + socket: socket.asCloudflareWebSocket(), + identity, + connectedAt: "2026-05-22T00:00:00.000Z", + consecutiveRejectedFrames: 0, + }; + const harness = roomHarness(room); + harness.peers.set(peer.id, peer); + harness.materializers.set("demo", fakeMaterializer(noopMaterializedResult())); + + // Fallback path for runtimes without setWebSocketAutoResponse: pings are + // text frames, but they must never ride the binary-only rejection. + for (let i = 0; i < 20; i += 1) { + await harness.handleMessage("demo", peer, LIVENESS_PING); + } + + assert.equal(socket.closed, false); + assert.equal(peer.consecutiveRejectedFrames, 0); + assert.equal(socket.sent.length, 20); + assert( + socket.sent.every((frame) => new TextDecoder().decode(frame) === LIVENESS_PONG), + "every ping is answered with a pong, not a rejection control frame", + ); + }); + + it("swallows pong send failures on a closing socket", async () => { + const room = new NotebookRoom(fakeState(), {} as Env); + const identity = authenticateDevRequest( + new Request("https://cloud.test/n/demo/sync?user=alice&operator=desktop:a&scope=editor"), + ); + const socket = new FakeSocket({ throwOnSend: true }); + const peer = { + id: "peer-a", + socket: socket.asCloudflareWebSocket(), + identity, + connectedAt: "2026-05-22T00:00:00.000Z", + consecutiveRejectedFrames: 0, + }; + const harness = roomHarness(room); + harness.peers.set(peer.id, peer); + + await harness.handleMessage("demo", peer, LIVENESS_PING); + + assert.equal(peer.consecutiveRejectedFrames, 0); + assert.equal(harness.peers.has(peer.id), true); + }); + it("does not count server-side room host failures toward peer close", async () => { const room = new NotebookRoom(fakeState(), {} as Env); const identity = authenticateDevRequest( diff --git a/apps/notebook-cloud/test/use-cloud-auth-session-identity.test.ts b/apps/notebook-cloud/test/use-cloud-auth-session-identity.test.ts new file mode 100644 index 000000000..f296edab2 --- /dev/null +++ b/apps/notebook-cloud/test/use-cloud-auth-session-identity.test.ts @@ -0,0 +1,101 @@ +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; +import { + cloudAppSessionsEqual, + nextCloudAppSessionReadyState, + type CloudAppSessionViewState, +} from "../viewer/use-cloud-auth.ts"; +import type { CloudAppSession } from "../viewer/app-session.ts"; + +// The mount-time /api/auth/session fetch re-confirms a session the page was +// already rendered with. Installing a fresh-but-content-identical session +// object changed the identity of every effect dependency derived from it +// (resolveSyncAuth → the live-room effect), tearing down and reconnecting the +// live room once per page load. The ready-state reducer keeps object +// identities stable so React bails out of the redundant update entirely. +describe("cloud app session identity stability", () => { + const session = (overrides: Partial = {}): CloudAppSession => ({ + provider: "oidc", + expires_at: 1_750_000_000, + ...overrides, + }); + + describe("cloudAppSessionsEqual", () => { + it("treats content-identical sessions as equal across object identities", () => { + assert.equal(cloudAppSessionsEqual(session(), session()), true); + }); + + it("compares by reference, null, and each field", () => { + const a = session(); + assert.equal(cloudAppSessionsEqual(a, a), true); + assert.equal(cloudAppSessionsEqual(null, null), true); + assert.equal(cloudAppSessionsEqual(a, null), false); + assert.equal(cloudAppSessionsEqual(null, a), false); + assert.equal(cloudAppSessionsEqual(a, session({ expires_at: 1_750_000_001 })), false); + }); + }); + + describe("nextCloudAppSessionReadyState", () => { + it("returns the CURRENT state object when the fetch only confirms it", () => { + const current: CloudAppSessionViewState = { + status: "ready", + session: session(), + error: null, + }; + const next = nextCloudAppSessionReadyState(current, session()); + assert.equal(next, current, "content-identical fetch must not produce a new state object"); + }); + + it("keeps the current SESSION object when only the wrapper would change", () => { + const keptSession = session(); + const current: CloudAppSessionViewState = { + status: "loading", + session: keptSession, + error: null, + }; + const next = nextCloudAppSessionReadyState(current, session()); + assert.notEqual(next, current, "loading → ready is a real transition"); + assert.equal(next.status, "ready"); + assert.equal(next.session, keptSession, "session identity survives the transition"); + assert.equal(next.error, null); + }); + + it("adopts a genuinely different session", () => { + const current: CloudAppSessionViewState = { + status: "ready", + session: session(), + error: null, + }; + const renewed = session({ expires_at: 1_750_009_999 }); + const next = nextCloudAppSessionReadyState(current, renewed); + assert.notEqual(next, current); + assert.equal(next.session, renewed); + assert.equal(next.status, "ready"); + }); + + it("transitions to ready with null when the fetch reports no session", () => { + const current: CloudAppSessionViewState = { + status: "ready", + session: session(), + error: null, + }; + const next = nextCloudAppSessionReadyState(current, null); + assert.notEqual(next, current); + assert.deepEqual(next, { status: "ready", session: null, error: null }); + }); + + it("clears a previous error even when the session content matches", () => { + const keptSession = session(); + const current: CloudAppSessionViewState = { + status: "error", + session: keptSession, + error: "fetch failed", + }; + const next = nextCloudAppSessionReadyState(current, session()); + assert.notEqual(next, current); + assert.equal(next.status, "ready"); + assert.equal(next.error, null); + assert.equal(next.session, keptSession); + }); + }); +}); diff --git a/apps/notebook-cloud/test/viewer-shared-cell-surface.test.ts b/apps/notebook-cloud/test/viewer-shared-cell-surface.test.ts index b3fa91246..d80888c1e 100644 --- a/apps/notebook-cloud/test/viewer-shared-cell-surface.test.ts +++ b/apps/notebook-cloud/test/viewer-shared-cell-surface.test.ts @@ -34,7 +34,13 @@ test("cloud viewer imports desktop notebook code only through public surfaces", for (const match of imports) { const importPath = match[1] ?? ""; - if (importPath.includes("/wasm/") || importPath.endsWith("/notebook-surface")) { + if ( + importPath.includes("/wasm/") || + importPath.endsWith("/notebook-surface") || + // Headless store surface: same public symbols, no component/CSS + // imports, so node-run tests can exercise the bridge directly. + importPath.endsWith("/notebook-surface-stores") + ) { continue; } offenders.push(`${fileName}: ${importPath}`); @@ -59,7 +65,7 @@ test("cloud projects live cells into the NotebookView stores", () => { ); assert.match( sessionSourceText, - /const applyResolvedCells = useCallback\(\(resolvedCells: ResolvedCell\[\]\) => \{[\s\S]*projectCloudCellsIntoNotebookViewStores\(resolvedCells\);[\s\S]*setCells\(resolvedCells\);/, + /const applyResolvedCells = useCallback\(\s*\(resolvedCells: ResolvedCell\[\]\) => \{[\s\S]*projectCloudCellsIntoNotebookViewStores\(resolvedCells\);[\s\S]*setCells\(resolvedCells\);/, ); assert.match(sessionSourceText, /applyResolvedCells\(syncCells\);/); assert.match(sessionSourceText, /applyResolvedCells\(progressiveCells\);/); diff --git a/apps/notebook-cloud/viewer/__tests__/use-cloud-app-session-status.test.tsx b/apps/notebook-cloud/viewer/__tests__/use-cloud-app-session-status.test.tsx new file mode 100644 index 000000000..4fc7123f0 --- /dev/null +++ b/apps/notebook-cloud/viewer/__tests__/use-cloud-app-session-status.test.tsx @@ -0,0 +1,93 @@ +import { act, renderHook, waitFor } from "@testing-library/react"; +import { describe, expect, it, vi, beforeEach } from "vite-plus/test"; +import type { CloudAppSession } from "../app-session"; + +// The mount-time /api/auth/session fetch usually CONFIRMS the session the +// page was rendered with. The hook must reduce that through +// nextCloudAppSessionReadyState so the session object identity is kept — +// the session feeds effect dependency chains (resolveSyncAuth → the live +// room effect), and a fresh-but-content-identical object tears down and +// reconnects the live room once per page load. These tests render the real +// hook, pinning the setState updater wiring that pure-reducer tests cannot. + +const mocks = vi.hoisted(() => ({ + readCloudAppSessionStatus: vi.fn<() => Promise<{ ok: true; session: CloudAppSession | null }>>(), +})); + +vi.mock("../app-session", async (importOriginal) => ({ + ...(await importOriginal()), + readCloudAppSessionStatus: mocks.readCloudAppSessionStatus, +})); + +import { useCloudAppSessionStatus } from "../use-cloud-auth"; + +describe("useCloudAppSessionStatus", () => { + beforeEach(() => { + mocks.readCloudAppSessionStatus.mockReset(); + }); + + const session = (overrides: Partial = {}): CloudAppSession => ({ + provider: "oidc", + expires_at: 1_750_000_000, + ...overrides, + }); + + it("keeps the session object reference across content-equal confirming fetches", async () => { + const initial = session(); + mocks.readCloudAppSessionStatus.mockResolvedValue({ ok: true, session: session() }); + + const { result } = renderHook(() => useCloudAppSessionStatus(initial)); + expect(result.current.session).toBe(initial); + + await waitFor(() => expect(mocks.readCloudAppSessionStatus).toHaveBeenCalledTimes(1)); + await act(async () => {}); + + expect(result.current.status).toBe("ready"); + // The wiring pin: the fetch returned a fresh-but-content-identical + // object, and the hook must keep the ORIGINAL reference. + expect(result.current.session).toBe(initial); + + // A manual refresh that confirms again keeps it too. + act(() => { + result.current.refreshAppSessionStatus(); + }); + await waitFor(() => expect(mocks.readCloudAppSessionStatus).toHaveBeenCalledTimes(2)); + await act(async () => {}); + expect(result.current.session).toBe(initial); + }); + + it("adopts a genuinely renewed session", async () => { + const initial = session(); + const renewed = session({ expires_at: 1_750_009_999 }); + mocks.readCloudAppSessionStatus.mockResolvedValue({ ok: true, session: renewed }); + + const { result } = renderHook(() => useCloudAppSessionStatus(initial)); + await waitFor(() => expect(mocks.readCloudAppSessionStatus).toHaveBeenCalledTimes(1)); + await act(async () => {}); + + expect(result.current.status).toBe("ready"); + expect(result.current.session).toBe(renewed); + }); + + it("moves loading to ready with the fetched session when mounted without one", async () => { + const fetched = session(); + mocks.readCloudAppSessionStatus.mockResolvedValue({ ok: true, session: fetched }); + + const { result } = renderHook(() => useCloudAppSessionStatus(null)); + expect(result.current.status).toBe("loading"); + + await waitFor(() => expect(result.current.status).toBe("ready")); + expect(result.current.session).toBe(fetched); + }); + + it("reports fetch failures without dropping the session it already has", async () => { + const initial = session(); + mocks.readCloudAppSessionStatus.mockRejectedValue(new Error("session endpoint down")); + + const { result } = renderHook(() => useCloudAppSessionStatus(initial)); + await waitFor(() => expect(result.current.status).toBe("error")); + + expect(result.current.error).toBe("session endpoint down"); + expect(result.current.session).toBe(initial); + }); +}); diff --git a/apps/notebook-cloud/viewer/cloud-viewer-session.ts b/apps/notebook-cloud/viewer/cloud-viewer-session.ts index 1de0e651c..6a6beca8b 100644 --- a/apps/notebook-cloud/viewer/cloud-viewer-session.ts +++ b/apps/notebook-cloud/viewer/cloud-viewer-session.ts @@ -54,6 +54,7 @@ import type { CloudViewerLoadingPolicy } from "./loading-policy"; import { markCloudViewerLoadMilestone } from "./load-milestones"; import { projectCloudCellsIntoNotebookViewStores, + resetCloudProjectionUnlessPreserved, resetCloudViewStoreProjection, } from "./notebook-view-store-bridge"; import { CloudViewerPresenceStore } from "./presence"; @@ -173,12 +174,31 @@ export function useCloudViewerSession({ const [connectionError, setConnectionError] = useState(null); const [connectAttempt, setConnectAttempt] = useState(0); - const applyResolvedCells = useCallback((resolvedCells: ResolvedCell[]) => { - projectCloudCellsIntoNotebookViewStores(resolvedCells); - setCells(resolvedCells); - }, []); + // Identity of the notebook whose cells are currently painted into the + // view stores — the flicker gate's "previous" side (see effect cleanup). + const paintedNotebookIdentityRef = useRef(null); + const applyResolvedCells = useCallback( + (resolvedCells: ResolvedCell[]) => { + projectCloudCellsIntoNotebookViewStores(resolvedCells); + if (resolvedCells.length > 0) { + paintedNotebookIdentityRef.current = `id:${config.notebookId}`; + } + setCells(resolvedCells); + }, + [config.notebookId], + ); - useEffect(() => resetCloudViewStoreProjection, []); + // True unmount: nothing is preserved across a session teardown — clear + // every store the projection paints (the live effect's cleanup preserves + // them for same-notebook re-runs, so it cannot be the unmount janitor). + useEffect( + () => () => { + resetCloudViewStoreProjection(); + resetRuntimeState(); + resetRuntimeStoresProjection(); + }, + [], + ); useEffect(() => { if (authRenewalKind === "refreshing") { @@ -657,6 +677,21 @@ export function useCloudViewerSession({ }; materializeLiveRuntimeRef.current = materializeLiveCellsSafely; + // Notebook-switch gate (desktop beforeBootstrap placement): the effect + // CLEANUP closes over its own run's config, so it can only ever compare + // the painted identity against itself — a real switch is visible only + // here, to the NEXT run. Before connecting, clear every projected store + // when the painted cells belong to a different notebook (or nothing + // usable is painted); a same-notebook re-run's paint survives untouched + // and is replaced wholesale by this run's materialization. + const preservedAcrossRuns = resetCloudProjectionUnlessPreserved({ + paintedNotebookIdentity: paintedNotebookIdentityRef.current, + nextNotebookIdentity: `id:${config.notebookId}`, + }); + if (!preservedAcrossRuns) { + paintedNotebookIdentityRef.current = null; + } + presenceStore.reset(); resetRuntimeState(); setConnectionError(null); @@ -897,9 +932,23 @@ export function useCloudViewerSession({ void teardownFlush.catch(() => undefined).then(() => persistenceAdapter.close()); } setCrdtCommWriter(null); - resetCloudViewStoreProjection(); - resetRuntimeState(); - resetRuntimeStoresProjection(); + // Flicker gate (desktop bootstrap-preservation pattern): the live + // effect re-runs for reasons that are NOT a notebook switch — OIDC + // refreshes, manual retries. With IDB seeding, paint #1 lands before + // those re-runs, and an unconditional clear here blanked a painted + // notebook into a full→empty→full flicker. The gate covers EVERY + // projected store together: CodeCell reads outputs and execution + // counts from the execution/output stores, so preserving cells while + // wiping those still flickered the dominant visual mass. Within this + // closure nextNotebookIdentity always equals the painted identity + // whenever cells are painted, so this honestly reduces to "painted + // with visible cells ⇒ preserve"; REAL notebook switches are cleared + // by the next run's body gate, and true unmount clears via the + // mount-scoped effect above. + resetCloudProjectionUnlessPreserved({ + paintedNotebookIdentity: paintedNotebookIdentityRef.current, + nextNotebookIdentity: `id:${config.notebookId}`, + }); resetPoolState(); livePresenceStore = null; presenceStore.reduceConnection("disconnected"); diff --git a/apps/notebook-cloud/viewer/live-sync.ts b/apps/notebook-cloud/viewer/live-sync.ts index 4d72100a2..a1a1dc333 100644 --- a/apps/notebook-cloud/viewer/live-sync.ts +++ b/apps/notebook-cloud/viewer/live-sync.ts @@ -14,7 +14,12 @@ import { } from "runtimed"; import { isConnectionScope, type ConnectionScope } from "../src/auth-shared"; import { identityDisplayLabel } from "../src/display-label"; -import { FrameType, type SessionControlMessage } from "../src/protocol"; +import { + FrameType, + LIVENESS_PING, + LIVENESS_PONG, + type SessionControlMessage, +} from "../src/protocol"; import { cloudPrototypeAuthFromWindow, cloudSyncAuthFromPrototypeAuthState, @@ -130,6 +135,17 @@ const RECONNECT_MAX_DELAY_MS = 30_000; /** ±50% full jitter on every retry delay. */ const RECONNECT_JITTER_RATIO = 0.5; +/** + * App-level liveness probe cadence. A zombie socket (OS-level offline, + * upstream loss with the interface up) keeps `readyState === OPEN` and + * buffers sends silently — without traffic that DEMANDS a reply, the only + * loss signal is the OS TCP retransmit abort, minutes later. The room DO + * answers `LIVENESS_PING` via `setWebSocketAutoResponse` (no DO wake), so a + * missed pong within the deadline means the link is dead: recycle it. + */ +const LIVENESS_PING_INTERVAL_MS = 20_000; +const LIVENESS_PONG_DEADLINE_MS = 10_000; + /** * Bound on the persisted-seed IndexedDB read at connect time. A hung IDB * open (corrupt browser profile) must degrade to bootstrap, not stall the @@ -143,6 +159,17 @@ interface PendingFrameAck { timeoutId: ReturnType; } +/** + * Browser timers are numbers; Node timers are objects whose `unref()` + * releases the event loop. The liveness probe's periodic timers must never + * keep a Node process (tests) alive on their own — a transport that is + * still connected when a test file finishes would otherwise wedge the + * runner forever. No-op in browsers. + */ +function unrefTimer(timer: ReturnType | ReturnType): void { + (timer as { unref?: () => void }).unref?.(); +} + function requestTimeoutMs(request: NotebookRequest): number { switch (request.type) { case "launch_kernel": @@ -657,6 +684,15 @@ export interface CloudWebSocketTransportOptions { reconnectBaseDelayMs?: number; reconnectMaxDelayMs?: number; handshakeTimeoutMs?: number; + /** + * Liveness probe tuning for tests. After each cloud_room_ready the + * transport sends `LIVENESS_PING` every `livenessPingIntervalMs`; if no + * `LIVENESS_PONG` arrives within `livenessPongDeadlineMs` of a ping, the + * connection is treated as lost. `livenessPingIntervalMs: 0` disables + * the probe entirely. + */ + livenessPingIntervalMs?: number; + livenessPongDeadlineMs?: number; /** Jitter source (default Math.random); injectable for deterministic tests. */ random?: () => number; } @@ -678,6 +714,8 @@ export class CloudWebSocketTransport implements NotebookTransport { private readonly reconnectBaseDelayMs: number; private readonly reconnectMaxDelayMs: number; private readonly handshakeTimeoutMs: number; + private readonly livenessPingIntervalMs: number; + private readonly livenessPongDeadlineMs: number; private readonly random: () => number; private socket: WebSocket | null = null; @@ -690,6 +728,8 @@ export class CloudWebSocketTransport implements NotebookTransport { private failedAttempts = 0; private retryTimer: ReturnType | null = null; private handshakeTimer: ReturnType | null = null; + private livenessPingTimer: ReturnType | null = null; + private livenessPongTimer: ReturnType | null = null; private manualDisconnect = false; private everReady = false; private readySettled = false; @@ -738,11 +778,28 @@ export class CloudWebSocketTransport implements NotebookTransport { } }; + /** + * navigator 'offline': proactively recycle the current socket. An + * OS-level offline window never fires WS `close`/`error` — the socket + * stays `OPEN` and buffers sends silently until the TCP retransmit abort + * minutes later. The browser telling us it is offline is trustworthy in + * the direction that matters; tear down now so status flips to + * reconnecting and the `online` handler can short-circuit recovery. + */ + private readonly handleBrowserOffline = () => { + if (this.manualDisconnect) return; + const socket = this.socket; + if (socket === null) return; + this.connectionLost(new Error("browser reported offline"), socket); + }; + constructor(options: CloudWebSocketTransportOptions) { this.options = options; this.reconnectBaseDelayMs = options.reconnectBaseDelayMs ?? RECONNECT_BASE_DELAY_MS; this.reconnectMaxDelayMs = options.reconnectMaxDelayMs ?? RECONNECT_MAX_DELAY_MS; this.handshakeTimeoutMs = options.handshakeTimeoutMs ?? HANDSHAKE_TIMEOUT_MS; + this.livenessPingIntervalMs = options.livenessPingIntervalMs ?? LIVENESS_PING_INTERVAL_MS; + this.livenessPongDeadlineMs = options.livenessPongDeadlineMs ?? LIVENESS_PONG_DEADLINE_MS; this.random = options.random ?? Math.random; this.ready = new Promise((resolve, reject) => { this.readyResolve = resolve; @@ -753,6 +810,7 @@ export class CloudWebSocketTransport implements NotebookTransport { this.ready.catch(() => undefined); if (typeof window !== "undefined") { window.addEventListener("online", this.handleBrowserOnline); + window.addEventListener("offline", this.handleBrowserOffline); } void this.connect(); } @@ -841,6 +899,7 @@ export class CloudWebSocketTransport implements NotebookTransport { this.teardownSocket(); this.connectionReady = false; this.clearHandshakeTimer(); + this.stopLivenessProbe(); // Pending FIFO frame ACKs cannot span sockets. this.rejectPendingFrameAcks(reason); // Frames queued from a dead connection are bound to that connection's @@ -919,6 +978,85 @@ export class CloudWebSocketTransport implements NotebookTransport { } } + /** + * (Re)start the liveness probe for the connection that just completed + * its cloud_room_ready handshake. The captured socket pins the probe to + * that connection: a superseding connect tears the old probe down via + * `connectionLost` → `stopLivenessProbe`, and the captured-socket guard + * makes a straggling tick harmless besides. + */ + private startLivenessProbe(socket: WebSocket): void { + this.stopLivenessProbe(); + if (this.livenessPingIntervalMs <= 0) return; + this.livenessPingTimer = setInterval(() => { + if (socket !== this.socket) { + this.stopLivenessProbe(); + return; + } + this.sendLivenessPing(socket); + }, this.livenessPingIntervalMs); + // Node-only (tests): a probe interval must never keep the process + // alive on its own. No-op in browsers, where timers are numbers. + unrefTimer(this.livenessPingTimer); + } + + private sendLivenessPing(socket: WebSocket): void { + if (socket.readyState !== WebSocket.OPEN) return; // close handler will recycle + try { + // Raw text send on purpose: the room's WebSocketRequestResponsePair + // matches the exact string and replies without waking the DO. The + // typed-frame channel stays binary-only. + socket.send(LIVENESS_PING); + } catch (error) { + const reason = + error instanceof Error + ? new Error(`cloud sync liveness ping failed: ${error.message}`) + : new Error(`cloud sync liveness ping failed: ${String(error)}`); + this.connectionLost(reason, socket); + return; + } + // One deadline per OUTSTANDING ping: if a pong is already overdue, + // keep the original (earlier) deadline rather than extending it. + if (this.livenessPongTimer !== null) return; + if (typeof document !== "undefined" && document.visibilityState === "hidden") { + // Background tabs throttle and suspend timers: a frozen deadline can + // fire on resume BEFORE the queued pong MessageEvent dispatches, + // declaring a healthy connection dead on every tab foreground. Send + // the ping (it keeps intermediaries warm) but skip enforcement while + // hidden — the next visible-tab ping re-arms the deadline. + return; + } + this.livenessPongTimer = setTimeout(() => { + this.livenessPongTimer = null; + if (socket !== this.socket) return; + // The link is zombie: readyState stays OPEN and sends buffer + // silently, but the room's auto-response never made it back. + this.connectionLost( + new Error( + `cloud sync liveness pong missed (no reply within ${this.livenessPongDeadlineMs}ms)`, + ), + socket, + ); + socket.close(); + }, this.livenessPongDeadlineMs); + unrefTimer(this.livenessPongTimer); + } + + private noteLivenessPong(): void { + if (this.livenessPongTimer !== null) { + clearTimeout(this.livenessPongTimer); + this.livenessPongTimer = null; + } + } + + private stopLivenessProbe(): void { + if (this.livenessPingTimer !== null) { + clearInterval(this.livenessPingTimer); + this.livenessPingTimer = null; + } + this.noteLivenessPong(); + } + async sendFrame(frameType: number, payload: Uint8Array): Promise { if (this.manualDisconnect) { throw new Error("cloud sync socket is closed"); @@ -1004,8 +1142,10 @@ export class CloudWebSocketTransport implements NotebookTransport { this.connectEpoch += 1; // invalidate any in-flight connect attempt this.clearRetryTimer(); this.clearHandshakeTimer(); + this.stopLivenessProbe(); if (typeof window !== "undefined") { window.removeEventListener("online", this.handleBrowserOnline); + window.removeEventListener("offline", this.handleBrowserOffline); } this.teardownSocket(); this.rejectPendingFrameAcks(new Error("cloud sync socket disconnected")); @@ -1020,6 +1160,21 @@ export class CloudWebSocketTransport implements NotebookTransport { } private async handleMessage(data: unknown, socket: WebSocket): Promise { + if (socket !== this.socket) return; // superseded connection + // ANY inbound message is liveness evidence, not just the strict pong: + // the pong is an ordinary frame serialized into the same in-order WS + // stream as sync frames, so during a large post-reconnect sync over a + // slow link it queues behind the backlog. A connection actively + // delivering frames is demonstrably alive — killing it on a fixed pong + // deadline would recycle exactly the connections least able to afford + // the resync churn. The probe's job is the silent zombie socket, and a + // zombie delivers nothing. + this.noteLivenessPong(); + // Liveness pongs are the only text frames the room ever sends; consume + // them before binary decoding (which rejects strings). + if (data === LIVENESS_PONG) { + return; + } const bytes = await bytesFromWebSocketMessage(data); if (socket !== this.socket) return; // superseded while decoding if (bytes.byteLength === 0) return; @@ -1028,7 +1183,7 @@ export class CloudWebSocketTransport implements NotebookTransport { const control = JSON.parse(new TextDecoder().decode(bytes.slice(1))) as SessionControlMessage; this.options.onControl?.(control); if (control.type === "cloud_room_ready") { - this.handleRoomReady(control); + this.handleRoomReady(control, socket); } else if (control.type === "cloud_frame_accepted") { this.resolveFrameAck(control.frame_type); } else if (control.type === "cloud_frame_rejected") { @@ -1044,13 +1199,14 @@ export class CloudWebSocketTransport implements NotebookTransport { this.emitFrame(frame); } - private handleRoomReady(control: CloudRoomReady): void { + private handleRoomReady(control: CloudRoomReady, socket: WebSocket): void { this.failedAttempts = 0; // backoff resets on the app-level ack this.everReady = true; // Before the roomReady$ emission: subscribers' resync flush sends on // this connection within the same tick. this.connectionReady = true; this.clearHandshakeTimer(); + this.startLivenessProbe(socket); this.setStatus("online"); if (!this.readySettled) { this.readySettled = true; diff --git a/apps/notebook-cloud/viewer/notebook-view-store-bridge.ts b/apps/notebook-cloud/viewer/notebook-view-store-bridge.ts index 9cac8b645..2b139ce0d 100644 --- a/apps/notebook-cloud/viewer/notebook-view-store-bridge.ts +++ b/apps/notebook-cloud/viewer/notebook-view-store-bridge.ts @@ -13,6 +13,12 @@ import { setNotebookQueueProjection, } from "@/components/notebook/state/execution-store"; import { deleteOutputs, setOutput } from "@/components/notebook/state/output-store"; +import { + getCellIdsSnapshot, + resetRuntimeState, + resetRuntimeStoresProjection, + shouldPreserveBootstrapProjection, +} from "../../notebook/src/notebook-surface-stores"; import type { ResolvedCell } from "./render-resolution"; let cloudOwnedExecutionIds = new Set(); @@ -86,6 +92,41 @@ export function resetCloudViewStoreProjection(): void { cloudOwnedExecutionIds = new Set(); } +/** + * Bootstrap-preservation gate over EVERY store the cloud projection paints + * (desktop pattern: useAutomergeNotebook's beforeBootstrap). CodeCell renders + * outputs exclusively through the execution/output stores — preserving the + * cell store while wiping those still blanks every output and execution + * count, which is the dominant visual mass of a painted notebook. So the + * gate keeps or clears them together; `resetPoolState` stays with the + * caller, unconditional, matching desktop. + * + * Stale-data safety on preserve: the next materialization replaces cells + * wholesale and `projectCloudCellsIntoNotebookViewStores` sweeps stale + * cloud-owned output/execution ids via the `cloudOwned*` difference sets, + * which this function leaves intact. + * + * Returns true when the painted projection was preserved. + */ +export function resetCloudProjectionUnlessPreserved(options: { + /** `id:`-prefixed identity of the notebook whose cells are painted, or null. */ + paintedNotebookIdentity: string | null; + /** `id:`-prefixed identity of the notebook this session targets. */ + nextNotebookIdentity: string; +}): boolean { + const preserve = shouldPreserveBootstrapProjection({ + previousIdentity: options.paintedNotebookIdentity, + nextIdentity: options.nextNotebookIdentity, + visibleCellCount: getCellIdsSnapshot().length, + }); + if (!preserve) { + resetCloudViewStoreProjection(); + resetRuntimeState(); + resetRuntimeStoresProjection(); + } + return preserve; +} + function resolvedCellToNotebookCell( cell: ResolvedCell, outputs: readonly NotebookStoreOutput[], diff --git a/apps/notebook-cloud/viewer/use-cloud-auth.ts b/apps/notebook-cloud/viewer/use-cloud-auth.ts index 71417c07a..58cfeaf31 100644 --- a/apps/notebook-cloud/viewer/use-cloud-auth.ts +++ b/apps/notebook-cloud/viewer/use-cloud-auth.ts @@ -22,12 +22,44 @@ interface CloudPrototypeAuthOptions { appSession?: CloudAppSession | null; } -interface CloudAppSessionViewState { +export interface CloudAppSessionViewState { status: "loading" | "ready" | "error"; session: CloudAppSession | null; error: string | null; } +export function cloudAppSessionsEqual( + a: CloudAppSession | null, + b: CloudAppSession | null, +): boolean { + if (a === b) return true; + if (!a || !b) return false; + return a.provider === b.provider && a.expires_at === b.expires_at; +} + +/** + * Ready-state reducer for a session-status fetch result that keeps OBJECT + * IDENTITIES stable when the fetch only confirms what we already have. + * + * The session object feeds effect dependency chains (resolveSyncAuth → the + * live-room effect), so installing a fresh-but-content-identical object on + * every mount fetch tears down and reconnects the live room gratuitously. + * Returning `current` unchanged lets React bail out of the re-render + * entirely. + */ +export function nextCloudAppSessionReadyState( + current: CloudAppSessionViewState, + fetchedSession: CloudAppSession | null, +): CloudAppSessionViewState { + const session = cloudAppSessionsEqual(current.session, fetchedSession) + ? current.session + : fetchedSession; + if (current.status === "ready" && current.session === session && current.error === null) { + return current; + } + return { status: "ready", session, error: null }; +} + export function useCloudPrototypeAuth( authConfig: CloudViewerAuthConfig, options?: CloudPrototypeAuthOptions, @@ -173,7 +205,7 @@ export function useCloudAppSessionStatus( void readCloudAppSessionStatus({ signal: controller.signal }) .then((status) => { if (controller.signal.aborted) return; - setState({ status: "ready", session: status.session, error: null }); + setState((current) => nextCloudAppSessionReadyState(current, status.session)); }) .catch((error: unknown) => { if (controller.signal.aborted) return; diff --git a/apps/notebook/src/notebook-surface-stores.ts b/apps/notebook/src/notebook-surface-stores.ts new file mode 100644 index 000000000..2e8b428bc --- /dev/null +++ b/apps/notebook/src/notebook-surface-stores.ts @@ -0,0 +1,18 @@ +/** + * Headless slice of the notebook public surface: store, projection, and + * preservation functions with no component (and therefore no CSS/asset) + * imports, so it loads under plain node test runners. + * + * `notebook-surface.ts` exports the same symbols for browser consumers that + * also need components; node-importable consumers (the cloud view-store + * bridge and its tests) import this module instead. Both files re-export + * the same `./lib` implementations, so there is no behavioral drift — + * only a difference in what else comes along for the ride. + */ +export { shouldPreserveBootstrapProjection } from "./lib/bootstrap-preservation"; +// Sourced from the store module directly: `./lib/notebook-cells` re-exports +// the same function through the `@/components/notebook` barrel, which pulls +// component CSS and breaks node loaders. +export { getCellIdsSnapshot } from "@/components/notebook/state/cell-store"; +export { resetRuntimeStoresProjection } from "./lib/project-runtime-stores"; +export { resetRuntimeState } from "./lib/runtime-state"; diff --git a/apps/notebook/src/notebook-surface.ts b/apps/notebook/src/notebook-surface.ts index 6aabb77fb..3dfb005c8 100644 --- a/apps/notebook/src/notebook-surface.ts +++ b/apps/notebook/src/notebook-surface.ts @@ -25,6 +25,7 @@ export { getNotebookCellsSnapshot, type NotebookCell, } from "./lib/notebook-cells"; +export { shouldPreserveBootstrapProjection } from "./lib/bootstrap-preservation"; export type { JupyterOutput } from "./types"; export { resetPoolState, setPoolState } from "./lib/pool-state"; export { diff --git a/vitest.config.ts b/vitest.config.ts index 542e597d5..6405309b7 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -49,6 +49,9 @@ export default defineConfig({ include: [ "src/**/__tests__/**/*.test.{ts,tsx}", "apps/notebook/src/**/__tests__/**/*.test.{ts,tsx}", + // Cloud viewer React hooks need a DOM renderer; the rest of the cloud + // suite stays on node:test (apps/notebook-cloud/test). + "apps/notebook-cloud/viewer/__tests__/**/*.test.{ts,tsx}", "apps/mcp-app/src/**/__tests__/**/*.test.{js,ts,tsx}", "packages/**/tests/**/*.test.{ts,tsx}", "plugins/nteract/pi/**/*.test.{ts,tsx}", From f14e339ecca46dac1d2733dce4ceeb2aa2d111f6 Mon Sep 17 00:00:00 2001 From: Quill <261289082+quillaid@users.noreply.github.com> Date: Thu, 11 Jun 2026 06:43:10 -0600 Subject: [PATCH 04/16] fix(notebook-cloud): cap snapshot blob-ref validation fan-out (#3575) --- apps/notebook-cloud/src/index.ts | 39 +++++++++++++++++++ .../notebook-cloud/test/worker-routes.test.ts | 12 +++++- 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/apps/notebook-cloud/src/index.ts b/apps/notebook-cloud/src/index.ts index a7f17a104..3d90add2f 100644 --- a/apps/notebook-cloud/src/index.ts +++ b/apps/notebook-cloud/src/index.ts @@ -128,6 +128,11 @@ const VIEWER_RUNTIME_WASM_ASSET_MANIFEST_PATH = "/assets/runtime-wasm-assets.jso const VIEWER_RUNTIMED_WASM_MODULE_NAME = "runtimed_wasm.js"; const VIEWER_RUNTIMED_WASM_NAME = "runtimed_wasm_bg.wasm"; const SNAPSHOT_BLOB_HEAD_CONCURRENCY = 16; +// One R2 HEAD is issued per referenced blob during snapshot-pair validation. +// Cap the total so a single publish cannot fan out unbounded billable R2 +// operations. Sized ~10x the largest blob_ref_count observed in +// snapshot_pair.validation.completed logs; raise it if legitimate notebooks hit it. +const MAX_SNAPSHOT_BLOB_REFS = 2000; const ULID_ALPHABET = "0123456789ABCDEFGHJKMNPQRSTVWXYZ"; const CREATE_NOTEBOOK_ID_ATTEMPTS = 8; @@ -2807,6 +2812,29 @@ async function validateSnapshotPair(options: { }; } + const blobRefs = snapshotBlobRefsOverCap(render); + if (blobRefs.over) { + cloudLog("warn", "snapshot_pair.validation.blob_refs_over_cap", { + notebook_id: options.notebookId, + notebook_heads_hash: options.notebookHeadsHash, + runtime_heads_hash: options.runtimeHeadsHash, + duration_ms: durationMs(startedAt), + blob_ref_count: blobRefs.count, + blob_ref_cap: blobRefs.cap, + counter: "snapshot_pair_validation_blob_ref_cap_rejections", + counter_delta: 1, + }); + return { + ok: false, + status: 422, + body: { + error: "snapshot references too many blobs", + blob_ref_count: blobRefs.count, + blob_ref_cap: blobRefs.cap, + }, + }; + } + const missingBlobs = await findMissingSnapshotBlobs(bucket, options.notebookId, render); if (missingBlobs.length > 0) { cloudLog("warn", "snapshot_pair.validation.missing_blobs", { @@ -2848,6 +2876,9 @@ async function findMissingSnapshotBlobs( render: unknown, ): Promise { const refs = collectSnapshotBlobRefs(render); + if (refs.length > MAX_SNAPSHOT_BLOB_REFS) { + throw new Error(`snapshot blob ref count ${refs.length} exceeds cap ${MAX_SNAPSHOT_BLOB_REFS}`); + } const missing: Array = []; for (let index = 0; index < refs.length; index += SNAPSHOT_BLOB_HEAD_CONCURRENCY) { @@ -2884,6 +2915,14 @@ function collectSnapshotBlobRefs(render: unknown): BlobRef[] { return Object.values(refs); } +export function snapshotBlobRefsOverCap( + render: unknown, + cap = MAX_SNAPSHOT_BLOB_REFS, +): { count: number; cap: number; over: boolean } { + const count = collectSnapshotBlobRefs(render).length; + return { count, cap, over: count > cap }; +} + function isRecord(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); } diff --git a/apps/notebook-cloud/test/worker-routes.test.ts b/apps/notebook-cloud/test/worker-routes.test.ts index 857214873..57160b51b 100644 --- a/apps/notebook-cloud/test/worker-routes.test.ts +++ b/apps/notebook-cloud/test/worker-routes.test.ts @@ -1,7 +1,7 @@ import { before, describe, it } from "node:test"; import assert from "node:assert/strict"; import { readFile } from "node:fs/promises"; -import worker from "../src/index.ts"; +import worker, { snapshotBlobRefsOverCap } from "../src/index.ts"; import { NOTEBOOK_CLOUD_APP_SESSION_COOKIE_NAME } from "../src/app-session.ts"; import { BEARER_AUTH_TOKEN_PROTOCOL_PREFIX, @@ -4276,6 +4276,16 @@ describe("Worker artifact routes", () => { "snapshot_pair.validation.missing_blobs", ); }); + + it("rejects snapshot renders that reference more blobs than the cap", () => { + const over = { + blob_urls: Object.fromEntries( + Array.from({ length: 5 }, (_, i) => [`sha256:${i}`, `https://x/${i}`]), + ), + }; + assert.deepEqual(snapshotBlobRefsOverCap(over, 4), { count: 5, cap: 4, over: true }); + assert.deepEqual(snapshotBlobRefsOverCap(over, 5), { count: 5, cap: 5, over: false }); + }); }); async function ownerPut( From 317feda4ac434347cf982dd40b61cffa0086cbc7 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 11 Jun 2026 07:11:09 -0600 Subject: [PATCH 05/16] chore(nix): update pnpmDeps hash (#3578) Co-authored-by: github-actions[bot] --- flake.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flake.nix b/flake.nix index 2f8bc3f5f..813c34b7e 100644 --- a/flake.nix +++ b/flake.nix @@ -97,7 +97,7 @@ src = filteredSrc; fetcherVersion = 2; # Update with: nix build .#pnpmDeps 2>&1 | grep 'got:' - hash = "sha256-OEsQ2f3Yv3A5tIgsBmRvKu/VmeauGJT9SExOHRTIbOY="; + hash = "sha256-go4sdKs4x+gVK6RJSh0ySO4Fy3pDsT/HdQmL3tyzqjQ="; }; jsBuild = pkgs.stdenv.mkDerivation { From 6ab47e09a07ec8ecc2911a2a4ef94fa34c4fd233 Mon Sep 17 00:00:00 2001 From: Quill <261289082+quillaid@users.noreply.github.com> Date: Thu, 11 Jun 2026 07:22:09 -0600 Subject: [PATCH 06/16] fix(cloud): apply connection actor to comms doc writes (#3579) * fix(cloud): apply connection actor to comms doc writes * test(cloud): find widget smoke sliders across output frames --- .../hosted-widget-cross-window-smoke.mjs | 5 +- .../test/room-materializer.test.ts | 62 +++++++++++++++++++ crates/runtimed-wasm/src/lib.rs | 3 + 3 files changed, 67 insertions(+), 3 deletions(-) diff --git a/apps/notebook-cloud/scripts/hosted-widget-cross-window-smoke.mjs b/apps/notebook-cloud/scripts/hosted-widget-cross-window-smoke.mjs index 81c9939dc..3aca8a4d5 100644 --- a/apps/notebook-cloud/scripts/hosted-widget-cross-window-smoke.mjs +++ b/apps/notebook-cloud/scripts/hosted-widget-cross-window-smoke.mjs @@ -277,9 +277,8 @@ async function openNotebookShell(page, href, timeout) { } async function waitForWidgetSlider(page, timeout) { - const cell = page.locator('[data-cell-type="code"]').first(); - await cell.waitFor({ state: "visible", timeout }); - const slider = cell + await page.locator('[data-cell-type="code"]').first().waitFor({ state: "visible", timeout }); + const slider = page .frameLocator('[data-slot="isolated-frame"]') .locator('[data-widget-type="IntSlider"]') .getByRole("slider") diff --git a/apps/notebook-cloud/test/room-materializer.test.ts b/apps/notebook-cloud/test/room-materializer.test.ts index 2761b97ca..17fe3b26d 100644 --- a/apps/notebook-cloud/test/room-materializer.test.ts +++ b/apps/notebook-cloud/test/room-materializer.test.ts @@ -1483,6 +1483,68 @@ describe("RoomMaterializer", () => { assert.equal(resolved.state?.value, 2); }); + it("allows loaded owner handles to author CommsDoc changes with the connection actor", async () => { + const state = fakeState(); + const materializer = new RoomMaterializer("demo", state, {} as Env); + const runtimeIdentity = authenticateDevRequest( + new Request( + "https://cloud.test/n/demo/sync?user=runtime&operator=runtime:py&scope=runtime_peer", + ), + ); + const runtimePeerConnection = { id: "peer-runtime", identity: runtimeIdentity }; + const runtimePeer = new RuntimeStatePeerHandle(runtimeIdentity.actorLabel); + await syncMaterializerWithRuntimePeer(materializer, runtimePeerConnection, runtimePeer); + runtimePeer.put_comm_json( + "comm-widget", + "jupyter.widget", + "@jupyter-widgets/controls", + "IntSliderModel", + JSON.stringify({ value: 7, description: "probe" }), + 0, + ); + assert.equal( + ( + await applyRuntimePeerChangesToMaterializer( + materializer, + runtimePeerConnection, + runtimePeer, + ) + ).changed, + true, + ); + assert.equal( + (await applyCommsPeerChangesToMaterializer(materializer, runtimePeerConnection, runtimePeer)) + .changed, + true, + ); + + const ownerIdentity = authenticateDevRequest( + new Request("https://cloud.test/n/demo/sync?user=alice&operator=browser:a&scope=owner"), + ); + const ownerPeerConnection = { id: "peer-owner", identity: ownerIdentity }; + const loadedOwner = NotebookHandle.load( + NotebookHandle.create_bootstrap("user:dev:seed/browser:seed").save(), + ); + loadedOwner.set_actor(ownerIdentity.actorLabel); + await syncMaterializerRuntimeStateWithClient(materializer, ownerPeerConnection, loadedOwner); + await syncMaterializerCommsDocWithClient(materializer, ownerPeerConnection, loadedOwner); + + assert.equal( + loadedOwner.set_comm_state_property("comm-widget", "value", JSON.stringify(11)), + true, + ); + const accepted = await applyCommsClientChangesToMaterializer( + materializer, + ownerPeerConnection, + loadedOwner, + ); + assert.equal( + accepted.changed, + true, + "loaded browser handles must not author CommsDoc changes with a raw random actor id", + ); + }); + it("fans out owner CommsDoc changes to already-open peers", async () => { const state = fakeState(); const materializer = new RoomMaterializer("demo", state, {} as Env); diff --git a/crates/runtimed-wasm/src/lib.rs b/crates/runtimed-wasm/src/lib.rs index 2053f1a7b..83d890444 100644 --- a/crates/runtimed-wasm/src/lib.rs +++ b/crates/runtimed-wasm/src/lib.rs @@ -2256,6 +2256,9 @@ impl NotebookHandle { pub fn set_actor(&mut self, actor_label: &str) { self.doc.set_actor(actor_label); self.state_doc.set_actor(actor_label); + self.comms_doc + .doc_mut() + .set_actor(automerge::ActorId::from(actor_label.as_bytes())); } /// Return the deduplicated, sorted list of actor labels that have From 9c0ce3590f1ce13488992bf96704cfc4da0ec4f4 Mon Sep 17 00:00:00 2001 From: Quill <261289082+quillaid@users.noreply.github.com> Date: Thu, 11 Jun 2026 08:19:35 -0600 Subject: [PATCH 07/16] fix(runtimed-wasm): preserve handle actor across doc loads and recovery rebuilds (#3580) --- crates/runtimed-wasm/src/lib.rs | 179 ++++++++++++++++++++++++++++---- 1 file changed, 159 insertions(+), 20 deletions(-) diff --git a/crates/runtimed-wasm/src/lib.rs b/crates/runtimed-wasm/src/lib.rs index 83d890444..fbb8a3126 100644 --- a/crates/runtimed-wasm/src/lib.rs +++ b/crates/runtimed-wasm/src/lib.rs @@ -2205,9 +2205,15 @@ impl NotebookHandle { /// (outputs, executions) alongside the notebook doc. Replacing the state /// doc also resets RuntimeStateDoc sync state so later room-host sync starts /// from the loaded snapshot, not the previous empty/bootstrap doc. + /// + /// Preserves the current actor so a load never silently discards an + /// authenticated actor set via `set_actor` (room hosts reject + /// non-`/` actors). pub fn load_state_doc(&mut self, bytes: &[u8]) -> Result<(), JsError> { - let doc = automerge::AutoCommit::load(bytes) + let actor = self.state_doc.doc().get_actor().clone(); + let mut doc = automerge::AutoCommit::load(bytes) .map_err(|e| JsError::new(&format!("load_state_doc failed: {}", e)))?; + doc.set_actor(actor); self.state_doc = RuntimeStateDoc::from_doc(doc); self.state_sync_state = sync::State::new(); self.prev_output_by_id.clear(); @@ -2216,9 +2222,15 @@ impl NotebookHandle { } /// Load a CommsDoc from saved bytes. + /// + /// Preserves the current actor so a load never silently discards an + /// authenticated actor set via `set_actor` (room hosts reject + /// non-`/` actors). pub fn load_comms_doc(&mut self, bytes: &[u8]) -> Result<(), JsError> { - let doc = automerge::AutoCommit::load(bytes) + let actor = self.comms_doc.doc().get_actor().clone(); + let mut doc = automerge::AutoCommit::load(bytes) .map_err(|e| JsError::new(&format!("load_comms_doc failed: {}", e)))?; + doc.set_actor(actor); self.comms_doc = CommsDoc::from_doc(doc); self.comms_sync_state = sync::State::new(); Ok(()) @@ -3414,32 +3426,29 @@ impl NotebookHandle { /// Rebuild the notebook doc via save→load to clear corrupted internal indices. /// - /// Mirrors `NotebookDoc::rebuild_from_save()` on the daemon side. The - /// `save()` path uses `op_set.export()` (safe even with corrupted indices), - /// and `load()` reconstructs all internal data structures from scratch. + /// Delegates to `NotebookDoc::rebuild_from_save`, which preserves the actor + /// and refuses cell-losing rebuilds. fn rebuild_doc(&mut self) { - let bytes = self.doc.save(); - if let Ok(doc) = - NotebookDoc::load_with_encoding(&bytes, notebook_doc::TextEncoding::Utf16CodeUnit) - { - self.doc = doc; - } + let _ = self.doc.rebuild_from_save(); } /// Rebuild the PoolDoc via save→load. + /// + /// PoolDoc has no crate-level `rebuild_from_save`, so preserves the actor manually. fn rebuild_pool_doc(&mut self) { + let actor = self.pool_doc.doc_mut().get_actor().clone(); let bytes = self.pool_doc.doc_mut().save(); - if let Ok(doc) = automerge::AutoCommit::load(&bytes) { + if let Ok(mut doc) = automerge::AutoCommit::load(&bytes) { + doc.set_actor(actor); *self.pool_doc.doc_mut() = doc; } } /// Rebuild the CommsDoc via save→load. + /// + /// Delegates to `CommsDoc::rebuild_from_save`, which preserves the actor. fn rebuild_comms_doc(&mut self) { - let bytes = self.comms_doc.doc_mut().save(); - if let Ok(doc) = automerge::AutoCommit::load(&bytes) { - *self.comms_doc.doc_mut() = doc; - } + let _ = self.comms_doc.rebuild_from_save(); } /// Normalize pool sync state via encode→decode round-trip. @@ -3449,11 +3458,10 @@ impl NotebookHandle { } /// Rebuild the RuntimeStateDoc via save→load. + /// + /// Delegates to `RuntimeStateDoc::rebuild_from_save`, which preserves the actor. fn rebuild_state_doc(&mut self) { - let bytes = self.state_doc.doc_mut().save(); - if let Ok(doc) = automerge::AutoCommit::load(&bytes) { - *self.state_doc.doc_mut() = doc; - } + let _ = self.state_doc.rebuild_from_save(); } /// Receive a typed frame from the daemon, demux by type byte, return events for the frontend. @@ -5052,4 +5060,135 @@ mod tests { assert!(buffer_paths.is_empty()); assert!(text_paths.is_empty()); } + + // -- Actor preservation tests (hotfix #3579 follow-up) ----------------- + + #[test] + fn set_actor_covers_all_docs_on_notebook_handle() { + let mut handle = NotebookHandle::create_bootstrap("user:dev:a/browser:x") + .expect("create bootstrap handle"); + + // Set a new actor + handle.set_actor("user:dev:b/browser:y"); + + // Verify all three docs have the new actor + let expected_actor = automerge::ActorId::from("user:dev:b/browser:y".as_bytes()); + + assert_eq!( + handle.doc.get_actor_id(), + "user:dev:b/browser:y", + "NotebookDoc actor mismatch" + ); + assert_eq!( + handle.state_doc.doc().get_actor(), + &expected_actor, + "RuntimeStateDoc actor mismatch" + ); + assert_eq!( + handle.comms_doc.doc().get_actor(), + &expected_actor, + "CommsDoc actor mismatch" + ); + } + + #[test] + fn set_actor_covers_both_docs_on_runtime_peer_handle() { + let mut handle = + RuntimeStatePeerHandle::new("user:dev:a/browser:x").expect("create handle"); + + // Set a new actor + handle.set_actor("user:dev:b/browser:y"); + + // Verify both docs have the new actor + let expected_actor = automerge::ActorId::from("user:dev:b/browser:y".as_bytes()); + + assert_eq!( + handle.state_doc.doc().get_actor(), + &expected_actor, + "RuntimeStateDoc actor mismatch" + ); + assert_eq!( + handle.comms_doc.doc().get_actor(), + &expected_actor, + "CommsDoc actor mismatch" + ); + } + + #[test] + fn load_comms_doc_preserves_actor() { + // Create handle A with actor X and write a comm + let mut handle_a = + NotebookHandle::create_bootstrap("user:dev:a/browser:x").expect("create handle A"); + let success = handle_a.set_comm_state_property("test-comm", "key", r#""value""#); + assert!(success, "write comm property"); + let bytes = handle_a.save_comms_doc(); + + // Create handle B, set actor Y, then load A's comms doc + let mut handle_b = + NotebookHandle::create_bootstrap("user:dev:tmp/browser:tmp").expect("create handle B"); + handle_b.set_actor("user:dev:b/browser:y"); + + handle_b + .load_comms_doc(&bytes) + .expect("load comms doc should succeed"); + + // Verify handle B's comms doc still has actor Y, not a random actor + let expected_actor = automerge::ActorId::from("user:dev:b/browser:y".as_bytes()); + assert_eq!( + handle_b.comms_doc.doc().get_actor(), + &expected_actor, + "load_comms_doc should preserve the handle's actor" + ); + + // Verify handle B can still write to the comms doc + let success = handle_b.set_comm_state_property("test-comm", "key2", r#""value2""#); + assert!(success, "should be able to write after load"); + } + + #[test] + fn load_state_doc_preserves_actor() { + // Create handle A with actor X + let mut handle_a = + NotebookHandle::create_bootstrap("user:dev:a/browser:x").expect("create handle A"); + let bytes = handle_a.state_doc.doc_mut().save(); + + // Create handle B, set actor Y, then load A's state doc + let mut handle_b = + NotebookHandle::create_bootstrap("user:dev:tmp/browser:tmp").expect("create handle B"); + handle_b.set_actor("user:dev:b/browser:y"); + + handle_b + .load_state_doc(&bytes) + .expect("load state doc should succeed"); + + // Verify handle B's state doc still has actor Y, not a random actor + let expected_actor = automerge::ActorId::from("user:dev:b/browser:y".as_bytes()); + assert_eq!( + handle_b.state_doc.doc().get_actor(), + &expected_actor, + "load_state_doc should preserve the handle's actor" + ); + } + + #[test] + fn rebuild_comms_doc_preserves_actor() { + let mut handle = + NotebookHandle::create_bootstrap("user:dev:test/browser:x").expect("create handle"); + + // Write a comm property so the doc is non-empty + let success = handle.set_comm_state_property("test-comm", "key", r#""value""#); + assert!(success, "write comm property"); + + let expected_actor = automerge::ActorId::from("user:dev:test/browser:x".as_bytes()); + + // Call rebuild directly (test has private field access) + handle.rebuild_comms_doc(); + + // Verify the actor is still the same + assert_eq!( + handle.comms_doc.doc().get_actor(), + &expected_actor, + "rebuild_comms_doc should preserve the actor" + ); + } } From b4297cd5eff5468d9211e149d36607234e0ed766 Mon Sep 17 00:00:00 2001 From: Quill Agent <261289082+quillaid@users.noreply.github.com> Date: Thu, 11 Jun 2026 03:54:30 +0000 Subject: [PATCH 08/16] feat(notebook): quiet connection/identity slot with desktop mount (#3421) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NotebookConnectionIdentity fills the reserved identity slot: the self actor's flattened avatar (shared actor projection, initials/icon only) paired with a connectivity dot — connectionStatus$'s first UI consumer. Runtime-state stores are deliberately not blanked while a transport reconnects, so the pulsing dot is what makes frozen kernel/execution chrome interpretable during the offline window. Hard quiet-chrome constraints from the pulled #3273/#3290/#3337/#3349 designs, enforced in component + tests: - renders NOTHING for a purely local desktop session (isRemoteNotebookContext: access.source !== 'local' or runtime.target.kind === 'runtime_peer') — conditionality is why #3290 was pulled; - flat rounded-md border-border/70 bg-muted/35, never rounded-full+shadow; - icon-only at every width: no visible text pill; label and status live in sr-only copy and the title tooltip; - state expresses as dot color (statusTone vocabulary: emerald online, amber pulse connecting/reconnecting, muted offline) and wrapper opacity, never copy; - errors/reconnect prompts stay in the notices stack; connection state never rides the kernel status surface. The component takes a structural NotebookConnectionStatusSource (an rxjs-free Observable subset), so shared src/ gains no dependency; NotebookActorAvatar is exported from NotebookIdentity with an optional status-tone override to carry the connection dot. Desktop mount: trailingControls on in App.tsx -> identityControls at the right end of the command toolbar, driven by the host transport's connectionStatus$ (stable for the app's lifetime). A purely local session renders nothing, exactly as before #3273. --- apps/notebook/src/App.tsx | 10 + .../notebook/NotebookConnectionIdentity.tsx | 133 ++++++++++ src/components/notebook/NotebookIdentity.tsx | 14 +- .../NotebookConnectionIdentity.test.tsx | 234 ++++++++++++++++++ src/components/notebook/index.ts | 6 + 5 files changed, 393 insertions(+), 4 deletions(-) create mode 100644 src/components/notebook/NotebookConnectionIdentity.tsx create mode 100644 src/components/notebook/__tests__/NotebookConnectionIdentity.test.tsx diff --git a/apps/notebook/src/App.tsx b/apps/notebook/src/App.tsx index 7478d9058..2bdf3a93b 100644 --- a/apps/notebook/src/App.tsx +++ b/apps/notebook/src/App.tsx @@ -42,6 +42,7 @@ import { DebugBanner, EnvBuildDecisionDialog, KernelLaunchErrorBanner, + NotebookConnectionIdentity, NotebookDocumentRail, NotebookDocumentShell, PoolErrorBanner, @@ -1514,6 +1515,15 @@ function AppContent() { updateStatus={updateStatus} updateVersion={updateVersion} onRestartToUpdate={restartToUpdate} + trailingControls={ + // Connection/identity slot: renders nothing for a purely local + // session (isRemoteNotebookContext) — conditionality is the + // point. The host transport is stable for the app's lifetime. + + } /> {globalFind.isOpen && ( ` — keeps the + * shared component free of a direct rxjs dependency while accepting the + * transports' `connectionStatus$` and the cloud session's bridge directly. + */ +export interface NotebookConnectionStatusSource { + subscribe(next: (status: ConnectionStatus) => void): { unsubscribe(): void }; +} + +/** + * NotebookConnectionIdentity — the connection/identity slot. + * + * Self-identity (the flattened avatar treatment) paired with a + * connectivity dot driven by the transport's `connectionStatus$` — its + * first UI consumer. Runtime-state stores are deliberately not blanked + * while a transport reconnects, so this dot is what makes frozen + * kernel/execution chrome interpretable during the offline window. + * + * Quiet-chrome rules (distilled from the pulled #3273/#3290/#3337/#3349 + * designs — hard constraints, not preferences): + * - renders NOTHING for a purely local desktop session + * (`isRemoteNotebookContext`); local identity is noise, not chrome; + * - flat `rounded-md border-border/70 bg-muted/35`, never + * rounded-full + shadow; + * - icon/avatar-first and icon-only at every width: no visible text pill — + * the label and status live in `sr-only` copy and the title tooltip; + * - state expresses as dot color / opacity, never copy; + * - errors and reconnect prompts belong to the notices stack, never here; + * - connection state never masquerades as kernel/runtime status. + */ +export interface NotebookConnectionIdentityProps { + capabilities: NotebookShellCapabilities; + /** + * Connection lifecycle from the live transport. Hosts must hand a source + * that survives transport replacement (cloud: the session's + * `CloudConnectionStatusBridge`; desktop: the host transport, which is + * stable for the app's lifetime). + */ + connectionStatus$: NotebookConnectionStatusSource; + className?: string; +} + +export function NotebookConnectionIdentity({ + capabilities, + connectionStatus$, + className, +}: NotebookConnectionIdentityProps) { + const status = useConnectionStatus(connectionStatus$); + + // Conditionality is the point (#3290): a purely local desktop session + // gets no identity chrome at all. + if (!isRemoteNotebookContext(capabilities)) { + return null; + } + + const actor = notebookToolbarActors(capabilities)[0]; + if (!actor) { + return null; + } + + const statusLabel = connectionStatusLabel(status); + const detail = `${actor.label} — ${statusLabel}`; + + return ( +
+ + {detail} +
+ ); +} + +/** + * Remote-context predicate: cloud is always remote; a desktop session is + * remote only when its access is server-assigned or a runtime peer is the + * compute target. + */ +export function isRemoteNotebookContext(capabilities: NotebookShellCapabilities): boolean { + return ( + capabilities.access.source !== "local" || capabilities.runtime.target?.kind === "runtime_peer" + ); +} + +function useConnectionStatus(status$: NotebookConnectionStatusSource): ConnectionStatus { + const [status, setStatus] = useState("connecting"); + useEffect(() => { + const subscription = status$.subscribe((next) => setStatus(next)); + return () => subscription.unsubscribe(); + }, [status$]); + return status; +} + +/** Existing statusTone vocabulary: emerald active, amber attention, muted offline. */ +function connectionDotTone(status: ConnectionStatus): string { + switch (status) { + case "online": + return "bg-emerald-500"; + case "connecting": + case "reconnecting": + return "animate-pulse bg-amber-500"; + case "offline": + return "bg-muted"; + } +} + +function connectionStatusLabel(status: ConnectionStatus): string { + switch (status) { + case "online": + return "Connected"; + case "connecting": + return "Connecting"; + case "reconnecting": + return "Reconnecting"; + case "offline": + return "Offline"; + } +} diff --git a/src/components/notebook/NotebookIdentity.tsx b/src/components/notebook/NotebookIdentity.tsx index ab8fcc740..3bb34f763 100644 --- a/src/components/notebook/NotebookIdentity.tsx +++ b/src/components/notebook/NotebookIdentity.tsx @@ -141,17 +141,21 @@ export function NotebookIdentityGroup({ ); } -function NotebookActorAvatar({ +export function NotebookActorAvatar({ actor, - icon: Icon, + icon, size = "default", showStatus = true, + statusClassName, }: { actor: NotebookActorIdentity; - icon: LucideIcon; + icon?: LucideIcon; size?: "sm" | "default"; showStatus?: boolean; + /** Override the status dot tone (e.g. connection state instead of actor status). */ + statusClassName?: string; }) { + const Icon = icon ?? actorIcon(actor.kind); return ( ); } diff --git a/src/components/notebook/__tests__/NotebookConnectionIdentity.test.tsx b/src/components/notebook/__tests__/NotebookConnectionIdentity.test.tsx new file mode 100644 index 000000000..88559d6e3 --- /dev/null +++ b/src/components/notebook/__tests__/NotebookConnectionIdentity.test.tsx @@ -0,0 +1,234 @@ +import { act, render } from "@testing-library/react"; +import { describe, expect, it } from "vite-plus/test"; +import type { ConnectionStatus } from "runtimed"; +import { readOnlyNotebookShellCapabilities } from "../capabilities"; +import { + NotebookConnectionIdentity, + isRemoteNotebookContext, + type NotebookConnectionStatusSource, +} from "../NotebookConnectionIdentity"; +import type { NotebookShellCapabilities } from "../capabilities"; + +/** Minimal BehaviorSubject-shaped source (rxjs-free, like the component). */ +class FakeStatusSource implements NotebookConnectionStatusSource { + private readonly listeners = new Set<(status: ConnectionStatus) => void>(); + + constructor(private value: ConnectionStatus) {} + + subscribe(next: (status: ConnectionStatus) => void): { unsubscribe(): void } { + next(this.value); + this.listeners.add(next); + return { unsubscribe: () => this.listeners.delete(next) }; + } + + next(status: ConnectionStatus): void { + this.value = status; + for (const listener of this.listeners) { + listener(status); + } + } +} + +function capabilities( + overrides: Partial = {}, +): NotebookShellCapabilities { + return { + ...readOnlyNotebookShellCapabilities, + ...overrides, + access: { + ...readOnlyNotebookShellCapabilities.access, + ...overrides.access, + }, + auth: { + ...readOnlyNotebookShellCapabilities.auth, + ...overrides.auth, + }, + runtime: { + ...readOnlyNotebookShellCapabilities.runtime, + ...overrides.runtime, + }, + }; +} + +function cloudCapabilities(): NotebookShellCapabilities { + return capabilities({ + access: { + level: "editor", + source: "cloud", + isPublic: false, + actorLabel: "user:anaconda:alice/browser:session-1", + identityLabel: "Alice", + }, + }); +} + +function localCapabilities(): NotebookShellCapabilities { + return capabilities({ + access: { + level: "owner", + source: "local", + isPublic: false, + actorLabel: "local:kyle/desktop:abc", + identityLabel: "Kyle", + }, + runtime: { + connected: true, + canWriteRuntimeState: true, + source: "local", + actorLabel: "local:kyle/desktop:abc", + identityLabel: "Kyle", + target: { kind: "local_daemon", status: "connected" }, + }, + }); +} + +function renderSlot( + caps: NotebookShellCapabilities, + status: ConnectionStatus = "online", +): { container: HTMLElement; source: FakeStatusSource } { + const source = new FakeStatusSource(status); + const { container } = render( + , + ); + return { container, source }; +} + +function slotElement(container: HTMLElement): HTMLElement { + const slot = container.querySelector('[data-slot="notebook-connection-identity"]'); + expect(slot).not.toBeNull(); + return slot!; +} + +function dotElement(container: HTMLElement): HTMLElement { + const dot = container.querySelector('[data-slot="avatar-badge"]'); + expect(dot).not.toBeNull(); + return dot!; +} + +describe("NotebookConnectionIdentity", () => { + it("renders nothing for a purely local desktop session", () => { + // Conditionality is the point (#3290): local identity is noise, not chrome. + const { container } = renderSlot(localCapabilities()); + expect(container.innerHTML).toBe(""); + }); + + it("renders for a local session attached to a runtime peer", () => { + const caps = localCapabilities(); + const { container } = renderSlot( + capabilities({ + ...caps, + access: caps.access, + runtime: { + ...caps.runtime, + source: "cloud", + target: { kind: "runtime_peer", status: "connected" }, + }, + }), + ); + expect(slotElement(container)).toBeTruthy(); + }); + + it.each([ + ["online", "bg-emerald-500", false, "Connected"], + ["connecting", "bg-amber-500", true, "Connecting"], + ["reconnecting", "bg-amber-500", true, "Reconnecting"], + ["offline", "bg-muted", false, "Offline"], + ] as Array<[ConnectionStatus, string, boolean, string]>)( + "renders the %s state with the statusTone dot", + (status, tone, pulses, label) => { + const { container } = renderSlot(cloudCapabilities(), status); + const slot = slotElement(container); + const dot = dotElement(container); + + expect(slot.dataset.state).toBe(status); + expect(dot.className).toContain(tone); + expect(dot.className.includes("animate-pulse")).toBe(pulses); + // State expresses as opacity/dot color, never copy: non-online dims. + expect(slot.className.includes("opacity-60")).toBe(status !== "online"); + // Detail lives in sr-only copy and the title tooltip only. + expect(slot.title).toContain(label); + const srOnly = slot.querySelector(".sr-only"); + expect(srOnly?.textContent).toContain(label); + expect(srOnly?.textContent).toContain("Alice"); + }, + ); + + it("keeps the dot live through a reconnect loop (stale-chrome motivation)", () => { + // Runtime-state stores are not blanked while the transport reconnects; + // the dot is what makes the frozen chrome interpretable, so it must + // track the loop rather than only steady states. + const { container, source } = renderSlot(cloudCapabilities(), "online"); + expect(slotElement(container).dataset.state).toBe("online"); + + act(() => source.next("reconnecting")); + expect(slotElement(container).dataset.state).toBe("reconnecting"); + expect(dotElement(container).className).toContain("animate-pulse"); + + act(() => source.next("online")); + expect(slotElement(container).dataset.state).toBe("online"); + expect(dotElement(container).className).toContain("bg-emerald-500"); + }); + + it("uses the flat quiet treatment, never a raised bubble", () => { + const { container } = renderSlot(cloudCapabilities()); + const slot = slotElement(container); + + expect(slot.className).toContain("rounded-md"); + expect(slot.className).toContain("border-border/70"); + expect(slot.className).toContain("bg-muted/35"); + // The pulled designs' raised-bubble look must never come back. + expect(slot.className).not.toContain("rounded-full"); + expect(slot.className).not.toContain("shadow"); + }); + + it("is icon-only at every width: no visible text pill", () => { + // Collapse-proof by construction — there is no label to truncate. + const { container } = renderSlot(cloudCapabilities()); + const slot = slotElement(container); + + const visibleText = Array.from(slot.querySelectorAll("*")) + .filter( + (element) => + !element.classList.contains("sr-only") && + !element.closest('[data-slot="avatar-fallback"]'), + ) + .flatMap((element) => + Array.from(element.childNodes) + .filter((node) => node.nodeType === Node.TEXT_NODE) + .map((node) => node.textContent?.trim() ?? ""), + ) + .filter((text) => text.length > 0); + expect(visibleText).toEqual([]); + + // The avatar fallback (initials) is the only visible glyph. + const fallback = slot.querySelector('[data-slot="avatar-fallback"]'); + expect(fallback?.textContent).toBe("AL"); + }); + + it("renders the actor avatar from the shared identity projection", () => { + const { container } = renderSlot(cloudCapabilities()); + const slot = slotElement(container); + expect(slot.dataset.actorKind).toBe("human"); + expect(slot.querySelector('[data-slot="notebook-actor-avatar"]')).not.toBeNull(); + }); +}); + +describe("isRemoteNotebookContext", () => { + it("treats cloud access as remote and local-only sessions as local", () => { + expect(isRemoteNotebookContext(cloudCapabilities())).toBe(true); + expect(isRemoteNotebookContext(localCapabilities())).toBe(false); + }); + + it("treats a runtime-peer compute target as remote even with local access", () => { + const caps = localCapabilities(); + expect( + isRemoteNotebookContext( + capabilities({ + ...caps, + access: caps.access, + runtime: { ...caps.runtime, target: { kind: "runtime_peer", status: "connected" } }, + }), + ), + ).toBe(true); + }); +}); diff --git a/src/components/notebook/index.ts b/src/components/notebook/index.ts index 03420d986..28e2232d3 100644 --- a/src/components/notebook/index.ts +++ b/src/components/notebook/index.ts @@ -141,6 +141,12 @@ export { type NotebookIdentityGroupProps, } from "./NotebookIdentity"; export { NotebookPresenceStatus, type NotebookPresenceStatusProps } from "./NotebookPresenceStatus"; +export { + NotebookConnectionIdentity, + isRemoteNotebookContext, + type NotebookConnectionIdentityProps, + type NotebookConnectionStatusSource, +} from "./NotebookConnectionIdentity"; export { NotebookToolbarIdentity, notebookToolbarActors, From cb32178547ed801741e4863495802db5fddb5f4d Mon Sep 17 00:00:00 2001 From: Quill Agent <261289082+quillaid@users.noreply.github.com> Date: Thu, 11 Jun 2026 03:54:47 +0000 Subject: [PATCH 09/16] feat(notebook-cloud): mount the connection slot on a switching status bridge (#3421) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CloudConnectionStatusBridge (live-sync.ts): one BehaviorSubject-backed connection-status source that stays stable for the session's lifetime. The transport survives transport-level reconnects, but initial-connect attempts and escalation teardowns still REPLACE it — the host facade's delegation captures whichever transport exists at subscribe time and never switches, so a subscriber could watch a dead object forever. The bridge attaches to each replacement transport via onTransportCreated, dedups across switches, and on escalation teardown detaches BEFORE the dispose and reports 'reconnecting' — the disposed transport's terminal 'offline' (manual-disconnect vocabulary) never surfaces while the session is retrying. useCloudViewerSession exposes connectionStatus$ from the bridge; notebook-viewer.tsx fills the formerly-null identityControls slot with NotebookConnectionIdentity driven by it. Tests: bridge replacement + dedup, teardown-retry masking, and the PR-2 reconnect loop reflected through one stable subscription; the viewer-render-props quiet-chrome regression now pins the new mount instead of identityControls={null}. --- apps/notebook-cloud/test/live-sync.test.ts | 79 +++++++++++++++++++ .../test/viewer-render-props.test.ts | 9 ++- .../viewer/cloud-viewer-session.ts | 27 +++++++ apps/notebook-cloud/viewer/live-sync.ts | 48 +++++++++++ .../notebook-cloud/viewer/notebook-viewer.tsx | 12 ++- 5 files changed, 173 insertions(+), 2 deletions(-) diff --git a/apps/notebook-cloud/test/live-sync.test.ts b/apps/notebook-cloud/test/live-sync.test.ts index fb69a6d01..c512bbc67 100644 --- a/apps/notebook-cloud/test/live-sync.test.ts +++ b/apps/notebook-cloud/test/live-sync.test.ts @@ -1,7 +1,9 @@ import { describe, it } from "node:test"; import assert from "node:assert/strict"; +import { BehaviorSubject } from "rxjs"; import { SyncEngine, type PersistedNotebookDoc, type SyncableHandle } from "runtimed"; import { + CloudConnectionStatusBridge, CloudRecoverableRejectionTracker, CloudWebSocketTransport, applyCloudRoomReady, @@ -1828,6 +1830,83 @@ describe("cloud transport reconnect loop", () => { }); }); +describe("cloud connection status bridge", () => { + it("switches to the replacement transport and dedups across the switch", () => { + const bridge = new CloudConnectionStatusBridge(); + const statuses: string[] = []; + bridge.status$.subscribe((status) => statuses.push(status)); + + // Initial-connect attempt: transport A. + const a = new BehaviorSubject<"connecting" | "online" | "offline" | "reconnecting">( + "connecting", + ); + bridge.attach({ connectionStatus$: a.asObservable() }); + a.next("online"); + + // The effect re-runs (manual retry / escalation): transport B replaces + // A. The single stable subscription must follow B and ignore A. + const b = new BehaviorSubject<"connecting" | "online" | "offline" | "reconnecting">("online"); + bridge.attach({ connectionStatus$: b.asObservable() }); + a.next("offline"); // dead transport — must not surface + b.next("reconnecting"); + b.next("online"); + + assert.deepEqual(statuses, ["connecting", "online", "reconnecting", "online"]); + assert.equal(bridge.current, "online"); + }); + + it("reports the retry loop on teardown instead of the disposed transport's offline", () => { + const bridge = new CloudConnectionStatusBridge(); + const statuses: string[] = []; + bridge.status$.subscribe((status) => statuses.push(status)); + + const transport = new BehaviorSubject<"connecting" | "online" | "offline" | "reconnecting">( + "online", + ); + bridge.attach({ connectionStatus$: transport.asObservable() }); + + // Escalation teardown: detach first, then the manual disconnect emits + // a terminal "offline" that must never surface — the session is + // retrying, not going offline. + bridge.noteTeardownRetry(); + transport.next("offline"); + + assert.deepEqual(statuses, ["connecting", "online", "reconnecting"]); + }); + + it("reflects the transport's own reconnect loop without a switch", async () => { + const fake = installFakeWebSocket(); + try { + const bridge = new CloudConnectionStatusBridge(); + const statuses: string[] = []; + bridge.status$.subscribe((status) => statuses.push(status)); + + const transport = createTransport({ reconnectBaseDelayMs: 1, random: () => 0.5 }); + bridge.attach(transport); + + const first = await waitForSocket(0); + first.open(); + first.ready("peer-1"); + await transport.ready; + + // PR-2's transport-level loop: same transport object, status walks + // online -> reconnecting -> online through the stable bridge. + first.close({ code: 1006 }); + await delayMs(10); + const second = await waitForSocket(1); + second.open(); + second.ready("peer-2"); + await drainMicrotasks(); + + assert.deepEqual(statuses, ["connecting", "online", "reconnecting", "online"]); + bridge.detach(); + transport.disconnect(); + } finally { + fake.restore(); + } + }); +}); + describe("cloud reconnect session policies", () => { it("re-establishes in the safe order: set_actor, resetForBootstrap, resetAndResync", () => { const calls: string[] = []; diff --git a/apps/notebook-cloud/test/viewer-render-props.test.ts b/apps/notebook-cloud/test/viewer-render-props.test.ts index 13bb0e616..109c6fca2 100644 --- a/apps/notebook-cloud/test/viewer-render-props.test.ts +++ b/apps/notebook-cloud/test/viewer-render-props.test.ts @@ -152,7 +152,14 @@ test("cloud viewer routes notebook header controls through the shared shell chro /authControls=\{[\s\S]*shouldShowCloudHeaderSignIn\(authState, \{[\s\S]*hasAppSession: Boolean\(appSessionStatus\.session\),[\s\S]*\}\) \? \(/, ); assert.match(sourceText, /authControls=\{[\s\S]*; liveMaterializedRef: MutableRefObject; liveRuntimeRef: MutableRefObject; notebookLanguageRef: MutableRefObject; @@ -168,6 +177,13 @@ export function useCloudViewerSession({ presenceStoreRef.current = new CloudViewerPresenceStore(); } const presenceStore = presenceStoreRef.current; + // Stable across effect re-runs: UI subscribers must not re-subscribe when + // a connect attempt or escalation teardown replaces the transport. + const connectionStatusBridgeRef = useRef(null); + if (connectionStatusBridgeRef.current === null) { + connectionStatusBridgeRef.current = new CloudConnectionStatusBridge(); + } + const connectionStatusBridge = connectionStatusBridgeRef.current; const [connectionScope, setConnectionScope] = useState(null); const [connectionPeerId, setConnectionPeerId] = useState(null); const [connectionActorLabel, setConnectionActorLabel] = useState(null); @@ -546,6 +562,9 @@ export function useCloudViewerSession({ const scheduleReconnect = (reason: Error) => { if (disposed) return; console.warn("[notebook-cloud] live room session teardown; reconnecting", reason); + // Detach BEFORE disposing: the manual disconnect's terminal "offline" + // must not surface — the session is retrying, not going offline. + connectionStatusBridge.noteTeardownRetry(); presenceStore.reduceConnection("disconnected"); setConnectionScope(null); setConnectionActorLabel(null); @@ -713,6 +732,9 @@ export function useCloudViewerSession({ onConnectionLost: handleConnectionLost, onTransportCreated: (transport) => { pendingTransport = transport; + // Follow the replacement transport so the stable status source + // reflects this attempt (and PR-2's reconnect loop within it). + connectionStatusBridge.attach(transport); }, onControl: (message) => { if (disposed) return; @@ -768,6 +790,7 @@ export function useCloudViewerSession({ // unauthorized; losing them is the intended outcome. The chain // is stashed so the next attempt arms strictly clear-then-arm. skipSeedOnceRef.current = true; + connectionStatusBridge.noteTeardownRetry(); pendingSeedDiscardRef.current = discardPersistedSeedAfterTeardown( disposeCurrentRuntime, persistenceSeed.clear, @@ -920,6 +943,9 @@ export function useCloudViewerSession({ window.removeEventListener("pagehide", flushPersistence); document.removeEventListener("visibilitychange", flushPersistenceWhenHidden); materializeLiveRuntimeRef.current = null; + // Stop following before the teardown disconnects emit "offline"; a + // re-running effect re-attaches its replacement transport. + connectionStatusBridge.detach(); const teardownFlush = disposeCurrentRuntime(); // An in-flight connect (runtime not yet resolved) owns a transport // whose retry loop would otherwise run forever after unmount. @@ -984,6 +1010,7 @@ export function useCloudViewerSession({ return { connectionActorLabel, connectionError, + connectionStatus$: connectionStatusBridge.status$, connectionPeerId, connectionScope, liveMaterializedRef, diff --git a/apps/notebook-cloud/viewer/live-sync.ts b/apps/notebook-cloud/viewer/live-sync.ts index a1a1dc333..fd670e9e4 100644 --- a/apps/notebook-cloud/viewer/live-sync.ts +++ b/apps/notebook-cloud/viewer/live-sync.ts @@ -573,6 +573,54 @@ export function shouldDiscardPersistedSeedOnRejection( return seededFromPersistence && isRecoverableCloudFrameRejection(message); } +/** + * Stable, switching connection-status source for UI consumers. + * + * The transport object survives transport-level reconnects, but + * initial-connect attempts and escalation teardowns still REPLACE it — a + * subscriber holding one transport's `connectionStatus$` would watch a + * dead object forever (the host facade has exactly that flaw). The bridge + * follows whichever transport is current and exposes one + * BehaviorSubject-backed observable that stays stable for the session's + * lifetime, deduplicating repeated values across switches. + */ +export class CloudConnectionStatusBridge { + private readonly _status$ = new BehaviorSubject("connecting"); + private subscription: { unsubscribe(): void } | null = null; + readonly status$: Observable = this._status$.asObservable(); + + get current(): ConnectionStatus { + return this._status$.getValue(); + } + + /** Follow a (replacement) transport's connection status. */ + attach(transport: Pick): void { + this.subscription?.unsubscribe(); + this.subscription = transport.connectionStatus$.subscribe((status) => this.next(status)); + } + + /** + * A session-level teardown is about to dispose the current transport and + * retry: stop following it (so the manual disconnect's terminal + * "offline" never surfaces) and report the retry loop instead. + */ + noteTeardownRetry(): void { + this.detach(); + this.next("reconnecting"); + } + + /** Stop following the current transport (effect cleanup). */ + detach(): void { + this.subscription?.unsubscribe(); + this.subscription = null; + } + + private next(status: ConnectionStatus): void { + if (status === this._status$.getValue()) return; + this._status$.next(status); + } +} + export type CloudRejectionDisposition = "absorb" | "resync_in_place" | "escalate"; /** diff --git a/apps/notebook-cloud/viewer/notebook-viewer.tsx b/apps/notebook-cloud/viewer/notebook-viewer.tsx index 463740fad..fd531a4bf 100644 --- a/apps/notebook-cloud/viewer/notebook-viewer.tsx +++ b/apps/notebook-cloud/viewer/notebook-viewer.tsx @@ -13,6 +13,7 @@ import type { NteractEmbedHostContextPatch } from "@/components/isolated/host-co import { NotebookNotice } from "@/components/notebook/NotebookNotice"; import type { NotebookRailPanelId } from "@/components/notebook-rail"; import { + NotebookConnectionIdentity, NotebookDocumentToolbar, navigateNotebookOutlineItem, NotebookDocumentRail, @@ -193,6 +194,7 @@ export function NotebookViewer({ connectionError, connectionPeerId, connectionScope, + connectionStatus$, liveMaterializedRef, liveRuntimeRef, notebookLanguageRef, @@ -845,7 +847,15 @@ export function NotebookViewer({ onRequestEditAccess={requestCloudEditAccess} /> } - identityControls={null} + identityControls={ + // Connection/identity slot: self-identity avatar + connectivity dot + // (the stable bridge survives transport replacement; the dot keeps + // frozen runtime chrome interpretable while reconnecting). + + } reserveCommandToolbar={editAccessPending} commandToolbar={{ runtime: toolbarRuntime, From 7958d4bad32b8b3a365f99af7e36e4f415ac1336 Mon Sep 17 00:00:00 2001 From: Quill Agent <261289082+quillaid@users.noreply.github.com> Date: Thu, 11 Jun 2026 03:54:47 +0000 Subject: [PATCH 10/16] docs(adr): record PR-3 landed shape - Component name (NotebookConnectionIdentity), centralized isRemoteNotebookContext gate, structural rxjs-free status source. - Desktop mount via host transport; cloud mount via CloudConnectionStatusBridge with teardown-retry masking semantics. - The slot is what makes the PR-2 stale-chrome limitation interpretable: the dot stays live through 'reconnecting'. - Tests paragraph updated to the landed coverage, including the identityControls={null} pin becoming a pin on the new mount. --- docs/adr/local-first-notebook-state.md | 63 +++++++++++++++++--------- 1 file changed, 42 insertions(+), 21 deletions(-) diff --git a/docs/adr/local-first-notebook-state.md b/docs/adr/local-first-notebook-state.md index c99eae3b4..4a9187bb4 100644 --- a/docs/adr/local-first-notebook-state.md +++ b/docs/adr/local-first-notebook-state.md @@ -386,23 +386,34 @@ real `createCloudConnectTarget`. ## PR 3 — Connection/identity slot -One shared component, mounted in slots that already exist and are empty: +One shared component — `NotebookConnectionIdentity` +(`src/components/notebook/`) — mounted in slots that already existed and +were empty: -- **Cloud:** `identityControls={null}` in `notebook-viewer.tsx` (right-most - header slot). -- **Desktop:** `trailingControls` → `identityControls` at the right end of the - command toolbar. +- **Cloud:** the `identityControls={null}` slot in `notebook-viewer.tsx` + (right-most header slot), now filled. +- **Desktop:** `trailingControls` on `` in `App.tsx` → + `identityControls` at the right end of the command toolbar, fed by the + host transport's `connectionStatus$` (stable for the app's lifetime; + TauriTransport reports online/offline today, which is honest). **Conditionality** (the reason #3290 pulled the previous attempt): the slot -renders **nothing** for a purely local desktop session. The predicate already -exists in `NotebookShellCapabilities`: `access.source !== "local"` or -`runtime.target.kind === "runtime_peer"`. Cloud is always remote. - -**Content:** self-identity (the flattened `NotebookIdentityBadge` treatment — -actor initials/avatar) paired with a connectivity dot driven by -`connectionStatus$` — its first UI consumer. Status vocabulary follows the -existing tones: emerald `online`, amber pulse `reconnecting`/`connecting`, -muted `offline`. +renders **nothing** for a purely local desktop session. The predicate is +centralized in the component (`isRemoteNotebookContext`): +`access.source !== "local"` or `runtime.target.kind === "runtime_peer"`. +Cloud is always remote, so hosts mount unconditionally. + +**Content:** self-identity (the flattened avatar treatment via the shared +actor projection — initials/avatar only, no visible label) paired with a +connectivity dot driven by `connectionStatus$` — its first UI consumer. The +component accepts a structural `NotebookConnectionStatusSource` (an +rxjs-free `Observable` subset) so shared `src/` takes no new dependency. +Status vocabulary follows the existing tones: emerald `online`, amber pulse +`reconnecting`/`connecting`, muted `offline`; non-online states also dim +the wrapper (opacity, not copy). This is the surface that makes the PR-2 +limitation interpretable: runtime-state stores are not blanked during the +offline window, and the pulsing dot stays live through `reconnecting` so +frozen kernel/execution chrome reads as "reconnecting", not as truth. **Aesthetic rules distilled from the three pulled designs** (#3273, #3290, #3337, #3349 — recorded so we do not relitigate them): @@ -421,13 +432,23 @@ muted `offline`. Plumbing fix included: the host facade's `connectionStatus$` delegation captures whichever transport exists at subscribe time and never switches — a subscriber can watch a dead transport forever. With PR 2 the transport object -survives reconnects, but initial-connect attempts may still replace it; the -session exposes a stable, switching connection-status source (small -BehaviorSubject bridge fed by the current transport) for the UI. - -Tests: component tests for each status × identity combination and the -local-session-renders-nothing gate; keep #3337's quiet-chrome regressions -green. +survives reconnects, but initial-connect attempts and escalation teardowns +still replace it; the session exposes `CloudConnectionStatusBridge` +(`live-sync.ts`) — a stable BehaviorSubject-backed source attached to each +replacement transport via `onTransportCreated`, deduplicating across +switches. On escalation teardown the bridge detaches BEFORE the dispose and +reports `"reconnecting"`, so the disposed transport's terminal `"offline"` +(manual-disconnect vocabulary) never surfaces while the session is retrying. + +Tests: component tests for every status × identity combination +(data-state + dot tone + opacity), the local-session-renders-nothing gate, +the runtime-peer remote case, the live-dot-through-reconnecting regression, +the flat-never-raised treatment, and icon-only-at-every-width (no visible +text outside the avatar — collapse-proof by construction); bridge tests for +transport replacement, teardown-retry masking of the disposed transport's +offline, and the PR-2 loop reflected without a switch; #3337's quiet-chrome +regressions stay green (the `identityControls={null}` pin became a pin on +the new mount). --- From f7cdc4a4f52434dbd14d042c9194eb5d4661aa26 Mon Sep 17 00:00:00 2001 From: Quill Agent <261289082+quillaid@users.noreply.github.com> Date: Thu, 11 Jun 2026 09:59:27 +0000 Subject: [PATCH 11/16] fix(notebook): real desktop dot, scoped link copy, SR announcements (#3421) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per the PR-3 adversarial review: - The desktop dot was a constant: TauriTransport's connectionStatus$ initializes 'online' and the app never disconnects it, so the slot could never transition — emerald through a daemon restart, the exact window its doc comment promised to make interpretable. The mount now feeds createDesktopConnectionStatusSource, derived from the host facade's daemon lifecycle events (daemon:ready -> online with ready-cache backfill for late mounts; daemon:disconnected -> reconnecting, since the host auto-reconnects; daemon:unavailable -> offline). The IPC transport's own subject stays untouched and honest about IPC. - Scoped copy: connectionLabel names the link the source measures ('Daemon connection' on desktop) in the title/sr-only copy and the announcements, so the dot never asserts daemon<->room health it does not observe (recorded as future work). - getCurrent(): optional synchronous snapshot on the source contract; first paint shows the real status instead of a one-frame 'connecting' flash. - Live region: status CHANGES (never the initial value — the first source delivery is the baseline) are announced via a polite sr-only region using the scoped copy; sources dedup so flaps don't spam. - aria-hidden on the avatar so SR users hear the sr-only copy, not 'AL Alice — Connected'. Tests hardened per review: fixtures use valid runtime-target shapes (attached/ready + label); no-visible-text now clone-and-strips so wrapper-level text nodes are caught; flat-never-raised walks the whole subtree banning shadow tokens (rounded-full stays wrapper-scoped — Avatar internals are legitimately round) with classList token checks; unmount-unsubscribe pinned via listenerCount; new tests for getCurrent first paint, live-region change-only announcements, aria-hidden, and scoped copy. Desktop coverage added: daemon-lifecycle source unit tests, a composition test feeding REAL desktopNotebookShellCapabilities output into isRemoteNotebookContext, toolbar trailingControls flow with real projection fixtures, and a source-text pin on the App.tsx mount (daemon source + scoped label, never the IPC transport). --- apps/notebook/src/App.tsx | 16 +- .../__tests__/notebook-toolbar.test.tsx | 76 ++++++++ .../desktop-connection-status.test.ts | 93 +++++++++ .../desktop-shell-capabilities.test.ts | 31 +++ .../src/lib/desktop-connection-status.ts | 64 +++++++ .../notebook/NotebookConnectionIdentity.tsx | 89 +++++++-- .../NotebookConnectionIdentity.test.tsx | 181 +++++++++++++----- 7 files changed, 480 insertions(+), 70 deletions(-) create mode 100644 apps/notebook/src/lib/__tests__/desktop-connection-status.test.ts create mode 100644 apps/notebook/src/lib/desktop-connection-status.ts diff --git a/apps/notebook/src/App.tsx b/apps/notebook/src/App.tsx index 2bdf3a93b..1d7dbc094 100644 --- a/apps/notebook/src/App.tsx +++ b/apps/notebook/src/App.tsx @@ -51,6 +51,7 @@ import { UntrustedBanner, } from "@/components/notebook"; import { GlobalFindBar } from "@/components/search"; +import { createDesktopConnectionStatusSource } from "./lib/desktop-connection-status"; import { CondaDependencyPanel as CondaDependencyHeader, DenoDependencyPanel as DenoDependencyHeader, @@ -640,6 +641,14 @@ function AppContent() { ], ); + // Connection/identity slot source: daemon lifecycle, stable for the + // app's lifetime (the dot must transition on daemon restarts). + const desktopConnectionStatus = useMemo( + () => createDesktopConnectionStatusSource(host.daemonEvents), + [host], + ); + useEffect(() => () => desktopConnectionStatus.dispose(), [desktopConnectionStatus]); + useEffect(() => { installExecutionPerformanceApi(); }, []); @@ -1518,10 +1527,13 @@ function AppContent() { trailingControls={ // Connection/identity slot: renders nothing for a purely local // session (isRemoteNotebookContext) — conditionality is the - // point. The host transport is stable for the app's lifetime. + // point. The source derives from daemon lifecycle events (the + // IPC transport's status is constant in practice), and the + // copy is scoped to the link it measures. } /> diff --git a/apps/notebook/src/components/__tests__/notebook-toolbar.test.tsx b/apps/notebook/src/components/__tests__/notebook-toolbar.test.tsx index 2c023cdbe..2e1a96bdc 100644 --- a/apps/notebook/src/components/__tests__/notebook-toolbar.test.tsx +++ b/apps/notebook/src/components/__tests__/notebook-toolbar.test.tsx @@ -19,6 +19,10 @@ import { type RuntimeLifecycle, type RuntimeStatusKey, } from "../../lib/kernel-status"; +import { readFileSync } from "node:fs"; +import { resolve } from "node:path"; +import { NotebookConnectionIdentity } from "@/components/notebook"; +import { desktopNotebookShellCapabilities } from "../../lib/desktop-shell-capabilities"; import { NotebookToolbar } from "../NotebookToolbar"; function makeEnvProgress(overrides: Partial): EnvProgressState { @@ -624,3 +628,75 @@ describe("NotebookToolbar", () => { }); }); }); + +describe("connection/identity slot wiring", () => { + const statusSource = { + getCurrent: () => "online" as const, + subscribe: () => ({ unsubscribe: () => {} }), + }; + + it("flows trailingControls into the toolbar identity slot for remote sessions", () => { + // Real projection output, not hand-rolled capability literals: the + // runtime_peer scope is what makes the slot render on desktop. + const capabilities = desktopNotebookShellCapabilities({ + canAcceptCellMutations: true, + sessionReady: true, + localActor: "user:anaconda:kyle/desktop:window", + connectionScope: "runtime_peer", + }); + const { container } = render( + + } + />, + ); + + const slot = container.querySelector('[data-slot="notebook-connection-identity"]'); + expect(slot).not.toBeNull(); + expect(slot?.getAttribute("title")).toContain("Daemon connection: Connected"); + }); + + it("renders no identity chrome for a purely local session", () => { + const capabilities = desktopNotebookShellCapabilities({ + canAcceptCellMutations: true, + sessionReady: true, + localActor: "local:kyle/desktop:window", + connectionScope: null, + }); + const { container } = render( + + } + />, + ); + + expect(container.querySelector('[data-slot="notebook-connection-identity"]')).toBeNull(); + }); + + it("App.tsx mounts the slot on the daemon-lifecycle source with scoped copy", () => { + // Source-text pin mirroring the cloud guardrail: deleting the desktop + // mount, feeding it the IPC transport again, or dropping the scoped + // connection label must fail here. + // vp test runs from the repo root. + const appSource = readFileSync(resolve(process.cwd(), "apps/notebook/src/App.tsx"), "utf8"); + expect(appSource).toMatch( + /trailingControls=\{[\s\S]{0,600}? void>(), + disconnected: new Set<() => void>(), + unavailable: new Set<() => void>(), + }; + return { + handlers, + fire(kind: keyof typeof handlers): void { + for (const handler of [...handlers[kind]]) { + handler(); + } + }, + daemonEvents: { + onReady: (cb: () => void) => { + handlers.ready.add(cb); + return () => handlers.ready.delete(cb); + }, + onDisconnected: (cb: () => void) => { + handlers.disconnected.add(cb); + return () => handlers.disconnected.delete(cb); + }, + onUnavailable: (cb: () => void) => { + handlers.unavailable.add(cb); + return () => handlers.unavailable.delete(cb); + }, + }, + }; +} + +describe("createDesktopConnectionStatusSource", () => { + it("walks the daemon lifecycle: connecting, online, reconnecting, online, offline", () => { + const fake = createFakeDaemonEvents(); + const source = createDesktopConnectionStatusSource(fake.daemonEvents); + const statuses: ConnectionStatus[] = []; + source.subscribe((status) => statuses.push(status)); + + // daemon:ready (incl. the host facade's cache backfill for late mounts). + fake.fire("ready"); + // daemon:disconnected — the host auto-reconnects, so this is a + // transition, not an outage verdict. + fake.fire("disconnected"); + // daemon restart completes. + fake.fire("ready"); + // daemon:unavailable is the terminal state. + fake.fire("unavailable"); + + expect(statuses).toEqual(["connecting", "online", "reconnecting", "online", "offline"]); + expect(source.getCurrent()).toBe("offline"); + }); + + it("dedups repeated lifecycle events", () => { + const fake = createFakeDaemonEvents(); + const source = createDesktopConnectionStatusSource(fake.daemonEvents); + const statuses: ConnectionStatus[] = []; + source.subscribe((status) => statuses.push(status)); + + fake.fire("ready"); + fake.fire("ready"); // path changes re-emit daemon:ready + expect(statuses).toEqual(["connecting", "online"]); + }); + + it("replays the current value to subscribers and supports getCurrent", () => { + const fake = createFakeDaemonEvents(); + const source = createDesktopConnectionStatusSource(fake.daemonEvents); + fake.fire("ready"); + + expect(source.getCurrent()).toBe("online"); + const statuses: ConnectionStatus[] = []; + source.subscribe((status) => statuses.push(status)); + expect(statuses).toEqual(["online"]); + }); + + it("unsubscribe and dispose detach cleanly", () => { + const fake = createFakeDaemonEvents(); + const source = createDesktopConnectionStatusSource(fake.daemonEvents); + const statuses: ConnectionStatus[] = []; + const subscription = source.subscribe((status) => statuses.push(status)); + + subscription.unsubscribe(); + fake.fire("ready"); + expect(statuses).toEqual(["connecting"]); // late events are inert + + source.dispose(); + expect(fake.handlers.ready.size).toBe(0); + expect(fake.handlers.disconnected.size).toBe(0); + expect(fake.handlers.unavailable.size).toBe(0); + }); +}); diff --git a/apps/notebook/src/lib/__tests__/desktop-shell-capabilities.test.ts b/apps/notebook/src/lib/__tests__/desktop-shell-capabilities.test.ts index ecb0c3331..ab5636497 100644 --- a/apps/notebook/src/lib/__tests__/desktop-shell-capabilities.test.ts +++ b/apps/notebook/src/lib/__tests__/desktop-shell-capabilities.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from "vite-plus/test"; +import { isRemoteNotebookContext } from "@/components/notebook"; import { desktopNotebookShellCapabilities } from "../desktop-shell-capabilities"; import { RUNTIME_STATUS } from "../kernel-status"; @@ -375,4 +376,34 @@ describe("desktopNotebookShellCapabilities", () => { scope: "editor", }); }); + + it("composes with the connection/identity slot gate on REAL projection output", () => { + // Drift guard: the slot's isRemoteNotebookContext must keep agreeing + // with what desktopNotebookShellCapabilities actually emits — a purely + // local session renders no identity chrome (#3290), a server-assigned + // scope does. + const local = desktopNotebookShellCapabilities({ + canAcceptCellMutations: true, + sessionReady: true, + localActor: "local:kyle/desktop:window", + connectionScope: null, + }); + expect(isRemoteNotebookContext(local)).toBe(false); + + const runtimePeer = desktopNotebookShellCapabilities({ + canAcceptCellMutations: true, + sessionReady: true, + localActor: "user:anaconda:kyle/desktop:window", + connectionScope: "runtime_peer", + }); + expect(isRemoteNotebookContext(runtimePeer)).toBe(true); + + const cloudScoped = desktopNotebookShellCapabilities({ + canAcceptCellMutations: true, + sessionReady: true, + localActor: "user:anaconda:kyle/desktop:window", + connectionScope: "editor", + }); + expect(isRemoteNotebookContext(cloudScoped)).toBe(true); + }); }); diff --git a/apps/notebook/src/lib/desktop-connection-status.ts b/apps/notebook/src/lib/desktop-connection-status.ts new file mode 100644 index 000000000..d450788b2 --- /dev/null +++ b/apps/notebook/src/lib/desktop-connection-status.ts @@ -0,0 +1,64 @@ +import type { HostDaemonEvents } from "@nteract/notebook-host"; +import type { ConnectionStatus } from "runtimed"; +import type { NotebookConnectionStatusSource } from "@/components/notebook"; + +export interface DesktopConnectionStatusSource extends NotebookConnectionStatusSource { + getCurrent(): ConnectionStatus; + /** Detach from the daemon events (app teardown). */ + dispose(): void; +} + +/** + * Desktop connection-status source for the connection/identity slot, + * derived from daemon lifecycle events. + * + * The Tauri IPC transport's own `connectionStatus$` is honest about IPC + * but constant in practice — the app never disconnects it, so a dot fed + * from it could never transition (it would sit emerald through a daemon + * restart, exactly the window where kernel/execution chrome freezes). The + * daemon link is the first hop of any remote context the slot renders + * for, and `daemon:ready` / `daemon:disconnected` / `daemon:unavailable` + * are the real lifecycle the desktop can report today. Daemon↔room link + * health is future work; the slot's copy is scoped to the daemon link via + * its `connectionLabel`. + * + * - `onReady` → "online" (the host facade backfills from its cache, so a + * source created after the daemon is already up still reaches "online") + * - `onDisconnected` → "reconnecting" (the host immediately starts its own + * reconnect — "reconnecting" is the truthful state, not "offline") + * - `onUnavailable` → "offline" + */ +export function createDesktopConnectionStatusSource( + daemonEvents: Pick, +): DesktopConnectionStatusSource { + let current: ConnectionStatus = "connecting"; + const listeners = new Set<(status: ConnectionStatus) => void>(); + const next = (status: ConnectionStatus) => { + if (status === current) return; + current = status; + for (const listener of listeners) { + listener(status); + } + }; + + const unlisteners = [ + daemonEvents.onReady(() => next("online")), + daemonEvents.onDisconnected(() => next("reconnecting")), + daemonEvents.onUnavailable(() => next("offline")), + ]; + + return { + getCurrent: () => current, + subscribe(listener) { + listener(current); + listeners.add(listener); + return { unsubscribe: () => listeners.delete(listener) }; + }, + dispose() { + for (const unlisten of unlisteners) { + unlisten(); + } + listeners.clear(); + }, + }; +} diff --git a/src/components/notebook/NotebookConnectionIdentity.tsx b/src/components/notebook/NotebookConnectionIdentity.tsx index e3ed196c6..148c7a657 100644 --- a/src/components/notebook/NotebookConnectionIdentity.tsx +++ b/src/components/notebook/NotebookConnectionIdentity.tsx @@ -12,16 +12,26 @@ import type { NotebookShellCapabilities } from "./capabilities"; */ export interface NotebookConnectionStatusSource { subscribe(next: (status: ConnectionStatus) => void): { unsubscribe(): void }; + /** + * Optional synchronous snapshot for first paint. BehaviorSubject-backed + * sources (the cloud bridge, the desktop daemon source) implement it so + * the first committed frame shows the real status instead of a one-frame + * "connecting" flash. + */ + getCurrent?(): ConnectionStatus; } /** * NotebookConnectionIdentity — the connection/identity slot. * * Self-identity (the flattened avatar treatment) paired with a - * connectivity dot driven by the transport's `connectionStatus$` — its - * first UI consumer. Runtime-state stores are deliberately not blanked - * while a transport reconnects, so this dot is what makes frozen - * kernel/execution chrome interpretable during the offline window. + * connectivity dot driven by a connection-status source — its first UI + * consumer. Runtime-state stores are deliberately not blanked while a + * transport reconnects, so this dot is what makes frozen kernel/execution + * chrome interpretable during the offline window — for the link the host + * actually measures: the cloud room transport on cloud, the daemon link on + * desktop (daemon↔room health is future work; `connectionLabel` scopes the + * copy to the measured link so the dot never overclaims). * * Quiet-chrome rules (distilled from the pulled #3273/#3290/#3337/#3349 * designs — hard constraints, not preferences): @@ -31,28 +41,39 @@ export interface NotebookConnectionStatusSource { * rounded-full + shadow; * - icon/avatar-first and icon-only at every width: no visible text pill — * the label and status live in `sr-only` copy and the title tooltip; - * - state expresses as dot color / opacity, never copy; + * - state expresses as dot color / opacity, never copy; status CHANGES are + * announced through a polite sr-only live region (quiet for the eyes is + * not silence for screen readers); * - errors and reconnect prompts belong to the notices stack, never here; * - connection state never masquerades as kernel/runtime status. */ export interface NotebookConnectionIdentityProps { capabilities: NotebookShellCapabilities; /** - * Connection lifecycle from the live transport. Hosts must hand a source - * that survives transport replacement (cloud: the session's - * `CloudConnectionStatusBridge`; desktop: the host transport, which is + * Connection lifecycle source. Hosts must hand a source that survives + * transport replacement (cloud: the session's + * `CloudConnectionStatusBridge`; desktop: the daemon-lifecycle source, * stable for the app's lifetime). */ connectionStatus$: NotebookConnectionStatusSource; + /** + * Names the link the source measures, e.g. "Daemon connection" on + * desktop. Scopes the title/sr-only copy and the live-region + * announcements so the dot never asserts health it does not observe. + */ + connectionLabel?: string; className?: string; } export function NotebookConnectionIdentity({ capabilities, connectionStatus$, + connectionLabel, className, }: NotebookConnectionIdentityProps) { - const status = useConnectionStatus(connectionStatus$); + const { status, announcedStatus } = useConnectionStatus(connectionStatus$); + const statusText = formatStatusText(status, connectionLabel); + const announcement = announcedStatus ? formatStatusText(announcedStatus, connectionLabel) : ""; // Conditionality is the point (#3290): a purely local desktop session // gets no identity chrome at all. @@ -65,8 +86,7 @@ export function NotebookConnectionIdentity({ return null; } - const statusLabel = connectionStatusLabel(status); - const detail = `${actor.label} — ${statusLabel}`; + const detail = `${actor.label} — ${statusText}`; return (
- + {/* The visual layer is hidden from the a11y tree (avatar initials + would otherwise leak ahead of the sr-only copy). */} + {detail} + {/* Announces status CHANGES only — empty on initial render so a mount + never speaks, and sources dedup repeated values so flaps don't + spam. */} + + {announcement} +
); } @@ -97,13 +127,40 @@ export function isRemoteNotebookContext(capabilities: NotebookShellCapabilities) ); } -function useConnectionStatus(status$: NotebookConnectionStatusSource): ConnectionStatus { - const [status, setStatus] = useState("connecting"); +function useConnectionStatus(status$: NotebookConnectionStatusSource): { + status: ConnectionStatus; + /** Set only on a CHANGE the source delivered after its initial value. */ + announcedStatus: ConnectionStatus | null; +} { + const [status, setStatus] = useState( + () => status$.getCurrent?.() ?? "connecting", + ); + const [announcedStatus, setAnnouncedStatus] = useState(null); useEffect(() => { - const subscription = status$.subscribe((next) => setStatus(next)); + // The first delivery (BehaviorSubject replay) is the baseline, never an + // announcement — only subsequent transitions speak. + let baseline: ConnectionStatus | null = null; + let delivered = false; + const subscription = status$.subscribe((next) => { + setStatus(next); + if (!delivered) { + delivered = true; + baseline = next; + return; + } + if (next !== baseline) { + baseline = next; + setAnnouncedStatus(next); + } + }); return () => subscription.unsubscribe(); }, [status$]); - return status; + return { status, announcedStatus }; +} + +function formatStatusText(status: ConnectionStatus, connectionLabel?: string): string { + const label = connectionStatusLabel(status); + return connectionLabel ? `${connectionLabel}: ${label}` : label; } /** Existing statusTone vocabulary: emerald active, amber attention, muted offline. */ diff --git a/src/components/notebook/__tests__/NotebookConnectionIdentity.test.tsx b/src/components/notebook/__tests__/NotebookConnectionIdentity.test.tsx index 88559d6e3..ab245ed0b 100644 --- a/src/components/notebook/__tests__/NotebookConnectionIdentity.test.tsx +++ b/src/components/notebook/__tests__/NotebookConnectionIdentity.test.tsx @@ -27,6 +27,23 @@ class FakeStatusSource implements NotebookConnectionStatusSource { listener(status); } } + + get listenerCount(): number { + return this.listeners.size; + } +} + +/** Source with a snapshot but NO replay on subscribe — isolates getCurrent. */ +class SnapshotOnlyStatusSource implements NotebookConnectionStatusSource { + constructor(private readonly value: ConnectionStatus) {} + + getCurrent(): ConnectionStatus { + return this.value; + } + + subscribe(): { unsubscribe(): void } { + return { unsubscribe: () => {} }; + } } function capabilities( @@ -77,7 +94,20 @@ function localCapabilities(): NotebookShellCapabilities { source: "local", actorLabel: "local:kyle/desktop:abc", identityLabel: "Kyle", - target: { kind: "local_daemon", status: "connected" }, + target: { kind: "local_daemon", status: "ready", label: "Local daemon" }, + }, + }); +} + +function runtimePeerCapabilities(): NotebookShellCapabilities { + const caps = localCapabilities(); + return capabilities({ + ...caps, + access: caps.access, + runtime: { + ...caps.runtime, + source: "cloud", + target: { kind: "runtime_peer", status: "attached", label: "Cloud room" }, }, }); } @@ -85,12 +115,13 @@ function localCapabilities(): NotebookShellCapabilities { function renderSlot( caps: NotebookShellCapabilities, status: ConnectionStatus = "online", -): { container: HTMLElement; source: FakeStatusSource } { + props: { connectionLabel?: string } = {}, +): { container: HTMLElement; source: FakeStatusSource; unmount: () => void } { const source = new FakeStatusSource(status); - const { container } = render( - , + const { container, unmount } = render( + , ); - return { container, source }; + return { container, source, unmount }; } function slotElement(container: HTMLElement): HTMLElement { @@ -105,6 +136,12 @@ function dotElement(container: HTMLElement): HTMLElement { return dot!; } +function liveRegion(container: HTMLElement): HTMLElement { + const region = container.querySelector('[aria-live="polite"]'); + expect(region).not.toBeNull(); + return region!; +} + describe("NotebookConnectionIdentity", () => { it("renders nothing for a purely local desktop session", () => { // Conditionality is the point (#3290): local identity is noise, not chrome. @@ -113,18 +150,7 @@ describe("NotebookConnectionIdentity", () => { }); it("renders for a local session attached to a runtime peer", () => { - const caps = localCapabilities(); - const { container } = renderSlot( - capabilities({ - ...caps, - access: caps.access, - runtime: { - ...caps.runtime, - source: "cloud", - target: { kind: "runtime_peer", status: "connected" }, - }, - }), - ); + const { container } = renderSlot(runtimePeerCapabilities()); expect(slotElement(container)).toBeTruthy(); }); @@ -141,10 +167,10 @@ describe("NotebookConnectionIdentity", () => { const dot = dotElement(container); expect(slot.dataset.state).toBe(status); - expect(dot.className).toContain(tone); - expect(dot.className.includes("animate-pulse")).toBe(pulses); + expect(dot.classList.contains(tone)).toBe(true); + expect(dot.classList.contains("animate-pulse")).toBe(pulses); // State expresses as opacity/dot color, never copy: non-online dims. - expect(slot.className.includes("opacity-60")).toBe(status !== "online"); + expect(slot.classList.contains("opacity-60")).toBe(status !== "online"); // Detail lives in sr-only copy and the title tooltip only. expect(slot.title).toContain(label); const srOnly = slot.querySelector(".sr-only"); @@ -153,6 +179,18 @@ describe("NotebookConnectionIdentity", () => { }, ); + it("scopes the copy to the measured link via connectionLabel", () => { + // Desktop measures the daemon link, not daemon<->room health — the + // copy must say which link it reports so the dot never overclaims. + const { container } = renderSlot(runtimePeerCapabilities(), "online", { + connectionLabel: "Daemon connection", + }); + const slot = slotElement(container); + + expect(slot.title).toContain("Daemon connection: Connected"); + expect(slot.querySelector(".sr-only")?.textContent).toContain("Daemon connection: Connected"); + }); + it("keeps the dot live through a reconnect loop (stale-chrome motivation)", () => { // Runtime-state stores are not blanked while the transport reconnects; // the dot is what makes the frozen chrome interpretable, so it must @@ -162,43 +200,91 @@ describe("NotebookConnectionIdentity", () => { act(() => source.next("reconnecting")); expect(slotElement(container).dataset.state).toBe("reconnecting"); - expect(dotElement(container).className).toContain("animate-pulse"); + expect(dotElement(container).classList.contains("animate-pulse")).toBe(true); act(() => source.next("online")); expect(slotElement(container).dataset.state).toBe("online"); - expect(dotElement(container).className).toContain("bg-emerald-500"); + expect(dotElement(container).classList.contains("bg-emerald-500")).toBe(true); + }); + + it("seeds first paint from getCurrent without waiting for a subscription replay", () => { + // SnapshotOnlyStatusSource never replays on subscribe — the rendered + // state can only come from the synchronous getCurrent snapshot. + const source = new SnapshotOnlyStatusSource("online"); + const { container } = render( + , + ); + expect(slotElement(container).dataset.state).toBe("online"); + }); + + it("announces status CHANGES politely, never the initial state", () => { + const { container, source } = renderSlot(cloudCapabilities(), "online", { + connectionLabel: "Live room", + }); + // Mounting must not speak. + expect(liveRegion(container).textContent).toBe(""); + + act(() => source.next("reconnecting")); + expect(liveRegion(container).textContent).toBe("Live room: Reconnecting"); + + act(() => source.next("online")); + expect(liveRegion(container).textContent).toBe("Live room: Connected"); + }); + + it("hides the avatar from the a11y tree so sr-only copy is the accessible text", () => { + // Without aria-hidden, SR users would hear "AL Alice — Connected". + const { container } = renderSlot(cloudCapabilities()); + const avatar = container.querySelector('[data-slot="notebook-actor-avatar"]'); + expect(avatar).not.toBeNull(); + expect(avatar?.closest('[aria-hidden="true"]')).not.toBeNull(); + + const srOnly = slotElement(container).querySelector(".sr-only"); + expect(srOnly?.closest('[aria-hidden="true"]')).toBeNull(); }); - it("uses the flat quiet treatment, never a raised bubble", () => { + it("unsubscribes from the source on unmount", () => { + const { source, unmount } = renderSlot(cloudCapabilities(), "online"); + expect(source.listenerCount).toBe(1); + + unmount(); + expect(source.listenerCount).toBe(0); + // Late emissions are inert (no listener left to call). + source.next("offline"); + }); + + it("uses the flat quiet treatment, never a raised bubble — across the whole subtree", () => { const { container } = renderSlot(cloudCapabilities()); const slot = slotElement(container); - expect(slot.className).toContain("rounded-md"); - expect(slot.className).toContain("border-border/70"); - expect(slot.className).toContain("bg-muted/35"); - // The pulled designs' raised-bubble look must never come back. - expect(slot.className).not.toContain("rounded-full"); - expect(slot.className).not.toContain("shadow"); + expect(slot.classList.contains("rounded-md")).toBe(true); + expect(slot.classList.contains("border-border/70")).toBe(true); + expect(slot.classList.contains("bg-muted/35")).toBe(true); + // The pulled designs' raised-bubble look must never come back. The + // wrapper must not be a pill (Avatar internals are legitimately + // rounded-full), and NOTHING in the subtree may carry a shadow. + expect(slot.classList.contains("rounded-full")).toBe(false); + for (const element of [slot, ...Array.from(slot.querySelectorAll("*"))]) { + for (const token of Array.from(element.classList)) { + expect(token.startsWith("shadow")).toBe(false); + } + } }); it("is icon-only at every width: no visible text pill", () => { // Collapse-proof by construction — there is no label to truncate. + // Clone-and-strip covers the wrapper's own text nodes and every + // nesting depth: remove sr-only copy and the avatar initials, then + // nothing visible may remain. const { container } = renderSlot(cloudCapabilities()); const slot = slotElement(container); - const visibleText = Array.from(slot.querySelectorAll("*")) - .filter( - (element) => - !element.classList.contains("sr-only") && - !element.closest('[data-slot="avatar-fallback"]'), - ) - .flatMap((element) => - Array.from(element.childNodes) - .filter((node) => node.nodeType === Node.TEXT_NODE) - .map((node) => node.textContent?.trim() ?? ""), - ) - .filter((text) => text.length > 0); - expect(visibleText).toEqual([]); + const clone = slot.cloneNode(true) as HTMLElement; + for (const hidden of Array.from( + clone.querySelectorAll('.sr-only, [data-slot="avatar-fallback"]'), + )) { + hidden.remove(); + } + expect(clone.textContent?.trim()).toBe(""); // The avatar fallback (initials) is the only visible glyph. const fallback = slot.querySelector('[data-slot="avatar-fallback"]'); @@ -220,15 +306,6 @@ describe("isRemoteNotebookContext", () => { }); it("treats a runtime-peer compute target as remote even with local access", () => { - const caps = localCapabilities(); - expect( - isRemoteNotebookContext( - capabilities({ - ...caps, - access: caps.access, - runtime: { ...caps.runtime, target: { kind: "runtime_peer", status: "connected" } }, - }), - ), - ).toBe(true); + expect(isRemoteNotebookContext(runtimePeerCapabilities())).toBe(true); }); }); From 1c9ae9575035a8ef7626b99b31ae5229d1b3aa23 Mon Sep 17 00:00:00 2001 From: Quill Agent <261289082+quillaid@users.noreply.github.com> Date: Thu, 11 Jun 2026 09:59:46 +0000 Subject: [PATCH 12/16] fix(notebook-cloud): close the bridge gap, implement the slot source contract (#3421) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Auth-refresh gap: the live-room effect cleanup detached the bridge and the refreshing re-run early-returns without attaching, so the bridge served stale 'online' while the transport was fully disconnected. The cleanup now uses noteTeardownRetry() — the gap reads as 'reconnecting' (a transition), and the disposed transports' terminal 'offline' still never surfaces. Harmless at real unmount. - CloudConnectionStatusBridge implements the slot's source contract directly (subscribe + getCurrent), and the session exposes the bridge itself so first paint reads the synchronous snapshot. - Tests: plain detach() severance (no late emissions, no spurious status), the source contract (replay + snapshot + unsubscribe); viewer-render-props slot pin is now module-scoped via viewerFileContaining with bounded windows and pins capabilities={shellCapabilities} (no cross-corpus false-pass), plus source guardrails for the session wiring order: attach-on-transport-created and retry-before-dispose in all three teardown paths (scheduleReconnect, poison-pill discard, effect cleanup). --- apps/notebook-cloud/test/live-sync.test.ts | 40 +++++++++++++++++++ .../test/viewer-render-props.test.ts | 38 ++++++++++++++++-- .../viewer/cloud-viewer-session.ts | 21 +++++----- apps/notebook-cloud/viewer/live-sync.ts | 16 ++++++++ 4 files changed, 102 insertions(+), 13 deletions(-) diff --git a/apps/notebook-cloud/test/live-sync.test.ts b/apps/notebook-cloud/test/live-sync.test.ts index c512bbc67..f911e3326 100644 --- a/apps/notebook-cloud/test/live-sync.test.ts +++ b/apps/notebook-cloud/test/live-sync.test.ts @@ -1874,6 +1874,46 @@ describe("cloud connection status bridge", () => { assert.deepEqual(statuses, ["connecting", "online", "reconnecting"]); }); + it("detach severs silently: no late emissions, no spurious status", () => { + const bridge = new CloudConnectionStatusBridge(); + const statuses: string[] = []; + bridge.status$.subscribe((status) => statuses.push(status)); + + const transport = new BehaviorSubject<"connecting" | "online" | "offline" | "reconnecting">( + "online", + ); + bridge.attach({ connectionStatus$: transport.asObservable() }); + + // Plain detach (effect cleanup without a retry in flight): the dead + // transport's later emissions must not surface, and detach itself must + // not emit anything. + bridge.detach(); + transport.next("offline"); + + assert.deepEqual(statuses, ["connecting", "online"]); + assert.equal(bridge.current, "online"); + }); + + it("implements the slot source contract: subscribe replay + getCurrent snapshot", () => { + const bridge = new CloudConnectionStatusBridge(); + const transport = new BehaviorSubject<"connecting" | "online" | "offline" | "reconnecting">( + "online", + ); + bridge.attach({ connectionStatus$: transport.asObservable() }); + + assert.equal(bridge.getCurrent(), "online"); + const statuses: string[] = []; + const subscription = bridge.subscribe((status) => statuses.push(status)); + assert.deepEqual(statuses, ["online"]); // replayed for late subscribers + + transport.next("reconnecting"); + assert.deepEqual(statuses, ["online", "reconnecting"]); + + subscription.unsubscribe(); + transport.next("online"); + assert.deepEqual(statuses, ["online", "reconnecting"]); // severed + }); + it("reflects the transport's own reconnect loop without a switch", async () => { const fake = installFakeWebSocket(); try { diff --git a/apps/notebook-cloud/test/viewer-render-props.test.ts b/apps/notebook-cloud/test/viewer-render-props.test.ts index 109c6fca2..ec5adf5d9 100644 --- a/apps/notebook-cloud/test/viewer-render-props.test.ts +++ b/apps/notebook-cloud/test/viewer-render-props.test.ts @@ -2,7 +2,12 @@ import assert from "node:assert/strict"; import { readFileSync } from "node:fs"; import { test } from "node:test"; import * as ts from "typescript"; -import { viewerCorpus, viewerFunctionSource, viewerModuleTexts } from "./viewer-source-corpus"; +import { + viewerCorpus, + viewerFileContaining, + viewerFunctionSource, + viewerModuleTexts, +} from "./viewer-source-corpus"; test("cloud notebook rendering uses shared cell chrome instead of report-mode cells", () => { const offenders: string[] = []; @@ -154,12 +159,37 @@ test("cloud viewer routes notebook header controls through the shared shell chro assert.match(sourceText, /authControls=\{[\s\S]* \{[\s\S]{0,400}?connectionStatusBridge\.attach\(transport\);/, + ); + assert.match( + sessionSourceText, + /const scheduleReconnect = \(reason: Error\) => \{[\s\S]{0,600}?connectionStatusBridge\.noteTeardownRetry\(\);[\s\S]{0,800}?disposeCurrentRuntime\(\);/, + ); + assert.match( + sessionSourceText, + /connectionStatusBridge\.noteTeardownRetry\(\);[\s\S]{0,400}?pendingSeedDiscardRef\.current = discardPersistedSeedAfterTeardown\(/, + ); + assert.match( + sessionSourceText, + /connectionStatusBridge\.noteTeardownRetry\(\);[\s\S]{0,600}?const teardownFlush = disposeCurrentRuntime\(\);/, + ); assert.match(sourceText, /useState\(initialCloudRailCollapsed\)/); assert.match(sourceText, /function initialCloudRailCollapsed/); assert.match(sourceText, /function initialCloudRailCollapsed\(\): boolean \{[\s\S]*return true;/); diff --git a/apps/notebook-cloud/viewer/cloud-viewer-session.ts b/apps/notebook-cloud/viewer/cloud-viewer-session.ts index fc56cecfd..ea6f5a56d 100644 --- a/apps/notebook-cloud/viewer/cloud-viewer-session.ts +++ b/apps/notebook-cloud/viewer/cloud-viewer-session.ts @@ -1,5 +1,4 @@ import { useCallback, useEffect, useRef, useState, type MutableRefObject } from "react"; -import type { Observable } from "rxjs"; import { IndexedDbStorageAdapter, NotebookDocPersistence, @@ -7,7 +6,6 @@ import { loadPersistedNotebookDoc, type BlobResolver, type CommChanges, - type ConnectionStatus, } from "runtimed"; import { applyWidgetCommBroadcastToStore, @@ -102,10 +100,11 @@ export interface CloudViewerSession { connectionScope: string | null; /** * Stable connection lifecycle across transport replacements (initial - * connect attempts and escalation teardowns) — fed by the session's - * CloudConnectionStatusBridge, the slot's connectivity-dot source. + * connect attempts and escalation teardowns) — the session's + * CloudConnectionStatusBridge, the slot's connectivity-dot source + * (subscribe + getCurrent for first paint). */ - connectionStatus$: Observable; + connectionStatus$: CloudConnectionStatusBridge; liveMaterializedRef: MutableRefObject; liveRuntimeRef: MutableRefObject; notebookLanguageRef: MutableRefObject; @@ -943,9 +942,13 @@ export function useCloudViewerSession({ window.removeEventListener("pagehide", flushPersistence); document.removeEventListener("visibilitychange", flushPersistenceWhenHidden); materializeLiveRuntimeRef.current = null; - // Stop following before the teardown disconnects emit "offline"; a - // re-running effect re-attaches its replacement transport. - connectionStatusBridge.detach(); + // Detach BEFORE the teardown disconnects emit their terminal + // "offline", and report "reconnecting": a re-running effect usually + // re-attaches a replacement transport, but the auth-refresh re-run + // early-returns without attaching — the bridge must read as a + // transition, not as stale "online", for that window. Harmless at + // real unmount (no subscribers remain). + connectionStatusBridge.noteTeardownRetry(); const teardownFlush = disposeCurrentRuntime(); // An in-flight connect (runtime not yet resolved) owns a transport // whose retry loop would otherwise run forever after unmount. @@ -1010,7 +1013,7 @@ export function useCloudViewerSession({ return { connectionActorLabel, connectionError, - connectionStatus$: connectionStatusBridge.status$, + connectionStatus$: connectionStatusBridge, connectionPeerId, connectionScope, liveMaterializedRef, diff --git a/apps/notebook-cloud/viewer/live-sync.ts b/apps/notebook-cloud/viewer/live-sync.ts index fd670e9e4..54688605f 100644 --- a/apps/notebook-cloud/viewer/live-sync.ts +++ b/apps/notebook-cloud/viewer/live-sync.ts @@ -593,6 +593,22 @@ export class CloudConnectionStatusBridge { return this._status$.getValue(); } + /** Synchronous snapshot (NotebookConnectionStatusSource contract). */ + getCurrent(): ConnectionStatus { + return this._status$.getValue(); + } + + /** + * Subscribe with a plain callback (NotebookConnectionStatusSource + * contract) — the bridge is handed to the connection/identity slot + * directly, so first paint can read getCurrent() instead of flashing + * "connecting". + */ + subscribe(next: (status: ConnectionStatus) => void): { unsubscribe(): void } { + const subscription = this._status$.subscribe(next); + return { unsubscribe: () => subscription.unsubscribe() }; + } + /** Follow a (replacement) transport's connection status. */ attach(transport: Pick): void { this.subscription?.unsubscribe(); From c6c81f3b5fc69485ef7fc7c6e6c58245afe57fc9 Mon Sep 17 00:00:00 2001 From: Quill Agent <261289082+quillaid@users.noreply.github.com> Date: Thu, 11 Jun 2026 09:59:46 +0000 Subject: [PATCH 13/16] docs(adr): correct the PR-3 desktop source and record review fixes - Desktop is fed by the daemon-lifecycle source, not the IPC transport (whose status is constant in practice); copy scoped via connectionLabel; daemon<->room link health recorded as future work. - getCurrent first-paint snapshot, change-only live-region announcements, aria-hidden avatar. - Bridge cleanup semantics: teardown paths AND the effect cleanup report 'reconnecting' so the auth-refresh re-run gap never reads as stale 'online'. - Tests paragraph updated to the landed coverage. --- docs/adr/local-first-notebook-state.md | 64 ++++++++++++++++++-------- 1 file changed, 46 insertions(+), 18 deletions(-) diff --git a/docs/adr/local-first-notebook-state.md b/docs/adr/local-first-notebook-state.md index 4a9187bb4..dc35bea61 100644 --- a/docs/adr/local-first-notebook-state.md +++ b/docs/adr/local-first-notebook-state.md @@ -393,9 +393,17 @@ were empty: - **Cloud:** the `identityControls={null}` slot in `notebook-viewer.tsx` (right-most header slot), now filled. - **Desktop:** `trailingControls` on `` in `App.tsx` → - `identityControls` at the right end of the command toolbar, fed by the - host transport's `connectionStatus$` (stable for the app's lifetime; - TauriTransport reports online/offline today, which is honest). + `identityControls` at the right end of the command toolbar, fed by a + daemon-lifecycle source (`createDesktopConnectionStatusSource`: + `daemon:ready` → online, `daemon:disconnected` → reconnecting — the host + auto-reconnects — `daemon:unavailable` → offline; the host facade's + ready-cache backfill covers late mounts). The Tauri IPC transport's own + `connectionStatus$` is deliberately NOT used: it is honest about IPC but + constant in practice (the app never disconnects it), so a dot fed from + it could never transition through a daemon restart. The desktop copy is + scoped to the measured link via `connectionLabel="Daemon connection"` — + daemon↔room link health for runtime-peer contexts is **future work**; + until it exists the dot must say which hop it reports. **Conditionality** (the reason #3290 pulled the previous attempt): the slot renders **nothing** for a purely local desktop session. The predicate is @@ -404,16 +412,22 @@ centralized in the component (`isRemoteNotebookContext`): Cloud is always remote, so hosts mount unconditionally. **Content:** self-identity (the flattened avatar treatment via the shared -actor projection — initials/avatar only, no visible label) paired with a +actor projection — initials/avatar only, no visible label, hidden from the +a11y tree so the sr-only copy is the single accessible text) paired with a connectivity dot driven by `connectionStatus$` — its first UI consumer. The component accepts a structural `NotebookConnectionStatusSource` (an -rxjs-free `Observable` subset) so shared `src/` takes no new dependency. -Status vocabulary follows the existing tones: emerald `online`, amber pulse -`reconnecting`/`connecting`, muted `offline`; non-online states also dim -the wrapper (opacity, not copy). This is the surface that makes the PR-2 +rxjs-free `Observable` subset with an optional `getCurrent()` snapshot so +first paint shows the real status, no one-frame "connecting" flash) so +shared `src/` takes no new dependency. Status vocabulary follows the +existing tones: emerald `online`, amber pulse `reconnecting`/`connecting`, +muted `offline`; non-online states also dim the wrapper (opacity, not +copy). Status CHANGES (never the initial value) are announced through a +polite sr-only live region using the scoped link copy — quiet for the eyes +is not silence for screen readers. This is the surface that makes the PR-2 limitation interpretable: runtime-state stores are not blanked during the offline window, and the pulsing dot stays live through `reconnecting` so -frozen kernel/execution chrome reads as "reconnecting", not as truth. +frozen kernel/execution chrome reads as "reconnecting", not as truth — for +the link each host actually measures. **Aesthetic rules distilled from the three pulled designs** (#3273, #3290, #3337, #3349 — recorded so we do not relitigate them): @@ -436,19 +450,33 @@ survives reconnects, but initial-connect attempts and escalation teardowns still replace it; the session exposes `CloudConnectionStatusBridge` (`live-sync.ts`) — a stable BehaviorSubject-backed source attached to each replacement transport via `onTransportCreated`, deduplicating across -switches. On escalation teardown the bridge detaches BEFORE the dispose and -reports `"reconnecting"`, so the disposed transport's terminal `"offline"` -(manual-disconnect vocabulary) never surfaces while the session is retrying. +switches. On escalation teardown — and in the effect cleanup, where the +auth-refresh re-run early-returns without attaching a replacement — the +bridge detaches BEFORE the dispose and reports `"reconnecting"`, so the +disposed transport's terminal `"offline"` (manual-disconnect vocabulary) +never surfaces and the gap reads as a transition, never as stale +`"online"`. The bridge implements the slot's source contract directly +(`subscribe` + `getCurrent`). Tests: component tests for every status × identity combination (data-state + dot tone + opacity), the local-session-renders-nothing gate, the runtime-peer remote case, the live-dot-through-reconnecting regression, -the flat-never-raised treatment, and icon-only-at-every-width (no visible -text outside the avatar — collapse-proof by construction); bridge tests for -transport replacement, teardown-retry masking of the disposed transport's -offline, and the PR-2 loop reflected without a switch; #3337's quiet-chrome -regressions stay green (the `identityControls={null}` pin became a pin on -the new mount). +scoped-link copy, `getCurrent` first paint, live-region announcements +(changes only, never the mount), aria-hidden avatar, unmount-unsubscribe, +the flat-never-raised treatment across the whole subtree, and +icon-only-at-every-width via clone-and-strip (wrapper text nodes included); +desktop tests for the daemon-lifecycle source (lifecycle walk, dedup, +replay/getCurrent, dispose), the real-projection composition with +`isRemoteNotebookContext`, the toolbar trailingControls flow with real +`desktopNotebookShellCapabilities` output, and a source-text pin on the +App.tsx mount (daemon source + scoped label, not the IPC transport); bridge +tests for transport replacement, plain-detach silence, the slot source +contract, teardown-retry masking of the disposed transport's offline, and +the PR-2 loop reflected without a switch; session wiring order +(attach-on-transport-created, retry-before-dispose in all three teardown +paths) pinned by source guardrails; #3337's quiet-chrome regressions stay +green (the `identityControls={null}` pin became a module-scoped pin on the +new mount). --- From 2fa13e4282ef3b94174d234470755e792b4d9035 Mon Sep 17 00:00:00 2001 From: Quill Agent <261289082+quillaid@users.noreply.github.com> Date: Thu, 11 Jun 2026 10:53:53 +0000 Subject: [PATCH 14/16] feat(notebook-cloud): debounced sustained-reconnecting line in the notices stack The connection/identity slot stays an 8px dot by design, so a sustained outage was only legible to someone watching the far right of the header. Surface ONE quiet notices-stack line ("Reconnecting." / "Your edits are kept locally and will sync when the connection returns.") once the live room status has stayed "reconnecting" for 3s, and clear it on online. The debounce lives in SustainedReconnectingTracker: reconnecting arms one timer, sub-debounce flaps cancel it silently, repeated reconnecting deliveries never re-arm or duplicate, and a replacement transport's pre-handshake "connecting" neither arms nor clears. Transport-level connectionError strings (socket drops, handshake timeouts, missed liveness pongs, browser offline) now route through this debounced line instead of the immediate per-drop "Live room needs attention." warning; access and sign-in diagnostics keep their dedicated notices. The chip and dot visuals are untouched. --- .../test/sustained-reconnecting.test.ts | 233 ++++++++++++++++++ .../notebook-cloud/viewer/notebook-viewer.tsx | 7 + apps/notebook-cloud/viewer/notices.tsx | 38 ++- .../viewer/use-sustained-reconnecting.ts | 95 +++++++ 4 files changed, 372 insertions(+), 1 deletion(-) create mode 100644 apps/notebook-cloud/test/sustained-reconnecting.test.ts create mode 100644 apps/notebook-cloud/viewer/use-sustained-reconnecting.ts diff --git a/apps/notebook-cloud/test/sustained-reconnecting.test.ts b/apps/notebook-cloud/test/sustained-reconnecting.test.ts new file mode 100644 index 000000000..9bc5bc049 --- /dev/null +++ b/apps/notebook-cloud/test/sustained-reconnecting.test.ts @@ -0,0 +1,233 @@ +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { describe, it, test } from "node:test"; +import * as React from "react"; +import { renderToStaticMarkup } from "react-dom/server"; +import type { CloudPrototypeAuthState } from "../viewer/collaborator-auth"; +import { CLOUD_CONNECTION_NO_ACCESS_DIAGNOSTIC } from "../viewer/connection-diagnostics"; +import { + CloudNotebookNotices, + cloudNotebookHasNotices, + isTransportReconnectError, +} from "../viewer/notices"; +import { SustainedReconnectingTracker } from "../viewer/use-sustained-reconnecting"; + +globalThis.React = React; + +// Field report: a moderate offline window produced zero UI change. The slot +// dot is 8px by design, so legibility comes from ONE debounced notices-stack +// line — present only while "reconnecting" outlives the debounce, cleared the +// moment the room is back, and silent across sub-debounce flaps. +describe("sustained reconnecting tracker", () => { + function tracked() { + const changes: boolean[] = []; + const tracker = new SustainedReconnectingTracker({ + debounceMs: 3_000, + onChange: (sustained) => changes.push(sustained), + }); + return { changes, tracker }; + } + + it("flips true only after reconnecting persists past the debounce", (t) => { + t.mock.timers.enable({ apis: ["setTimeout"] }); + const { changes, tracker } = tracked(); + + tracker.next("reconnecting"); + t.mock.timers.tick(2_999); + assert.deepEqual(changes, [] as boolean[], "no line inside the debounce window"); + t.mock.timers.tick(1); + assert.deepEqual(changes, [true]); + tracker.dispose(); + }); + + it("clears on online", (t) => { + t.mock.timers.enable({ apis: ["setTimeout"] }); + const { changes, tracker } = tracked(); + + tracker.next("reconnecting"); + t.mock.timers.tick(3_000); + tracker.next("online"); + assert.deepEqual(changes, [true, false]); + tracker.dispose(); + }); + + it("stays silent across sub-debounce flaps", (t) => { + t.mock.timers.enable({ apis: ["setTimeout"] }); + const { changes, tracker } = tracked(); + + for (let i = 0; i < 5; i += 1) { + tracker.next("reconnecting"); + t.mock.timers.tick(1_000); + tracker.next("online"); + t.mock.timers.tick(1_000); + } + // The recovered windows must not accumulate into a phantom line. + t.mock.timers.tick(60_000); + assert.deepEqual(changes, [] as boolean[]); + tracker.dispose(); + }); + + it("fires once per outage no matter how many reconnecting deliveries arrive", (t) => { + t.mock.timers.enable({ apis: ["setTimeout"] }); + const { changes, tracker } = tracked(); + + tracker.next("reconnecting"); + t.mock.timers.tick(1_000); + tracker.next("reconnecting"); // re-delivery while armed: no re-arm + t.mock.timers.tick(2_000); + assert.deepEqual(changes, [true], "the ORIGINAL deadline holds"); + tracker.next("reconnecting"); // while sustained: no-op + t.mock.timers.tick(10_000); + assert.deepEqual(changes, [true]); + tracker.dispose(); + }); + + it("treats a replacement transport's connecting as neutral", (t) => { + t.mock.timers.enable({ apis: ["setTimeout"] }); + const { changes, tracker } = tracked(); + + // connecting never arms: initial connect is not an outage. + tracker.next("connecting"); + t.mock.timers.tick(60_000); + assert.deepEqual(changes, [] as boolean[]); + + // And it never clears: a session-level retry creates a fresh transport + // that reports "connecting" before its first handshake — the room is + // still down, so the line stays until online. + tracker.next("reconnecting"); + t.mock.timers.tick(3_000); + tracker.next("connecting"); + assert.deepEqual(changes, [true]); + tracker.next("online"); + assert.deepEqual(changes, [true, false]); + tracker.dispose(); + }); + + it("clears on terminal offline and cancels on dispose", (t) => { + t.mock.timers.enable({ apis: ["setTimeout"] }); + const { changes, tracker } = tracked(); + + tracker.next("reconnecting"); + t.mock.timers.tick(3_000); + tracker.next("offline"); // manual disconnect: 'Reconnecting' would lie + assert.deepEqual(changes, [true, false]); + + tracker.next("reconnecting"); + tracker.dispose(); // unmount mid-debounce + t.mock.timers.tick(60_000); + assert.deepEqual(changes, [true, false]); + }); +}); + +describe("sustained reconnecting notice", () => { + const anonymousAuth: CloudPrototypeAuthState = { + mode: "anonymous", + token: null, + user: null, + oidcClaims: null, + requestedScope: "viewer", + problem: null, + }; + + function renderNotices( + overrides: Partial>, + ): string { + return renderToStaticMarkup( + React.createElement(CloudNotebookNotices, { + authState: anonymousAuth, + authRenewal: { kind: "idle", message: null }, + connectionError: null, + hasReadableSnapshot: true, + status: { kind: "ready", message: "Ready" }, + onResetAuth: () => {}, + ...overrides, + }), + ); + } + + it("classifies transport-loss reasons, not diagnostics", () => { + assert.equal(isTransportReconnectError("browser reported offline"), true); + assert.equal(isTransportReconnectError("cloud sync socket closed (1006)"), true); + assert.equal(isTransportReconnectError("cloud sync socket failed"), true); + assert.equal( + isTransportReconnectError("cloud sync liveness pong missed (no reply within 10000ms)"), + true, + ); + assert.equal( + isTransportReconnectError("cloud room handshake did not complete within 30000ms"), + true, + ); + assert.equal(isTransportReconnectError(CLOUD_CONNECTION_NO_ACCESS_DIAGNOSTIC), false); + assert.equal(isTransportReconnectError("websocket failed"), false); + }); + + it("shows the single quiet line while reconnecting is sustained", () => { + const html = renderNotices({ + sustainedReconnecting: true, + connectionError: "cloud sync socket closed (1006)", + }); + assert.match(html, /Reconnecting\./); + assert.match(html, /Your edits are kept locally and will sync when the connection returns\./); + assert.doesNotMatch(html, /Live room needs attention/); + assert.doesNotMatch(html, /cloud sync socket closed/); + assert.equal( + (html.match(/data-slot="notebook-notice"/g) ?? []).length, + 1, + "one line, not a reconnect line plus a per-drop warning", + ); + }); + + it("surfaces nothing for a transport drop inside the debounce window", () => { + for (const error of [ + "cloud sync socket closed (1006)", + "browser reported offline", + "cloud room handshake did not complete within 30000ms", + ]) { + assert.equal( + cloudNotebookHasNotices({ + authState: anonymousAuth, + authRenewal: { kind: "idle", message: null }, + connectionError: error, + hasReadableSnapshot: true, + status: { kind: "ready", message: "Ready" }, + }), + false, + `${error} must wait for the sustained flag`, + ); + assert.equal(renderNotices({ connectionError: error }), ""); + } + }); + + it("clears the line when sustained reconnecting ends", () => { + assert.equal(renderNotices({ sustainedReconnecting: false }), ""); + assert.equal( + cloudNotebookHasNotices({ + authState: anonymousAuth, + authRenewal: { kind: "idle", message: null }, + connectionError: null, + hasReadableSnapshot: true, + status: { kind: "ready", message: "Ready" }, + sustainedReconnecting: false, + }), + false, + ); + }); + + it("keeps access diagnostics as their own immediate notice", () => { + const html = renderNotices({ connectionError: CLOUD_CONNECTION_NO_ACCESS_DIAGNOSTIC }); + assert.match(html, /Notebook access needed/); + }); +}); + +test("notebook viewer wires the debounced status line into the notices stack", () => { + const viewerSource = readFileSync( + new URL("../viewer/notebook-viewer.tsx", import.meta.url), + "utf8", + ); + assert.match( + viewerSource, + /const sustainedReconnecting = useSustainedReconnecting\(connectionStatus\$\);/, + ); + assert.match(viewerSource, /sustainedReconnecting=\{sustainedReconnecting\}/); + assert.match(viewerSource, /sustainedReconnecting,\n/); +}); diff --git a/apps/notebook-cloud/viewer/notebook-viewer.tsx b/apps/notebook-cloud/viewer/notebook-viewer.tsx index fd531a4bf..97a2aa3cf 100644 --- a/apps/notebook-cloud/viewer/notebook-viewer.tsx +++ b/apps/notebook-cloud/viewer/notebook-viewer.tsx @@ -65,6 +65,7 @@ import { markCloudViewerLoadMilestone } from "./load-milestones"; import { cloudPresenceHasRuntimePeer, cloudPresenceRuntimePeerCount } from "./presence"; import type { ResolvedCell } from "./render-resolution"; import { CloudNotebookNotices, cloudNotebookHasNotices } from "./notices"; +import { useSustainedReconnecting } from "./use-sustained-reconnecting"; import type { ViewerStatus } from "./notice-types"; import type { CloudNotebookAccessRequest } from "./sharing-client"; import { CloudSharingControls } from "./sharing-controls"; @@ -223,6 +224,10 @@ export function NotebookViewer({ }), [blobResolver, liveRuntimeRef], ); + // Sustained-outage legibility: the connection/identity slot stays an 8px + // dot by design, so once "reconnecting" outlives the debounce the notices + // stack carries the one calm line (and clears it when the room is back). + const sustainedReconnecting = useSustainedReconnecting(connectionStatus$); const presenceSnapshot = useSyncExternalStore( presenceStore.subscribe, presenceStore.getSnapshot, @@ -898,6 +903,7 @@ export function NotebookViewer({ diagnostics: accessRequestNotice, hasAppSession: Boolean(appSessionStatus.session), hasReadableSnapshot: notebookHasReadableSnapshot, + sustainedReconnecting, status: noticeStatus, }); const notices = hasNotices ? ( @@ -908,6 +914,7 @@ export function NotebookViewer({ diagnostics={accessRequestNotice} hasAppSession={Boolean(appSessionStatus.session)} hasReadableSnapshot={notebookHasReadableSnapshot} + sustainedReconnecting={sustainedReconnecting} status={noticeStatus} onResetAuth={resetPrototypeAuth} onSignInAgain={authConfig.oidc ? beginNotebookOidcAuth : undefined} diff --git a/apps/notebook-cloud/viewer/notices.tsx b/apps/notebook-cloud/viewer/notices.tsx index 205b154ec..55a19c8c3 100644 --- a/apps/notebook-cloud/viewer/notices.tsx +++ b/apps/notebook-cloud/viewer/notices.tsx @@ -20,18 +20,38 @@ export interface CloudNotebookNoticesProps { connectionError: string | null; hasAppSession?: boolean; hasReadableSnapshot?: boolean; + /** + * The live-room status has been "reconnecting" past the debounce window + * (`useSustainedReconnecting`). Renders the single quiet reconnect line; + * transport-loss `connectionError` strings are routed here instead of + * the per-drop connection notice, so brief blips surface nothing and a + * real outage surfaces one calm sentence. + */ + sustainedReconnecting?: boolean; status: ViewerStatus; diagnostics?: ReactNode; onResetAuth: () => void; onSignInAgain?: () => void | Promise; } +/** + * Transport-level connection-loss reasons (socket drops, handshake + * timeouts, missed liveness pongs, the browser offline event). The retry + * loop owns recovery, so these surface through the debounced sustained- + * reconnecting line rather than an immediate per-drop warning. Access and + * sign-in diagnostics keep their dedicated notices. + */ +export function isTransportReconnectError(error: string): boolean { + return error === "browser reported offline" || /^cloud (sync|room) /.test(error); +} + export function cloudNotebookHasNotices({ authState, authRenewal, connectionError, hasAppSession = false, hasReadableSnapshot = false, + sustainedReconnecting = false, status, diagnostics, }: Omit): boolean { @@ -50,6 +70,7 @@ export function cloudNotebookHasNotices({ return ( shouldShowAuthNotice || shouldShowAuthRenewalNotice || + sustainedReconnecting || Boolean(connectionNotice) || Boolean(diagnostics) || shouldShowStatusNotice @@ -62,6 +83,7 @@ export function CloudNotebookNotices({ connectionError, hasAppSession = false, hasReadableSnapshot = false, + sustainedReconnecting = false, status, diagnostics, onResetAuth, @@ -74,6 +96,7 @@ export function CloudNotebookNotices({ connectionError, hasAppSession, hasReadableSnapshot, + sustainedReconnecting, status, diagnostics, }) @@ -126,6 +149,12 @@ export function CloudNotebookNotices({ ) : null} + {sustainedReconnecting ? ( + } title="Reconnecting."> + Your edits are kept locally and will sync when the connection returns. + + ) : null} + {connectionNotice ? ( void): { unsubscribe(): void }; +} + +/** + * Debounce "reconnecting" into a single sustained flag: + * + * - `reconnecting` arms one timer; the flag flips true only if the status + * is STILL reconnecting when the debounce elapses. Repeated reconnecting + * deliveries while armed (or already sustained) are no-ops, so flapping + * connections cannot spam the notices stack. + * - `online` (and the terminal manual-disconnect `offline`) cancels the + * pending timer and clears the flag. + * - `connecting` is neutral: a replacement transport reports "connecting" + * before its first handshake, and that is neither recovery (the line + * must not clear before the room is back) nor a fresh loss. + */ +export class SustainedReconnectingTracker { + private timer: ReturnType | null = null; + private sustained = false; + + constructor( + private readonly options: { + debounceMs: number; + onChange: (sustained: boolean) => void; + }, + ) {} + + next(status: ConnectionStatus): void { + if (status === "reconnecting") { + if (this.sustained || this.timer !== null) return; + this.timer = setTimeout(() => { + this.timer = null; + this.sustained = true; + this.options.onChange(true); + }, this.options.debounceMs); + return; + } + if (status === "online" || status === "offline") { + this.clearTimer(); + if (this.sustained) { + this.sustained = false; + this.options.onChange(false); + } + } + // "connecting" falls through: neither arms nor clears. + } + + dispose(): void { + this.clearTimer(); + } + + private clearTimer(): void { + if (this.timer !== null) { + clearTimeout(this.timer); + this.timer = null; + } + } +} + +/** + * True while the live-room connection has been "reconnecting" for at + * least `debounceMs`. Drives the quiet notices-stack line — the + * connection/identity slot stays an 8px dot by design, so a sustained + * outage needs one legible sentence somewhere calm. + */ +export function useSustainedReconnecting( + source: ReconnectingStatusSource, + debounceMs: number = SUSTAINED_RECONNECTING_DEBOUNCE_MS, +): boolean { + const [sustained, setSustained] = useState(false); + + useEffect(() => { + const tracker = new SustainedReconnectingTracker({ debounceMs, onChange: setSustained }); + const subscription = source.subscribe((status) => tracker.next(status)); + return () => { + subscription.unsubscribe(); + tracker.dispose(); + setSustained(false); + }; + }, [source, debounceMs]); + + return sustained; +} From 9ff946788821fee702b9832d98be282c8e038c9b Mon Sep 17 00:00:00 2001 From: Quill Agent <261289082+quillaid@users.noreply.github.com> Date: Thu, 11 Jun 2026 11:45:34 +0000 Subject: [PATCH 15/16] fix(notebook-cloud): classify only exact link-loss shapes as reconnect noise The broad prefix match (/^cloud (sync|room) /) swallowed every transport-wrapped failure into the debounced calm "Reconnecting." line - including genuinely terminal ones. A mid-session auth/access revocation surfaces as wrapped connect-target or socket failures, and the actionable diagnostics path only runs pre-ready, so an affected user saw an infinite quiet reconnect with no sign-in CTA. The classifier now matches the transport's own link-loss reasons shape by shape, anchored (socket close/failure, browser offline, handshake timeout, liveness ping/pong misses). Connect-target resolution failures, socket creation failures, protocol decode failures, and the session escalation's "cloud room rejected frame: ..." keep the existing "Live room needs attention." warning with its action. Tests pin both routes, including the rendered notice for non-link failures. --- .../test/sustained-reconnecting.test.ts | 53 ++++++++++++++++++- apps/notebook-cloud/viewer/notices.tsx | 31 ++++++++--- 2 files changed, 77 insertions(+), 7 deletions(-) diff --git a/apps/notebook-cloud/test/sustained-reconnecting.test.ts b/apps/notebook-cloud/test/sustained-reconnecting.test.ts index 9bc5bc049..113064cea 100644 --- a/apps/notebook-cloud/test/sustained-reconnecting.test.ts +++ b/apps/notebook-cloud/test/sustained-reconnecting.test.ts @@ -145,10 +145,23 @@ describe("sustained reconnecting notice", () => { ); } - it("classifies transport-loss reasons, not diagnostics", () => { + it("classifies exactly the transport's own link-loss shapes", () => { + // The calm-reconnect funnel: link-level losses the retry loop owns. assert.equal(isTransportReconnectError("browser reported offline"), true); assert.equal(isTransportReconnectError("cloud sync socket closed (1006)"), true); + assert.equal( + isTransportReconnectError("cloud sync socket closed (1008): too many rejected frames"), + true, + ); assert.equal(isTransportReconnectError("cloud sync socket failed"), true); + assert.equal( + isTransportReconnectError("cloud sync socket send failed: socket is closing"), + true, + ); + assert.equal( + isTransportReconnectError("cloud sync liveness ping failed: synthetic send failure"), + true, + ); assert.equal( isTransportReconnectError("cloud sync liveness pong missed (no reply within 10000ms)"), true, @@ -157,10 +170,48 @@ describe("sustained reconnecting notice", () => { isTransportReconnectError("cloud room handshake did not complete within 30000ms"), true, ); + }); + + it("keeps terminal-looking failures on the actionable diagnostic route", () => { + // The transport wraps NON-link failures in similar prefixes; a broad + // prefix match routed mid-session terminal auth/access failures into + // the perpetual calm "Reconnecting." line with no CTA. These must keep + // the warning notice. + assert.equal( + isTransportReconnectError("cloud sync connect target failed: Unable to read app session"), + false, + ); + assert.equal( + isTransportReconnectError("cloud sync socket creation failed: invalid URL"), + false, + ); + assert.equal( + isTransportReconnectError("cloud sync socket message failed: unexpected token"), + false, + ); + assert.equal( + isTransportReconnectError("cloud room rejected frame: notebook sync rejected"), + false, + ); assert.equal(isTransportReconnectError(CLOUD_CONNECTION_NO_ACCESS_DIAGNOSTIC), false); assert.equal(isTransportReconnectError("websocket failed"), false); }); + it("renders the actionable warning for non-link transport failures", () => { + const html = renderNotices({ + connectionError: "cloud sync connect target failed: Unable to read app session", + }); + assert.match(html, /Live room needs attention/); + assert.match(html, /cloud sync connect target failed/); + assert.match(html, /Use anonymous/, "the warning keeps its action"); + assert.doesNotMatch(html, /Your edits are kept locally/); + + const rejectionHtml = renderNotices({ + connectionError: "cloud room rejected frame: notebook sync rejected", + }); + assert.match(rejectionHtml, /Live room needs attention/); + }); + it("shows the single quiet line while reconnecting is sustained", () => { const html = renderNotices({ sustainedReconnecting: true, diff --git a/apps/notebook-cloud/viewer/notices.tsx b/apps/notebook-cloud/viewer/notices.tsx index 55a19c8c3..b65b0cd31 100644 --- a/apps/notebook-cloud/viewer/notices.tsx +++ b/apps/notebook-cloud/viewer/notices.tsx @@ -35,14 +35,33 @@ export interface CloudNotebookNoticesProps { } /** - * Transport-level connection-loss reasons (socket drops, handshake - * timeouts, missed liveness pongs, the browser offline event). The retry - * loop owns recovery, so these surface through the debounced sustained- - * reconnecting line rather than an immediate per-drop warning. Access and - * sign-in diagnostics keep their dedicated notices. + * EXACT connection-loss shapes minted by CloudWebSocketTransport when the + * LINK itself drops (socket close/failure, browser offline, liveness + * probe misses, handshake that never completed). The retry loop owns + * recovery for these, so they surface through the debounced sustained- + * reconnecting line rather than an immediate per-drop warning. + * + * Anchored shape-by-shape on purpose: the transport wraps OTHER failures + * in similar prefixes — connect-target resolution ("cloud sync connect + * target failed: ..."), socket creation, protocol decode, and the session + * escalation's "cloud room rejected frame: ..." — and those carry + * actionable detail that must keep the warning notice and its action. + * A terminal auth/access failure routed into the perpetual calm + * "Reconnecting." line would loop forever with no CTA. Keep this list in + * lockstep with the connectionLost reasons in live-sync.ts. */ +const TRANSPORT_RECONNECT_ERROR_SHAPES: readonly RegExp[] = [ + /^browser reported offline$/, + /^cloud sync socket failed$/, + /^cloud sync socket closed \(\d+\)/, + /^cloud sync socket send failed: /, + /^cloud room handshake did not complete within \d+ms$/, + /^cloud sync liveness ping failed: /, + /^cloud sync liveness pong missed \(no reply within \d+ms\)$/, +]; + export function isTransportReconnectError(error: string): boolean { - return error === "browser reported offline" || /^cloud (sync|room) /.test(error); + return TRANSPORT_RECONNECT_ERROR_SHAPES.some((shape) => shape.test(error)); } export function cloudNotebookHasNotices({ From 65577bca84a6bf0bc23dc2ad17556430e3187ae8 Mon Sep 17 00:00:00 2001 From: Quill Agent <261289082+quillaid@users.noreply.github.com> Date: Thu, 11 Jun 2026 11:46:40 +0000 Subject: [PATCH 16/16] test(notebook-cloud): render useSustainedReconnecting through a real hook harness The tracker had thorough unit coverage but the hook itself was never executed, leaving its React wiring unpinned - dropping tracker.dispose() from the effect cleanup passed every suite. Render the real hook against a BehaviorSubject-shaped fake source: debounce boundary drives the returned flag, sub-debounce flaps stay silent, unmount unsubscribes and disposes the pending timer (timer count reaches zero), and a source identity change mid-outage pins the intended clear-then-re-arm-via-replay behavior. --- .../use-sustained-reconnecting.test.tsx | 132 ++++++++++++++++++ 1 file changed, 132 insertions(+) create mode 100644 apps/notebook-cloud/viewer/__tests__/use-sustained-reconnecting.test.tsx diff --git a/apps/notebook-cloud/viewer/__tests__/use-sustained-reconnecting.test.tsx b/apps/notebook-cloud/viewer/__tests__/use-sustained-reconnecting.test.tsx new file mode 100644 index 000000000..6db668621 --- /dev/null +++ b/apps/notebook-cloud/viewer/__tests__/use-sustained-reconnecting.test.tsx @@ -0,0 +1,132 @@ +import { act, renderHook } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; +import type { ConnectionStatus } from "runtimed"; +import { + useSustainedReconnecting, + type ReconnectingStatusSource, +} from "../use-sustained-reconnecting"; + +// The tracker has thorough unit coverage; these tests render the REAL hook +// so the React wiring is pinned too: subscription on mount, debounce timers +// driving state, unsubscribe + timer disposal on unmount, and the intended +// behavior when the source object identity changes mid-outage. + +class FakeStatusSource implements ReconnectingStatusSource { + listeners = new Set<(status: ConnectionStatus) => void>(); + current: ConnectionStatus = "connecting"; + + subscribe(next: (status: ConnectionStatus) => void): { unsubscribe(): void } { + this.listeners.add(next); + // BehaviorSubject contract (CloudConnectionStatusBridge): replay the + // current status to new subscribers. + next(this.current); + return { unsubscribe: () => this.listeners.delete(next) }; + } + + emit(status: ConnectionStatus): void { + this.current = status; + for (const listener of this.listeners) { + listener(status); + } + } +} + +describe("useSustainedReconnecting", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it("flips true only after reconnecting outlives the debounce, and clears on online", () => { + const source = new FakeStatusSource(); + const { result } = renderHook(() => useSustainedReconnecting(source, 3_000)); + expect(result.current).toBe(false); + + act(() => { + source.emit("reconnecting"); + vi.advanceTimersByTime(2_999); + }); + expect(result.current).toBe(false); + + act(() => { + vi.advanceTimersByTime(1); + }); + expect(result.current).toBe(true); + + act(() => { + source.emit("online"); + }); + expect(result.current).toBe(false); + }); + + it("stays silent across sub-debounce flaps", () => { + const source = new FakeStatusSource(); + const { result } = renderHook(() => useSustainedReconnecting(source, 3_000)); + + act(() => { + for (let i = 0; i < 5; i += 1) { + source.emit("reconnecting"); + vi.advanceTimersByTime(1_000); + source.emit("online"); + vi.advanceTimersByTime(1_000); + } + vi.advanceTimersByTime(60_000); + }); + expect(result.current).toBe(false); + }); + + it("unsubscribes and disposes the pending timer on unmount", () => { + const source = new FakeStatusSource(); + const { unmount } = renderHook(() => useSustainedReconnecting(source, 3_000)); + expect(source.listeners.size).toBe(1); + + act(() => { + source.emit("reconnecting"); + }); + unmount(); + expect(source.listeners.size).toBe(0); + + // The armed debounce timer must be gone: advancing past it neither + // throws nor updates state on an unmounted component. + act(() => { + vi.advanceTimersByTime(60_000); + }); + expect(vi.getTimerCount()).toBe(0); + }); + + it("clears then re-arms via replay when the source object changes mid-outage", () => { + const first = new FakeStatusSource(); + first.current = "reconnecting"; + const { result, rerender } = renderHook( + ({ source }: { source: ReconnectingStatusSource }) => useSustainedReconnecting(source, 3_000), + { initialProps: { source: first as ReconnectingStatusSource } }, + ); + + act(() => { + vi.advanceTimersByTime(3_000); + }); + expect(result.current).toBe(true); + + // A new source identity remounts the subscription: the flag clears + // (the old tracker is disposed), and the replacement source's replayed + // "reconnecting" re-arms a FRESH debounce window. + const second = new FakeStatusSource(); + second.current = "reconnecting"; + rerender({ source: second }); + expect(result.current).toBe(false); + expect(first.listeners.size).toBe(0); + expect(second.listeners.size).toBe(1); + + act(() => { + vi.advanceTimersByTime(2_999); + }); + expect(result.current).toBe(false); + act(() => { + vi.advanceTimersByTime(1); + }); + expect(result.current).toBe(true); + }); +});