mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-10-25 14:00:40 +00:00
test(web): pin the two mechanisms that keep a settings reload from vouching for the previous identity's invitations (BUG-3237) (#1588)
* test(web): pin the two mechanisms that keep a settings reload from vouching for the previous identity's invitations (BUG-3237) BUG-3237 does not reproduce in production. The filed state (load() stamps the page epoch at entry, so pageIdentityHeld() vouches for the previous session's invitations mid-reload) needs the epoch to move WITHOUT the user id. Only the existing settingsIdentityFence harness does that, deliberately. A real identity change moves both, and two mechanisms each close the window: the (user, workspace)-keyed load effect clears the lists before load(), and loading = true swaps the page for the spinner. New settingsLoadIdentityOrder.svelte.test.ts drives a coupled mock (user and epoch move together, both reactive and probed). It has a no-flip control, a settled flip, an identity change mid-load with the old answer landing late, and a workspace switch mid-load. It passes on main. The mutants: clear removed, 5 pass; loading swap removed, 5 pass; both removed, the settled leg fails; loadGen check removed, 2 legs fail; user dropped from the key, 2 legs fail. It is the only test in the tree that fails with both removed, which makes it the pin for that pair. A comment on load() names both mechanisms, so neither is removed on the theory that the other covers it. It is comment-only, and the TASK-3097 identity gate is unchanged. A reactive $app/state double (src/test/mocks/reactivePage.svelte.ts) is added, because the default alias is a plain object: reassigning page.params never re-ran the route's $derived, so the first run of the workspace-switch leg issued no second load at all. Claude-Session: https://claude.ai/code/session_016PXRzXhokr61CLbbu3Txj9 * test(web): the BUG-3237 pin clicks the copy link during the pending reload, so the both-removed mutant fails on the copy itself (codex round 1) Claude-Session: https://claude.ai/code/session_016PXRzXhokr61CLbbu3Txj9
This commit is contained in:
xarmian
@@ -375,6 +375,22 @@
|
||||
// was loaded, not the one current when the module first evaluated. A
|
||||
// route component that survives an identity change without remounting
|
||||
// would otherwise keep comparing against a stale epoch for ever.
|
||||
//
|
||||
// STAMPING AT ENTRY, BEFORE THE DATA LANDS, IS SAFE HERE, and only
|
||||
// because of two OTHER mechanisms (BUG-3237). The roles board and the
|
||||
// library call this ordering a defect, because there the stamp vouches
|
||||
// for data the round-trip has not yet replaced. On this page nothing is
|
||||
// left to vouch for:
|
||||
// 1. the (user, workspace)-keyed load effect clears name, context,
|
||||
// collections, members and invitations BEFORE it calls load(),
|
||||
// and a real identity change moves the user id with the epoch;
|
||||
// 2. `loading = true` above swaps the whole page for the spinner, so
|
||||
// no control rendered from any list is clickable mid-load.
|
||||
// EACH ONE ALONE covers the other's removal, and removing both lets a
|
||||
// Copy invite link click copy the previous session's join URL.
|
||||
// Removing one because the other "covers it" leaves no margin at all.
|
||||
// `settingsLoadIdentityOrder.svelte.test.ts` is the pin: it is the only
|
||||
// test that fails with both removed.
|
||||
identityEpochAtLoad = authStore.identityEpoch;
|
||||
try {
|
||||
await workspaceStore.setCurrent(slug);
|
||||
|
||||
+275
@@ -0,0 +1,275 @@
|
||||
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 {
|
||||
bindReactiveEpoch,
|
||||
bindReactiveUserId,
|
||||
isEpochReactive,
|
||||
isUserIdReactive,
|
||||
} from '../../../../test/identityEpochMock.svelte';
|
||||
|
||||
/**
|
||||
* BUG-3237: `load()` stamps `identityEpochAtLoad` at ENTRY, before its data
|
||||
* lands, so `pageIdentityHeld()` answers yes for the whole round-trip. The
|
||||
* filed concern: during a reload that follows an identity change, the Copy
|
||||
* invite link handler could copy a join link from the PREVIOUS session's
|
||||
* `invitations`.
|
||||
*
|
||||
* This suite measures that against an identity change as production makes it:
|
||||
* the epoch and the user id move TOGETHER (`notifyIdentityChange` bumps the
|
||||
* epoch only when the session's user id differs). `settingsIdentityFence`
|
||||
* deliberately moves the epoch alone, which isolates the handler fences but
|
||||
* also produces a state no real identity change can reach, so it cannot answer
|
||||
* this question.
|
||||
*
|
||||
* RESULT: it does not reproduce. Two mechanisms each close it on their own:
|
||||
* the (user, workspace)-keyed load effect clears the lists before load(), and
|
||||
* `loading = true` swaps the page for the spinner. With either one removed
|
||||
* this suite stays green; with BOTH removed the settled-page leg fails, which
|
||||
* is the filed defect. This suite is the regression pin for that pair, and it
|
||||
* is the only test in the tree that fails with both removed (BUG-3237 trail,
|
||||
* mutants M1/M5/M6). The dropped `loadGen` check and a load key without the
|
||||
* user each fail two legs here as well.
|
||||
*
|
||||
* A CONTROL leg (no flip) shows that the invitation and its copy button do
|
||||
* render and that the copy writes, so "not on screen" cannot pass because the
|
||||
* page rendered nothing.
|
||||
*/
|
||||
|
||||
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((slug: string) => {
|
||||
const d = deferred<unknown>();
|
||||
memberLists.push({ slug, d });
|
||||
return d.promise;
|
||||
}),
|
||||
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);
|
||||
|
||||
/** Every invitation email and copy button currently in the document. */
|
||||
function onScreen() {
|
||||
return {
|
||||
emails: [...document.querySelectorAll('.inv-email')].map((e) => e.textContent?.trim()),
|
||||
copyButtons: [...document.querySelectorAll('.copy-link-btn')] as HTMLButtonElement[],
|
||||
};
|
||||
}
|
||||
|
||||
async function openMembers() {
|
||||
await waitFor(() => expect(screen.getByRole('tab', { name: /Members/ })).toBeTruthy());
|
||||
screen.getByRole('tab', { name: /Members/ }).click();
|
||||
flushSync();
|
||||
}
|
||||
|
||||
async function answer(index: number, who: string) {
|
||||
await waitFor(() => expect(memberLists.length).toBeGreaterThan(index));
|
||||
memberLists[index]!.d.resolve(membersFor(who));
|
||||
}
|
||||
|
||||
describe('BUG-3237: the settings load never vouches for the previous identity\'s invitations', () => {
|
||||
it('PRECONDITION: the faked epoch and user id are both reactive', () => {
|
||||
expect(isEpochReactive(() => auth.identityEpoch, () => auth.bumpEpoch())).toBe(true);
|
||||
expect(isUserIdReactive(() => auth.userId, (id) => auth.setUser(id))).toBe(true);
|
||||
// The page double too: the workspace-switch leg is vacuous without it.
|
||||
let runs = 0;
|
||||
const stop = $effect.root(() => {
|
||||
$effect(() => {
|
||||
void page.params.workspace;
|
||||
runs += 1;
|
||||
});
|
||||
});
|
||||
flushSync();
|
||||
const baseline = runs;
|
||||
page.params = { username: 'dave', workspace: 'probe' };
|
||||
flushSync();
|
||||
stop();
|
||||
expect(runs).toBeGreaterThan(baseline);
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
memberLists.length = 0;
|
||||
copies.length = 0;
|
||||
auth.reset();
|
||||
page.params = { username: 'dave', workspace: 'ws' };
|
||||
window.location.hash = '';
|
||||
vi.stubGlobal('confirm', () => true);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
window.location.hash = '';
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
it('CONTROL: an uninterrupted load renders the invitation and its copy link works', async () => {
|
||||
render(SettingsPage);
|
||||
await answer(0, 'A');
|
||||
await openMembers();
|
||||
await waitFor(() => expect(onScreen().emails).toContain('invitee-of-A@example.com'));
|
||||
onScreen().copyButtons[0]!.click();
|
||||
await waitFor(() => expect(copies).toEqual(['https://pad.test/join/A']));
|
||||
});
|
||||
|
||||
it('a settled page: the moment the identity changes, the previous invitations are gone and nothing is copyable', async () => {
|
||||
render(SettingsPage);
|
||||
await answer(0, 'A');
|
||||
await openMembers();
|
||||
await waitFor(() => expect(onScreen().emails).toContain('invitee-of-A@example.com'));
|
||||
|
||||
auth.swapTo('u2');
|
||||
flushSync();
|
||||
// The CONSEQUENCE first (codex r1 on #1588): click whatever copy button
|
||||
// is on screen while the new identity's load is pending. With both
|
||||
// mechanisms removed, the previous session's button is still there and
|
||||
// `pageIdentityHeld()` already vouches for it, so this is where the
|
||||
// filed defect shows up, not only in the DOM assertions below.
|
||||
for (const b of onScreen().copyButtons) b.click();
|
||||
await new Promise((r) => setTimeout(r, 20));
|
||||
expect(copies).not.toContain('https://pad.test/join/A');
|
||||
expect(onScreen().emails).not.toContain('invitee-of-A@example.com');
|
||||
expect(onScreen().copyButtons).toHaveLength(0);
|
||||
|
||||
// The reload for the new identity lands; only its own data shows.
|
||||
await answer(1, 'B');
|
||||
await openMembers();
|
||||
await waitFor(() => expect(onScreen().emails).toContain('invitee-of-B@example.com'));
|
||||
expect(onScreen().emails).not.toContain('invitee-of-A@example.com');
|
||||
});
|
||||
|
||||
it('an identity change mid-load: the previous identity\'s late answer never lands, even after the new one', async () => {
|
||||
render(SettingsPage);
|
||||
await waitFor(() => expect(memberLists.length).toBe(1));
|
||||
|
||||
auth.swapTo('u2');
|
||||
flushSync();
|
||||
// The new identity's load answers FIRST, then the old one arrives late.
|
||||
await answer(1, 'B');
|
||||
await openMembers();
|
||||
await waitFor(() => expect(onScreen().emails).toContain('invitee-of-B@example.com'));
|
||||
memberLists[0]!.d.resolve(membersFor('A'));
|
||||
await new Promise((r) => setTimeout(r, 20));
|
||||
flushSync();
|
||||
|
||||
expect(onScreen().emails).toEqual(['invitee-of-B@example.com']);
|
||||
for (const b of onScreen().copyButtons) b.click();
|
||||
await new Promise((r) => setTimeout(r, 20));
|
||||
expect(copies).not.toContain('https://pad.test/join/A');
|
||||
expect(copies).toContain('https://pad.test/join/B');
|
||||
});
|
||||
|
||||
it('a workspace switch mid-load: the previous workspace\'s late answer never lands', async () => {
|
||||
render(SettingsPage);
|
||||
await waitFor(() => expect(memberLists.length).toBe(1));
|
||||
expect(memberLists[0]!.slug).toBe('ws');
|
||||
|
||||
page.params = { username: 'dave', workspace: 'ws2' };
|
||||
flushSync();
|
||||
await answer(1, 'W2');
|
||||
expect(memberLists[1]!.slug).toBe('ws2');
|
||||
await openMembers();
|
||||
await waitFor(() => expect(onScreen().emails).toContain('invitee-of-W2@example.com'));
|
||||
memberLists[0]!.d.resolve(membersFor('W1'));
|
||||
await new Promise((r) => setTimeout(r, 20));
|
||||
flushSync();
|
||||
|
||||
expect(onScreen().emails).toEqual(['invitee-of-W2@example.com']);
|
||||
});
|
||||
});
|
||||
@@ -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>,
|
||||
});
|
||||
Reference in New Issue
Block a user