Skip to content

Commit 0a6bd08

Browse files
bloveclaude
andauthored
refactor(examples-chat): URL is the sole source of truth for active thread (#518)
* refactor(examples-chat): URL is the sole source of truth for active thread 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 `/<next-mode>/<id>`), 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 /<mode>/<threadId> 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) <noreply@anthropic.com> * test(examples-chat): update e2e specs for URL-as-truth 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> * test(examples-chat): correct lifecycle 'New chat' URL expectation 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> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 120e7eb commit 0a6bd08

10 files changed

Lines changed: 193 additions & 98 deletions

docs/superpowers/specs/2026-05-20-url-thread-routing-design.md

Lines changed: 102 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -5,60 +5,105 @@
55
Make the active LangGraph thread part of the URL so links to specific
66
conversations on the canonical demo are shareable and survive reload.
77

8-
## Current state
8+
## URL is the source of truth
99

10-
`DemoShell.threadIdSignal = signal<string | null>(persistence.read('threadId') ?? null)`.
11-
The agent watches the signal; `onThreadId` callbacks write it back +
12-
persist to localStorage. Routes are `/embed`, `/popup`, `/sidebar`
13-
all stateless paths; the active thread lives only in localStorage.
14-
Sharing `/embed` always lands on whichever thread that browser last
15-
used (or a fresh one).
10+
The URL is the **sole** source of truth for the active thread. The
11+
shell does not persist the active thread to localStorage:
1612

17-
## URL shape
13+
- `/embed`, `/popup`, `/sidebar` — bare mode paths mean "no active
14+
thread" (welcome state).
15+
- `/embed/<id>`, `/popup/<id>`, `/sidebar/<id>` — that thread, in that
16+
presentation mode.
1817

19-
```
20-
/<mode>/:threadId?
21-
```
18+
Sharing `/embed/abc` lands on thread `abc`. Sharing `/embed` always
19+
lands on the welcome state, regardless of what the recipient's browser
20+
last used. There is no localStorage fallback to "last active thread"
21+
— that conflates user intent with browser-local state and breaks
22+
shareability.
2223

