fix(settings): the workspace name and context fill in after a cold load, instead of loading blank (BUG-3260, v0.16.1)

Backport of #1621 (f36436f8) to release-0.16.x for v0.16.1 (TASK-3269).

On a cold load of Settings -> General, the page's load() and the layout both
called workspaceStore.setCurrent for the same slug. The page's call was
superseded and returned before `current` was written, so load() seeded the
name as "" and the context as {}. One Save then wrote the empty context back
over the workspace's real one.

The +page.svelte change is #1621's, line for line.

Tests, adapted to the release line:
- bug3260NameOnColdLoad.svelte.test.ts, from #1621 unchanged. On this branch
  its DEFECT leg fails against v0.16.0's page and passes with the fix; the two
  control legs pass on both.
- web/src/test/mocks/reactivePage.svelte.ts and
  web/src/test/identityEpochMock.svelte.ts, copied from main. The suite imports
  them, and v0.16.0 predates them. Both are test-only, and each imports nothing
  but svelte.
- Not carried: #1621's edits to settingsIdentityGate.test.ts and
  permission-walk-workspace.spec.ts. Neither file exists at v0.16.0.

Claude-Session: https://claude.ai/code/session_016PXRzXhokr61CLbbu3Txj9
(cherry picked from commit f36436f802)

web/src/routes/[username]/[workspace]/settings/+page.svelte

@@ -231,6 +231,8 @@
 		untrack(() => {
 			wsName = '';
 			contextEditor = '';
+			nameEdited = false;
+			contextEdited = false;
 			collections = [];
 			members = [];
 			invitations = [];
@@ -238,6 +240,31 @@
 		});
 	});
 
+	// The name and context fields are SEEDED FROM THE STORE, reactively, not
+	// snapshotted inside load() (BUG-3260). load() used to read
+	// `workspaceStore.current` right after `await setCurrent(slug)`. But a
+	// setCurrent that a concurrent call supersedes (the layout makes one for
+	// the same slug on every cold load) returns BEFORE it writes `current`. So
+	// the snapshot read an unset store: an empty name, and a `{}` context that
+	// saveContext would then have written back as "no context".
+	//
+	// Seeded only once the store has SETTLED for this user
+	// (`membershipKnown`) and holds this workspace, so a previous identity's
+	// workspace is never shown. Re-seeded whenever `current` changes, unless
+	// the user has typed in that field since the page's (user, workspace) key
+	// last changed.
+	let nameEdited = false;
+	let contextEdited = false;
+	$effect(() => {
+		void sessionUserId;
+		const cur = workspaceStore.current;
+		if (!workspaceStore.membershipKnown || !cur || cur.slug !== wsSlug) return;
+		untrack(() => {
+			if (!nameEdited) wsName = cur.name;
+			if (!contextEdited) contextEditor = formatContextEditor(cur.context);
+		});
+	});
+
 	// BUG-2265: keep the collections list fresh when another client changes a
 	// collection, so opening the edit modal seeds a CURRENT
 	// expected_updated_at instead of a stale one. UNIFIED generation (Codex):
