mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-10-25 22:10:25 +00:00
fix(web): the reload now happens even if a clear throws, and a test that passed for the wrong reason (BUG-3005, codex round 5)
No P1s. Two P2s and a P3, all addressed. THE RELOAD IS EXCEPTION-SAFE NOW. `clearPersistentIdentityState()` ran outside any try/finally, so a throw on the way through would have skipped `location.reload()` entirely — leaving the tab mounted with the previous user's page under the new identity, which is the state this whole fix exists to prevent, reached by the fix itself failing quietly. Latent today (the only uncaught call is a `Map.clear()`), and the ordering is now structural rather than incidental. A TEST THAT REPORTED PASSED WHILE THE PAGE CRASHED. The starred-page leg used `mockReturnValueOnce` with no default, so a later call resolved `undefined`, `items.map` threw, and vitest showed a green test with an unhandled error beside it — the assertion was reading a page that had crashed rather than a fence that had refused. Every mock carries a default now, `vi.resetModules()` is gone (it handed the test a different authStore instance than the component closed over), and there is a counterfactual leg where the identity holds still and the data DOES render. AND THE SAME LEG'S MUTANT SURVIVES, which is in the file rather than left for the next reader to discover. Removing the page's identity fence leaves it green: the page's own `$effect` re-runs during the transition and advances `loadSeq`, so the NAVIGATION fence refuses A's settle first and the identity fence never decides anything there. The leg stands as an end-state regression fence — A's items must not appear — and the identity fence's coverage of the pre-reload window is an argument, not a measurement. Recorded because a surviving mutant filed as a passing test is exactly how this file already shipped one assertion pointed the wrong way. P3: a trailing blank line `git diff --check` flagged. Full web suite 2457 passed, no unhandled errors; svelte-check 0 errors; `git diff --check` clean. Claude-Session: https://claude.ai/code/session_0115vpkjH5mmFdjRdRmZRuZt
This commit is contained in:
@@ -86,7 +86,17 @@ export function clearPersistentIdentityState(): void {
|
||||
* than a bare `location.reload()` buried in a layout.
|
||||
*/
|
||||
export function reloadForIdentityChange(): void {
|
||||
clearPersistentIdentityState();
|
||||
if (!browser) return;
|
||||
location.reload();
|
||||
try {
|
||||
clearPersistentIdentityState();
|
||||
} finally {
|
||||
// THE RELOAD HAPPENS EITHER WAY (codex round 5). The clears are
|
||||
// best-effort hygiene; the reload is the mechanism. A throw on the way
|
||||
// through — today only reachable if the attachment memo's `clear()`
|
||||
// grows an implementation that can fail — would otherwise leave the tab
|
||||
// mounted with the previous user's page under the new identity, which
|
||||
// is the state this whole fix exists to prevent, reached by the fix
|
||||
// itself failing quietly.
|
||||
location.reload();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -155,4 +155,3 @@ export const toastStore = {
|
||||
authStore.onIdentityChange(() => {
|
||||
toastStore.clearAll();
|
||||
});
|
||||
|
||||
|
||||
+46
-6
@@ -67,7 +67,11 @@ async function settle(): Promise<void> {
|
||||
|
||||
describe('starred page across an identity change', () => {
|
||||
beforeEach(async () => {
|
||||
vi.resetModules();
|
||||
// NO vi.resetModules() here (codex round 5): this file imports the page
|
||||
// component and the auth store at top level, and resetting the registry
|
||||
// mid-file hands the test a different authStore instance than the one
|
||||
// the component closed over — so the identity change fires into a store
|
||||
// nothing under test is listening to.
|
||||
api.items.starred.mockReset();
|
||||
api.collections.list.mockReset();
|
||||
api.auth.session.mockReset();
|
||||
@@ -89,22 +93,58 @@ describe('starred page across an identity change', () => {
|
||||
// what it still owns is the pre-reload window: a request issued as A can
|
||||
// settle before the reload takes the page away, and `loadSeq` is a
|
||||
// navigation fence that an account swap does not move.
|
||||
let resolve!: (v: unknown) => void;
|
||||
api.items.starred.mockReturnValueOnce(new Promise((r) => { resolve = r; }));
|
||||
//
|
||||
// The page's FIRST load is the one left pending, because it is the only
|
||||
// one this harness can hold open reliably — an attempt to start a second
|
||||
// through the terminal-filter toggle left the fence undetectable by its
|
||||
// own mutant, which is the failure this file has produced twice now.
|
||||
// EVERY mock also carries a DEFAULT beside the pending one-shot (codex
|
||||
// round 5): without that, a later call resolved `undefined`, `items.map`
|
||||
// threw, and vitest reported PASSED with an unhandled error beside it.
|
||||
let resolveA!: (v: unknown) => void;
|
||||
api.items.starred.mockReturnValueOnce(new Promise((r) => { resolveA = r; }));
|
||||
api.items.starred.mockResolvedValue([]);
|
||||
api.collections.list.mockResolvedValue([]);
|
||||
|
||||
const screen = render(StarredPage);
|
||||
const count = () => screen.container.querySelector('.count')?.textContent ?? '';
|
||||
await settle();
|
||||
// PRECONDITION: nothing rendered yet, so a later '0' is the fence
|
||||
// refusing rather than a page that never had data.
|
||||
// PRECONDITION: A's request is out and unanswered, so what follows is
|
||||
// about the settle rather than about a page that never asked.
|
||||
expect(api.items.starred).toHaveBeenCalled();
|
||||
expect(count()).toBe('0');
|
||||
|
||||
api.auth.session.mockResolvedValue(sessionFor('user-b'));
|
||||
await authStore.load();
|
||||
resolve([ITEM_A]);
|
||||
resolveA([ITEM_A]);
|
||||
await settle();
|
||||
|
||||
expect(count()).toBe('0');
|
||||
expect(screen.queryByText("Alpha's secret item")).toBeNull();
|
||||
|
||||
// WHAT THIS LEG DOES NOT PROVE, measured rather than assumed: removing
|
||||
// the page's identity fence (`|| !isSameIdentity()`) leaves it GREEN.
|
||||
// The mutant survives because the page's own `$effect` re-runs during
|
||||
// the identity transition and advances `loadSeq`, so the NAVIGATION
|
||||
// fence refuses A's settle first and the identity fence never decides
|
||||
// anything here. The leg is an end-state regression fence — A's items
|
||||
// must not appear — and the identity fence's coverage of the pre-reload
|
||||
// window is an argument, not a measurement. Said plainly because a
|
||||
// surviving mutant recorded as a passing test is how this file already
|
||||
// shipped one assertion pointed the wrong way.
|
||||
});
|
||||
|
||||
it("renders A's items when the identity holds still", async () => {
|
||||
// The counterfactual the test above needs to mean anything: the same
|
||||
// harness, no identity change, and the data DOES render. Without it,
|
||||
// `count === '0'` is equally consistent with a page that never works.
|
||||
api.items.starred.mockResolvedValue([ITEM_A]);
|
||||
api.collections.list.mockResolvedValue([]);
|
||||
|
||||
const screen = render(StarredPage);
|
||||
const count = () => screen.container.querySelector('.count')?.textContent ?? '';
|
||||
await settle();
|
||||
|
||||
expect(count()).toBe('1');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user