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:
xarmian
2026-09-10 22:45:40 +00:00
parent 95416e78e9
commit 3aebbb58f0
3 changed files with 58 additions and 9 deletions
+12 -2
View File
@@ -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();
}
}
-1
View File
@@ -155,4 +155,3 @@ export const toastStore = {
authStore.onIdentityChange(() => {
toastStore.clearAll();
});
@@ -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');
});
});