From 5b3ea78bfd82f650e030cb362f3994637abd14e4 Mon Sep 17 00:00:00 2001 From: Brian Love Date: Thu, 21 May 2026 12:54:06 -0700 Subject: [PATCH 1/3] refactor(examples-chat): URL is the sole source of truth for active thread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #500 introduced URL-based thread routing with the intent that the URL would replace localStorage as the persistence layer. PR #514 partly walked that back by re-introducing a localStorage `threadId` fallback to fix mode-switch sync — but that fallback conflates URL state with browser-local state and silently teleports users to old threads when they navigate to bare-mode URLs (paste link, back button). This finishes the URL-as-truth migration: - Drops `threadId` from `PaletteState`. - Removes the persistence write effect + persistence-read fallback in the URL→signal sync and `threadIdSignal` initialiser. - Removes the persistence clear in `validateUrlThreadId`'s 404 handler. - Keeps PR #514's `untracked` guard on the URL→signal effect — that guard prevents the stamp-in-progress signal from being cleared during the async URL navigation gap. It works without the persistence layer. - Keeps PR #504's `UrlMatcher` collapse (the stream-survival fix). - Keeps PR #500's `getThread()` validator + 404 redirect. Mode-switch UI continues to preserve the active thread across mode boundaries via `onModeChange` (URL navigation to `//`), which was the bug PR #514 was trying to fix. That path didn't need localStorage — it just needed the URL navigation to carry the id. Tests: - "does not write the active thread id to localStorage (URL is the source of truth)" — new - "ignores any legacy persisted threadId — bare mode URLs start fresh" — new (covers users who upgrade with legacy localStorage state) - "hydrates the active thread id from // URLs" — new - "does not clear an agent-created thread id while URL navigation is still pending" — retained from PR #514 Spec at `docs/superpowers/specs/2026-05-20-url-thread-routing-design.md` rewritten to match the simplified architecture; was describing the pre-#504 6-route world and the pre-#514 sync flow. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../2026-05-20-url-thread-routing-design.md | 143 +++++++++++++----- .../app/shell/demo-shell.component.spec.ts | 26 +++- .../src/app/shell/demo-shell.component.ts | 45 +++--- .../shell/palette-persistence.service.spec.ts | 9 +- .../app/shell/palette-persistence.service.ts | 1 - 5 files changed, 152 insertions(+), 72 deletions(-) diff --git a/docs/superpowers/specs/2026-05-20-url-thread-routing-design.md b/docs/superpowers/specs/2026-05-20-url-thread-routing-design.md index 4bd4c7fb..c6322d41 100644 --- a/docs/superpowers/specs/2026-05-20-url-thread-routing-design.md +++ b/docs/superpowers/specs/2026-05-20-url-thread-routing-design.md @@ -5,60 +5,105 @@ Make the active LangGraph thread part of the URL so links to specific conversations on the canonical demo are shareable and survive reload. -## Current state +## URL is the source of truth -`DemoShell.threadIdSignal = signal(persistence.read('threadId') ?? null)`. -The agent watches the signal; `onThreadId` callbacks write it back + -persist to localStorage. Routes are `/embed`, `/popup`, `/sidebar` — -all stateless paths; the active thread lives only in localStorage. -Sharing `/embed` always lands on whichever thread that browser last -used (or a fresh one). +The URL is the **sole** source of truth for the active thread. The +shell does not persist the active thread to localStorage: -## URL shape +- `/embed`, `/popup`, `/sidebar` — bare mode paths mean "no active + thread" (welcome state). +- `/embed/`, `/popup/`, `/sidebar/` — that thread, in that + presentation mode. -``` -//:threadId? -``` +Sharing `/embed/abc` lands on thread `abc`. Sharing `/embed` always +lands on the welcome state, regardless of what the recipient's browser +last used. There is no localStorage fallback to "last active thread" +— that conflates user intent with browser-local state and breaks +shareability. -`:threadId` is optional. Angular doesn't support `?` syntax for -optional params, so each mode gets two route entries: +## Route shape + +Each mode gets a single route entry via `UrlMatcher` that accepts both +`` and `/` shapes under one entry. This is +critical: a per-shape pair (two entries) causes Angular to destroy and +remount the mode component when navigating `/embed` → `/embed/`, +which kills any in-flight agent stream (see PR #500/#504 history). ```ts -{ path: 'embed', component: EmbedMode }, -{ path: 'embed/:threadId', component: EmbedMode }, -{ path: 'popup', component: PopupMode }, -{ path: 'popup/:threadId', component: PopupMode }, -{ path: 'sidebar', component: SidebarMode }, -{ path: 'sidebar/:threadId', component: SidebarMode }, +function modeMatcher(modeName: string): UrlMatcher { + return (segments) => { + if (segments.length === 0 || segments[0].path !== modeName) return null; + if (segments.length === 1) return { consumed: segments, posParams: {} }; + if (segments.length === 2) { + return { consumed: segments, posParams: { threadId: segments[1] } }; + } + return null; + }; +} + +export const routes: Routes = [ + { path: '', pathMatch: 'full', redirectTo: 'embed' }, + { + path: '', + loadComponent: () => import('./shell/demo-shell.component').then((m) => m.DemoShell), + children: [ + { matcher: modeMatcher('embed'), loadComponent: () => import('./modes/embed-mode.component').then((m) => m.EmbedMode) }, + { matcher: modeMatcher('popup'), loadComponent: () => import('./modes/popup-mode.component').then((m) => m.PopupMode) }, + { matcher: modeMatcher('sidebar'), loadComponent: () => import('./modes/sidebar-mode.component').then((m) => m.SidebarMode) }, + ], + }, + { path: '**', redirectTo: 'embed' }, +]; ``` -## URL ↔ signal sync (in DemoShell) +DemoShell parses the URL itself (via `router.url` + a NavigationEnd +`toSignal`) rather than reading param maps from `route.firstChild`, +because the param data lives on the matched route under `posParams` +and is more reliably read this way. -URL is the source of truth when present; localStorage falls back when -the URL has no id. +## URL ↔ signal sync (in DemoShell) Two reactive flows in DemoShell, with guards against render loops: -1. **URL → signal.** `toSignal(route.firstChild.paramMap)` (the active - mode component owns the param). An `effect` reads the URL's - `threadId` and writes it into `threadIdSignal` if-and-only-if it - differs from the current value. +1. **URL → signal.** A `toSignal(NavigationEnd)` pipes the current URL + through `parseUrl()` to extract `{mode, threadId}`. An `effect` + reads the URL's `threadId` and writes it into `threadIdSignal` iff + it differs from the current value. The signal read is `untracked` + so the effect only fires on URL changes, not on imperative signal + writes (critical for the stamp-in-progress async gap — see below). + 2. **signal → URL.** A second `effect` reads `threadIdSignal` + the current `mode()` and `router.navigate(['/', mode, id])` if the URL - doesn't already match. Uses `replaceUrl: false` so the back button - walks through visited threads. + doesn't already match. Uses default `replaceUrl: false` so the + back button walks through visited threads. + +The compare-and-set guard in flow 1 prevents the obvious +URL→signal→URL loop: by the time the signal→URL effect fires, the +values match and `router.navigate` is skipped. + +### Stamp-in-progress invariant + +When the agent auto-creates a thread mid-send, the `onThreadId` +callback fires immediately and sets `threadIdSignal`. The signal→URL +effect then navigates asynchronously. During the gap, the URL is +still bare. The URL→signal effect MUST NOT clear the just-set signal +back to `null` during this window — that would lose the agent's +allocation. This is enforced by: -The "if it differs" guard is the only thing preventing the obvious -URL→signal→URL→signal loop. Both effects already short-circuit -because Angular signal writes are no-ops when the value is unchanged, -but `router.navigate` doesn't short-circuit, so the explicit URL -comparison in flow #2 is required. +- The URL→signal effect tracks only `urlThreadId()`, not the signal. + Imperative signal writes don't refire it. +- The signal read inside the effect is `untracked`. + +There is no test covering the no-nav-loop invariant directly; the +"does not clear an agent-created thread id while URL navigation is +still pending" test (`demo-shell.component.spec.ts`) covers the +stamp-in-progress case. ## Invalid id handling When a route loads with a `:threadId` the user has never seen (typo, -deleted thread, link from another browser), we silently redirect to -the bare mode path: +deleted thread, link from another browser), silently redirect to the +bare mode path: ```ts const thread = await threadsSvc.getThread(id); @@ -67,20 +112,25 @@ if (!thread) router.navigate(['/', mode()], { replaceUrl: true }); `replaceUrl: true` so the back button doesn't reload the broken URL. -This requires a new method on `LangGraphThreadsAdapter`: +Validation runs as a separate `effect` from the URL→signal sync, with +a `lastValidated` closure variable to dedupe — `getThread` is async +and we don't want to re-hit the server on every signal flip that +round-trips the same id. + +Requires `LangGraphThreadsAdapter.getThread()`: ```ts async getThread(threadId: string): Promise ``` -Wraps `client.threads.get(id)`. Returns `null` on 404 (caught from -the SDK's thrown error); rethrows on other failures so genuine +Wraps `client.threads.get(id)`. Returns `null` on 404 and 422 (the +latter for malformed UUIDs); rethrows on other failures so genuine network errors don't get masked as "thread missing." ## Mode switching preserves thread `/embed/abc` → click "Popup" tab → `/popup/abc`. The `onModeChange` -handler already exists; updates to include the current thread id: +handler navigates with the current id: ```ts protected onModeChange(next: DemoMode | string): void { @@ -89,22 +139,33 @@ protected onModeChange(next: DemoMode | string): void { } ``` +This is the **only** mechanism that preserves the active thread +across mode boundaries. There is no localStorage backstop — direct +URL navigation to `/popup` (e.g. paste link, back button) clears the +active thread. + ## Out of scope - Server-side render of ``/og:* tags for richer link previews - Restoring scroll position to the last-read message on reload - Authentication / private threads — these URLs are already public on the demo and that's fine +- Round-tripping agent knobs (model, effort, theme, ...) via query + params — see follow-up #494 ## Test plan - `LangGraphThreadsAdapter.getThread()` — returns `Thread` for an - existing id, returns `null` for a missing id, rethrows on other - errors + existing id, returns `null` for a missing id (404 or 422), rethrows + on other errors - Demo route loads `/embed/<existing-id>` → `threadIdSignal()` === that id, messages from that thread render - Demo route loads `/embed/<bogus-id>` → silently redirects to `/embed`, fresh chat +- Bare-mode route loads → `threadIdSignal()` is `null` regardless of + any legacy localStorage state +- Agent-allocated thread id survives the URL navigation async gap + (stamp-in-progress invariant) - Click a thread in the sidenav → URL updates to `/<mode>/<id>` - Click mode toggle while a thread is active → URL switches mode but keeps the id diff --git a/examples/chat/angular/src/app/shell/demo-shell.component.spec.ts b/examples/chat/angular/src/app/shell/demo-shell.component.spec.ts index 41f6e311..f255c51e 100644 --- a/examples/chat/angular/src/app/shell/demo-shell.component.spec.ts +++ b/examples/chat/angular/src/app/shell/demo-shell.component.spec.ts @@ -145,7 +145,9 @@ describe('DemoShell — URL thread sync', () => { threadsAdapterProvider, provideRouter([ { path: 'embed', component: DemoShell }, + { path: 'embed/:threadId', component: DemoShell }, { path: 'popup', component: DemoShell }, + { path: 'popup/:threadId', component: DemoShell }, { path: '', pathMatch: 'full', redirectTo: 'embed' }, { path: '**', redirectTo: 'embed' }, ]), @@ -168,7 +170,7 @@ describe('DemoShell — URL thread sync', () => { expect(cmp.threadIdSignal()).toBe('thread-created-by-agent'); }); - it('persists an agent-created thread id for bare mode route fallback', () => { + it('does not write the active thread id to localStorage (URL is the source of truth)', () => { const fx = TestBed.createComponent(DemoShell); fx.detectChanges(); @@ -180,13 +182,14 @@ describe('DemoShell — URL thread sync', () => { fx.detectChanges(); const raw = localStorage.getItem('ngaf-chat-demo:palette'); - expect(raw ? JSON.parse(raw).threadId : null).toBe('thread-created-by-agent'); + const stored = raw ? JSON.parse(raw) : {}; + expect(stored.threadId).toBeUndefined(); }); - it('falls back to the persisted active thread on bare mode routes', async () => { + it('ignores any legacy persisted threadId — bare mode URLs start fresh', async () => { localStorage.setItem( 'ngaf-chat-demo:palette', - JSON.stringify({ threadId: 'persisted-thread' }), + JSON.stringify({ threadId: 'legacy-persisted-thread' }), ); const router = TestBed.inject(Router); await router.navigateByUrl('/popup'); @@ -197,6 +200,19 @@ describe('DemoShell — URL thread sync', () => { const cmp = fx.componentInstance as unknown as { threadIdSignal: { (): string | null }; }; - expect(cmp.threadIdSignal()).toBe('persisted-thread'); + expect(cmp.threadIdSignal()).toBeNull(); + }); + + it('hydrates the active thread id from /<mode>/<threadId> URLs', async () => { + const router = TestBed.inject(Router); + await router.navigateByUrl('/embed/url-thread'); + + const fx = TestBed.createComponent(DemoShell); + fx.detectChanges(); + + const cmp = fx.componentInstance as unknown as { + threadIdSignal: { (): string | null }; + }; + expect(cmp.threadIdSignal()).toBe('url-thread'); }); }); diff --git a/examples/chat/angular/src/app/shell/demo-shell.component.ts b/examples/chat/angular/src/app/shell/demo-shell.component.ts index 4d6a6444..b9d5e9a8 100644 --- a/examples/chat/angular/src/app/shell/demo-shell.component.ts +++ b/examples/chat/angular/src/app/shell/demo-shell.component.ts @@ -107,27 +107,29 @@ export class DemoShell { } }); - // Persist and refresh whenever the active thread changes (e.g. after - // create or switch) so bare mode routes can restore the current thread - // and the panel stays up to date. + // Refresh threads list whenever the active thread changes (e.g. after + // create or switch) so the panel stays up to date. The effect also + // covers the initial load (fires synchronously on first reactive read). effect(() => { - const id = this.threadIdSignal(); - this.persistence.write('threadId', id); + void this.threadIdSignal(); void this.threadsSvc.refresh(); }); - // URL → signal. Explicit URL thread ids win (paste link, back/forward, - // programmatic navigation). Bare mode URLs fall back to the persisted - // active thread, which keeps conversations attached across mode routes. - // The compare-and-set guard breaks the obvious URL→signal→URL loop: - // by the time the signal→URL effect below fires, both values match - // and `router.navigate` is skipped. + // URL → signal. When the URL's threadId changes (paste link, back/ + // forward, programmatic navigation), reflect it into threadIdSignal. + // Tracks ONLY the URL — the signal read is untracked so this effect + // does not re-fire when the signal is set imperatively (e.g. by the + // agent's onThreadId callback during the stamp-in-progress async gap + // before the URL catches up). + // + // No localStorage fallback: URL is the source of truth. Bare-mode URLs + // (`/embed`, `/popup`, `/sidebar`) explicitly mean "no active thread" + // — the welcome state. Mode-switch UI uses `onModeChange` to preserve + // the active thread across mode boundaries via URL navigation. effect(() => { const urlId = this.urlThreadId(); - const nextId = urlId ?? untracked(() => this.persistence.read('threadId') ?? null); - const currentId = untracked(() => this.threadIdSignal()); - if (nextId !== currentId) { - this.threadIdSignal.set(nextId); + if (urlId !== untracked(() => this.threadIdSignal())) { + this.threadIdSignal.set(urlId); } }); @@ -311,11 +313,15 @@ export class DemoShell { { value: 'material-light', label: 'Material light' }, ]); - /** Active thread id. URL is the source of truth when it contains an - * explicit thread id; bare mode URLs fall back to the last active - * thread so mode switches like `/embed` -> `/popup` preserve context. */ + /** Active thread id. URL is the sole source of truth (see urlState + * above); this signal initialises from the URL on construction and is + * kept in sync by the bidirectional effects in the constructor. The + * agent watches this signal directly. + * + * Mode switches that should preserve the active thread go through + * `onModeChange`, which navigates to `/<next-mode>/<id>` directly. */ protected readonly threadIdSignal = signal<string | null>( - parseUrl(this.router.url).threadId ?? this.persistence.read('threadId') ?? null, + parseUrl(this.router.url).threadId, ); /** Title of the currently-selected thread, or 'New chat' if none. The @@ -432,7 +438,6 @@ export class DemoShell { if (thread) return; if (this.threadIdSignal() === threadId) { this.threadIdSignal.set(null); - this.persistence.write('threadId', null); } await this.router.navigate(['/', this.mode()], { replaceUrl: true }); } diff --git a/examples/chat/angular/src/app/shell/palette-persistence.service.spec.ts b/examples/chat/angular/src/app/shell/palette-persistence.service.spec.ts index 75071765..2b9a746a 100644 --- a/examples/chat/angular/src/app/shell/palette-persistence.service.spec.ts +++ b/examples/chat/angular/src/app/shell/palette-persistence.service.spec.ts @@ -13,7 +13,6 @@ describe('PalettePersistence', () => { const svc = TestBed.runInInjectionContext(() => new PalettePersistence()); expect(svc.read('model')).toBeNull(); expect(svc.read('effort')).toBeNull(); - expect(svc.read('threadId')).toBeNull(); expect(svc.read('drawerOpen')).toBeNull(); }); @@ -39,10 +38,10 @@ describe('PalettePersistence', () => { it('clearing a key with null removes it from storage', () => { const svc = TestBed.runInInjectionContext(() => new PalettePersistence()); - svc.write('threadId', 'abc'); - expect(svc.read('threadId')).toBe('abc'); - svc.write('threadId', null); - expect(svc.read('threadId')).toBeNull(); + svc.write('selectedProjectId', 'abc'); + expect(svc.read('selectedProjectId')).toBe('abc'); + svc.write('selectedProjectId', null); + expect(svc.read('selectedProjectId')).toBeNull(); }); it('survives malformed storage (returns null and does not throw)', () => { diff --git a/examples/chat/angular/src/app/shell/palette-persistence.service.ts b/examples/chat/angular/src/app/shell/palette-persistence.service.ts index 7d0c3e72..85356770 100644 --- a/examples/chat/angular/src/app/shell/palette-persistence.service.ts +++ b/examples/chat/angular/src/app/shell/palette-persistence.service.ts @@ -8,7 +8,6 @@ interface PaletteState { effort?: string | null; genUiMode?: string | null; theme?: string | null; - threadId?: string | null; drawerOpen?: boolean | null; sidenavMode?: 'expanded' | 'collapsed' | null; selectedProjectId?: string | null; From 9b813011cc80bcb5b8703208e9f85e0ddb9fae49 Mon Sep 17 00:00:00 2001 From: Brian Love <brian@liveloveapp.com> Date: Thu, 21 May 2026 14:10:30 -0700 Subject: [PATCH 2/3] test(examples-chat): update e2e specs for URL-as-truth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four specs were reading the active thread id from \`localStorage['ngaf-chat-demo:palette'].threadId\` — which no longer exists after the persistence layer was dropped. One spec asserted cross-mode persistence via bare /<mode> navigation, which now lands on the welcome state by design (URL is the sole source of truth). Changes: - New helper \`activeThreadIdFromUrl(page)\` in test-helpers.ts — parses \`/<mode>/<threadId>\` URL shape. - lifecycle.spec.ts:27 — "New chat (sidenav)…" now asserts URL flips to bare /embed on welcome state, then sends again to verify a fresh thread id replaces the prior one (reads from URL, not localStorage). - mode-routing.spec.ts:39 — "cross-mode persistence…" captures the thread id after first send, then navigates to /<other-mode>/<id> explicitly. Bare /<mode> would clear the thread by design. - model-picker.spec.ts:12 — reads threadId from URL via the helper. - regenerate.spec.ts:5 — same. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --- examples/chat/angular/e2e/lifecycle.spec.ts | 29 +++++++++++-------- .../chat/angular/e2e/mode-routing.spec.ts | 16 ++++++++-- .../chat/angular/e2e/model-picker.spec.ts | 8 ++--- examples/chat/angular/e2e/regenerate.spec.ts | 7 ++--- examples/chat/angular/e2e/test-helpers.ts | 14 +++++++++ 5 files changed, 48 insertions(+), 26 deletions(-) diff --git a/examples/chat/angular/e2e/lifecycle.spec.ts b/examples/chat/angular/e2e/lifecycle.spec.ts index ab611245..dbfe7d91 100644 --- a/examples/chat/angular/e2e/lifecycle.spec.ts +++ b/examples/chat/angular/e2e/lifecycle.spec.ts @@ -1,6 +1,7 @@ // SPDX-License-Identifier: MIT import { test, expect } from '@playwright/test'; import { + activeThreadIdFromUrl, messageInput, openDemo, sendButton, @@ -32,15 +33,15 @@ test('lifecycle: New chat (sidenav) starts a fresh thread and restores welcome s await sendButton(page).click(); await waitForFinalAssistant(page); - const threadIdBefore = await page.evaluate(() => { - const raw = localStorage.getItem('ngaf-chat-demo:palette'); - return raw ? (JSON.parse(raw) as { threadId?: string | null }).threadId ?? null : null; - }); + // After the first send the agent allocates a thread id and stamps + // it into the URL via the signal→URL effect: /embed/<thread-id>. + await expect(page).toHaveURL(/\/embed\/[A-Za-z0-9-]+$/); + const threadIdBefore = await activeThreadIdFromUrl(page); // The toolbar "New conversation" button was removed; the sidenav's // "New chat" pill is now the only affordance for starting a fresh - // thread. It creates a new thread server-side (rather than clearing - // local state) and routes the UI back to the welcome surface. + // thread. It routes the UI back to bare /embed (welcome state); the + // next send allocates a fresh thread id. await page.getByRole('button', { name: 'New chat' }).first().click(); await expect( @@ -48,12 +49,16 @@ test('lifecycle: New chat (sidenav) starts a fresh thread and restores welcome s ).toBeVisible(); await expect(page.locator('chat-message')).toHaveCount(0); - const threadIdAfter = await page.evaluate(() => { - const raw = localStorage.getItem('ngaf-chat-demo:palette'); - return raw ? (JSON.parse(raw) as { threadId?: string | null }).threadId ?? null : null; - }); - // A fresh thread id was persisted, and it's different from the one we - // had before clicking New chat. + // URL should be back to bare /embed (no thread id) on welcome state. + await expect(page).toHaveURL(/\/embed$/); + + // Send again to allocate a new thread id, then verify URL holds a + // different id than before. + await messageInput(page).fill('say hi briefly'); + await sendButton(page).click(); + await waitForFinalAssistant(page); + await expect(page).toHaveURL(/\/embed\/[A-Za-z0-9-]+$/); + const threadIdAfter = await activeThreadIdFromUrl(page); expect(threadIdAfter).toBeTruthy(); expect(threadIdAfter).not.toBe(threadIdBefore); }); diff --git a/examples/chat/angular/e2e/mode-routing.spec.ts b/examples/chat/angular/e2e/mode-routing.spec.ts index 43017919..75307620 100644 --- a/examples/chat/angular/e2e/mode-routing.spec.ts +++ b/examples/chat/angular/e2e/mode-routing.spec.ts @@ -1,6 +1,7 @@ // SPDX-License-Identifier: MIT import { test, expect } from '@playwright/test'; import { + activeThreadIdFromUrl, closeChatDevtools, messageInput, openDemo, @@ -44,14 +45,23 @@ test('cross-mode persistence: conversation follows embed, popup, and sidebar', a await sendButton(page).click(); await waitForFinalAssistant(page); - await page.goto('/popup'); + // Capture the active thread id from the URL — post-URL-as-truth the + // path is /embed/<thread-id>. Cross-mode persistence works by + // navigating to /<other-mode>/<thread-id> directly (bare-mode URLs + // intentionally do NOT restore the last thread — that would conflate + // user intent with browser-local state). + await expect(page).toHaveURL(/\/embed\/[A-Za-z0-9-]+$/); + const threadId = await activeThreadIdFromUrl(page); + expect(threadId).toBeTruthy(); + + await page.goto(`/popup/${threadId}`); await closeChatDevtools(page); await page.locator('.chat-popup__launcher button.chat-launcher-button').click(); await expect( page.getByRole('dialog').locator('chat-message[data-role="assistant"]'), ).toContainText(/hi/i, { timeout: 30_000 }); - await page.goto('/sidebar'); + await page.goto(`/sidebar/${threadId}`); await closeChatDevtools(page); // Sidebar mode auto-opens the panel; assert the existing conversation // is visible without a launcher click. @@ -59,7 +69,7 @@ test('cross-mode persistence: conversation follows embed, popup, and sidebar', a page.getByRole('complementary').locator('chat-message[data-role="assistant"]'), ).toContainText(/hi/i, { timeout: 30_000 }); - await page.goto('/embed'); + await page.goto(`/embed/${threadId}`); await expect(page.locator('embed-mode chat-message[data-role="assistant"]')).toContainText(/hi/i, { timeout: 30_000, }); diff --git a/examples/chat/angular/e2e/model-picker.spec.ts b/examples/chat/angular/e2e/model-picker.spec.ts index 2dee1a4c..ec2e2b5f 100644 --- a/examples/chat/angular/e2e/model-picker.spec.ts +++ b/examples/chat/angular/e2e/model-picker.spec.ts @@ -1,6 +1,7 @@ // SPDX-License-Identifier: MIT import { test, expect } from '@playwright/test'; import { + activeThreadIdFromUrl, messageInput, openDemo, sendButton, @@ -45,12 +46,7 @@ test('model picker: configured models render, persist, and reach backend state', await sendButton(page).click(); await waitForFinalAssistant(page); - const threadId = await page.evaluate(() => { - const raw = localStorage.getItem('ngaf-chat-demo:palette'); - return raw - ? (JSON.parse(raw) as { threadId?: string }).threadId - : undefined; - }); + const threadId = await activeThreadIdFromUrl(page); expect(threadId).toBeTruthy(); const state = await fetch( `http://localhost:2024/threads/${threadId}/state` diff --git a/examples/chat/angular/e2e/regenerate.spec.ts b/examples/chat/angular/e2e/regenerate.spec.ts index 6a9af1d6..22c85d28 100644 --- a/examples/chat/angular/e2e/regenerate.spec.ts +++ b/examples/chat/angular/e2e/regenerate.spec.ts @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT import { test, expect } from '@playwright/test'; -import { sendPromptAndWait } from './test-helpers'; +import { activeThreadIdFromUrl, sendPromptAndWait } from './test-helpers'; test('regenerate: re-running keeps 1 user / 1 assistant in the conversation', async ({ page, @@ -36,10 +36,7 @@ test('regenerate: re-running keeps 1 user / 1 assistant in the conversation', as await expect(userMessages).toHaveCount(1); await expect(assistantMessages).toHaveCount(1); - const threadId = await page.evaluate(() => { - const raw = localStorage.getItem('ngaf-chat-demo:palette'); - return raw ? (JSON.parse(raw) as { threadId?: string }).threadId : undefined; - }); + const threadId = await activeThreadIdFromUrl(page); expect(threadId).toBeTruthy(); const state = await fetch(`http://localhost:2024/threads/${threadId}/state`).then((r) => r.json() as Promise<{ values?: { messages?: unknown[] }; next?: unknown[] }>, diff --git a/examples/chat/angular/e2e/test-helpers.ts b/examples/chat/angular/e2e/test-helpers.ts index c5ab320b..2fca646a 100644 --- a/examples/chat/angular/e2e/test-helpers.ts +++ b/examples/chat/angular/e2e/test-helpers.ts @@ -44,6 +44,20 @@ export function sendButton(page: Page): Locator { return page.getByRole('button', { name: 'Send message' }); } +/** + * Read the active thread id from the URL. URL is the source of truth + * for the active thread post-URL-as-truth migration; bare-mode paths + * (`/embed`, `/popup`, `/sidebar`) return `null`. + * + * Use this in place of `JSON.parse(localStorage.getItem(...)).threadId`, + * which no longer exists. + */ +export async function activeThreadIdFromUrl(page: Page): Promise<string | null> { + const url = new URL(page.url()); + const segments = url.pathname.split('/').filter(Boolean); + return segments.length >= 2 ? segments[1] : null; +} + /** * Locate the chat-select trigger inside a toolbar field by its label. * From 5c421c2d1ea8b2abb6cfd5e6aa5ff4a539fdd093 Mon Sep 17 00:00:00 2001 From: Brian Love <brian@liveloveapp.com> Date: Thu, 21 May 2026 14:17:43 -0700 Subject: [PATCH 3/3] test(examples-chat): correct lifecycle 'New chat' URL expectation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sidenav 'New chat' button calls \`onNewThread\` which creates a new thread server-side and sets \`threadIdSignal\` to the new id — the signal→URL effect then navigates to /embed/<new-thread-id>. The URL does NOT go back to bare /embed; the welcome state renders because the new thread is empty, not because the URL is bare. Drops the incorrect \`expect(page).toHaveURL(/\\/embed\$/)\` assertion and removes the redundant second send. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --- examples/chat/angular/e2e/lifecycle.spec.ts | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/examples/chat/angular/e2e/lifecycle.spec.ts b/examples/chat/angular/e2e/lifecycle.spec.ts index dbfe7d91..3d753088 100644 --- a/examples/chat/angular/e2e/lifecycle.spec.ts +++ b/examples/chat/angular/e2e/lifecycle.spec.ts @@ -40,8 +40,8 @@ test('lifecycle: New chat (sidenav) starts a fresh thread and restores welcome s // The toolbar "New conversation" button was removed; the sidenav's // "New chat" pill is now the only affordance for starting a fresh - // thread. It routes the UI back to bare /embed (welcome state); the - // next send allocates a fresh thread id. + // thread. It creates a new thread server-side and navigates to + // /embed/<new-thread-id>; the empty thread renders the welcome state. await page.getByRole('button', { name: 'New chat' }).first().click(); await expect( @@ -49,14 +49,7 @@ test('lifecycle: New chat (sidenav) starts a fresh thread and restores welcome s ).toBeVisible(); await expect(page.locator('chat-message')).toHaveCount(0); - // URL should be back to bare /embed (no thread id) on welcome state. - await expect(page).toHaveURL(/\/embed$/); - - // Send again to allocate a new thread id, then verify URL holds a - // different id than before. - await messageInput(page).fill('say hi briefly'); - await sendButton(page).click(); - await waitForFinalAssistant(page); + // URL holds a fresh thread id, different from the one we had before. await expect(page).toHaveURL(/\/embed\/[A-Za-z0-9-]+$/); const threadIdAfter = await activeThreadIdFromUrl(page); expect(threadIdAfter).toBeTruthy();