23-
`:threadId` is optional. Angular doesn't support `?` syntax for
24-
optional params, so each mode gets two route entries:
24+
## Route shape
25+
26+
Each mode gets a single route entry via `UrlMatcher` that accepts both
27+
`<mode>` and `<mode>/<threadId>` shapes under one entry. This is
28+
critical: a per-shape pair (two entries) causes Angular to destroy and
29+
remount the mode component when navigating `/embed``/embed/<id>`,
30+
which kills any in-flight agent stream (see PR #500/#504 history).
2531

2632
```ts
27-
{ path: 'embed', component: EmbedMode },
28-
{ path: 'embed/:threadId', component: EmbedMode },
29-
{ path: 'popup', component: PopupMode },
30-
{ path: 'popup/:threadId', component: PopupMode },
31-
{ path: 'sidebar', component: SidebarMode },
32-
{ path: 'sidebar/:threadId', component: SidebarMode },
33+
function modeMatcher(modeName: string): UrlMatcher {
34+
return (segments) => {
35+
if (segments.length === 0 || segments[0].path !== modeName) return null;
36+
if (segments.length === 1) return { consumed: segments, posParams: {} };
37+
if (segments.length === 2) {
38+
return { consumed: segments, posParams: { threadId: segments[1] } };
39+
}
40+
return null;
41+
};
42+
}
43+
44+
export const routes: Routes = [
45+
{ path: '', pathMatch: 'full', redirectTo: 'embed' },
46+
{
47+
path: '',
48+
loadComponent: () => import('./shell/demo-shell.component').then((m) => m.DemoShell),
49+
children: [
50+
{ matcher: modeMatcher('embed'), loadComponent: () => import('./modes/embed-mode.component').then((m) => m.EmbedMode) },
51+
{ matcher: modeMatcher('popup'), loadComponent: () => import('./modes/popup-mode.component').then((m) => m.PopupMode) },
52+
{ matcher: modeMatcher('sidebar'), loadComponent: () => import('./modes/sidebar-mode.component').then((m) => m.SidebarMode) },
53+
],
54+
},
55+
{ path: '**', redirectTo: 'embed' },
56+
];
3357
```
3458

35-
## URL ↔ signal sync (in DemoShell)
59+
DemoShell parses the URL itself (via `router.url` + a NavigationEnd
60+
`toSignal`) rather than reading param maps from `route.firstChild`,
61+
because the param data lives on the matched route under `posParams`
62+
and is more reliably read this way.
3663

37-
URL is the source of truth when present; localStorage falls back when
38-
the URL has no id.
64+
## URL ↔ signal sync (in DemoShell)
3965

4066
Two reactive flows in DemoShell, with guards against render loops:
4167

42-
1. **URL → signal.** `toSignal(route.firstChild.paramMap)` (the active
43-
mode component owns the param). An `effect` reads the URL's
44-
`threadId` and writes it into `threadIdSignal` if-and-only-if it
45-
differs from the current value.
68+
1. **URL → signal.** A `toSignal(NavigationEnd)` pipes the current URL
69+
through `parseUrl()` to extract `{mode, threadId}`. An `effect`
70+
reads the URL's `threadId` and writes it into `threadIdSignal` iff
71+
it differs from the current value. The signal read is `untracked`
72+
so the effect only fires on URL changes, not on imperative signal
73+
writes (critical for the stamp-in-progress async gap — see below).
74+
4675
2. **signal → URL.** A second `effect` reads `threadIdSignal` + the
4776
current `mode()` and `router.navigate(['/', mode, id])` if the URL
48-
doesn't already match. Uses `replaceUrl: false` so the back button
49-
walks through visited threads.
77+
doesn't already match. Uses default `replaceUrl: false` so the
78+
back button walks through visited threads.
79+
80+
The compare-and-set guard in flow 1 prevents the obvious
81+
URL→signal→URL loop: by the time the signal→URL effect fires, the
82+
values match and `router.navigate` is skipped.
83+
84+
### Stamp-in-progress invariant
85+
86+
When the agent auto-creates a thread mid-send, the `onThreadId`
87+
callback fires immediately and sets `threadIdSignal`. The signal→URL
88+
effect then navigates asynchronously. During the gap, the URL is
89+
still bare. The URL→signal effect MUST NOT clear the just-set signal
90+
back to `null` during this window — that would lose the agent's
91+
allocation. This is enforced by:
5092

51-
The "if it differs" guard is the only thing preventing the obvious
52-
URL→signal→URL→signal loop. Both effects already short-circuit
53-
because Angular signal writes are no-ops when the value is unchanged,
54-
but `router.navigate` doesn't short-circuit, so the explicit URL
55-
comparison in flow #2 is required.
93+
- The URL→signal effect tracks only `urlThreadId()`, not the signal.
94+
Imperative signal writes don't refire it.
95+
- The signal read inside the effect is `untracked`.
96+
97+
There is no test covering the no-nav-loop invariant directly; the
98+
"does not clear an agent-created thread id while URL navigation is
99+
still pending" test (`demo-shell.component.spec.ts`) covers the
100+
stamp-in-progress case.
56101

57102
## Invalid id handling
58103

59104
When a route loads with a `:threadId` the user has never seen (typo,
60-
deleted thread, link from another browser), we silently redirect to
61-
the bare mode path:
105+
deleted thread, link from another browser), silently redirect to the
106+
bare mode path:
62107

