mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-10-25 22:10:25 +00:00
fix(web): fence every async commit point on the library page on the signed-in identity (BUG-3084) (#1378)
Surface 3 of 7. Three async handlers and FOUR DEFERRED TIMERS — a population of 7, not the 3 a function-level count sees. The originally filed survey reported 3; the widened instrument's number is confirmed again here on the current tree rather than carried from the survey. What this fixes ---------------------------------------------------------------------- `activateConvention` and `activatePlaybook` COMPOSE A WRITE from state chosen under the previous identity: the entry the user clicked came from a list `loadData` fetched then. A click after an identity change is the current epoch, so every entry capture passes and the previous session's choice is activated into whatever workspace is on screen now. Both now take `pageIdentityHeld()` at entry, the same question the roles board's drag handlers needed. The four timers are the distinctive class. Each fires three seconds after its handler returned and clears `toast` — by which time the page may belong to someone else and be showing THEIR toast. Clearing it then is a commit by a session that has ended. Driven with fake timers rather than reasoned about. `loadData` had no identity bookkeeping at all, and its `$effect` is keyed on `wsSlug` alone, so nothing re-ran it when the identity moved. It now has an entry capture, per-arm fences, a generation guard, and an `onIdentityChange` subscription that resets transient state before re-loading. The rules carried from surfaces 1 and 2, not re-derived ---------------------------------------------------------------------- - The epoch is re-stamped AFTER the awaited state is assigned, never before the await, and on BOTH arms. Too early and the guard vouches for data it has not replaced; missing on a path and a transient error pins the fence shut for ever. Bounded on both sides in the guard. - A continuation that has LOST the identity writes no shared interaction state — `resetTransientState()` owns that reset, and a stale continuation writing it again can only clobber what the NEW user is doing. `activatingTitle` gates both handlers at their first line, so this is not cosmetic: a stale release permits a second concurrent activation. - `loadGen` gates the state assignments, not merely `loading`. Different question from the identity fence and neither implies the other — both loads can belong to the same user, which is what a workspace switch produces here. - The transient-state reset is ENUMERATED against the file's own `$state` declarations, with every non-member named and justified, so a new piece of interaction state fails the guard rather than waiting for a reviewer (CONVE-35). Evidence ---------------------------------------------------------------------- - Mutation matrix 8/8 detected, positive control green. Four of the eight are caught by a BEHAVIOURAL leg independently of the source guard, so the semantics are measured and not merely the syntax. - One mutant (L6, a stale `finally` releasing the activate gate) survived its first instrument. The instrument was fixed rather than the mutant dropped — the continuation rule cannot govern a `finally`, because the success-path check always sits in its window, so the two arms are named explicitly instead of pretending a regex expresses "which check governs which statement". - `npx vitest run` from `web/`: 226 files / 3104 tests, 0 failures. - `npx svelte-check --threshold error`: 0 errors. Codex round 1 ---------------------------------------------------------------------- No P1. Three P2, all taken. - **Double loads on every identity change.** `loadData` reads `authStore.identityEpoch` synchronously via `captureIdentity()` before its first await, so the `$effect` took a dependency on the epoch and re-ran on an identity change — on top of the listener that already re-loads for that event. Eight requests where four were wanted, with `loadGen` quietly discarding half: correct on screen, a mystery in a network log. Untracked, naming the workspace as the real key, the way settings (#1370) suppresses the same class. **This is a class, and the merged collection page has it too** (#1374's listener plus a `$effect` calling `loadCollection`, which reads the epoch the same way). Out of scope here — merged code, a different surface — and reported to the lead as its own unit rather than widened into this PR. - **A fenced timer that clears nothing passed every test.** The stale-timer leg proves the toast SURVIVES a timer armed by the previous session; it says nothing about the timer doing its job when the identity is unchanged, and the source guard only looked for `identityHeld(` in the callback. Fixing a stale-write by making the write never happen is the failure this family already shipped once, as a fence that latched shut (#1372). Added: a control leg that advances the clock under an unchanged identity and requires the toast to clear, and a guard assertion that every timer still assigns `toast = null`. - **`activatePlaybook` had no driven coverage** — every behavioural leg clicked the convention button, so the near-identical playbook handler was vouched for only by source text. It now has its own control and its own click-after-the-change leg. Mutation matrix: 11/11 detected at this round, both positive controls green. Codex round 2 ---------------------------------------------------------------------- No P1. Two P2, both the same rule as round 1's and both taken — the instrument could not tell a commit correctly REFUSED from a commit that never existed. - **The load path had no driven leg at all.** The source guard permits the fence to sit after the assignments, and nothing held a load open across an identity change to notice. The implementation ordering was right; the instrument could not say so. Added a leg that holds the reload open, moves the identity, then resolves it with different content and requires that content not to be painted. - **Two of the four timers live in CATCH arms, and nothing executed them.** A dead branch carrying both `identityHeld()` and `toast = null` satisfied the source guard and the whole suite — source text vouching for a path nothing runs. This round drove the CONVENTION failure arm and the playbook success arm, plus a stale-timer leg on the convention failure path. It did NOT reach the playbook failure arm; round 3 found that, and the wording here has been corrected rather than left reading as though the catch-arm class had been closed. Per the lead's ruling, the rule those three rounds keep re-teaching is now in the shared instrument's header (`identityFenceSource.ts`), so surfaces 4-7 meet it before writing their first assertion: every rule in that core asks a REFUSAL question, none can distinguish refusal from absence, and a surface therefore owes one case that keeps USING the page after an identity change, one unchanged-identity case per fenced commit proving it still happens, and a driven leg per handler rather than per handler SHAPE. Mutation matrix: 14/14 detected at this round, three positive controls green. Codex round 3 ---------------------------------------------------------------------- No P1, and no correctness defect in the page code at all — both findings are coverage, both on the playbook path, and one of them is that round 2's own write-up overclaimed. - `activatePlaybook`'s LATE-RESPONSE fence had no driven leg. Round 2 gave that handler a control and a click-after-the-change leg, which between them prove the request starts and that a preflight refusal works, and say nothing about the fence after the await. Removing that fence left the entire suite passing. - The playbook FAILURE timer — the fourth of four — still had no driven leg. Round 2 named "two of the four timers live in catch arms" as the problem and then drove one of them. The prose above is corrected in place; a write-up that describes a class and fixes half of it is how a reviewer stops being able to trust the next one. Both now have driven legs, and both mutants (dropping the post-await fence; unfencing the playbook failure timer) are detected. Round 3 also confirmed the load-window leg discriminates: it starts from a rendered baseline, holds a second load, supplies unique content, and moves the epoch before resolving, so without the fence that content would be painted. Mutation matrix: 17/17 detected, four positive controls green. Full suite: 226 files / 3114 tests, 0 failures; svelte-check 0 errors. Claude-Session: https://claude.ai/code/session_01PDYjP67KTAcqpFpo3ijAAA
This commit is contained in:
xarmian
@@ -1,11 +1,58 @@
|
||||
<script lang="ts">
|
||||
import { onDestroy, untrack } from 'svelte';
|
||||
import { page } from '$app/state';
|
||||
import { api } from '$lib/api/client';
|
||||
import { authStore } from '$lib/stores/auth.svelte';
|
||||
import { createScrollRestoration } from '$lib/scroll/restore.svelte';
|
||||
import Chip from '$lib/components/common/Chip.svelte';
|
||||
import { statusColor } from '$lib/utils/fieldColors';
|
||||
import type { LibraryCategory, LibraryConvention, PlaybookCategory, LibraryPlaybook, Item } from '$lib/types';
|
||||
|
||||
/**
|
||||
* IDENTITY FENCE — surface 3 of 7 (BUG-3084). Every async commit point on
|
||||
* this page is checked against the signed-in identity, so a response or a
|
||||
* click that lands after a sign-in or sign-out cannot commit the previous
|
||||
* session's data. Same shape as the collection page (#1372/#1374), settings
|
||||
* (#1370) and the roles board (#1375); the rules below are the family's,
|
||||
* carried here rather than re-derived.
|
||||
*
|
||||
* WHAT IS DIFFERENT HERE. The population is 7, not the 3 a function-level
|
||||
* count sees: four DEFERRED TIMERS (`setTimeout(… , 3000)` clearing the
|
||||
* toast) are commit points that fire long after their handler returned, and
|
||||
* the filed survey missed all four (BUG-3084 checkpoint 6).
|
||||
*
|
||||
* `activateConvention` / `activatePlaybook` COMPOSE A WRITE from state
|
||||
* chosen under the previous identity — the `convention` / `playbook` the
|
||||
* user clicked came from a list `loadData` fetched then. A click after an
|
||||
* identity change is the current epoch, so every entry capture passes; the
|
||||
* question that catches it is `pageIdentityHeld()`, exactly as on the roles
|
||||
* board's drag handlers.
|
||||
*
|
||||
* `identityHeld(captured)` takes a handler's own ENTRY capture, because
|
||||
* `loadData` re-stamps the page epoch and a handler comparing against that
|
||||
* can be defeated by a concurrent load.
|
||||
*
|
||||
* `pageIdentityHeld()` asks whether this PAGE still belongs to the
|
||||
* signed-in user.
|
||||
*/
|
||||
// `$state` for the same reason the roles board needs it: `pageIdentityHeld()`
|
||||
// is read from an `$effect` below, and an untracked value would leave that
|
||||
// effect with whichever answer it saw first.
|
||||
let identityEpochAtLoad = $state(authStore.identityEpoch);
|
||||
|
||||
/** The epoch as of now — captured at a handler's entry, before its awaits. */
|
||||
function captureIdentity(): number {
|
||||
return authStore.identityEpoch;
|
||||
}
|
||||
|
||||
function identityHeld(captured: number): boolean {
|
||||
return authStore.identityEpoch === captured;
|
||||
}
|
||||
|
||||
function pageIdentityHeld(): boolean {
|
||||
return authStore.identityEpoch === identityEpochAtLoad;
|
||||
}
|
||||
|
||||
let wsSlug = $derived(page.params.workspace ?? '');
|
||||
let username = $derived(page.params.username ?? '');
|
||||
|
||||
@@ -54,11 +101,66 @@
|
||||
return convention.surfaces?.join(', ') || 'all';
|
||||
}
|
||||
|
||||
/**
|
||||
* Every piece of TRANSIENT INTERACTION STATE on this page — something a
|
||||
* click started and a later response or timer would finish. Distinct from
|
||||
* the page's DATA (the four collections `loadData` replaces) and from its
|
||||
* identity bookkeeping.
|
||||
*
|
||||
* The source guard holds this against the file's own `$state` declarations,
|
||||
* so a new piece of interaction state cannot arrive without being
|
||||
* dispositioned here (BUG-3084, the roles board's checkpoint-14 lesson
|
||||
* carried forward rather than re-learned).
|
||||
*/
|
||||
function resetTransientState() {
|
||||
activatingTitle = null;
|
||||
toast = null;
|
||||
}
|
||||
|
||||
/**
|
||||
* RE-LOAD ON AN IDENTITY CHANGE. The `$effect` below is keyed on `wsSlug`
|
||||
* alone, so it does NOT re-run when the identity moves — and
|
||||
* `routes/+layout.svelte` deliberately does not reload on an anonymous ->
|
||||
* signed-in transition. Without this the page keeps a dead epoch and every
|
||||
* `pageIdentityHeld()` is false for ever: both activate handlers go
|
||||
* silently inert and never recover (#1372's regression, repaired in #1374).
|
||||
*
|
||||
* The reset runs FIRST: `activatingTitle` gates both handlers at their very
|
||||
* first line, so a stale value latches them shut, and `toast` would keep the
|
||||
* previous session's message on screen for whoever is signed in now.
|
||||
*/
|
||||
const stopIdentityWatch = authStore.onIdentityChange(() => {
|
||||
resetTransientState();
|
||||
if (wsSlug) void loadData(wsSlug);
|
||||
});
|
||||
onDestroy(stopIdentityWatch);
|
||||
|
||||
$effect(() => {
|
||||
if (wsSlug) loadData(wsSlug);
|
||||
// UNTRACKED (codex round 1 [P2]). `loadData` reads `authStore.identityEpoch`
|
||||
// synchronously via `captureIdentity()` before its first await, so
|
||||
// without this the effect takes a dependency on the epoch and re-runs
|
||||
// when the identity moves — on top of the listener above, which already
|
||||
// re-loads for exactly that event. Two loads, eight requests, and
|
||||
// `loadGen` quietly discarding half of them: correct on screen, wasteful
|
||||
// on the wire, and the kind of thing that reads as a mystery in a
|
||||
// network log.
|
||||
//
|
||||
// The key this effect is FOR is the workspace. Settings (#1370) suppresses
|
||||
// the same class the same way; naming the real key is what makes the
|
||||
// suppression safe rather than a blanket silencing.
|
||||
const ws = wsSlug;
|
||||
if (ws) untrack(() => loadData(ws));
|
||||
});
|
||||
|
||||
// Bumped by every `loadData()`. The identity fence and this ask different
|
||||
// questions and neither implies the other: the fence asks whether the
|
||||
// SIGNED-IN USER changed, this asks whether a NEWER LOAD for the same user
|
||||
// already landed. A workspace switch produces exactly the second.
|
||||
let loadGen = 0;
|
||||
|
||||
async function loadData(ws: string) {
|
||||
const epochAtEntry = captureIdentity();
|
||||
const myLoad = ++loadGen;
|
||||
loading = true;
|
||||
try {
|
||||
const [libraryRes, playbookRes, existingConventions, existingPlaybooks] = await Promise.all([
|
||||
@@ -67,47 +169,121 @@
|
||||
api.items.listByCollection(ws, 'conventions', { all: true }).catch(() => [] as Item[]),
|
||||
api.items.listByCollection(ws, 'playbooks', { all: true }).catch(() => [] as Item[]),
|
||||
]);
|
||||
if (!identityHeld(epochAtEntry)) return;
|
||||
if (myLoad !== loadGen) return;
|
||||
categories = libraryRes.categories;
|
||||
playbookCategories = playbookRes.categories;
|
||||
// THE SHARPEST READS ON THIS PAGE. These two sets are what the
|
||||
// activate handlers consult to decide whether a library entry is
|
||||
// already in the workspace, so a set belonging to the previous
|
||||
// session's workspace makes the NEW user's activate button either
|
||||
// dead or duplicating.
|
||||
activeConventionTitles = new Set(existingConventions.map((item) => item.title));
|
||||
activePlaybookTitles = new Set(existingPlaybooks.map((item) => item.title));
|
||||
// RE-STAMPED HERE, AFTER the data it vouches for has landed — never
|
||||
// before the await (BUG-3084, the roles board's checkpoint-14
|
||||
// lesson). `pageIdentityHeld()` means "this page's DATA belongs to
|
||||
// the signed-in user", and the activate handlers compose writes from
|
||||
// that data. Re-stamping at the top answers yes for the whole
|
||||
// round-trip while the four collections are still the previous
|
||||
// session's, so the guard would vouch for data it has not replaced.
|
||||
identityEpochAtLoad = epochAtEntry;
|
||||
} catch {
|
||||
if (!identityHeld(epochAtEntry)) return;
|
||||
if (myLoad !== loadGen) return;
|
||||
categories = [];
|
||||
playbookCategories = [];
|
||||
// CLEARED here too, because the re-stamp below vouches for whatever
|
||||
// is left and a failed load replaces neither set.
|
||||
activeConventionTitles = new Set();
|
||||
activePlaybookTitles = new Set();
|
||||
// RE-STAMPED ON THE ERROR PATH TOO: omitting it pins the page inert
|
||||
// for ever on a transient network error, which is the outage #1374
|
||||
// was opened to repair, and is the worse failure of the two.
|
||||
identityEpochAtLoad = epochAtEntry;
|
||||
} finally {
|
||||
loading = false;
|
||||
// NOT identity-fenced: this must run on every exit path or the page
|
||||
// is pinned at its skeleton, and the flag discloses nothing about
|
||||
// either user. GENERATION-gated though, so only the NEWEST load may
|
||||
// declare the page loaded.
|
||||
if (myLoad === loadGen) loading = false;
|
||||
}
|
||||
}
|
||||
|
||||
async function activateConvention(convention: LibraryConvention) {
|
||||
if (activeConventionTitles.has(convention.title) || activatingTitle) return;
|
||||
// BOTH QUESTIONS. `pageIdentityHeld()` first, for the reason the roles
|
||||
// board's drag handlers need it: the convention this writes was chosen
|
||||
// from a list `loadData` fetched under the PREVIOUS identity, and a
|
||||
// click after the change is the current epoch, so an entry capture
|
||||
// passes and the previous session's choice is activated into whatever
|
||||
// workspace is on screen now.
|
||||
if (!pageIdentityHeld()) return;
|
||||
const epochAtEntry = captureIdentity();
|
||||
activatingTitle = convention.title;
|
||||
try {
|
||||
await api.library.activate(wsSlug, convention);
|
||||
if (!identityHeld(epochAtEntry)) return;
|
||||
activeConventionTitles = new Set([...activeConventionTitles, convention.title]);
|
||||
toast = `Activated: ${convention.title}`;
|
||||
setTimeout(() => (toast = null), 3000);
|
||||
// FENCED, and the timer is the reason this surface's population is
|
||||
// 7 rather than 3: it commits 3 seconds after the handler returned,
|
||||
// by which time the identity may have moved. Clearing then would
|
||||
// wipe a toast belonging to whoever is signed in NOW. The identity
|
||||
// listener already clears `toast`, so the stale timer has nothing
|
||||
// legitimate left to do.
|
||||
setTimeout(() => {
|
||||
if (!identityHeld(epochAtEntry)) return;
|
||||
toast = null;
|
||||
}, 3000);
|
||||
} catch {
|
||||
if (!identityHeld(epochAtEntry)) return;
|
||||
toast = `Failed to activate: ${convention.title}`;
|
||||
setTimeout(() => (toast = null), 3000);
|
||||
setTimeout(() => {
|
||||
if (!identityHeld(epochAtEntry)) return;
|
||||
toast = null;
|
||||
}, 3000);
|
||||
} finally {
|
||||
activatingTitle = null;
|
||||
// FENCED, unlike `loading` above, and the asymmetry is deliberate.
|
||||
// `activatingTitle` is cleared by `resetTransientState()` on every
|
||||
// identity change, so a stale continuation clearing it again can
|
||||
// only release a gate the NEW user's own click is holding — which
|
||||
// permits a second concurrent activation.
|
||||
if (identityHeld(epochAtEntry)) activatingTitle = null;
|
||||
}
|
||||
}
|
||||
|
||||
async function activatePlaybook(playbook: LibraryPlaybook) {
|
||||
if (activePlaybookTitles.has(playbook.title) || activatingTitle) return;
|
||||
// BOTH QUESTIONS. `pageIdentityHeld()` first, for the reason the roles
|
||||
// board's drag handlers need it: the playbook this writes was chosen
|
||||
// from a list `loadData` fetched under the PREVIOUS identity, and a
|
||||
// click after the change is the current epoch, so an entry capture
|
||||
// passes and the previous session's choice is activated into whatever
|
||||
// workspace is on screen now.
|
||||
if (!pageIdentityHeld()) return;
|
||||
const epochAtEntry = captureIdentity();
|
||||
activatingTitle = playbook.title;
|
||||
try {
|
||||
await api.library.activatePlaybook(wsSlug, playbook);
|
||||
if (!identityHeld(epochAtEntry)) return;
|
||||
activePlaybookTitles = new Set([...activePlaybookTitles, playbook.title]);
|
||||
toast = `Activated: ${playbook.title}`;
|
||||
setTimeout(() => (toast = null), 3000);
|
||||
// See the timer note in activateConvention.
|
||||
setTimeout(() => {
|
||||
if (!identityHeld(epochAtEntry)) return;
|
||||
toast = null;
|
||||
}, 3000);
|
||||
} catch {
|
||||
if (!identityHeld(epochAtEntry)) return;
|
||||
toast = `Failed to activate: ${playbook.title}`;
|
||||
setTimeout(() => (toast = null), 3000);
|
||||
setTimeout(() => {
|
||||
if (!identityHeld(epochAtEntry)) return;
|
||||
toast = null;
|
||||
}, 3000);
|
||||
} finally {
|
||||
activatingTitle = null;
|
||||
// See the note in activateConvention.
|
||||
if (identityHeld(epochAtEntry)) activatingTitle = null;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,517 @@
|
||||
import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { cleanup, render, waitFor } from '@testing-library/svelte';
|
||||
import { tick } from 'svelte';
|
||||
|
||||
/**
|
||||
* BUG-3084 surface 3 — the BEHAVIOURAL half. `libraryIdentityFence.test.ts`
|
||||
* beside this file owns the POPULATION; this owns the SEMANTICS.
|
||||
*
|
||||
* THE MOVE EACH TEST MAKES: start a handler, hold its request open, move the
|
||||
* identity, then resolve, and assert the commit did not land.
|
||||
*
|
||||
* ONE CONTROL PER CASE, because "nothing was committed" is satisfied just as
|
||||
* well by a handler that never ran or a mount that failed.
|
||||
*
|
||||
* WHAT THIS SURFACE HAS that the earlier ones did not: a DEFERRED TIMER class.
|
||||
* Four `setTimeout(…, 3000)` callbacks clear the toast long after their handler
|
||||
* returned, and they are driven here with fake timers rather than reasoned
|
||||
* about — the filed survey missed all four, so they get the sharpest legs.
|
||||
*
|
||||
* And the leg every instrument set in this family was missing until #1374:
|
||||
* one case that moves the identity and then KEEPS USING the page. An
|
||||
* instrument set that only ever asks "did it refuse?" can only measure
|
||||
* under-protection, and the failure on the other side — a fence that latches
|
||||
* shut and never lifts — is invisible to all of it.
|
||||
*/
|
||||
|
||||
const auth = vi.hoisted(() => {
|
||||
let epoch = 0;
|
||||
return {
|
||||
get identityEpoch() { return epoch; },
|
||||
get userId() { return 'u1'; },
|
||||
get user() { return { id: 'u1', name: 'A', email: 'a@example.com' }; },
|
||||
bumpEpoch() { epoch++; },
|
||||
resetEpoch() { epoch = 0; },
|
||||
identityFence() { const c = epoch; return () => epoch === c; },
|
||||
listeners: [] as Array<() => void>,
|
||||
onIdentityChange(cb: () => void) {
|
||||
this.listeners.push(cb);
|
||||
return () => { this.listeners = this.listeners.filter((l) => l !== cb); };
|
||||
},
|
||||
notify() { for (const l of [...this.listeners]) l(); },
|
||||
clear() {},
|
||||
};
|
||||
});
|
||||
vi.mock('$lib/stores/auth.svelte', () => ({ authStore: auth }));
|
||||
|
||||
const libraryCalls = vi.hoisted(() => [] as Array<(v: unknown) => void>);
|
||||
const activateCalls = vi.hoisted(
|
||||
() => [] as Array<{ args: unknown[]; resolve: (v: unknown) => void; reject: (e: unknown) => void }>
|
||||
);
|
||||
// `activatePlaybook` gets its own driven legs rather than riding on the
|
||||
// convention handler's (codex round 1 [P2]). The two are near-identical, which
|
||||
// is exactly why source-text assertions alone would not notice one of them
|
||||
// going inert.
|
||||
const playbookCalls = vi.hoisted(
|
||||
() => [] as Array<{ args: unknown[]; resolve: (v: unknown) => void; reject: (e: unknown) => void }>
|
||||
);
|
||||
|
||||
vi.mock('$lib/api/client', () => ({
|
||||
api: {
|
||||
library: {
|
||||
get: vi.fn(() => new Promise((resolve) => { libraryCalls.push(resolve as (v: unknown) => void); })),
|
||||
getPlaybooks: vi.fn(async () => ({ categories: [{ name: 'agent-workflows', playbooks: [PLAYBOOK] }] })),
|
||||
activate: vi.fn((...args: unknown[]) => new Promise((resolve, reject) => {
|
||||
activateCalls.push({ args, resolve: resolve as (v: unknown) => void, reject });
|
||||
})),
|
||||
activatePlaybook: vi.fn((...args: unknown[]) => new Promise((resolve, reject) => {
|
||||
playbookCalls.push({ args, resolve: resolve as (v: unknown) => void, reject });
|
||||
})),
|
||||
},
|
||||
items: { listByCollection: vi.fn(async () => []) },
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock('$lib/scroll/restore.svelte', () => ({
|
||||
createScrollRestoration: () => ({ snapshot: { capture: () => null, restore: () => {} } }),
|
||||
}));
|
||||
|
||||
import { api } from '$lib/api/client';
|
||||
import { page } from '$app/state';
|
||||
import LibraryPage from './+page.svelte';
|
||||
|
||||
const PLAYBOOK = {
|
||||
title: 'Ship',
|
||||
description: 'd',
|
||||
content: 'body',
|
||||
category: 'agent-workflows',
|
||||
trigger: 'manual',
|
||||
scope: 'all',
|
||||
invocation_slug: 'ship',
|
||||
arguments: [],
|
||||
};
|
||||
|
||||
const CONVENTION = {
|
||||
title: 'Use conventional commits',
|
||||
description: 'd',
|
||||
content: 'body',
|
||||
category: 'git',
|
||||
trigger: 'on-commit',
|
||||
priority: 'should',
|
||||
surfaces: ['all'],
|
||||
};
|
||||
|
||||
async function mountPage() {
|
||||
libraryCalls.length = 0;
|
||||
activateCalls.length = 0;
|
||||
playbookCalls.length = 0;
|
||||
page.params = { username: 'dave', workspace: 'ws' };
|
||||
page.url = new URL('http://localhost/dave/ws/library');
|
||||
const r = render(LibraryPage);
|
||||
const resolveLibrary = await waitFor(() => {
|
||||
if (libraryCalls.length === 0) throw new Error('no library fetch yet');
|
||||
return libraryCalls[0]!;
|
||||
});
|
||||
resolveLibrary({ categories: [{ name: 'git', conventions: [CONVENTION] }] });
|
||||
await waitFor(() => {
|
||||
if (!document.querySelector('.activate-btn')) throw new Error('library not rendered yet');
|
||||
});
|
||||
return r;
|
||||
}
|
||||
|
||||
function clickActivate(): void {
|
||||
const btn = document.querySelector('.activate-btn') as HTMLButtonElement | null;
|
||||
expect(btn, 'the activate button did not render — nothing to click').not.toBeNull();
|
||||
btn!.click();
|
||||
}
|
||||
|
||||
function flipIdentity(): void {
|
||||
const before = auth.identityEpoch;
|
||||
auth.bumpEpoch();
|
||||
if (auth.identityEpoch === before) {
|
||||
throw new Error('the identity epoch did not move — this test would pass against an unfenced page');
|
||||
}
|
||||
}
|
||||
|
||||
describe('the library page stops a commit when the identity moves mid-flight', () => {
|
||||
beforeEach(() => { auth.resetEpoch(); auth.listeners.length = 0; });
|
||||
afterEach(() => { cleanup(); vi.useRealTimers(); });
|
||||
|
||||
it('CONTROL: activating a convention issues the write and shows the toast', async () => {
|
||||
await mountPage();
|
||||
clickActivate();
|
||||
const call = await waitFor(() => {
|
||||
if (activateCalls.length === 0) throw new Error('no activate call yet');
|
||||
return activateCalls[0]!;
|
||||
});
|
||||
call.resolve({});
|
||||
await waitFor(() => {
|
||||
expect(
|
||||
document.querySelector('.toast')?.textContent,
|
||||
'the control showed no toast — the legs below would measure nothing'
|
||||
).toContain('Activated');
|
||||
});
|
||||
});
|
||||
|
||||
it('a click AFTER an identity move never issues the activation at all', async () => {
|
||||
// The order an entry capture cannot see: the click IS the current epoch,
|
||||
// so every capture passes, while the `convention` it carries came from
|
||||
// the list the PREVIOUS identity loaded.
|
||||
await mountPage();
|
||||
flipIdentity();
|
||||
clickActivate();
|
||||
await tick();
|
||||
await tick();
|
||||
expect(
|
||||
activateCalls.length,
|
||||
"a convention chosen from the previous identity's library was activated into the " +
|
||||
'workspace on screen now'
|
||||
).toBe(0);
|
||||
});
|
||||
|
||||
it('an activation IN FLIGHT when the identity moves commits nothing', async () => {
|
||||
await mountPage();
|
||||
clickActivate();
|
||||
const call = await waitFor(() => {
|
||||
if (activateCalls.length === 0) throw new Error('no activate call yet');
|
||||
return activateCalls[0]!;
|
||||
});
|
||||
flipIdentity();
|
||||
call.resolve({});
|
||||
await tick();
|
||||
await tick();
|
||||
expect(
|
||||
document.querySelector('.toast'),
|
||||
"the previous session's activation reported success to whoever is signed in now"
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it('a timer armed by the previous session does not clear the NEW toast', async () => {
|
||||
// THE DEFERRED CLASS, driven rather than reasoned about. The timer is
|
||||
// armed under identity A and fires 3s later; by then the page may belong
|
||||
// to identity B and be showing B's own toast. Clearing it then is a
|
||||
// commit by a session that has ended.
|
||||
vi.useFakeTimers();
|
||||
await mountPage();
|
||||
clickActivate();
|
||||
const call = await waitFor(() => {
|
||||
if (activateCalls.length === 0) throw new Error('no activate call yet');
|
||||
return activateCalls[0]!;
|
||||
});
|
||||
call.resolve({});
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
await tick();
|
||||
expect(
|
||||
document.querySelector('.toast')?.textContent,
|
||||
'no toast was shown, so the timer under test was never armed'
|
||||
).toContain('Activated');
|
||||
|
||||
// Identity moves; the page paints a toast belonging to the NEW session.
|
||||
flipIdentity();
|
||||
const stale = document.querySelector('.toast');
|
||||
expect(stale, 'precondition: a toast is on screen when the timer fires').not.toBeNull();
|
||||
|
||||
await vi.advanceTimersByTimeAsync(3100);
|
||||
await tick();
|
||||
|
||||
// The listener is NOT notified here, deliberately: this leg is about the
|
||||
// timer's own fence, not about the reload clearing up after it.
|
||||
expect(
|
||||
document.querySelector('.toast'),
|
||||
"a timer armed by the previous session cleared a toast belonging to whoever is signed " +
|
||||
'in now — a commit by a session that has ended'
|
||||
).not.toBeNull();
|
||||
});
|
||||
|
||||
it('a LOAD in flight when the identity moves writes none of the previous data', async () => {
|
||||
// The load path had no driven leg at all (codex round 2 [P2]): the source
|
||||
// guard permits the fence to sit AFTER the assignments, and nothing held
|
||||
// a load open across an identity change to notice. The implementation
|
||||
// ordering was right; the instrument could not say so.
|
||||
await mountPage();
|
||||
const before = document.querySelectorAll('.activate-btn').length;
|
||||
expect(before, 'precondition: the first load rendered entries').toBeGreaterThan(0);
|
||||
|
||||
// A second load, held open, whose response belongs to the previous
|
||||
// identity by the time it resolves.
|
||||
auth.notify();
|
||||
await tick();
|
||||
const reload = await waitFor(() => {
|
||||
if (libraryCalls.length < 2) throw new Error('no second load yet');
|
||||
return libraryCalls[1]!;
|
||||
});
|
||||
flipIdentity();
|
||||
reload({
|
||||
categories: [
|
||||
{ name: 'git', conventions: [CONVENTION, { ...CONVENTION, title: 'Second' }] },
|
||||
],
|
||||
});
|
||||
await tick();
|
||||
await tick();
|
||||
|
||||
expect(
|
||||
document.body.textContent,
|
||||
"a load that resolved after the identity moved painted the previous session's library"
|
||||
).not.toContain('Second');
|
||||
});
|
||||
|
||||
it('the FAILURE path arms a fenced timer too, and it clears', async () => {
|
||||
// Only the success timer was driven (codex round 2 [P2]). Two of the four
|
||||
// timers live in catch arms, so a dead branch carrying both
|
||||
// `identityHeld()` and `toast = null` satisfied the source guard and the
|
||||
// whole suite — source text vouching for a path nothing executes.
|
||||
vi.useFakeTimers();
|
||||
await mountPage();
|
||||
clickActivate();
|
||||
const call = await waitFor(() => {
|
||||
if (activateCalls.length === 0) throw new Error('no activate call yet');
|
||||
return activateCalls[0]!;
|
||||
});
|
||||
call.reject(new Error('nope'));
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
await tick();
|
||||
expect(
|
||||
document.querySelector('.toast')?.textContent,
|
||||
'the failure path showed no toast — its timer was never armed, so this leg measures nothing'
|
||||
).toContain('Failed to activate');
|
||||
|
||||
await vi.advanceTimersByTimeAsync(3100);
|
||||
await tick();
|
||||
expect(
|
||||
document.querySelector('.toast'),
|
||||
"the failure path's timer never clears its toast"
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it('a FAILURE-path timer armed by the previous session does not clear the NEW toast', async () => {
|
||||
vi.useFakeTimers();
|
||||
await mountPage();
|
||||
clickActivate();
|
||||
const call = await waitFor(() => {
|
||||
if (activateCalls.length === 0) throw new Error('no activate call yet');
|
||||
return activateCalls[0]!;
|
||||
});
|
||||
call.reject(new Error('nope'));
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
await tick();
|
||||
expect(document.querySelector('.toast'), 'precondition: a toast is on screen').not.toBeNull();
|
||||
|
||||
flipIdentity();
|
||||
await vi.advanceTimersByTimeAsync(3100);
|
||||
await tick();
|
||||
expect(
|
||||
document.querySelector('.toast'),
|
||||
'a failure-path timer armed by the previous session cleared a toast belonging to whoever ' +
|
||||
'is signed in now'
|
||||
).not.toBeNull();
|
||||
});
|
||||
|
||||
it("the PLAYBOOK success path arms a fenced timer too, and it clears", async () => {
|
||||
vi.useFakeTimers();
|
||||
await mountPage();
|
||||
clickActivatePlaybook();
|
||||
await tick();
|
||||
const btn = document.querySelector('.activate-btn') as HTMLButtonElement | null;
|
||||
expect(btn, 'the playbook activate button did not render').not.toBeNull();
|
||||
btn!.click();
|
||||
const call = await waitFor(() => {
|
||||
if (playbookCalls.length === 0) throw new Error('no playbook activate call yet');
|
||||
return playbookCalls[0]!;
|
||||
});
|
||||
call.resolve({});
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
await tick();
|
||||
expect(
|
||||
document.querySelector('.toast')?.textContent,
|
||||
'the playbook success path showed no toast — its timer was never armed'
|
||||
).toContain('Activated');
|
||||
|
||||
await vi.advanceTimersByTimeAsync(3100);
|
||||
await tick();
|
||||
expect(
|
||||
document.querySelector('.toast'),
|
||||
"the playbook path's timer never clears its toast"
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it('a playbook activation IN FLIGHT when the identity moves commits nothing', async () => {
|
||||
// The late-response half of the playbook path (codex round 3 [P2]).
|
||||
// Round 2 gave the playbook handler a control and a click-after-the-change
|
||||
// leg, which between them prove the request STARTS and that a preflight
|
||||
// refusal works — and say nothing about the post-await fence. Removing
|
||||
// that fence left the whole suite passing.
|
||||
await mountPage();
|
||||
clickActivatePlaybook();
|
||||
await tick();
|
||||
const btn = document.querySelector('.activate-btn') as HTMLButtonElement | null;
|
||||
expect(btn, 'the playbook activate button did not render').not.toBeNull();
|
||||
btn!.click();
|
||||
const call = await waitFor(() => {
|
||||
if (playbookCalls.length === 0) throw new Error('no playbook activate call yet');
|
||||
return playbookCalls[0]!;
|
||||
});
|
||||
flipIdentity();
|
||||
call.resolve({});
|
||||
await tick();
|
||||
await tick();
|
||||
expect(
|
||||
document.querySelector('.toast'),
|
||||
"the previous session's playbook activation reported success to whoever is signed in now"
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it("the PLAYBOOK failure path arms a fenced timer too, and it clears", async () => {
|
||||
// The FOURTH timer, and the one round 2 claimed coverage of without
|
||||
// having it (codex round 3 [P2]). Two of the four live in catch arms;
|
||||
// round 2 drove the convention one and stopped.
|
||||
vi.useFakeTimers();
|
||||
await mountPage();
|
||||
clickActivatePlaybook();
|
||||
await tick();
|
||||
const btn = document.querySelector('.activate-btn') as HTMLButtonElement | null;
|
||||
expect(btn, 'the playbook activate button did not render').not.toBeNull();
|
||||
btn!.click();
|
||||
const call = await waitFor(() => {
|
||||
if (playbookCalls.length === 0) throw new Error('no playbook activate call yet');
|
||||
return playbookCalls[0]!;
|
||||
});
|
||||
call.reject(new Error('nope'));
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
await tick();
|
||||
expect(
|
||||
document.querySelector('.toast')?.textContent,
|
||||
'the playbook failure path showed no toast — its timer was never armed'
|
||||
).toContain('Failed to activate');
|
||||
|
||||
await vi.advanceTimersByTimeAsync(3100);
|
||||
await tick();
|
||||
expect(
|
||||
document.querySelector('.toast'),
|
||||
"the playbook failure path's timer never clears its toast"
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it('CONTROL: the timer DOES clear the toast when the identity is unchanged', async () => {
|
||||
// Pairs with the stale-timer leg above, which on its own is satisfied by
|
||||
// a timer that clears nothing at all (codex round 1 [P2]). Fixing a
|
||||
// stale-write by making the write never happen is the failure this
|
||||
// family already shipped once.
|
||||
vi.useFakeTimers();
|
||||
await mountPage();
|
||||
clickActivate();
|
||||
const call = await waitFor(() => {
|
||||
if (activateCalls.length === 0) throw new Error('no activate call yet');
|
||||
return activateCalls[0]!;
|
||||
});
|
||||
call.resolve({});
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
await tick();
|
||||
expect(document.querySelector('.toast'), 'no toast to clear').not.toBeNull();
|
||||
|
||||
await vi.advanceTimersByTimeAsync(3100);
|
||||
await tick();
|
||||
expect(
|
||||
document.querySelector('.toast'),
|
||||
'the toast never clears under an unchanged identity — the fence removed the behaviour ' +
|
||||
'instead of conditioning it'
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
// ── activatePlaybook, driven rather than asserted about ────────────────
|
||||
|
||||
function clickActivatePlaybook(): void {
|
||||
const tab = [...document.querySelectorAll('button')].find(
|
||||
(b) => b.textContent?.trim().toLowerCase().includes('playbook')
|
||||
) as HTMLButtonElement | undefined;
|
||||
expect(tab, 'the playbooks tab did not render').toBeDefined();
|
||||
tab!.click();
|
||||
}
|
||||
|
||||
it('CONTROL: activating a playbook issues its own write', async () => {
|
||||
await mountPage();
|
||||
clickActivatePlaybook();
|
||||
await tick();
|
||||
const btn = document.querySelector('.activate-btn') as HTMLButtonElement | null;
|
||||
expect(btn, 'the playbook activate button did not render').not.toBeNull();
|
||||
btn!.click();
|
||||
await waitFor(() => {
|
||||
expect(
|
||||
playbookCalls.length,
|
||||
'the playbook control issued no write — the leg below would measure nothing'
|
||||
).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
it('a playbook click AFTER an identity move never issues the activation', async () => {
|
||||
await mountPage();
|
||||
clickActivatePlaybook();
|
||||
await tick();
|
||||
flipIdentity();
|
||||
const btn = document.querySelector('.activate-btn') as HTMLButtonElement | null;
|
||||
expect(btn, 'the playbook activate button did not render').not.toBeNull();
|
||||
btn!.click();
|
||||
await tick();
|
||||
await tick();
|
||||
expect(
|
||||
playbookCalls.length,
|
||||
"a playbook chosen from the previous identity's library was activated into the workspace " +
|
||||
'on screen now'
|
||||
).toBe(0);
|
||||
});
|
||||
|
||||
it('recovers after an identity change and activates under the NEW identity', async () => {
|
||||
// "Did it ever resume?" — the opposite of every leg above. A fence that
|
||||
// latches shut and never lifts is not a safe guard, it is an outage with
|
||||
// good intentions, and it is harder to notice than what it replaced.
|
||||
await mountPage();
|
||||
|
||||
flipIdentity();
|
||||
auth.notify();
|
||||
await tick();
|
||||
|
||||
const reload = await waitFor(() => {
|
||||
if (libraryCalls.length < 2) throw new Error('the identity change did not trigger a reload');
|
||||
return libraryCalls[1]!;
|
||||
});
|
||||
reload({ categories: [{ name: 'git', conventions: [CONVENTION] }] });
|
||||
|
||||
await waitFor(() => {
|
||||
if (!document.querySelector('.activate-btn')) {
|
||||
throw new Error('the library never came back after the identity change');
|
||||
}
|
||||
});
|
||||
|
||||
const before = activateCalls.length;
|
||||
clickActivate();
|
||||
await waitFor(() => {
|
||||
expect(
|
||||
activateCalls.length,
|
||||
'the library is rendered but activating commits nothing — the fence latched shut and ' +
|
||||
'never lifted'
|
||||
).toBe(before + 1);
|
||||
});
|
||||
});
|
||||
|
||||
it('CONTROL: the identity-change reload clears the previous toast', async () => {
|
||||
// Pairs with the timer leg: that one proves the stale TIMER must not
|
||||
// clear the new toast; this proves the listener DOES clear the old one,
|
||||
// so "the toast survived" can never be read as the page doing nothing.
|
||||
await mountPage();
|
||||
clickActivate();
|
||||
const call = await waitFor(() => {
|
||||
if (activateCalls.length === 0) throw new Error('no activate call yet');
|
||||
return activateCalls[0]!;
|
||||
});
|
||||
call.resolve({});
|
||||
await waitFor(() => {
|
||||
if (!document.querySelector('.toast')) throw new Error('no toast yet');
|
||||
});
|
||||
flipIdentity();
|
||||
auth.notify();
|
||||
await tick();
|
||||
expect(
|
||||
document.querySelector('.toast'),
|
||||
"the previous session's toast is still on screen after the identity changed"
|
||||
).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,406 @@
|
||||
/**
|
||||
* BUG-3084 surface 3 — the SOURCE half. `libraryIdentityFence.svelte.test.ts`
|
||||
* beside this file owns the SEMANTICS; this owns the POPULATION.
|
||||
*
|
||||
* Separating the two is the reusable part of this family. The behavioural suite
|
||||
* can only speak for the handlers it drives; this one enumerates every commit
|
||||
* point on the page and fails when a new member arrives, which is what stops a
|
||||
* later handler being added without a fence and nobody noticing.
|
||||
*
|
||||
* Every rule here is the family's, carried from surfaces 1 and 2 rather than
|
||||
* re-derived — including the two that were learned the hard way:
|
||||
* - the re-stamp must come AFTER the state it vouches for, and on every path
|
||||
* that leaves the page usable (roles board checkpoint 14);
|
||||
* - a continuation that has LOST the identity writes no shared interaction
|
||||
* state, because the identity listener already reset it and someone else
|
||||
* owns it now (roles board, codex round 4).
|
||||
*/
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { readFenceSource, withoutCatchArms } from '../../../../test/identityFenceSource';
|
||||
|
||||
const src = readFenceSource(new URL('./+page.svelte', import.meta.url));
|
||||
const CODE = src.code;
|
||||
|
||||
describe('the library page fences every async commit point', () => {
|
||||
it('declares the page-level fence helpers, re-stamped after the load lands', () => {
|
||||
expect(
|
||||
CODE,
|
||||
'identityEpochAtLoad is not $state, so the $effect reading pageIdentityHeld() would not ' +
|
||||
're-run when the epoch is re-stamped'
|
||||
).toMatch(/let\s+identityEpochAtLoad\s*=\s*\$state\(authStore\.identityEpoch\)/);
|
||||
expect(CODE).toMatch(
|
||||
/function captureIdentity\(\)\s*:\s*number\s*\{[\s\S]*?return authStore\.identityEpoch/
|
||||
);
|
||||
expect(CODE).toMatch(
|
||||
/function identityHeld\(captured: number\)\s*:\s*boolean\s*\{[\s\S]*?authStore\.identityEpoch === captured/
|
||||
);
|
||||
expect(CODE).toMatch(
|
||||
/function pageIdentityHeld\(\)\s*:\s*boolean\s*\{[\s\S]*?authStore\.identityEpoch === identityEpochAtLoad/
|
||||
);
|
||||
|
||||
// THE RE-STAMP POSITION, bounded on BOTH sides because each bound has
|
||||
// its own failure mode: too early and the guard vouches for data it has
|
||||
// not replaced; missing on a path and a transient error pins the fence
|
||||
// shut for ever.
|
||||
const load = src.asyncFunctions().get('loadData');
|
||||
expect(load, 'loadData() was renamed — re-point this guard').toBeDefined();
|
||||
|
||||
const firstAwait = load!.indexOf('await');
|
||||
expect(firstAwait, 'loadData no longer awaits — this guard measures nothing').toBeGreaterThan(-1);
|
||||
expect(
|
||||
load!.slice(0, firstAwait),
|
||||
'loadData re-stamps identityEpochAtLoad BEFORE its await: every fence reading it then ' +
|
||||
"vouches for the previous session's data for the whole round-trip"
|
||||
).not.toMatch(/identityEpochAtLoad\s*=/);
|
||||
|
||||
const tail = load!.slice(firstAwait);
|
||||
const restamp = tail.indexOf('identityEpochAtLoad =');
|
||||
expect(restamp, 'loadData never re-stamps — the fence can never recover').toBeGreaterThan(-1);
|
||||
expect(
|
||||
tail.slice(0, restamp),
|
||||
'the epoch is re-stamped before the sets the activate handlers read are replaced'
|
||||
).toContain('activeConventionTitles = new Set(');
|
||||
expect(
|
||||
(tail.match(/identityEpochAtLoad\s*=/g) ?? []).length,
|
||||
'identityEpochAtLoad is re-stamped on only one path out of loadData — the other leaves ' +
|
||||
'the page permanently fenced against its own user'
|
||||
).toBe(2);
|
||||
});
|
||||
|
||||
it('subscribes to identity changes, re-loads, resets, and unsubscribes', () => {
|
||||
expect(
|
||||
CODE,
|
||||
'the page does not subscribe to authStore.onIdentityChange. Its $effect is keyed on wsSlug ' +
|
||||
'alone, so nothing re-runs the load when the identity moves and pageIdentityHeld() ' +
|
||||
'refuses for ever after a sign-in'
|
||||
).toMatch(/authStore\.onIdentityChange\(/);
|
||||
|
||||
const sub = CODE.slice(CODE.indexOf('authStore.onIdentityChange('));
|
||||
const listener = sub.slice(0, sub.indexOf('\t});'));
|
||||
expect(listener, 'the identity-change listener does not re-run loadData').toMatch(/loadData\(/);
|
||||
expect(
|
||||
listener,
|
||||
'the listener does not reset transient interaction state: activatingTitle gates both ' +
|
||||
'activate handlers at their first line, so a stale value latches them shut'
|
||||
).toMatch(/resetTransientState\(\)/);
|
||||
expect(CODE).toMatch(/onDestroy\(stopIdentityWatch\)/);
|
||||
});
|
||||
|
||||
it('the recovery never leaves a fence vouching for state it has not replaced', () => {
|
||||
// THE FAMILY PROPERTY, satisfied two ways. Surface 1 (#1374) and
|
||||
// settings (#1370) clear before re-loading; the roles board and this
|
||||
// page re-stamp late. Asserted as a disjunction so a later surface may
|
||||
// hold it either way — but NEITHER is the failure this catches.
|
||||
const sub = CODE.slice(CODE.indexOf('authStore.onIdentityChange('));
|
||||
const listener = sub.slice(0, sub.indexOf('\t});'));
|
||||
const clearsFirst =
|
||||
/activeConventionTitles\s*=\s*new Set\(\)/.test(listener) &&
|
||||
listener.indexOf('activeConventionTitles') < listener.indexOf('loadData(');
|
||||
|
||||
const load = src.asyncFunctions().get('loadData')!;
|
||||
const firstAwait = load.indexOf('await');
|
||||
const tail = firstAwait > -1 ? load.slice(firstAwait) : '';
|
||||
const restamp = tail.indexOf('identityEpochAtLoad =');
|
||||
const restampsLate =
|
||||
firstAwait > -1 &&
|
||||
!/identityEpochAtLoad\s*=/.test(load.slice(0, firstAwait)) &&
|
||||
restamp > -1 &&
|
||||
tail.slice(0, restamp).includes('activeConventionTitles = new Set(');
|
||||
|
||||
expect(
|
||||
clearsFirst || restampsLate,
|
||||
'the identity-change reload neither clears the state the page-level fence vouches for nor ' +
|
||||
'defers the re-stamp until the new data lands. Either is fine; neither means every ' +
|
||||
"pageIdentityHeld() during the reload answers TRUE over the previous session's data."
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('fences the handlers that WRITE state the previous identity chose', () => {
|
||||
// The roles board's handleDndFinalize class. An entry capture asks "has
|
||||
// the identity moved since this work started"; these handlers' work
|
||||
// starts with a CLICK, always the current epoch, over a library entry
|
||||
// chosen against the list the PREVIOUS identity loaded.
|
||||
const MUST_HOLD_THE_PAGE = {
|
||||
activateConvention: 'activates a convention chosen from the previous list',
|
||||
activatePlaybook: 'activates a playbook chosen from the previous list',
|
||||
};
|
||||
const missing: string[] = [];
|
||||
for (const [name, why] of Object.entries(MUST_HOLD_THE_PAGE)) {
|
||||
const body = src.asyncFunctions().get(name);
|
||||
expect(body, `${name}() was renamed — re-point this guard`).toBeDefined();
|
||||
const head = body!.indexOf('await') > -1 ? body!.slice(0, body!.indexOf('await')) : body!;
|
||||
if (!head.includes('pageIdentityHeld()')) missing.push(`${name} (${why})`);
|
||||
}
|
||||
expect(
|
||||
missing,
|
||||
'these handlers write state the PREVIOUS identity chose but check only an entry capture, ' +
|
||||
'which a click after the identity change always passes'
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it('fences every DEFERRED TIMER, which is why this population is 7 and not 3', () => {
|
||||
// The four timers are the members a function-level count cannot see, and
|
||||
// the filed survey missed all four (BUG-3084 checkpoint 6). Each fires
|
||||
// three seconds after its handler returned and writes `toast` — a commit
|
||||
// point whose whole nature is to land late.
|
||||
const timers = src.deferredTimers();
|
||||
expect(timers.length, 'the timer population changed — disposition the new member').toBe(4);
|
||||
const unfenced = timers
|
||||
.filter((t) => !/identityHeld\(/.test(t.body))
|
||||
.map((t) => t.body.slice(0, 60).replace(/\s+/g, ' '));
|
||||
expect(
|
||||
unfenced,
|
||||
'these deferred callbacks commit without checking the identity, so a timer armed by the ' +
|
||||
"previous session clears a toast belonging to whoever is signed in now"
|
||||
).toEqual([]);
|
||||
|
||||
// AND THEY MUST STILL DO THEIR JOB (codex round 1 [P2]). A fence is only
|
||||
// half the requirement: a callback that checks the identity and then
|
||||
// clears nothing satisfies the assertion above while leaving the toast
|
||||
// on screen for ever. Fixing a stale-write bug by making the write never
|
||||
// happen is the failure this family already shipped once, in the shape
|
||||
// of a fence that latched shut (#1372).
|
||||
const inert = timers
|
||||
.filter((t) => !/toast = null/.test(t.body))
|
||||
.map((t) => t.body.slice(0, 60).replace(/\s+/g, ' '));
|
||||
expect(
|
||||
inert,
|
||||
'these deferred callbacks no longer clear the toast at all — a fence that removes the ' +
|
||||
'behaviour instead of conditioning it'
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it('the identity-change listener resets every piece of transient interaction state', () => {
|
||||
// ENUMERATED rather than sampled (CONVE-35). On the roles board this
|
||||
// class produced findings in two consecutive review rounds before it was
|
||||
// enumerated; carried here so this surface does not repeat that.
|
||||
const reset = CODE.slice(CODE.indexOf('function resetTransientState()'));
|
||||
const resetBody = reset.slice(0, reset.indexOf('\n\t}'));
|
||||
|
||||
const NOT_TRANSIENT: Record<string, string> = {
|
||||
identityEpochAtLoad: 'identity bookkeeping — only loadData may write it',
|
||||
categories: 'page DATA — loadData replaces it on both arms',
|
||||
playbookCategories: 'page DATA — loadData replaces it on both arms',
|
||||
activeConventionTitles: 'page DATA — loadData replaces it on both arms',
|
||||
activePlaybookTitles: 'page DATA — loadData replaces it on both arms',
|
||||
loading: "owned by loadData's generation-guarded finally",
|
||||
activeTab:
|
||||
'a view preference read from the URL, not interaction state: it belongs to the ' +
|
||||
'viewer and carries nothing about either identity',
|
||||
};
|
||||
|
||||
const declared = [...CODE.matchAll(/^\s*let\s+(\w+)\s*(?::[^=]*)?=\s*\$state/gm)].map((m) => m[1]!);
|
||||
expect(declared.length, 'no $state declarations found — re-point this guard').toBeGreaterThan(5);
|
||||
|
||||
const undispositioned = declared.filter(
|
||||
(n) => !(n in NOT_TRANSIENT) && !new RegExp(`\\b${n}\\s*=`).test(resetBody)
|
||||
);
|
||||
expect(
|
||||
undispositioned,
|
||||
'these $state values are neither page data nor reset by resetTransientState(). Each is ' +
|
||||
'transient interaction state that survives an identity change: decide which it is and ' +
|
||||
'either reset it or name it in this guard.'
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it('a lost-identity exit writes no shared interaction state', () => {
|
||||
// The roles board's round-4 reversal, carried here so this surface never
|
||||
// acquires the defect it corrected. `resetTransientState()` owns these;
|
||||
// a stale continuation writing them again can only clobber what the NEW
|
||||
// user is doing.
|
||||
const SHARED = ['activatingTitle', 'toast'];
|
||||
const offenders: string[] = [];
|
||||
for (const [name, body] of src.asyncFunctions()) {
|
||||
if (name === 'loadData') continue;
|
||||
|
||||
for (const m of body.matchAll(/if \(!(?:page)?[Ii]dentityHeld\([^)]*\)\)\s*\{([\s\S]*?)\}/g)) {
|
||||
const arm = m[1] ?? '';
|
||||
for (const v of SHARED) {
|
||||
if (new RegExp(`\\b${v}\\s*=`).test(arm)) offenders.push(`${name}: ${v} (lost-identity arm)`);
|
||||
}
|
||||
}
|
||||
|
||||
// A continuation — after an await — must be checked SINCE that await.
|
||||
// "Since the last await", not "on the same line": both spellings are
|
||||
// correct and the family uses both (CONVE-30).
|
||||
for (const m of body.matchAll(
|
||||
new RegExp(`\\b(${SHARED.join('|')})\\s*=\\s*(?:null|false|true)`, 'g')
|
||||
)) {
|
||||
const at = m.index ?? 0;
|
||||
const before = body.slice(0, at);
|
||||
const lastAwait = before.lastIndexOf('await ');
|
||||
if (lastAwait === -1) continue;
|
||||
if (/identityHeld\(/.test(body.slice(lastAwait, at + m[0].length))) continue;
|
||||
offenders.push(`${name}: ${m[1]} (continuation with no identity check since its await)`);
|
||||
}
|
||||
}
|
||||
expect(
|
||||
offenders,
|
||||
'these lost-identity exits write shared interaction state that the identity listener has ' +
|
||||
'already reset, so they can clobber what the NEW user is doing'
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it("a stale activation does not release the new identity's activate gate", () => {
|
||||
// The `finally` counterpart, and it needs its own assertion because the
|
||||
// continuation rule above CANNOT see it: that rule asks whether an
|
||||
// identity check appears between the last await and the write, and a
|
||||
// `finally` always has the success-path check somewhere in that window.
|
||||
// The window is wide enough to be satisfied by a check that does not
|
||||
// govern this write. Rather than pretend a regex can express "which
|
||||
// check governs which statement", the two `finally` arms are named.
|
||||
// (Found by mutant L6 surviving the first version of these assertions —
|
||||
// the same way the roles board's M22 was found.)
|
||||
//
|
||||
// A deliberate ASYMMETRY with `loading` in loadData, which stays
|
||||
// unconditional: nothing else clears `loading` and pinning the page at
|
||||
// its skeleton is the worse failure, whereas `activatingTitle` IS
|
||||
// cleared by `resetTransientState()` on every identity change — so a
|
||||
// stale continuation clearing it again can only release a gate the NEW
|
||||
// user's own click is holding, permitting a second concurrent activation.
|
||||
for (const name of ['activateConvention', 'activatePlaybook']) {
|
||||
const body = src.asyncFunctions().get(name);
|
||||
expect(body, `${name}() was renamed — re-point this guard`).toBeDefined();
|
||||
const fin = body!.indexOf('} finally {');
|
||||
expect(fin, `${name} no longer has a finally arm — re-point this guard`).toBeGreaterThan(-1);
|
||||
expect(
|
||||
body!.slice(fin),
|
||||
`${name} clears activatingTitle unconditionally, so a stale request releases the ` +
|
||||
"new user's activate gate behind their back"
|
||||
).toMatch(/if \(identityHeld\(epochAtEntry\)\) activatingTitle = null/);
|
||||
}
|
||||
});
|
||||
|
||||
it('the load effect is keyed on the workspace, not on the identity', () => {
|
||||
// `loadData` reads `authStore.identityEpoch` synchronously before its
|
||||
// first await, so an un-untracked effect takes a dependency on the epoch
|
||||
// and re-runs on every identity change — on top of the listener that
|
||||
// already re-loads for that event (codex round 1 [P2]). Correct on
|
||||
// screen, double on the wire.
|
||||
const eff = CODE.slice(CODE.indexOf('$effect(() => {'));
|
||||
const body = eff.slice(0, eff.indexOf('\t});'));
|
||||
expect(
|
||||
body,
|
||||
'the load effect calls loadData tracked, so it re-runs on an identity change as well as ' +
|
||||
'a workspace change and every load is issued twice'
|
||||
).toMatch(/untrack\(\(\) => loadData\(/);
|
||||
});
|
||||
|
||||
it('a stale load may not write the page it no longer owns', () => {
|
||||
// Different question from the identity fence, and neither implies the
|
||||
// other: both loads can belong to the SAME user, which is exactly what a
|
||||
// workspace switch produces on this page.
|
||||
const load = src.asyncFunctions().get('loadData')!;
|
||||
expect(load, 'loadData does not take a load generation').toMatch(/const myLoad = \+\+loadGen/);
|
||||
expect(
|
||||
(load.match(/if \(myLoad !== loadGen\) return;/g) ?? []).length,
|
||||
'loadData does not gate its state writes on the load generation, so a stale load can ' +
|
||||
'overwrite a newer one for the same user'
|
||||
).toBeGreaterThanOrEqual(2);
|
||||
expect(load.indexOf('if (myLoad !== loadGen) return;')).toBeLessThan(
|
||||
load.indexOf('categories = libraryRes.categories')
|
||||
);
|
||||
expect(
|
||||
load,
|
||||
'loadData clears `loading` unconditionally, so a stale load can declare a newer one finished'
|
||||
).toMatch(/if \(myLoad === loadGen\) loading = false/);
|
||||
});
|
||||
|
||||
it('the error path clears the state its own re-stamp will vouch for', () => {
|
||||
const load = src.asyncFunctions().get('loadData')!;
|
||||
const cat = load.indexOf('} catch {');
|
||||
expect(cat, 'loadData no longer has a catch arm — re-point this guard').toBeGreaterThan(-1);
|
||||
const arm = load.slice(cat, load.indexOf('} finally {', cat));
|
||||
expect(arm, 'a failed load leaves the previous identity\'s convention set in place').toMatch(
|
||||
/activeConventionTitles = new Set\(\)/
|
||||
);
|
||||
expect(arm, 'a failed load leaves the previous identity\'s playbook set in place').toMatch(
|
||||
/activePlaybookTitles = new Set\(\)/
|
||||
);
|
||||
expect(arm, 'a failed load never re-stamps, pinning the fence shut for ever').toMatch(
|
||||
/identityEpochAtLoad\s*=/
|
||||
);
|
||||
expect(arm.indexOf('activeConventionTitles = new Set()')).toBeLessThan(
|
||||
arm.indexOf('identityEpochAtLoad =')
|
||||
);
|
||||
});
|
||||
|
||||
it('enumerates the population it claims to cover', () => {
|
||||
// COUNTS ASSERTED so a new member cannot arrive unnoticed. These are the
|
||||
// widened instrument's four populations; the originally filed survey saw
|
||||
// only the first and reported 3 (BUG-3084 checkpoint 6).
|
||||
expect(src.asyncFunctions().size, 'a top-level async function was added or removed').toBe(3);
|
||||
expect(src.markupAsyncArrows(), 'an inline async arrow appeared in the markup').toHaveLength(0);
|
||||
expect(src.nestedAsyncCallbacks(), 'an async callback arrived').toHaveLength(0);
|
||||
expect(src.deferredTimers(), 'a setTimeout/setInterval arrived or left').toHaveLength(4);
|
||||
});
|
||||
|
||||
it('captures the identity at ENTRY in every async handler, before its first await', () => {
|
||||
const missing: string[] = [];
|
||||
for (const [name, body] of src.asyncFunctions()) {
|
||||
if (!body.includes('await')) continue;
|
||||
if (!body.slice(0, body.indexOf('await')).includes('captureIdentity()')) missing.push(name);
|
||||
}
|
||||
expect(
|
||||
missing,
|
||||
'these async handlers do not capture the identity before their first await. A fence ' +
|
||||
'comparing against anything else can be defeated by a concurrent load re-stamping it.'
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it('never lets a handler compare against the PAGE load epoch after an await', () => {
|
||||
// `pageIdentityHeld()` is legitimate BEFORE the await — it is the
|
||||
// "does this page's data belong to the signed-in user" question. After
|
||||
// an await it is the wrong instrument, because a concurrent load can
|
||||
// re-stamp the value it reads to exactly what the handler expects.
|
||||
const offenders: string[] = [];
|
||||
for (const [name, body] of src.asyncFunctions()) {
|
||||
if (name === 'loadData') continue;
|
||||
const first = body.indexOf('await');
|
||||
if (first === -1) continue;
|
||||
if (body.slice(first).includes('pageIdentityHeld()')) offenders.push(name);
|
||||
}
|
||||
expect(
|
||||
offenders,
|
||||
'these handlers compare against the PAGE load epoch after an await, which a concurrent ' +
|
||||
'load can re-stamp to the value the handler is about to read'
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it('checks the fence at least once per await in every async handler', () => {
|
||||
const short: string[] = [];
|
||||
for (const [name, body] of src.asyncFunctions()) {
|
||||
const awaits = body.split('await ').length - 1;
|
||||
if (awaits === 0) continue;
|
||||
const checks =
|
||||
body.split('identityHeld(').length - 1 + body.split('pageIdentityHeld()').length - 1;
|
||||
if (checks < awaits) short.push(`${name} (${checks} checks for ${awaits} awaits)`);
|
||||
}
|
||||
expect(
|
||||
short,
|
||||
'these async handlers have fewer identity checks than awaits, so at least one continuation ' +
|
||||
'commits unguarded'
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it('guards the SUCCESS continuation, not merely the failure one', () => {
|
||||
// The window is the body with every CATCH ARM EXCISED. Splitting at the
|
||||
// first `} catch` reports correct code as unguarded when a handler
|
||||
// commits after its try/catch; balancing arms separately does the same
|
||||
// for an await in a nested try. Both were tried on surface 1.
|
||||
const unguarded: string[] = [];
|
||||
for (const [name, body] of src.asyncFunctions()) {
|
||||
const successOnly = withoutCatchArms(body);
|
||||
const firstAwait = successOnly.indexOf('await ');
|
||||
if (firstAwait === -1) continue;
|
||||
if (!/identityHeld\(|pageIdentityHeld\(\)/.test(successOnly.slice(firstAwait))) {
|
||||
unguarded.push(name);
|
||||
}
|
||||
}
|
||||
expect(
|
||||
unguarded,
|
||||
'these handlers have no identity check between their first await and the end of their try ' +
|
||||
'block, so the path a SUCCESSFUL request takes commits unguarded'
|
||||
).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -24,6 +24,41 @@
|
||||
// worse than one that is red.
|
||||
import { readFileSync } from 'node:fs';
|
||||
|
||||
/**
|
||||
* REFUSAL IS NOT ABSENCE — the rule every surface in this family has now paid
|
||||
* for at least once, placed here so surfaces 4-7 meet it before they write
|
||||
* their first assertion (lead's ruling, BUG-3084 day 68).
|
||||
*
|
||||
* Every rule in this file asks a REFUSAL question: is a fence present, is it
|
||||
* positioned before the commit, is there one per await, does it read the right
|
||||
* epoch. Not one of them can tell a commit that was correctly REFUSED from a
|
||||
* commit that never existed — and the second is a defect, not a fix.
|
||||
*
|
||||
* What that has actually cost, three times, in three costumes:
|
||||
* - #1372 shipped a fence that could detect the bad state and never leave it.
|
||||
* Ten guard assertions, nine behavioural legs and a 10/10 mutation matrix,
|
||||
* and not one asked whether the page still WORKED afterwards. Every mutant
|
||||
* weakened a fence, so the matrix could only ever measure under-protection.
|
||||
* - Surface 3's deferred timers: a callback that checked the identity and
|
||||
* then cleared nothing satisfied the guard AND the whole behavioural suite.
|
||||
* "Fix the stale write by making the write never happen" was passing.
|
||||
* - Surface 3's `activatePlaybook`: a handler near-identical to its sibling
|
||||
* was vouched for by source text alone, because every leg clicked the
|
||||
* sibling's button. Near-identical is exactly when a source assertion feels
|
||||
* sufficient and is not.
|
||||
*
|
||||
* So a surface using this core owes, alongside its refusal rules:
|
||||
* 1. one case that moves the identity and then KEEPS USING the page;
|
||||
* 2. for each fenced commit, one case under an UNCHANGED identity proving the
|
||||
* commit still happens;
|
||||
* 3. a driven leg per handler, not per handler SHAPE — including the failure
|
||||
* arms, whose code a source guard reads and nothing executes.
|
||||
*
|
||||
* A guard that latches into the safe state is not a safe guard; it is an outage
|
||||
* with good intentions, and it is harder to notice than the flapping it
|
||||
* replaced, precisely because nothing looks wrong.
|
||||
*/
|
||||
|
||||
export interface EnumeratedBlock {
|
||||
/** How this block is named in a failure message. */
|
||||
label: string;
|
||||
|
||||
Reference in New Issue
Block a user