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

On a direct load or reload, Settings → General showed an empty workspace
Name and a `{}` Workspace Context, for the owner too. In-app navigation
filled them.

Cause: a cold load makes two concurrent setCurrent calls for the same slug,
the page's load() and the layout's. setCurrent drops a superseded call's
writes, returning before `current = resolved`, so the page's `await
setCurrent(slug)` could return with `current` unset. load() then
snapshotted `current?.name ?? ''` and `current?.context ?? {}`, and nothing
wrote them again. The context half could lose data: saveContext treats `{}`
as "strip the context", so one Save click on the blank editor deleted the
workspace's context.

The page is the only caller that snapshots `current` after awaiting
setCurrent, so the fix is local and the store's contract is unchanged. An
effect seeds both fields from `workspaceStore.current`:
- only once the store has settled for this user (membershipKnown) and holds
  this slug;
- again whenever `current` changes, unless the user has typed in that field
  since the (user, workspace) key last changed.

An identity change nulls `current` and membershipKnown, so a previous
identity's workspace is never shown.

Tests:
- bug3260NameOnColdLoad.svelte.test.ts reproduces the superseded call. It
  was red on the unfixed page (it showed a stale workspace's name) and is
  green with the fix. A late-user-id leg, the first hypothesis, is kept and
  documented as not reproducing.
- settingsIdentityGate: load()'s reviewed hash is updated, with the reason;
  it now commits strictly less.
- The walk's General-name leg drops its test.fail and passes in the browser
  for all five members.

Claude-Session: https://claude.ai/code/session_016PXRzXhokr61CLbbu3Txj9

web/e2e/permission-walk-workspace.spec.ts

@@ -109,9 +109,7 @@ for (const key of MEMBERS) {
 	});
 
 	test(`${key}: General name field holds the workspace name`, async ({ page }) => {
-		// BUG-3260: on a direct load the field is empty for every member.
-		// Remove once the fix lands.
-		test.fail(true, 'BUG-3260');
+		// BUG-3260: a direct load used to leave this empty for every member.
 		await openSettingsTab(page, key, 'general');
 		await expect(page.locator('#ws-name')).toHaveValue(/^Walk /);
 	});

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

@@ -290,6 +290,8 @@
 		untrack(() => {
 			wsName = '';
 			contextEditor = '';
+			nameEdited = false;
+			contextEdited = false;
 			collections = [];
 			members = [];
 			invitations = [];
@@ -297,6 +299,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):
@@ -395,8 +422,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;
@@ -447,12 +474,14 @@
 	}
 
 	function resetContextEditor() {
+		contextEdited = false;
 		contextEditor = formatContextEditor(workspaceStore.current?.context);
 		contextError = '';
 		contextStatus = 'idle';
 	}
 
 	function clearContextEditor() {
+		contextEdited = true;
 		contextEditor = '{}';
 		contextError = '';
 		contextStatus = 'idle';
@@ -853,6 +882,7 @@
 								id="ws-name"
 								type="text"
 								bind:value={wsName}
+								oninput={() => (nameEdited = true)}
 								readonly={!isOwner}
 								onkeydown={(e) => isOwner && e.key === 'Enter' && saveName()}
 							/>
@@ -924,6 +954,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/routes/[username]/[workspace]/settings/settingsIdentityGate.test.ts

@@ -16,8 +16,8 @@ identityGateSuite({
 		asyncFunctions: {
 			refreshCollections: { reviewed: '99eebed18312', why: 'collectionsGen and the workspace, then identityHeld(epochAtEntry), before the commit; a failure commits nothing' },
 			load: {
-				reviewed: '1070cdd4b0df',
-				why: 'loadGen after each await (collectionsGen too for the collections write). It has no identity check, and it re-stamps identityEpochAtLoad BEFORE its awaits, where roles and library re-stamp after the data lands. The advisory list on TASK-3097 carries that',
+				reviewed: 'fedd925ecc86',
+				why: 'loadGen after each await (collectionsGen too for the collections write). It has no identity check, and it re-stamps identityEpochAtLoad BEFORE its awaits, where roles and library re-stamp after the data lands. The advisory list on TASK-3097 carries that. Re-reviewed for BUG-3260: it no longer writes the name or context (an effect seeds them from the settled store), so it commits strictly less than before',
 			},
 			saveName: { reviewed: '9d02b62e22dd', why: `${ENTRY}; the finally clears the button's own busy flag unfenced, on purpose` },
 			saveContext: { reviewed: '74dda10addda', why: `${ENTRY}, including between the update and the shared-store setCurrent; the finally clears its own busy flag` },