63108
```ts
64109
const thread = await threadsSvc.getThread(id);
@@ -67,20 +112,25 @@ if (!thread) router.navigate(['/', mode()], { replaceUrl: true });
67112

68113
`replaceUrl: true` so the back button doesn't reload the broken URL.
69114

70-
This requires a new method on `LangGraphThreadsAdapter`:
115+
Validation runs as a separate `effect` from the URL→signal sync, with
116+
a `lastValidated` closure variable to dedupe — `getThread` is async
117+
and we don't want to re-hit the server on every signal flip that
118+
round-trips the same id.
119+
120+
Requires `LangGraphThreadsAdapter.getThread()`:
71121

72122
```ts
73123
async getThread(threadId: string): Promise<Thread | null>
74124
```
75125

76-
Wraps `client.threads.get(id)`. Returns `null` on 404 (caught from
77-
the SDK's thrown error); rethrows on other failures so genuine
126+
Wraps `client.threads.get(id)`. Returns `null` on 404 and 422 (the
127+
latter for malformed UUIDs); rethrows on other failures so genuine
78128
network errors don't get masked as "thread missing."
79129

80130
## Mode switching preserves thread
81131

82132
`/embed/abc` → click "Popup" tab → `/popup/abc`. The `onModeChange`
83-
handler already exists; updates to include the current thread id:
133+
handler navigates with the current id:
84134

85135
```ts
86136
protected onModeChange(next: DemoMode | string): void {
@@ -89,22 +139,33 @@ protected onModeChange(next: DemoMode | string): void {
89139
}
90140
```
91141

142+
This is the **only** mechanism that preserves the active thread
143+
across mode boundaries. There is no localStorage backstop — direct
144+
URL navigation to `/popup` (e.g. paste link, back button) clears the
145+
active thread.
146+
92147
## Out of scope
93148

94149
- Server-side render of `<title>`/og:* tags for richer link previews
95150
- Restoring scroll position to the last-read message on reload
96151
- Authentication / private threads — these URLs are already public on
97152
the demo and that's fine
153+
- Round-tripping agent knobs (model, effort, theme, ...) via query
154+
params — see follow-up #494
98155

99156
## Test plan
100157

101158
- `LangGraphThreadsAdapter.getThread()` — returns `Thread` for an
102-
existing id, returns `null` for a missing id, rethrows on other
103-
errors
159+
existing id, returns `null` for a missing id (404 or 422), rethrows
160+
on other errors
104161
- Demo route loads `/embed/<existing-id>``threadIdSignal()` ===
105162
that id, messages from that thread render
106163
- Demo route loads `/embed/<bogus-id>` → silently redirects to
107164
`/embed`, fresh chat
165+
- Bare-mode route loads → `threadIdSignal()` is `null` regardless of
166+
any legacy localStorage state
167+
- Agent-allocated thread id survives the URL navigation async gap
168+
(stamp-in-progress invariant)
108169
- Click a thread in the sidenav → URL updates to `/<mode>/<id>`
109170
- Click mode toggle while a thread is active → URL switches mode but
110171
keeps the id

examples/chat/angular/e2e/lifecycle.spec.ts

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// SPDX-License-Identifier: MIT
22
import { test, expect } from '@playwright/test';
33
import {
4+
activeThreadIdFromUrl,
45
messageInput,
56
openDemo,
67
sendButton,
@@ -32,28 +33,25 @@ test('lifecycle: New chat (sidenav) starts a fresh thread and restores welcome s
3233
await sendButton(page).click();
3334
await waitForFinalAssistant(page);
3435

35-
const threadIdBefore = await page.evaluate(() => {
36-
const raw = localStorage.getItem('ngaf-chat-demo:palette');
37-
return raw ? (JSON.parse(raw) as { threadId?: string | null }).threadId ?? null : null;
38-
});
36+
// After the first send the agent allocates a thread id and stamps
37+
// it into the URL via the signal→URL effect: /embed/<thread-id>.
38+
await expect(page).toHaveURL(/\/embed\/[A-Za-z0-9-]+$/);
39+
const threadIdBefore = await activeThreadIdFromUrl(page);
3940

4041
// The toolbar "New conversation" button was removed; the sidenav's
4142
// "New chat" pill is now the only affordance for starting a fresh
42-
// thread. It creates a new thread server-side (rather than clearing
43-
// local state) and routes the UI back to the welcome surface.
43+
// thread. It creates a new thread server-side and navigates to
44+
// /embed/<new-thread-id>; the empty thread renders the welcome state.
4445
await page.getByRole('button', { name: 'New chat' }).first().click();
4546

4647
await expect(
4748
page.getByRole('heading', { name: 'How can I help?' })
4849
).toBeVisible();
4950
await expect(page.locator('chat-message')).toHaveCount(0);
5051

51-
const threadIdAfter = await page.evaluate(() => {
52-
const raw = localStorage.getItem('ngaf-chat-demo:palette');
53-
return raw ? (JSON.parse(raw) as { threadId?: string | null }).threadId ?? null : null;
54-
});
55-
// A fresh thread id was persisted, and it's different from the one we
56-
// had before clicking New chat.
52+
// URL holds a fresh thread id, different from the one we had before.
53+
await expect(page).toHaveURL(/\/embed\/[A-Za-z0-9-]+$/);
54+
const threadIdAfter = await activeThreadIdFromUrl(page);
5755
expect(threadIdAfter).toBeTruthy();
5856
expect(threadIdAfter).not.toBe(threadIdBefore);
5957
});

examples/chat/angular/e2e/mode-routing.spec.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// SPDX-License-Identifier: MIT
22
import { test, expect } from '@playwright/test';
33
import {
4+
activeThreadIdFromUrl,
45
closeChatDevtools,
56
messageInput,
67
openDemo,
@@ -44,22 +45,31 @@ test('cross-mode persistence: conversation follows embed, popup, and sidebar', a
4445
await sendButton(page).click();
4546
await waitForFinalAssistant(page);
4647

47-
await page.goto('/popup');
48+
// Capture the active thread id from the URL — post-URL-as-truth the
49+
// path is /embed/<thread-id>. Cross-mode persistence works by
50+
// navigating to /<other-mode>/<thread-id> directly (bare-mode URLs
51+
// intentionally do NOT restore the last thread — that would conflate
52+
// user intent with browser-local state).
53+
await expect(page).toHaveURL(/\/embed\/[A-Za-z0-9-]+$/);
54+
const threadId = await activeThreadIdFromUrl(page);
55+
expect(threadId).toBeTruthy();
56+
57+
await page.goto(`/popup/${threadId}`);
4858
await closeChatDevtools(page);
4959
await page.locator('.chat-popup__launcher button.chat-launcher-button').click();
5060
await expect(
5161
page.getByRole('dialog').locator('chat-message[data-role="assistant"]'),
5262
).toContainText(/hi/i, { timeout: 30_000 });
5363

54-
await page.goto('/sidebar');
64+
await page.goto(`/sidebar/${threadId}`);
5565
await closeChatDevtools(page);
5666
// Sidebar mode auto-opens the panel; assert the existing conversation
5767
// is visible without a launcher click.
5868
await expect(
5969
page.getByRole('complementary').locator('chat-message[data-role="assistant"]'),
6070
).toContainText(/hi/i, { timeout: 30_000 });
6171

62-
await page.goto('/embed');
72+
await page.goto(`/embed/${threadId}`);
6373
await expect(page.locator('embed-mode chat-message[data-role="assistant"]')).toContainText(/hi/i, {
6474
timeout: 30_000,
6575
});

examples/chat/angular/e2e/model-picker.spec.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// SPDX-License-Identifier: MIT
22
import { test, expect } from '@playwright/test';
33
import {
4+
activeThreadIdFromUrl,
45
messageInput,
56
openDemo,
67
sendButton,
@@ -45,12 +46,7 @@ test('model picker: configured models render, persist, and reach backend state',
4546
await sendButton(page).click();
4647
await waitForFinalAssistant(page);
4748

48-
const threadId = await page.evaluate(() => {
49-
const raw = localStorage.getItem('ngaf-chat-demo:palette');
50-
return raw
51-
? (JSON.parse(raw) as { threadId?: string }).threadId
52-
: undefined;
53-
});
49+
const threadId = await activeThreadIdFromUrl(page);
5450
expect(threadId).toBeTruthy();
5551
const state = await fetch(
5652
`http://localhost:2024/threads/${threadId}/state`

examples/chat/angular/e2e/regenerate.spec.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// SPDX-License-Identifier: MIT
22
import { test, expect } from '@playwright/test';
3-
import { sendPromptAndWait } from './test-helpers';
3+
import { activeThreadIdFromUrl, sendPromptAndWait } from './test-helpers';
44

55
test('regenerate: re-running keeps 1 user / 1 assistant in the conversation', async ({
66
page,
@@ -36,10 +36,7 @@ test('regenerate: re-running keeps 1 user / 1 assistant in the conversation', as
3636
await expect(userMessages).toHaveCount(1);
3737
await expect(assistantMessages).toHaveCount(1);
3838

39-
const threadId = await page.evaluate(() => {
40-
const raw = localStorage.getItem('ngaf-chat-demo:palette');
41-
return raw ? (JSON.parse(raw) as { threadId?: string }).threadId : undefined;
42-
});
39+
const threadId = await activeThreadIdFromUrl(page);
4340
expect(threadId).toBeTruthy();
4441
const state = await fetch(`http://localhost:2024/threads/${threadId}/state`).then((r) =>
4542
r.json() as Promise<{ values?: { messages?: unknown[] }; next?: unknown[] }>,

examples/chat/angular/e2e/test-helpers.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,20 @@ export function sendButton(page: Page): Locator {
4444
return page.getByRole('button', { name: 'Send message' });
4545
}
4646

47+
/**
48+
* Read the active thread id from the URL. URL is the source of truth
49+
* for the active thread post-URL-as-truth migration; bare-mode paths
50+
* (`/embed`, `/popup`, `/sidebar`) return `null`.
51+
*
52+
* Use this in place of `JSON.parse(localStorage.getItem(...)).threadId`,
53+
* which no longer exists.
54+
*/
55+
export async function activeThreadIdFromUrl(page: Page): Promise<string | null> {
56+
const url = new URL(page.url());
57+
const segments = url.pathname.split('/').filter(Boolean);
58+
return segments.length >= 2 ? segments[1] : null;
59+
}
60+
4761
/**
4862
* Locate the chat-select trigger inside a toolbar field by its label.
4963
*

0 commit comments

Comments
 (0)