@@ -307,8 +334,8 @@
 		try {
 			await workspaceStore.setCurrent(slug);
 			if (myLoad !== loadGen) return;
-			wsName = workspaceStore.current?.name ?? '';
-			contextEditor = JSON.stringify(workspaceStore.current?.context ?? {}, null, 2);
+			// The name and context are seeded by the effect after the load key
+			// (BUG-3260), not read here: `current` may not be set yet.
 			const fresh = await api.collections.list(slug);
 			if (myLoad !== loadGen) return;
 			if (myColl === collectionsGen) collections = fresh;
@@ -345,12 +372,14 @@
 	}
 
 	function resetContextEditor() {
+		contextEdited = false;
 		contextEditor = formatContextEditor(workspaceStore.current?.context);
 		contextError = '';
 		contextStatus = 'idle';
 	}
 
 	function clearContextEditor() {
+		contextEdited = true;
 		contextEditor = '{}';
 		contextError = '';
 		contextStatus = 'idle';
@@ -650,6 +679,7 @@
 								id="ws-name"
 								type="text"
 								bind:value={wsName}
+								oninput={() => (nameEdited = true)}
 								readonly={!isOwner}
 								onkeydown={(e) => isOwner && e.key === 'Enter' && saveName()}
 							/>
@@ -721,6 +751,7 @@
 						id="workspace-context"
 						class="context-editor mono"
 						bind:value={contextEditor}
+						oninput={() => (contextEdited = true)}
 						spellcheck="false"
 						readonly={!isOwner}
 						rows="18"

web/src/routes/[username]/[workspace]/settings/bug3260NameOnColdLoad.svelte.test.ts

@@ -0,0 +1,201 @@
+import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest';
+import { cleanup, render, screen, waitFor } from '@testing-library/svelte';
+import { flushSync } from 'svelte';
+import { page } from '$app/state';
+import SettingsPage from './+page.svelte';
+import { api } from '$lib/api/client';
+import { workspaceStore } from '$lib/stores/workspace.svelte';
+import {
+	bindReactiveEpoch,
+	bindReactiveUserId,
+	isEpochReactive,
+	isUserIdReactive,
+} from '../../../../test/identityEpochMock.svelte';
+
+/**
+ * BUG-3260: Settings -> General showed an EMPTY workspace name on a direct load
+ * or reload (measured in e2e, owner included), while in-app navigation filled
+ * it. This suite reproduces the load orders in isolation.
+ *
+ * MECHANISM (traced in e2e): on a cold load the page's load() and the layout
+ * both call workspaceStore.setCurrent for the same slug at once. The page's
+ * call is superseded: it returns before `current` is written, because
+ * setCurrent drops a stale call's writes. load() then read `current` while it
+ * was still unset, and seeded the name as "" and the context as {}.
+ * Nothing wrote either again.
+ *
+ * CONTROL: an uncontended load, as after in-app navigation, when `current` is
+ * already this workspace.
+ * DEFECT LEG: the page's setCurrent is superseded by a concurrent one, and
+ * the superseded call's fetch answers first.
+ * A leg where the user id arrives late is kept: it was the first hypothesis,
+ * and it does not reproduce.
+ */
+
+type Deferred<T> = { promise: Promise<T>; resolve: (v: T) => void };
+function deferred<T>(): Deferred<T> {
+	let resolve!: (v: T) => void;
+	const promise = new Promise<T>((res) => {
+		resolve = res;
+	});
+	return { promise, resolve };
+}
+
+/** Outstanding members.list calls, in call order, with the slug each asked for. */
+const memberLists: Array<{ slug: string; d: Deferred<unknown> }> = [];
+const copies = vi.hoisted(() => [] as string[]);
+
+function membersFor(who: string) {
+	return {
+		members: [{ user_id: `m-${who}`, user_name: `Member ${who}`, user_email: `${who}@example.com`, role: 'editor' }],
+		invitations: [
+			{ id: `inv-${who}`, email: `invitee-of-${who}@example.com`, role: 'editor', code: `code-${who}`, join_url: `https://pad.test/join/${who}` },
+		],
+	};
+}
+
+vi.mock('$lib/utils/clipboard', () => ({
+	copyToClipboard: async (s: string) => {
+		copies.push(s);
+		return true;
+	},
+}));
+
+vi.mock('$lib/stores/toast.svelte', () => ({
+	toastStore: { show: () => 'toast-id', dismiss: () => {}, get toasts() { return []; } },
+	quietExternalToasts: () => false,
+}));
+
+vi.mock('$lib/api/client', () => ({
+	api: {
+		workspaces: {
+			get: vi.fn(async (slug: string) => ({ id: `id-${slug}`, slug, name: slug.toUpperCase(), context: {} })),
+			me: vi.fn(async () => ({ role: 'owner', collection_grants: [], item_grants: [] })),
+			list: vi.fn(async () => []),
+			update: vi.fn(async () => ({})),
+			delete: vi.fn(async () => ({})),
+			restore: vi.fn(async () => ({})),
+		},
+		collections: { list: vi.fn(async () => []) },
+		members: {
+			list: vi.fn(async () => ({ members: [], invitations: [] })),
+			getMemberCollectionAccess: vi.fn(async () => ({ collection_access: 'all', collection_ids: [] })),
+			setMemberCollectionAccess: vi.fn(async () => ({})),
+			remove: vi.fn(async () => ({})),
+		},
+	},
+	isPlanLimitError: () => false,
+	planLimitMessage: () => '',
+	PadApiError: class extends Error {},
+}));
+
+vi.mock('$app/state', async () => ({ page: (await import('../../../../test/mocks/reactivePage.svelte')).page }));
+vi.mock('$lib/services/sse.svelte', () => ({ sseService: { onItemEvent: () => () => {} } }));
+vi.mock('$app/navigation', () => ({ goto: () => Promise.resolve() }));
+
+const auth = vi.hoisted(() => {
+	const hook = {
+		read: null as null | (() => number),
+		write: null as null | ((n: number) => void),
+		readUser: null as null | (() => string),
+		writeUser: null as null | ((id: string) => void),
+	};
+	let fallbackEpoch = 0;
+	let fallbackUser = 'u1';
+	const getEpoch = () => (hook.read ? hook.read() : fallbackEpoch);
+	const setEpoch = (n: number) => (hook.write ? hook.write(n) : (fallbackEpoch = n));
+	const getUser = () => (hook.readUser ? hook.readUser() : fallbackUser);
+	const setUser = (id: string) => (hook.writeUser ? hook.writeUser(id) : (fallbackUser = id));
+	return {
+		__hook: hook,
+		get identityEpoch() { return getEpoch(); },
+		get userId() { return getUser(); },
+		setUser,
+		bumpEpoch() { setEpoch(getEpoch() + 1); },
+		/** A real identity change: user id and epoch move in the same step. */
+		swapTo(id: string) {
+			setUser(id);
+			setEpoch(getEpoch() + 1);
+		},
+		reset() {
+			setUser('u1');
+			setEpoch(0);
+		},
+		identityFence() {
+			const captured = getEpoch();
+			return () => getEpoch() === captured;
+		},
+		onIdentityChange() { return () => {}; },
+		clear() {},
+	};
+});
+
+vi.mock('$lib/stores/auth.svelte', () => ({ authStore: auth }));
+
+bindReactiveEpoch(auth.__hook);
+bindReactiveUserId(auth.__hook);
+
+
+function nameField(): HTMLInputElement | null {
+	return document.querySelector('#ws-name');
+}
+
+describe('BUG-3260: the General name field holds the workspace name however the page was reached', () => {
+	beforeEach(() => {
+		memberLists.length = 0;
+		copies.length = 0;
+		auth.reset();
+		page.params = { username: 'dave', workspace: 'ws' };
+		window.location.hash = '';
+	});
+
+	afterEach(() => {
+		cleanup();
+		window.location.hash = '';
+	});
+
+	it('CONTROL: user known before mount (in-app navigation)', async () => {
+		render(SettingsPage);
+		await waitFor(() => expect(nameField()?.value).toBe('WS'));
+	});
+
+	it('cold load: the page mounts before the session user is known, and the user arrives after', async () => {
+		auth.setUser('');
+		render(SettingsPage);
+		await waitFor(() => expect(nameField()).not.toBeNull());
+		auth.swapTo('u1');
+		flushSync();
+		await waitFor(() => expect(nameField()?.value).toBe('WS'));
+	});
+
+	it('DEFECT: the page setCurrent is superseded by a concurrent one (the layout call), and its fetch answers first', async () => {
+		const get = vi.mocked(api.workspaces.get);
+		const first = deferred<unknown>();
+		const second = deferred<unknown>();
+		// Any call beyond the two this leg controls answers with a name that
+		// cannot be mistaken for the fix working.
+		get.mockImplementation(async () => ({ id: 'id-ws', slug: 'ws', name: 'UNPLANNED', context: {} }) as never);
+		get.mockImplementationOnce(() => first.promise as never);
+		get.mockImplementationOnce(() => second.promise as never);
+
+		render(SettingsPage); // load() -> setCurrent('ws') -> get #1, pending
+		await waitFor(() => expect(get).toHaveBeenCalledTimes(1));
+		void workspaceStore.setCurrent('ws'); // the layout's call -> get #2; supersedes #1
+		await waitFor(() => expect(get).toHaveBeenCalledTimes(2));
+
+		first.resolve({ id: 'id-ws', slug: 'ws', name: 'Real Name', context: { repo: 'x' } });
+		// The superseded call's continuation, and load() after it, run to
+		// completion before the winning call's fetch lands, as traced in e2e.
+		await new Promise((r) => setTimeout(r, 0));
+		await waitFor(() => expect(vi.mocked(api.members.list)).toHaveBeenCalled());
+		second.resolve({ id: 'id-ws', slug: 'ws', name: 'Real Name', context: { repo: 'x' } });
+
+		// 'Real Name' comes only from these two answers. The store is a module
+		// singleton, so an earlier leg's workspace ('WS') can still be current
+		// here; a load that read the store at the wrong moment shows that
+		// instead, or '' on a real cold load.
+		await waitFor(() => expect(nameField()?.value).toBe('Real Name'));
+		const ctx = document.querySelector<HTMLTextAreaElement>('#workspace-context');
+		await waitFor(() => expect(ctx?.value ?? '').toContain('"repo"'));
+	});
+});

web/src/test/identityEpochMock.svelte.ts

@@ -0,0 +1,162 @@
+/**
+ * The family's fake `authStore` for identity-fence suites (BUG-3084).
+ *
+ * WHY THIS EXISTS, and it is not convenience. Each surface's suite grew its own
+ * double with `let epoch = 0` — a plain closure variable. That models the
+ * store's VALUES correctly and its REACTIVITY not at all, and the real
+ * `identityEpoch` is `$state`: reading it inside an `$effect` creates a
+ * DEPENDENCY, so an identity change re-runs that effect.
+ *
+ * A non-reactive double silently converts every "did this effect re-run?"
+ * question into "does this compile?". That is not a coverage gap; it is an
+ * instrument answering a different question than its name. It hid a real defect
+ * on the collection page through ten source assertions, ten behavioural legs, a
+ * 10/10 mutation matrix, four review rounds and two lead reads — the search
+ * dispatch effect re-armed its debounce with the previous user's typed text and
+ * captured the NEW epoch, so the fence inside the timer passed and the old
+ * query went to the server under the new identity.
+ *
+ * The general form, which is the part worth carrying to other families:
+ *
+ *   A test double that models a dependency's VALUES but not its REACTIVITY
+ *   turns every question about re-running into a question about compiling.
+ *   Where the subject is a framework effect, the double's reactivity IS the
+ *   thing under test.
+ *
+ * MECHANICS. `vi.hoisted` runs before the Svelte runtime is initialised, so
+ * `$state` cannot be declared inside a hoisted factory. This module declares it
+ * at module scope — legal in a `.svelte.ts` — and a suite binds its hoisted
+ * object to it with `bindReactiveEpoch`. Probed against a known answer before
+ * being relied on, rather than assumed to work.
+ */
+
+import { flushSync } from 'svelte';
+
+let epochSignal = $state(0);
+
+/** The reactive epoch. Reading this inside an `$effect` creates a dependency. */
+export function readEpoch(): number {
+	return epochSignal;
+}
+
+export function writeEpoch(next: number): void {
+	epochSignal = next;
+}
+
+export function bumpEpoch(): void {
+	epochSignal += 1;
+}
+
+export function resetEpoch(): void {
+	epochSignal = 0;
+}
+
+/**
+ * Bind a hoisted mock object's epoch hook to the reactive signal above.
+ *
+ * The hoisted object holds `{ read, write }` slots and delegates to them when
+ * they are set, falling back to a plain variable when they are not — so a suite
+ * that forgets to bind still runs, but its "did this re-run?" assertions are
+ * then measuring nothing. That is exactly the failure this module exists to
+ * remove, which is why `assertReactive` below is offered alongside it.
+ */
+export function bindReactiveEpoch(hook: {
+	read: null | (() => number);
+	write: null | ((n: number) => void);
+}): void {
+	hook.read = readEpoch;
+	hook.write = writeEpoch;
+}
+
+/**
+ * Proves the SUBJECT'S OWN epoch path is reactive.
+ *
+ * Takes the mock's getter and its bump, because the first version of this
+ * function did NOT: it read and wrote this module's signal directly, so it
+ * returned true even with both hooks disconnected and every PRECONDITION leg
+ * built on it passed against exactly the non-reactive double it existed to
+ * detect. Codex reproduced that. A guard that cannot fail for the case it is
+ * named after is the failure it is named after, wearing its own badge.
+ *
+ * Pass the values the PAGE will see: `() => authStore.identityEpoch` and
+ * `() => authStore.bumpEpoch()` from the suite's own mock. Nothing here
+ * references the module signal, so a suite that forgot `bindReactiveEpoch`
+ * reads its inert fallback and this returns false.
+ *
+ * Returns rather than throws, so the caller owns the assertion message.
+ */
+export function isEpochReactive(read: () => number, bump: () => void): boolean {
+	let runs = 0;
+	const stop = $effect.root(() => {
+		$effect(() => {
+			read();
+			runs += 1;
+		});
+	});
+	flushSync();
+	const baseline = runs;
+	const before = read();
+	bump();
+	flushSync();
+	const moved = read() !== before;
+	const reran = runs > baseline;
+	stop();
+	// Both halves, because either alone is satisfiable by a broken double: a
+	// bump that does not move the value proves nothing about tracking, and a
+	// re-run with an unmoved value would mean the effect depends on something
+	// else entirely.
+	return moved && reran;
+}
+
+/**
+ * The reactive USER ID, for surfaces whose recovery is keyed on
+ * `authStore.userId` rather than driven by an `onIdentityChange` listener
+ * (settings, the dashboard: a `(sessionUserId, wsSlug)` effect drops the data
+ * and reloads). The real getter reads the `session` `$state`, so it is a
+ * dependency; a suite whose double returns a plain variable turns "did the
+ * keyed effect re-run?" into "does this compile?" — the same failure as the
+ * epoch, one field over (BUG-3084 surface 4 found it on its first recovery
+ * leg: the reload never came, and the mock was why).
+ */
+let userIdSignal = $state('u1');
+
+export function readUserId(): string {
+	return userIdSignal;
+}
+
+export function writeUserId(next: string): void {
+	userIdSignal = next;
+}
+
+export function bindReactiveUserId(hook: {
+	readUser: null | (() => string);
+	writeUser: null | ((id: string) => void);
+}): void {
+	hook.readUser = readUserId;
+	hook.writeUser = writeUserId;
+}
+
+/**
+ * Proves the SUBJECT'S OWN userId path is reactive — same contract as
+ * `isEpochReactive`: pass the mock's getter and setter, never this module's.
+ */
+export function isUserIdReactive(read: () => string, set: (id: string) => void): boolean {
+	let runs = 0;
+	const stop = $effect.root(() => {
+		$effect(() => {
+			read();
+			runs += 1;
+		});
+	});
+	flushSync();
+	const baseline = runs;
+	const before = read();
+	set(before + '-moved');
+	flushSync();
+	const moved = read() !== before;
+	const reran = runs > baseline;
+	set(before);
+	flushSync();
+	stop();
+	return moved && reran;
+}

web/src/test/mocks/reactivePage.svelte.ts

@@ -0,0 +1,21 @@
+// A REACTIVE `$app/state` page double, for suites that navigate between routes
+// while a component is mounted. The default alias (`app-state.ts`) is a plain
+// object, so reassigning `page.params` there never re-runs a `$derived` that
+// reads it, and a "switch workspace mid-load" leg silently measures nothing
+// (BUG-3237: the first run of its workspace-switch leg issued no second load).
+//
+// Use from a suite with:
+//   vi.mock('$app/state', async () => ({ page: (await import('<path>/reactivePage.svelte')).page }));
+// `$state` must live at module scope in a `.svelte.ts` file; a hoisted mock
+// factory runs before the Svelte runtime is ready to declare it.
+
+export const page = $state({
+	params: {} as Record<string, string>,
+	url: new URL('http://localhost/'),
+	route: { id: null as string | null },
+	status: 200,
+	error: null as unknown,
+	data: {} as Record<string, unknown>,
+	form: null as unknown,
+	state: {} as Record<string, unknown>,
+});