fix(web): four singleton stores kept the previous user's data across an identity change (BUG-3005) (#1325)

* feat(web): an ORDERED identity epoch, so a store can tell "still me" from "me again" (BUG-3005, unit 1)

Four singleton stores hold per-user state and none of them can currently ask
the question they need to ask when a response comes back: was this issued
under the identity that is signed in now.

The tree's existing answer is a captured user id compared at settle
(`workspace.svelte.ts`). That answers "is this a DIFFERENT user", which is a
weaker question and comes apart on sign out of A, in as B, back in as A: a
request issued during the first A session settles, compares equal, and commits
data from a session that has ended. An ordinal cannot be confused that way —
the epoch it captured is strictly lower than the one it settles into.

`authStore.identityEpoch` bumps on exactly the transitions `onIdentityChange`
already fires on, and `identityFence()` is the capture-and-compare helper, so
four stores do not hand-roll it four ways. That is not a hypothetical: this
store and `workspace.svelte.ts` hand-rolled the same three parts of a
single-flight loader and drifted three times in one afternoon, which is why
`singleFlight.ts` exists.

Two orderings are load-bearing and each has a test:

- NOT bumped on the baseline, matching `notifyIdentityChange`'s own early
  return. The first resolution of the session is not a change from anything,
  and a fence captured before an identity existed has nothing stale to refuse.
- Bumped BEFORE the listeners, not after. A listener's expected shape is "drop
  state, reload it", and that reload captures a fence while the listener is
  still running — under the old epoch it would refuse its own response.

Distinct from `generation`, which bumps only on `clear()` and fences this
module's own /auth/session fetches; a sign-in as a different user through
`load()` moves the identity without touching it, so it cannot serve as the
epoch.

Five tests. Mutants: fence on the user id instead of the epoch (2 fail,
including the A->B->A straggler this exists for), bump after the listeners
(1 fail, alone), bump on the baseline too (3 fail). Full web suite 2416 passed.

No consumer yet — the stores are unit 2. Nothing changes behaviour on its own.

* fix(web): starredStore drops the previous user's stars, and refuses a settle from a session that has ended (BUG-3005, unit 2a)

`starredStore` holds a PER-USER set and was only ever cleared inside `load()`,
which is keyed by workspace slug. A same-route account swap starts no new load,
so A's starred ids stayed rendered inside B's session with no request having
been made for them.

Two halves, because they fail independently:

- an `authStore.onIdentityChange` subscription clears the store when nothing is
  racing at all, which is the common case;
- an identity fence on every settle, for the case where something is. Both
  arms of `load` commit — the catch arm publishes an empty set and flips
  `loaded` — so fencing only the success path would leave the store telling B
  that B's stars are loaded.

The navigation fence stays and is not subsumed: a workspace switch under one
identity moves no epoch, and an account swap moves no workspace.

FIVE TESTS, AND ONE OF THEM TOOK THREE TRIES TO BE WORTH ANYTHING. The
toggle-revert mutant SURVIVED twice:

- first because the identity-change reset blanks `currentWs`, so the workspace
  fence was doing the work — fixed by having B sign back into the SAME
  workspace before the revert fires;
- then because the test reverted a failed STAR, which is a delete, and a
  delete is invisible in an empty set. The write that actually leaks is the
  revert of a failed UNSTAR, which is an ADD into the next user's set.

Both are recorded in the test's own comment, because "this assertion is
pointed the wrong way" is not visible from a green run. Mutants now: drop the
subscription (2 fail), drop the fence on load's success arm (1), on its catch
arm (1), on the toggle revert (1, and only against the corrected test).

All 291 store tests pass.

* fix(web): collectionStore drops the previous user's collections and items, and fences every settle (BUG-3005, unit 2b)

`collections`, `items`, `itemsWorkspace`, `collectionsWorkspace` and
`activeItem` all belong to a user and none of them is keyed by one. The loader
is keyed by workspace SLUG and the workspace layout drives it from a slug-keyed
effect, so a same-route sign-in as a different user starts no new load at all —
Roles, the item picker and the editor's link gate then render A's collection
names and item titles inside B's session.

The store's only fence was `isLatest`, which is a NAVIGATION fence and cannot
see this: a same-route account swap supersedes nothing, so A's load is still
the latest one for that workspace and commits.

Both halves, as with starredStore:

- an `onIdentityChange` subscription clearing the store, for the ordinary case
  where nothing is racing;
- an identity fence captured before the first await on `loadCollections`,
  `loadItems` (both arms — they assign through different branches and stamp
  `itemsWorkspace` differently) and `loadItem`.

`loadItem` returns NULL rather than the fetched item when the identity moved.
The return value matters as much as the store write: the caller renders what it
is handed, so handing back A's item puts it on B's screen with `activeItem`
untouched.

The FRESHNESS STAMPS clear with the arrays, and that is load-bearing rather
than tidiness. Emptying `collections` while leaving `collectionsWorkspace` set
makes `collectionsAreFreshFor(ws)` answer true for an empty list — the one
answer that stops TASK-2200's recovery from re-fetching, so the new user's
sidebar would stay permanently empty. It has its own test and its own mutant.

`loadItems` and `loadItem` have no production callers today (the item list is
maintained through addItem/updateItemInList/removeItem from SSE, and
`activeItem` through setActiveItem). They are fenced anyway: they are public
API and this item names exactly that state as retained.

Seven tests, six mutants, five of them dying to exactly one test: drop the
subscription (2 fail), drop the fence on loadCollections (1), on each items arm
(1 each), make loadItem return the item anyway (1), leave the stamps set in
clear() (1). Full web suite 2428 passed.

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

* fix(web): localIndex and localSearch drop every workspace when the signed-in user changes (BUG-3005, unit 2c)

`localIndex` reset only when `bootstrap()` noticed a userId mismatch, which
makes the invalidation depend on somebody calling bootstrap. A route that does
not bootstrap the index on an identity change kept the previous user's rows in
RAM — and `localSearch`, whose only reset callers live inside `localIndex`,
kept a full-text index built from their item titles and bodies. That fourth
store is not named on the item; the enumeration this unit re-ran found it,
which is what CONVE-18 asks for.

The per-workspace shape is the other half. `reset(ws)` names one slug, so the
sign-out path dropped the workspace the layout happened to be looking at and a
tab that had visited two kept the other.

`localIndex.resetAll()` sweeps every workspace it holds and then calls
`localSearch.resetAll()`. The second call is not redundant with the per-
workspace resets it just performed: the two maps are keyed independently and
nothing keeps them in step, so an index whose workspace state is already gone
survives a sweep driven from localIndex's keys alone. It has its own test and
its own mutant.

The bootstrap mismatch check STAYS. It covers a different case — a first
bootstrap for a workspace whose cached state predates this tab's identity
signal, where nothing changed within this tab so nothing fired.

Four tests, three mutants: drop the subscription (3 fail), sweep only the first
workspace (1, alone), drop the localSearch.resetAll belt (1, alone). Full web
suite 2432 passed; svelte-check 0 errors.

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

* fix(web): the store fix routed the starred PAGE around its own filter, and three more singletons held user state (BUG-3005, codex round 1)

Round 1's largest finding is that the four stores were not the population, and
the sharpest instance is one my own fix made worse.

THE STARRED PAGE. `items` is `starredStore.loaded ? filter(fetchedItems) :
fetchedItems` — an unfiltered fallback for the pre-load window. The store's
identity reset sets `loaded = false`, so the fix routed the page around its
only filter and handed B the full `fetchedItems` array it had fetched as A. Its
load effect is keyed on the workspace slug, so a same-route swap started no
reload to correct it. The page now drops its own copy on the identity signal,
invalidates any load in flight, and reloads; its settle carries an identity
fence beside the `seq` one, which is a navigation fence and does not move on an
account swap.

THREE MORE STORES, none of which looks like per-user state to a sweep for
per-user FIELDS:

- `adminStore.stats` — instance-wide DATA, but authorization-scoped: only an
  admin can read it, and it must not outlive the admin who did.
- `titleStore` — its parts are retired by a PATH stamp, and an account swap
  moves no path, so A's workspace name and item ref stayed in the browser tab.
  Cosmetic, and cheap enough that leaving one member of a class unfixed costs
  more than fixing it.
- `editorStore` — `dirty` and `saveStatus` are read by GUARDS, not displayed:
  A's dirty editor makes B's unsaved-draft prompt and SSE archive suppression
  act on a document B never opened. Needs a WIDER clear than `resetForDoc`,
  which keeps `saveStatus` on purpose because it handles a document change
  within one session; an identity change ends the session. Its own test.

AND ONE FALSE CLAIM OF MINE. Unit 2b's message said every post-await write in
collectionStore was fenced. `finally { loading = false }` was not, so A's load
settling under B's in-flight one cleared B's flag and could flash empty-state
mid-load. Fenced, with `clear()` now owning the flag so nothing is stuck true.

THREE THINGS DOCUMENTED RATHER THAN CHANGED, each with the reasoning in the
code:

- The baseline exemption's residual. A request issued before `/auth/session`
  resolves settles under a fence that still says current, so a cross-tab
  identity change inside that window commits. It cannot be closed by refusing
  pre-baseline settles: the root layout issues the workspace list CONCURRENTLY
  with the session — that concurrency is why `identityEstablished` exists — so
  refusing them empties the app on every cold start. Nothing inside the tab can
  tell the two cases apart.
- The contract bumping-before-listeners places on listeners: a fence captured
  inside a listener passes, so a listener must reload for the identity signed
  in NOW. Every listener here clears and either stops or reloads from the
  current route.
- The durable wipe `resetAll` inherits from `reset`. Not needed for isolation
  (the cache is namespaced per user+workspace) and it costs a returning user
  their warm cache, but it is what sign-out has done since TASK-1360 and
  leaving a signed-out user's item titles on a shared browser's disk is the
  wrong default to adopt silently. What this widens is the pre-existing race
  between a fast re-login's hydrate and a wipe still in flight; that is a
  persistence-layer fix, not a change to this sweep.

Six new tests. Five mutants, each dying to exactly one: unfence `loading`,
drop the admin subscription, drop the title subscription, use `resetForDoc`
instead of `clear` for the editor, drop the starred page's subscription.

The admin test loads REAL stats through a stubbed fetch first — asserting null
before and after would have passed against a store with no clear at all, which
is the failure this branch has already produced once.

Full web suite 2438 passed; svelte-check 0 errors.

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

* fix(web): a reset that dropped the data but not the in-flight slot, plus the layout that outlived the identity (BUG-3005, codex round 2)

Round 2 found four defects, one of which my round-1 reset CREATED, and the
lead's three folded rows land with them.

THE SINGLE-FLIGHT SLOT — the reset was actively harmful. `clear()` dropped the
collections but left the previous identity's promise published, so B's
`ensureCollections` JOINED A's request, A's identity fence then refused to
commit, and B was answered with no data and no request of its own. A
permanently empty sidebar, which is the exact failure the cleared freshness
stamp was there to prevent. `invalidate()` on the keyed single-flight abandons
the slot AND bumps the sequence; `workspaceStore` gets the same line, because
it is the same defect one file away and BUG-2991 did not have to be filed for
it to be true there too.

THE WORKSPACE LAYOUT outlived the identity. Its per-workspace effect is keyed
on the slug, so a same-route swap re-ran nothing and the layout kept every
connection it made for the previous user. Two halves:

- the SSE stream is a leak rather than staleness. `sseService.connect` is
  idempotent per workspace, so B's tab kept the EventSource A opened — still
  authorized, because signing in as B replaces the cookie without destroying
  A's session row, which is precisely the case BUG-3007's server-side close
  cannot see. Explicit `disconnect()` first. BUG-3011 tracks the server half.
- fourteen leaf route components own user-scoped state and key their reloads on
  the slug. `{#key authStore.identityEpoch}` remounts them structurally rather
  than fixing fourteen pages and leaving the fifteenth to be remembered.

AND THE REMOUNT SHIPPED BROKEN FIRST. `identityEpoch` was a plain `let`, so the
template never saw it change and the block never re-ran. Nothing about the code
looked wrong; a test that counted mounts is what found it. It is `$state` now,
with the reason in the comment, because it is the only value in that file read
from a template.

Also from round 2: `adminStore.loadStats` had no fence, so A's statistics landed
after the clear; `clear()` set `loading = true` and the admin layout loads only
on mount, which pinned the console on "Loading admin data…" forever; and the
starred page reloaded its private copy but not the shared store, so every OTHER
view showed all items unstarred.

MY BASELINE COMMENT WAS FACTUALLY WRONG and codex was right to call it. I wrote
that the root layout issues the workspace list concurrently with
/auth/session — taken from the neighbouring comment rather than read from the
layout. `loadAll()` is gated on `authReady`, and no children render until then,
so nothing can issue a fenced request before the baseline. The residual is
therefore currently UNREACHABLE rather than narrow, and the exemption stays for
a different reason: the bump and the listener notification are one event, and
firing listeners on the baseline is a live hazard. Corrected in place, with what
is actually true.

The lead's three folded rows: SSE (above), `toastStore` (the tray is a readable
log of A's actions — toast text names items), and the editor's
attachment-metadata memo, cleared from the ROOT layout rather than by
self-subscribing, because that module is deliberately rune-free and importing
the auth store breaks every `node`-project test with "$state is not defined"
before an assertion runs.

Eight new tests. Mutants, each dying to exactly one test: no flight
invalidation, invalidate without the sequence bump (pinned by an A->B->A leg in
`workspaceStore`, where the captured user id AGREES and the bump is the only
guard left), unfenced `loadStats`, toast history kept, the starred page not
reloading the store, no SSE disconnect, no remount.

Full web suite 2446 passed; svelte-check 0 errors.

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

* fix(web): stop enumerating surfaces — an identity change clears what a reload cannot drop, then reloads (BUG-3005, codex round 3 + lead ruling)

Three review rounds each found another layer of surfaces holding the previous
user's data: three stores, then fourteen route pages plus a connection and two
caches, then the console subtree, share pages, the command palette and an open
dialog. The population is "everything in the tab that ever held a fetch", and a
per-surface fix cannot terminate — it is a list somebody has to keep complete
forever. The lead's ruling, and it is the right one: reload.

`reloadForIdentityChange` clears the state a reload does NOT drop — the
palette's recent searches in localStorage, collab cursors in sessionStorage,
the attachment-metadata memo — and then reloads. What remains an enumeration is
bounded by STORAGE KEYS rather than by every module in the app, and a grep for
the storage APIs can check it. The clears run FIRST and that ordering has its
own test: reloading first tears the page down before they execute.

REMOVED, because one mechanism is the point: the `{#key}` remount added in
round 2, and the starred page's own listener. The starred page had three reload
paths on a single identity change. The remount's absence is pinned by a test
rather than left implicit, so re-adding it has to be deliberate.

KEPT, because none of it is redundant with the reload: the epoch, the four
store fences, and the SSE disconnect. They cover the window between the identity
changing and the page actually going away, during which in-flight responses are
still settling into live stores. The SSE close bounds that window to zero for a
stream that would otherwise stay authorized — signing in as B replaces the
cookie without destroying A's session row, which is the case BUG-3007's
server-side close cannot see (BUG-3011).

THE TWO WRITES NEED THEIR OWN RULE, AND THE RELOAD MAKES THEM WORSE rather than
better. A teardown that PERSISTS must discard when the identity moved, because
the reload's unload fires those handlers under the NEW cookie:

- `ItemDetail`'s raw-markdown saver flushes pending edits with `keepalive: true`
  from three sites, a request deliberately built to outlive the page. It now
  refuses when the epoch differs from the one captured when the item was
  LOADED — the identity whose typing the editor contains.
- `openChildrenDialog` holds the fetched 409 child list AND continuations that
  perform the mutation on confirm. It is mounted in the root layout, outside
  everything the workspace layout controls. All pending requests now resolve
  FALSE, so the producer takes its cancel path; rejecting would surface
  unhandled rejections at call sites that only ever expected a boolean.

Grepped the class (`keepalive: true`): three sites, all funnelling through the
one saver callback that now carries the guard.

MY BASELINE COMMENT WAS WRONG A SECOND TIME. Round 2 corrected it to "no child
renders before authReady"; round 3 found the share-page branch sets `authReady`
WITHOUT calling `authStore.load()`, and the auth-failure path proceeds
deliberately. So the residual is narrow, not unreachable. Corrected again, and
this time the exceptions are named rather than the rule restated.

Nine tests across the new mechanism; every mutant dies to exactly one: reload
before clear, no palette clear, a collab clear that takes the whole of
sessionStorage, the dialog abandoning only its active entry, no SSE disconnect,
a reinstated remount.

Full web suite 2450 passed; svelte-check 0 errors.

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

* fix(web): the identity reload was firing the teardown writes it was meant to outrun (BUG-3005, codex round 4)

Round 4 found two P1s, and both are defects the RELOAD created — which is the
shape of this whole unit by now: each fix's own mechanism becomes the next
round's hazard.

THE UNLOAD HANDLER WAS PERSISTING UNDER THE NEW COOKIE. A reload fires
`beforeunload`, and both persistence paths there use `keepalive` — a request
deliberately built to outlive the page. Round 3's guard covered the raw saver
only. The collab flush was unguarded, and MY GREP FOR THE CLASS MISSED IT: I
grepped `keepalive: true`, and that call passes the flag positionally as
`flushNow(ctx, true)`. An enumeration by literal, in the unit whose entire
lesson is that enumerations by shape are wrong.

THE UNLOAD HANDLER COULD ALSO CANCEL THE RELOAD. `preventDefault()` on a dirty
editor raises the native "unsaved changes" prompt, and "Stay" cancels the
navigation. The identity has already changed and the stores are already
cleared, so the answer would be the new user sitting in the previous user's
page shell — with the `{#key}` remount removed in round 3, nothing left to
rebuild it. The prompt protects the author's unsaved work, and after an
identity change the author is not the one sitting there.

Both close the same way: when the epoch differs from the one captured at LOAD,
the handler returns before it flushes or prompts. The $effect cleanup is a third
door into the same write and is gated too.

Also from round 4: `pad-last-route-<ws>` and `pad-last-scroll-<ws>-<pathname>`
are keyed by WORKSPACE with no user in them. The scroll key embeds the pathname
in the KEY, so a private item slug is legible from a key listing without
reading a single value. Both are cleared now, and the test asserts an ordinary
UI preference SURVIVES, because a clear that took all of localStorage would be
a different and undocumented change.

ACCEPTED, NOT FIXED, with the reasoning: `persistWipe` is fire-and-forget, so
an IndexedDB database can outlive the reload if its delete has not landed. It
is residual data on disk rather than a cross-user read — the databases are
named `pad-local-index-{userId}-{workspace}`, so the next user cannot open the
previous one's — and making the reload await an unbounded IDB delete trades a
privacy-hygiene residual for a tab that may not reload at all.

Six tests. The ItemDetail ones are SOURCE guards, the pattern this directory
already uses for that file (~7,900 lines of collab, SSE and pane wiring), and
the header says what a source guard cannot do rather than leaving it implied.
Mutants: remove the beforeunload guard, remove the collab cleanup gate, remove
the route/scroll clear — each dies to exactly one test.

Full web suite 2456 passed; svelte-check 0 errors.

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

* 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

* fix(web): a sign-in from an unauthenticated tab must not reload (BUG-3005, E2E failure + lead ruling)

E2E failed on 3aebbb58 and the suite was right. `web/e2e/lib/collab-helpers.ts`
signs in by fetching `/api/v1/auth/login` FROM INSIDE a mounted page, which is
an identity change from `''` to a real user — and the reload took the page out
from under Playwright.

The narrowing is correct on its own terms, which is why it lands whatever the
log finally says. The reload exists so one user's data cannot be visible to the
next, so which transitions need it is a question about WHOSE data is being
dropped: an anonymous baseline holds nobody's private data. `user -> user` and
`user -> ''` reload; `'' -> user` does not.

It also takes a full page load out of the primary login path, where it was
racing the login flow's own navigation. That cost was never intended and would
have shipped unnoticed if the E2E suite had not been driving a real login.

`onIdentityChange` listeners now receive the PREVIOUS established user id.
Every other listener ignores it and drops state as before; only the root layout
needs to tell a sign-in from a sign-out or a swap, and reading it from the
signal is better than the alternative of a second store field that can disagree
with the one the notification was computed from.

Three legs, and the mutant that removes the guard kills all three — the swap and
sign-out ones on the exact CALL COUNT rather than on an absence, because
`afterEach` clears the store so every test starts from an established empty
identity and its own setup performs a sign-in. That is written into the test
file, since a leg failing for a reason other than its name is how a suite stops
meaning what it says.

Full web suite 2460 passed; svelte-check 0 errors.

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

* fix(web): sign-out clears and lets its own navigation leave; only a swap reloads (BUG-3005, E2E account-delete failure)

The E2E failure was `account-delete.spec.ts:147` with
`net::ERR_ABORTED; maybe frame was detached?` and
`Navigation to /console/settings is interrupted by another navigation to
/login`. A NAVIGATION RACE, not a rule about identity transitions — and not the
sign-in I had guessed at before the log was readable. Recording that because
the previous commit's narrowing was proposed on a wrong diagnosis; it is right
on its own terms, and it was not this.

Both sign-out sites already navigate away by themselves. Account deletion does
`authStore.clear()` then `window.location.href = '/login'`, deliberately and
with a comment saying so. My listener fires inside `clear()` and started a
reload of the current URL; the delete flow then set `location.href`. Two
navigations, one aborts.

So the split, per the lead's ruling:

- SWAP (user -> user): reload. Nothing else navigates, so the reload is the
  whole mechanism.
- SIGN-OUT (user -> ''): run the persistent clears and return. The clears still
  have to happen — a hard navigation drops the tab's memory, not localStorage,
  sessionStorage or IndexedDB — but the reload is redundant with a navigation
  that is already underway, and racing it is what broke the flow.
- and the console logout now navigates HARD. `goto('/login')` is an SPA
  navigation that leaves every singleton, component and cache where it was,
  which is this bug's original defect arriving through the sign-out path
  itself. Account deletion has always done this; the two sites agree now.

The property is unchanged: every transition away from a real user drops that
user's in-tab state, by reload on a swap and by hard navigation on a sign-out.

Four legs in the root-layout test (swap reloads and does NOT take the clear-only
branch; sign-out clears and does NOT reload; sign-in does neither) and a source
guard on the console logout, since jsdom does not perform an assignment to
`window.location`. Mutants: sign-out reloading again, sign-out doing neither,
and the logout reverting to `goto` — each dies.

Full web suite 2464 passed; 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 19:42:26 -04:00
committed by GitHub
parent d9ea533728
commit 57820422a2
33 changed files with 2374 additions and 24 deletions
+4 -1
View File
@@ -128,7 +128,10 @@ export type ForceRefreshHandler = () => void;
* reconnect would land on a stale-cursor force_refresh even though
* B has been a stable session the whole time.
*/
const CURSOR_STORAGE_PREFIX = 'pad:collab:cursor:';
// Exported so the identity-change clear can find these keys without copying
// the literal (BUG-3005). A second spelling of a storage prefix is a clear that
// silently stops matching.
export const CURSOR_STORAGE_PREFIX = 'pad:collab:cursor:';
function cursorStorageKey(itemID: string): string {
return `${CURSOR_STORAGE_PREFIX}${itemID}`;
@@ -92,6 +92,29 @@ export type AttachmentMetadataResult =
const cache = new Map<string, Promise<AttachmentMetadataResult>>();
// The key carries no USER (BUG-3005). Every entry is a HEAD result obtained
// under the signed-in user's authorization, and this cache lives for the page
// lifetime by design — so after a same-route account swap, B reading the same
// attachment id in the same workspace is answered from A's probe with no
// request made.
//
// CLEARED ON THE SIGNAL RATHER THAN KEYED BY USER, which was the other option
// and is the stronger one in general. Keying would mean four call sites
// composing the key and two PREFIX SCANS
// (`invalidateAttachmentMetadataForWorkspace`) agreeing with them on a userId
// that can move between the fetch and the invalidation — a new way to write an
// entry nothing can ever delete. The strength a key would add over a clear is
// coverage of requests issued before an identity is established, and nothing
// here can be in that window: the root layout renders no children until
// `authReady`, so no editor exists to probe an attachment before the session
// has resolved.
//
// The subscription is NOT registered here, and that is not tidiness: this
// module is deliberately rune-free and is imported by tests in vitest's `node`
// project, so importing `auth.svelte` breaks them with `$state is not defined`
// before a single assertion runs. The root layout owns the call instead — see
// `clearAttachmentMetadataCache` below.
/**
* Is this result a DURABLE fact worth keeping for the page's lifetime?
*
@@ -236,6 +259,17 @@ export function invalidateAttachmentMetadata(workspaceSlug: string, uuid: string
* what answers: a genuinely deleted row 404s again and re-latches. The cache was
* never the authority — it was only ever a memo of one.
*/
/**
* Drop EVERY entry, for every workspace (BUG-3005).
*
* Called from the root layout on an identity change. Exported rather than
* self-subscribing so this module keeps no dependency on the auth store — see
* the note on `cache` for why that dependency is not free here.
*/
export function clearAttachmentMetadataCache(): void {
cache.clear();
}
export function invalidateAttachmentMetadataForWorkspace(workspaceSlug: string): void {
if (!workspaceSlug) return;
const prefix = `${workspaceSlug}:`;
+52 -1
View File
@@ -223,6 +223,11 @@
// stores after unmount (PLAN-2105 / TASK-2112; Codex rounds 1-2 P1). Plain
// counter — not reactive; it only fences async writes.
let loadGeneration = 0;
// The identity this component's editor contents belong to (BUG-3005).
// Captured at load rather than at save: the markdown in the editor was
// typed under whoever was signed in when the item was loaded, and that is
// the identity a pending write may legitimately be persisted as.
let identityEpochAtLoad = authStore.identityEpoch;
// UNIFIED collection-snapshot generation (BUG-2265 Codex): bumped by EVERY
// operation that assigns `collection` — loadData's collection fetch, the SSE
@@ -1596,6 +1601,10 @@
async function loadData() {
const myGen = ++loadGeneration;
// Re-stamp the identity this editor's contents belong to (BUG-3005). A
// load is where the contents are replaced, so it is where the claim
// "this markdown was typed by the current user" becomes true again.
identityEpochAtLoad = authStore.identityEpoch;
// The item whose links `itemLinks` currently describes, captured BEFORE
// this load can replace `item`. `loadData` is not only a first load or a
// switch: the edit-collection handler calls it for a SAME-item reload
@@ -2572,7 +2581,12 @@
// [P1] of TASK-1319.
const skipFlush = skipFlushOnNextCleanup;
skipFlushOnNextCleanup = false;
if (!rawMode && !skipFlush) {
// The same discard rule as the beforeunload handler and the raw
// saver (BUG-3005): this cleanup also runs on an identity change,
// and a Y.Doc snapshot written then carries the wrong user's
// cookie.
const identityHeld = authStore.identityEpoch === identityEpochAtLoad;
if (!rawMode && !skipFlush && identityHeld) {
collabFlusher.flushNow(ctx, true);
}
provider.destroy();
@@ -2599,6 +2613,25 @@
$effect(() => {
if (typeof window === 'undefined') return;
const onBeforeUnload = (event: BeforeUnloadEvent) => {
// WHEN THE IDENTITY MOVED, THIS HANDLER DOES NOTHING AT ALL
// (BUG-3005, codex round 4). Two separate defects, both created by
// the fix that reloads the tab on an identity change:
//
// - it FLUSHES. Both paths below persist editor content with
// `keepalive`, and the unload they run in is the identity
// reload's own — so A's Y.Doc snapshot and A's pending markdown
// would be PATCHed carrying B's cookie. The raw saver has its
// own guard; the collab flush did not, and my grep for
// `keepalive: true` missed it because it takes the flag
// positionally. Guarding here covers both by construction.
// - it PROMPTS. `preventDefault()` on a dirty editor raises the
// native "unsaved changes" dialog, and "Stay" CANCELS the
// reload — leaving B in A's page shell with the stores already
// cleared and no remount left to rebuild it. The prompt exists
// to protect the author's unsaved work, and after an identity
// change the author is not the one sitting here.
if (authStore.identityEpoch !== identityEpochAtLoad) return;
// Collab path: flush the live Y.Doc snapshot (unchanged).
const ctx = activeCollabContext;
if (ctx) collabFlusher.flushNow(ctx, true);
@@ -3637,6 +3670,24 @@
debounceMs: 1200,
save: (markdown, { keepalive }) => {
if (!item) return;
// DISCARD RATHER THAN PERSIST WHEN THE IDENTITY MOVED (BUG-3005).
//
// This markdown was typed by whoever was signed in when they typed
// it. If a different user is signed in now, writing it means one
// account's unsaved edit is persisted as another's — a WRITE under
// the wrong identity, which is a different class from the display
// leaks the rest of that fix is about.
//
// The keepalive path is why this is load-bearing rather than
// theoretical: an identity change RELOADS this tab, the reload fires
// `beforeunload`, and that handler flushes pending raw markdown with
// `keepalive: true` — a request deliberately built to outlive the
// page, and it would go out carrying the NEW user's cookie. The fix
// that reloads is what makes this reachable, so it ships with it.
//
// Compared against the epoch captured when this item was LOADED,
// which is the identity the editor's contents belong to.
if (authStore.identityEpoch !== identityEpochAtLoad) return;
// HT-2176 Option A (TASK-2172): NO peeking recheck here. NEW raw input
// is blocked at the editor (`readonly={!canEdit || peeking}`), so this
// only ever fires for a debounced save the user INITIATED before the
@@ -0,0 +1,81 @@
// Node-project test (no DOM): a SOURCE-level guard that ItemDetail's teardown
// writes DISCARD when the signed-in identity has moved (BUG-3005, codex round
// 4).
//
// WHY THIS IS LOAD-BEARING RATHER THAN BELT-AND-BRACES. A real identity change
// now RELOADS the tab, and a reload fires `beforeunload`. Both persistence
// paths in that handler use `keepalive`, a request deliberately built to
// outlive the page — so without a guard, the previous user's Y.Doc snapshot and
// pending markdown are PATCHed carrying the NEW user's cookie. The fix that
// reloads is what makes this reachable; it ships with it.
//
// The prompt half matters as much as the flush half: `preventDefault()` on a
// dirty editor raises the native "unsaved changes" dialog, and "Stay" CANCELS
// the reload — leaving the new user in the previous user's page shell with the
// stores already cleared and (since round 3) no remount left to rebuild it.
//
// WHY SOURCE TEXT AND NOT A RENDER: same reasoning as
// `itemDetailLinksSurviveFailedRefresh.test.ts` in this directory — ItemDetail
// is ~7,900 lines of collab, SSE and pane wiring, and the property is
// structural. WHAT A SOURCE GUARD CANNOT DO, stated so it is not mistaken for
// proof: it checks spellings, not behaviour. It would not catch a guard that
// compares the wrong two values, or one made unreachable by an earlier return.
import { describe, expect, it } from 'vitest';
import { readFileSync } from 'node:fs';
import { resolve } from 'node:path';
const SRC = readFileSync(resolve(__dirname, './ItemDetail.svelte'), 'utf8');
/** Strip line and block comments so a guard cannot be satisfied by prose. */
function stripComments(src: string): string {
return src.replace(/\/\*[\s\S]*?\*\//g, '').replace(/^[ \t]*\/\/.*$/gm, '');
}
const CODE = stripComments(SRC);
describe('ItemDetail teardown writes under an identity change', () => {
it('captures the identity epoch at LOAD and re-stamps it in loadData', () => {
// The comparison is only meaningful against the identity whose typing
// the editor holds, which is the one current when the item was loaded —
// not the one current when the save fires.
expect(CODE).toMatch(/let\s+identityEpochAtLoad\s*=\s*authStore\.identityEpoch/);
const loadData = CODE.slice(CODE.indexOf('async function loadData()'));
const upToFirstAwait = loadData.slice(0, loadData.indexOf('await'));
expect(upToFirstAwait).toContain('identityEpochAtLoad = authStore.identityEpoch');
});
it('makes the identity check the FIRST thing beforeunload does', () => {
// Ahead of both flushes AND ahead of `preventDefault`, or the handler
// still writes or still blocks the reload.
const start = CODE.indexOf('const onBeforeUnload = (event: BeforeUnloadEvent) => {');
expect(start, 'the beforeunload handler was renamed or removed').toBeGreaterThan(-1);
const body = CODE.slice(start, start + 1200);
const guard = body.indexOf('authStore.identityEpoch !== identityEpochAtLoad');
expect(guard, 'beforeunload does not check the identity epoch').toBeGreaterThan(-1);
for (const persisting of ['collabFlusher.flushNow', 'rawContentSaver.flushNow', 'preventDefault']) {
const at = body.indexOf(persisting);
expect(at, `${persisting} is no longer in the handler — re-point this guard`).toBeGreaterThan(-1);
expect(at, `${persisting} runs BEFORE the identity check`).toBeGreaterThan(guard);
}
});
it('gates the collab cleanup flush on the identity too', () => {
// The $effect cleanup is a second door into the same write, and it runs
// on the same identity change. My own grep for `keepalive: true` missed
// it, because this one passes the flag positionally.
expect(CODE).toMatch(
/const\s+identityHeld\s*=\s*authStore\.identityEpoch\s*===\s*identityEpochAtLoad/,
);
expect(CODE).toMatch(/if\s*\(!rawMode\s*&&\s*!skipFlush\s*&&\s*identityHeld\)/);
});
it('gates the raw saver, which is the third door', () => {
const saver = CODE.slice(CODE.indexOf('const rawContentSaver = createContentSaver('));
const save = saver.slice(saver.indexOf('save: (markdown'));
const guard = save.indexOf('authStore.identityEpoch !== identityEpochAtLoad');
const patch = save.indexOf('api.items');
expect(guard).toBeGreaterThan(-1);
expect(patch).toBeGreaterThan(guard);
});
});
@@ -75,7 +75,17 @@ vi.mock('$lib/services/sse.svelte', () => ({
}));
vi.mock('$lib/stores/auth.svelte', () => ({
authStore: { userId: 'user-1', user: { id: 'user-1', role: 'member' } },
// `onIdentityChange` and `identityFence` are part of the store's surface as
// of BUG-3005: several singletons subscribe at MODULE scope, so a partial
// mock throws while the component tree is still being imported, before any
// assertion here runs.
authStore: {
userId: 'user-1',
user: { id: 'user-1', role: 'member' },
identityEpoch: 0,
identityFence: () => () => true,
onIdentityChange: () => () => {},
},
}));
// Controllable so the TASK-2474 peek leg can set the timeline's OWN canEdit
+36 -3
View File
@@ -2,6 +2,8 @@
// Admin store – shared state & utilities for the admin section
// ---------------------------------------------------------------------------
import { authStore } from './auth.svelte';
// ---- Interfaces -----------------------------------------------------------
export interface AdminUser {
@@ -115,12 +117,19 @@ let error = $state('');
async function loadStats() {
loading = true;
error = '';
// The identity that ASKED (BUG-3005, codex round 2). Clearing on the signal
// is not enough on its own: a request already in flight settles afterwards
// and writes the previous admin's statistics into the new session.
const isSameIdentity = authStore.identityFence();
try {
stats = await adminFetch('/admin/stats');
const result = await adminFetch('/admin/stats');
if (!isSameIdentity()) return;
stats = result;
} catch (e) {
if (!isSameIdentity()) return;
error = e instanceof Error ? e.message : 'Failed to load';
} finally {
loading = false;
if (isSameIdentity()) loading = false;
}
}
@@ -134,5 +143,29 @@ export const adminStore = {
get error() {
return error;
},
loadStats
loadStats,
/**
* Drop the loaded statistics (BUG-3005). `stats` is instance-wide DATA but
* it is authorization-scoped: only an admin can read it, so it must not
* outlive the admin who did.
*/
clear() {
stats = null;
// FALSE, not true (codex round 2). The admin layout loads on MOUNT, and
// a same-route admin-to-admin swap mounts nothing — leaving this true
// would pin the console on "Loading admin data…" forever. The layout
// re-issues the load on the same signal; until it lands the honest
// state is "nothing loaded", not "loading".
loading = false;
error = '';
}
};
// An admin signing out — or a swap to a non-admin on the same console route —
// must not leave the previous admin's statistics on screen. The admin layout
// loads on mount, and a same-route identity change mounts nothing (BUG-3005,
// codex round 1).
authStore.onIdentityChange(() => {
adminStore.clear();
});
+133 -3
View File
@@ -31,7 +31,7 @@ let generation = 0;
//
// The direction of the dependency is unchanged: `workspace.svelte.ts` imports
// this module and registers here, so this module still imports nothing of it.
const identityListeners = new Set<() => void>();
const identityListeners = new Set<(previousUserId: string) => void>();
// The id whose listeners have already been notified. Compared rather than
// assumed, so a `load()` that returns the SAME user (the common case — the root
// layout fetches the session on every cold start) notifies nobody and cannot
@@ -46,6 +46,73 @@ let notifiedUserId = '';
// baseline. Every later transition — including back to '' on sign-out — fires.
let identityEstablished = false;
// The identity EPOCH (BUG-3005): a monotonic counter bumped on exactly the
// transitions `onIdentityChange` fires on.
//
// WHY A COUNTER AND NOT THE USER ID. A store that starts a request as one user
// and commits its response later needs to ask "is this still the identity I
// issued under?", and a user id answers a WEAKER question — "is this a
// different user?". The two come apart on sign out of A, in as B, back in as
// A: a request issued during the FIRST A session settles, compares its
// captured id against the current one, finds them equal, and commits data
// belonging to a session that has ended. An ordinal cannot be confused that
// way, because the epoch it captured is strictly lower than the one it settles
// into. `workspace.svelte.ts`'s `userId` fence has exactly that hole; it is
// narrower than the primary defect (it needs a request to outlive two identity
// changes) and it is the same class.
//
// NOT BUMPED ON THE BASELINE, matching `notifyIdentityChange`'s own early
// return: the first resolution of the session is not a change from anything,
// and a fence captured before an identity was established has nothing stale to
// refuse.
//
// THE RESIDUAL THAT LEAVES, stated rather than discovered (codex round 1).
// A request issued before the baseline resolves carries whatever cookie the
// browser had; the baseline then reports whoever that cookie belongs to. Those
// are the same principal unless the cookie CHANGED mid-flight — another tab
// signing out and in — in which case the pre-baseline request settles under a
// fence that still says current, and commits.
//
// CORRECTED (codex round 2). This comment previously justified the exemption by
// saying the root layout issues the workspace list concurrently with
// /auth/session, so refusing pre-baseline settles would empty the app. That is
// FALSE, and I had taken it from the `identityEstablished` comment below rather
// than reading the layout: `workspaceStore.loadAll()` is gated on `authReady`
// (routes/+layout.svelte), which flips only after `authStore.load()` resolves,
// and the layout renders NO children until then — so no route can issue a
// fenced request before the baseline either.
//
// What that means for the residual: on the ORDINARY path it needs a fence
// captured by something that runs before any child renders, and the only
// pre-baseline request there is `authStore.load()` itself, which takes no
// fence. TWO EXCEPTIONS, both real (codex round 3, correcting this comment a
// second time): the share-page branch sets `authReady` WITHOUT calling
// `authStore.load()` at all, and the auth-FAILURE path proceeds deliberately —
// so children can render and fetch with no baseline established in either. The
// residual is therefore narrow rather than unreachable, and it is a property of
// the callers rather than of this code.
//
// The exemption itself stays for a different and simpler reason: the bump and
// the listener notification are the same event, and firing listeners on the
// baseline is a live hazard the `identityEstablished` comment below documents.
// Separating them would mean two signals where one is honest.
//
// The honest boundary is therefore: the epoch covers identity changes THIS TAB
// observed, and a cross-tab change during the pre-baseline window is outside
// it.
//
// Distinct from `generation` above, which bumps ONLY on `clear()` and fences
// this module's own `/auth/session` fetches. A sign-in as a different user
// through `load()` moves the identity without touching `generation`, so
// `generation` cannot serve as the epoch — it would miss the swap.
// `$state`, unlike `generation` and `notifiedUserId` beside it, because this
// one is READ FROM A TEMPLATE: the workspace layout keys its leaf-page block on
// it (`{#key authStore.identityEpoch}`) to remount route components whose own
// state is not identity-scoped. A plain `let` is invisible to the template and
// the block never re-runs — which is exactly how the first version of that
// remount shipped, silently, until a test counted mounts (codex round 2).
let identityEpoch = $state(0);
function notifyIdentityChange() {
const id = session?.user?.id ?? '';
if (!identityEstablished) {
@@ -54,8 +121,34 @@ function notifyIdentityChange() {
return;
}
if (id === notifiedUserId) return;
const previousUserId = notifiedUserId;
notifiedUserId = id;
for (const fn of identityListeners) fn();
// BEFORE the listeners, not after. A listener may capture a fence while it
// runs — dropping state and immediately reloading is the expected shape —
// and a fence captured under the OLD epoch would refuse that reload's own
// response.
//
// THE CONTRACT THIS PLACES ON LISTENERS (codex round 1): a fence captured
// inside a listener passes, so a listener must reload for the identity that
// is signed in NOW and never re-issue a request built from the identity it
// was just told about. The fence cannot check this for them — it compares
// epochs, and by the time a listener runs the epoch is already the new one.
// Every listener on this branch satisfies it, and most satisfy it by doing
// nothing further: clearing state and stopping cannot re-issue anything.
// The two that DO reload — the workspace layout and the starred page — read
// the workspace from the live route rather than from anything captured
// before the change (codex round 2 read "stops" as a claim that they all
// reload, so this says which is which).
identityEpoch++;
// The PREVIOUS established identity is passed to listeners, because one of
// them has to distinguish a sign-IN from a sign-out or a swap (BUG-3005,
// lead ruling after the E2E failure). The reload exists to stop one user's
// data being visible to the next, and an ANONYMOUS baseline holds nobody's
// private data — so `'' -> user` needs no reload, while `user -> user` and
// `user -> ''` do. Reloading on sign-in also put a full page load in the
// middle of the primary login path, racing whatever navigation that flow
// was doing.
for (const fn of identityListeners) fn(previousUserId);
}
export const authStore = {
@@ -155,12 +248,49 @@ export const authStore = {
* Fired only on a real change of user id, so a session refetch that returns
* the same user is silent. See `identityListeners` above for why this exists
* rather than a call at each sign-out site.
*
* The listener receives the PREVIOUS established user id — `''` when the tab
* was unauthenticated. Most listeners ignore it and simply drop state; the
* root layout uses it to reload on a sign-out or a swap but NOT on a
* sign-in.
*/
onIdentityChange(fn: () => void): () => void {
onIdentityChange(fn: (previousUserId: string) => void): () => void {
identityListeners.add(fn);
return () => identityListeners.delete(fn);
},
/**
* The current identity epoch — a monotonic counter of identity CHANGES.
* Read it to compare two moments; use `identityFence()` to guard a settle.
*/
get identityEpoch() { return identityEpoch; },
/**
* Capture the current identity, and return a predicate that says whether it
* is still the current one.
*
* The intended shape, for any store that reads user-scoped data:
*
* const isSameIdentity = authStore.identityFence();
* const result = await api.something();
* if (!isSameIdentity()) return; // issued as someone else; drop it
*
* Offered as a helper rather than left to each call site because this store
* and `workspace.svelte.ts` already hand-rolled the same three parts of a
* single-flight loader and drifted three times in one afternoon
* (`singleFlight.ts`'s own note). A fence is smaller and there are more
* places that need one.
*
* It is NOT a navigation fence and does not replace one. A workspace switch
* under one identity leaves the epoch alone, which is correct — that race is
* about WHICH WORKSPACE the answer describes, not about who asked. Stores
* that can race a switch need both, and neither subsumes the other.
*/
identityFence(): () => boolean {
const captured = identityEpoch;
return () => identityEpoch === captured;
},
clear() {
session = null;
generation++;
@@ -0,0 +1,117 @@
import { describe, expect, it, vi, beforeEach } from 'vitest';
/**
* BUG-3005 — `authStore.identityEpoch` / `identityFence()`.
*
* The epoch exists so a store that issues a request as one user can refuse the
* response if it settles as another. Its whole claim over the user-id
* comparison already in the tree is that it is ORDERED: A → B → A must not
* look like "no change".
*/
const session = vi.hoisted(() => ({ value: null as unknown }));
vi.mock('$lib/api/client', () => ({
api: { auth: { session: vi.fn(async () => session.value) } },
}));
function user(id: string) {
return { authenticated: true, user: { id, email: `${id}@example.com` } };
}
describe('authStore identity epoch', () => {
beforeEach(() => {
vi.resetModules();
session.value = null;
});
it('does not move on the FIRST resolution of the session', async () => {
// Same reasoning as onIdentityChange's silence on the baseline: a cold
// start has nothing stale, and a fence captured before an identity was
// established must not start refusing once one appears.
const { authStore } = await import('./auth.svelte');
const isSameIdentity = authStore.identityFence();
session.value = user('u1');
await authStore.load();
expect(authStore.identityEpoch).toBe(0);
expect(isSameIdentity()).toBe(true);
});
it('moves on sign-out, and a fence taken before it refuses', async () => {
const { authStore } = await import('./auth.svelte');
session.value = user('u1');
await authStore.load();
const isSameIdentity = authStore.identityFence();
// PRECONDITION: current before the change, or "it refused" is a claim
// about a fence that never accepted anything.
expect(isSameIdentity()).toBe(true);
authStore.clear();
expect(authStore.identityEpoch).toBe(1);
expect(isSameIdentity()).toBe(false);
});
it('REFUSES a straggler from the first A session after A -> B -> A', async () => {
// The reason this is an ordinal and not the user id. Every id comparison
// in this scenario reports "same user", because it IS the same user —
// and the session that issued the request is gone.
const { authStore } = await import('./auth.svelte');
session.value = user('a');
await authStore.load();
// A request issued in A's first session captures its fence here.
const issuedInFirstA = authStore.identityFence();
const userIdAtIssue = authStore.userId;
authStore.clear();
session.value = user('b');
await authStore.load();
authStore.clear();
session.value = user('a');
await authStore.load();
// The comparison the tree already had says CURRENT — this is the hole.
expect(authStore.userId).toBe(userIdAtIssue);
// The epoch says otherwise, which is the fix.
expect(issuedInFirstA()).toBe(false);
expect(authStore.identityEpoch).toBe(4);
});
it('stays put when a session refetch returns the SAME user', async () => {
// The counterfactual for every test above: a fence that refuses on a
// no-op refetch would drop live data on the root layout's ordinary
// session poll, which is a worse defect than the one being fixed.
const { authStore } = await import('./auth.svelte');
session.value = user('u1');
await authStore.load();
const isSameIdentity = authStore.identityFence();
const before = authStore.identityEpoch;
await authStore.load();
expect(authStore.identityEpoch).toBe(before);
expect(isSameIdentity()).toBe(true);
});
it('is already bumped when identity listeners run', async () => {
// A listener's expected shape is "drop state, reload it". The reload
// captures a fence WHILE the listener runs, so a bump ordered after the
// listeners would hand it the old epoch and refuse its own response.
const { authStore } = await import('./auth.svelte');
session.value = user('u1');
await authStore.load();
let fenceTakenInsideListener: (() => boolean) | null = null;
authStore.onIdentityChange(() => {
fenceTakenInsideListener = authStore.identityFence();
});
authStore.clear();
expect(fenceTakenInsideListener).not.toBeNull();
expect(fenceTakenInsideListener!()).toBe(true);
});
});
+78 -5
View File
@@ -158,6 +158,13 @@ export const collectionStore = {
// authenticated one. Same source as every `bootstrap` caller passes, so
// the two cannot disagree.
const userId = authStore.userId || null;
// The IDENTITY this load is issued under (BUG-3005). Captured out here,
// before `run` and therefore before any await, so it names the identity
// that ASKED rather than whichever one is signed in when the response
// lands. `isLatest` is a navigation fence and does not cover this: a
// same-route account swap supersedes nothing, so the older load is still
// "latest" and commits A's collection names into B's session.
const isSameIdentity = authStore.identityFence();
// ALWAYS ISSUES — `run` never coalesces. The join is `ensureCollections`'s
// alone, via `inFlightFor`, because every other caller here knows the list
// MOVED and a request issued before that move cannot answer it.
@@ -179,7 +186,7 @@ export const collectionStore = {
// switch that resolved first) has superseded this one, so writing
// `collections`/`collectionsWorkspace` here would clobber the newer
// workspace's data with this older load (Codex review).
if (!isLatest()) return;
if (!isLatest() || !isSameIdentity()) return;
collections = result;
// Stamp the array with its source workspace so consumers can tell
// it apart from a stale previous-workspace load (see
@@ -196,26 +203,47 @@ export const collectionStore = {
},
async loadItems(ws: string, collectionSlug?: string, params?: Record<string, string | number | boolean | undefined>) {
const isSameIdentity = authStore.identityFence();
loading = true;
try {
if (collectionSlug) {
items = await api.items.listByCollection(ws, collectionSlug, params);
const result = await api.items.listByCollection(ws, collectionSlug, params);
// A response issued as someone else must not become this
// session's item list (BUG-3005). Assigned through a local
// first, because `items = await ...` publishes whatever comes
// back before any check can run.
if (!isSameIdentity()) return;
items = result;
// Partial load — the stored array no longer represents the
// full workspace, so invalidate the freshness stamp. A
// downstream caller asking `itemsAreFreshFor(ws)` will
// correctly trigger a full re-load.
itemsWorkspace = null;
} else {
items = await api.items.list(ws, params);
const result = await api.items.list(ws, params);
if (!isSameIdentity()) return;
items = result;
itemsWorkspace = ws;
}
} finally {
loading = false;
// FENCED TOO (codex round 1, P2). `loading` is module state written
// after an await like any other: A's load settling while B's is
// still in flight would clear B's flag and flash empty-state or
// error UI mid-load. `clear()` resets the flag on the identity
// change itself, so nothing is left stuck true.
if (isSameIdentity()) loading = false;
}
},
async loadItem(ws: string, slug: string) {
activeItem = await api.items.get(ws, slug);
const isSameIdentity = authStore.identityFence();
const result = await api.items.get(ws, slug);
// Returns null rather than the fetched item when the identity moved:
// the caller asked on behalf of a session that has ended, and handing
// it the item would put it on screen for whoever is signed in now
// (BUG-3005). `activeItem` is left as the reset found it.
if (!isSameIdentity()) return null;
activeItem = result;
return activeItem;
},
@@ -242,4 +270,49 @@ export const collectionStore = {
activeItem = null;
}
},
/**
* Drop everything this store holds. Exported for the identity subscription
* below and for tests; there is no other caller.
*/
clear() {
collections = [];
items = [];
itemsWorkspace = null;
collectionsWorkspace = null;
activeItem = null;
// The flag any in-flight load will now decline to clear (see the fenced
// `finally` in `loadItems`). Reset here so the identity change itself
// owns it and no consumer is left waiting on a load that will never
// report finished.
loading = false;
// And the SINGLE-FLIGHT SLOT (codex round 2). Dropping the data while
// leaving A's promise published made this reset actively harmful: B's
// `ensureCollections` joined A's request, A's identity fence then
// refused to commit, and B was answered with no data and no request of
// its own — a permanently empty sidebar, which is the exact failure the
// cleared freshness stamp was supposed to prevent.
collectionsFlight.invalidate();
},
};
// Drop the previous user's collections, items and open item when the signed-in
// user changes (BUG-3005).
//
// Every piece of state here belongs to a user and none of it is keyed by one:
// the loader is keyed by workspace SLUG, and the workspace layout drives it
// from a slug-keyed effect, so a same-route sign-in as a different user starts
// no new load at all. Routes that read `collections` — Roles, the item picker,
// the editor's link gate — then render A's collection names and item titles
// inside B's session.
//
// The FRESHNESS STAMPS are cleared with the arrays, deliberately. Leaving
// `collectionsWorkspace` set while emptying `collections` would make
// `collectionsAreFreshFor(ws)` answer true for an empty list, which is the one
// answer that stops TASK-2200's recovery from re-fetching.
//
// Module scope, registered once, never unsubscribed: singleton store, tab
// lifetime.
authStore.onIdentityChange(() => {
collectionStore.clear();
});
@@ -0,0 +1,191 @@
import { describe, expect, it, vi, beforeEach } from 'vitest';
/**
* BUG-3005 — `collectionStore` retains `collections`, `items` and `activeItem`
* globally, and its loader is keyed only by workspace SLUG.
*
* The workspace layout drives it from a slug-keyed effect, so a same-route
* sign-in as a different user starts no new load at all: A's collection names
* and item titles stay on screen inside B's session, with no request having
* been made for them. And a load issued as A can commit after B signs in,
* because the store's only fence is a NAVIGATION fence that a same-route swap
* does not move.
*/
const api = vi.hoisted(() => ({
collections: { list: vi.fn() },
items: { list: vi.fn(), listByCollection: vi.fn(), get: vi.fn() },
}));
vi.mock('$lib/api/client', () => ({ api }));
// The durable cache is a different concern (TASK-2946); stub it inert so this
// file tests the identity fence and nothing else.
vi.mock('./localIndexPersistence', () => ({
hydrateCollections: vi.fn(async () => null),
persistCollections: vi.fn(async () => {}),
readDurableEpoch: vi.fn(async () => null),
}));
// A REAL epoch and fence. A stub that always answered "still current" would let
// every assertion below pass against a store with no fence at all.
const auth = vi.hoisted(() => {
const listeners = new Set<() => void>();
let epoch = 0;
return {
userId: 'user-1',
get identityEpoch() { return epoch; },
identityFence() {
const captured = epoch;
return () => epoch === captured;
},
onIdentityChange(fn: () => void) {
listeners.add(fn);
return () => listeners.delete(fn);
},
fireIdentityChange() {
epoch++;
for (const fn of listeners) fn();
},
resetListeners() {
listeners.clear();
epoch = 0;
},
};
});
vi.mock('./auth.svelte', () => ({ authStore: auth }));
const collection = (slug: string) => ({ id: slug, slug, name: slug, is_default: true, sort_order: 1 });
const item = (id: string) => ({ id, slug: id, title: id });
describe('collectionStore across an identity change', () => {
beforeEach(() => {
vi.resetModules();
auth.resetListeners();
api.collections.list.mockReset();
api.items.list.mockReset();
api.items.listByCollection.mockReset();
api.items.get.mockReset();
});
it('drops collections, items and the open item when the identity changes', async () => {
const { collectionStore } = await import('./collections.svelte');
api.collections.list.mockResolvedValueOnce([collection('tasks')]);
await collectionStore.loadCollections('ws');
api.items.list.mockResolvedValueOnce([item('item-a')]);
await collectionStore.loadItems('ws');
api.items.get.mockResolvedValueOnce(item('item-a'));
await collectionStore.loadItem('ws', 'item-a');
// PRECONDITION: all three are populated, or "they were dropped" is a
// claim about state that was never there.
expect(collectionStore.collections).toHaveLength(1);
expect(collectionStore.items).toHaveLength(1);
expect(collectionStore.activeItem).not.toBeNull();
expect(collectionStore.collectionsAreFreshFor('ws')).toBe(true);
auth.fireIdentityChange();
expect(collectionStore.collections).toHaveLength(0);
expect(collectionStore.items).toHaveLength(0);
expect(collectionStore.activeItem).toBeNull();
});
it('clears the FRESHNESS STAMPS with the arrays, so recovery re-fetches', async () => {
// Emptying `collections` while leaving `collectionsWorkspace` set would
// make `collectionsAreFreshFor(ws)` answer true for an empty list — the
// one answer that stops TASK-2200's recovery from ever re-fetching, so
// the sidebar would stay permanently empty for the new user.
const { collectionStore } = await import('./collections.svelte');
api.collections.list.mockResolvedValueOnce([collection('tasks')]);
await collectionStore.loadCollections('ws');
api.items.list.mockResolvedValueOnce([item('item-a')]);
await collectionStore.loadItems('ws');
expect(collectionStore.itemsAreFreshFor('ws')).toBe(true);
auth.fireIdentityChange();
expect(collectionStore.collectionsAreFreshFor('ws')).toBe(false);
expect(collectionStore.itemsAreFreshFor('ws')).toBe(false);
});
it('refuses a collections load that RESOLVES after the identity changed', async () => {
// The navigation fence cannot catch this: the workspace never changed,
// so this load is still the latest one for `ws`.
const { collectionStore } = await import('./collections.svelte');
let resolve!: (v: unknown) => void;
api.collections.list.mockReturnValueOnce(new Promise((r) => { resolve = r; }));
const inflight = collectionStore.loadCollections('ws');
auth.fireIdentityChange();
resolve([collection('tasks')]);
await inflight;
expect(collectionStore.collections).toHaveLength(0);
expect(collectionStore.collectionsAreFreshFor('ws')).toBe(false);
});
it('refuses an items load that resolves after the identity changed', async () => {
const { collectionStore } = await import('./collections.svelte');
let resolve!: (v: unknown) => void;
api.items.list.mockReturnValueOnce(new Promise((r) => { resolve = r; }));
const inflight = collectionStore.loadItems('ws');
auth.fireIdentityChange();
resolve([item('item-a')]);
await inflight;
expect(collectionStore.items).toHaveLength(0);
expect(collectionStore.itemsAreFreshFor('ws')).toBe(false);
});
it('refuses a COLLECTION-FILTERED items load that resolves after the change', async () => {
// The other arm of the same method. It assigns through a different
// branch and stamps `itemsWorkspace` differently, so fencing one arm
// leaves the other publishing another user's items.
const { collectionStore } = await import('./collections.svelte');
let resolve!: (v: unknown) => void;
api.items.listByCollection.mockReturnValueOnce(new Promise((r) => { resolve = r; }));
const inflight = collectionStore.loadItems('ws', 'tasks');
auth.fireIdentityChange();
resolve([item('item-a')]);
await inflight;
expect(collectionStore.items).toHaveLength(0);
});
it('returns null from loadItem when the identity moved, and does not publish it', async () => {
// The return value matters as much as the store write: the caller renders
// what it is handed, so handing back A's item puts it on B's screen even
// with `activeItem` untouched.
const { collectionStore } = await import('./collections.svelte');
let resolve!: (v: unknown) => void;
api.items.get.mockReturnValueOnce(new Promise((r) => { resolve = r; }));
const inflight = collectionStore.loadItem('ws', 'item-a');
auth.fireIdentityChange();
resolve(item('item-a'));
expect(await inflight).toBeNull();
expect(collectionStore.activeItem).toBeNull();
});
it('still commits an ordinary load when the identity holds still', async () => {
// The counterfactual. A fence that refused everything would leave every
// user with an empty sidebar, which is worse than the leak.
const { collectionStore } = await import('./collections.svelte');
api.collections.list.mockResolvedValueOnce([collection('tasks')]);
await collectionStore.loadCollections('ws');
api.items.list.mockResolvedValueOnce([item('item-a')]);
await collectionStore.loadItems('ws');
api.items.get.mockResolvedValueOnce(item('item-a'));
const loaded = await collectionStore.loadItem('ws', 'item-a');
expect(collectionStore.collections).toHaveLength(1);
expect(collectionStore.items).toHaveLength(1);
expect(loaded).not.toBeNull();
expect(collectionStore.collectionsAreFreshFor('ws')).toBe(true);
});
});
+23
View File
@@ -1,3 +1,5 @@
import { authStore } from './auth.svelte';
// Module-singleton editor scalars — ONE `dirty`/`lastSaveTime`/`saveStatus`
// for the whole page, regardless of how many <ItemDetail> instances are
// mounted. That's fine today (only one is ever live), but PLAN-2154 (the
@@ -38,4 +40,25 @@ export const editorStore = {
enterRaw() { mode = 'raw'; },
enterEdit() { mode = 'edit'; },
resetForDoc() { mode = 'edit'; dirty = false; externalChange = false; },
/**
* Drop every editor scalar (BUG-3005). Wider than `resetForDoc`, which
* deliberately keeps `saveStatus` and `lastSaveTime` because it is a
* DOCUMENT change within one session. An identity change ends the session.
*/
clear() {
mode = 'edit';
saveStatus = 'idle';
dirty = false;
lastSaveTime = 0;
externalChange = false;
},
};
// These scalars survive an SPA identity swap, and they are not merely stale —
// they are consumed by GUARDS. `dirty` drives unsaved-draft prompts and SSE
// archive/delete suppression, so A's dirty editor makes B's navigation and save
// paths act on a document B never opened (BUG-3005, codex round 1).
authStore.onIdentityChange(() => {
editorStore.clear();
});
@@ -0,0 +1,132 @@
import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest';
import { readFileSync } from 'node:fs';
import { resolve } from 'node:path';
/**
* BUG-3005 — what a real identity change does to the tab.
*
* The fix stopped being per-surface after three review rounds each found
* another layer, so the mechanism is now: clear the state a reload does NOT
* drop, then reload. These tests pin both halves, and the storage keys, because
* a clear that stops matching its key is silent.
*/
vi.mock('$app/environment', () => ({ browser: true }));
vi.mock('$lib/components/editor/attachment-metadata', () => ({
clearAttachmentMetadataCache: vi.fn(),
}));
import {
clearPersistentIdentityState,
reloadForIdentityChange,
RECENT_SEARCHES_KEY,
LAST_ROUTE_PREFIX,
LAST_SCROLL_PREFIX,
} from './identityReload.svelte';
import { CURSOR_STORAGE_PREFIX } from '$lib/collab/wsProvider.svelte';
import { clearAttachmentMetadataCache } from '$lib/components/editor/attachment-metadata';
describe('clearPersistentIdentityState', () => {
beforeEach(() => {
localStorage.clear();
sessionStorage.clear();
vi.mocked(clearAttachmentMetadataCache).mockClear();
});
it('drops the palette\'s recent searches', async () => {
// Search terms carry private item names and project context, and the key
// has no user in it.
localStorage.setItem(RECENT_SEARCHES_KEY, JSON.stringify(['alphas secret project']));
// PRECONDITION: it is really there.
expect(localStorage.getItem(RECENT_SEARCHES_KEY)).not.toBeNull();
clearPersistentIdentityState();
expect(localStorage.getItem(RECENT_SEARCHES_KEY)).toBeNull();
});
it('drops every collab cursor, and leaves unrelated session keys alone', async () => {
// Cursors are keyed by item id ALONE, so B opening an item A edited in
// this tab would inherit A's op-log position. The negative half matters
// as much: a clear that took the whole of sessionStorage would be a
// different, undocumented change.
sessionStorage.setItem(`${CURSOR_STORAGE_PREFIX}item-a`, '42');
sessionStorage.setItem(`${CURSOR_STORAGE_PREFIX}item-b`, '7');
sessionStorage.setItem('pad-unrelated', 'keep me');
clearPersistentIdentityState();
expect(sessionStorage.getItem(`${CURSOR_STORAGE_PREFIX}item-a`)).toBeNull();
expect(sessionStorage.getItem(`${CURSOR_STORAGE_PREFIX}item-b`)).toBeNull();
expect(sessionStorage.getItem('pad-unrelated')).toBe('keep me');
});
it('drops route and scroll memory, which carry private item paths', async () => {
// `pad-last-scroll-...` embeds the pathname in its own KEY, so a private
// item slug is legible from the key list without reading any value. Both
// are keyed by workspace with no user in them.
localStorage.setItem(`${LAST_ROUTE_PREFIX}ws`, '/alice/ws/ideas/alphas-secret');
localStorage.setItem(`${LAST_SCROLL_PREFIX}ws-/alice/ws/ideas/alphas-secret`, '420');
localStorage.setItem('pad-theme', 'dark');
clearPersistentIdentityState();
expect(localStorage.getItem(`${LAST_ROUTE_PREFIX}ws`)).toBeNull();
expect(localStorage.getItem(`${LAST_SCROLL_PREFIX}ws-/alice/ws/ideas/alphas-secret`)).toBeNull();
// And leaves an ordinary UI preference alone — the clear is scoped, not
// a localStorage wipe.
expect(localStorage.getItem('pad-theme')).toBe('dark');
});
it('LAST_ROUTE_PREFIX still matches the key workspace-route.ts builds', async () => {
const src = readFileSync(
resolve(__dirname, '../utils/workspace-route.ts'),
'utf8',
);
expect(src).toContain(LAST_ROUTE_PREFIX);
});
it('drops the attachment-metadata memo', async () => {
clearPersistentIdentityState();
expect(clearAttachmentMetadataCache).toHaveBeenCalledTimes(1);
});
it('RECENT_SEARCHES_KEY still matches the literal in CommandPalette.svelte', async () => {
// The palette is a .svelte file and cannot export the constant, so the
// clear duplicates it. A rename on either side would leave a clear that
// silently matches nothing — the failure this whole mechanism exists to
// avoid, reintroduced one string at a time.
const src = readFileSync(
resolve(__dirname, '../components/search/CommandPalette.svelte'),
'utf8',
);
const match = src.match(/RECENT_SEARCHES_KEY\s*=\s*'([^']+)'/);
expect(match, 'CommandPalette.svelte no longer declares RECENT_SEARCHES_KEY').not.toBeNull();
expect(match![1]).toBe(RECENT_SEARCHES_KEY);
});
});
describe('reloadForIdentityChange', () => {
const original = window.location;
afterEach(() => {
Object.defineProperty(window, 'location', { value: original, configurable: true });
});
it('clears first, then reloads', async () => {
const order: string[] = [];
vi.mocked(clearAttachmentMetadataCache).mockImplementation(() => { order.push('clear'); });
const reload = vi.fn(() => { order.push('reload'); });
Object.defineProperty(window, 'location', {
value: { ...original, reload },
configurable: true,
});
reloadForIdentityChange();
// ORDER, not just both: reloading first would tear the page down before
// the clears ran, leaving the palette history and collab cursors for the
// next user — the exact state the reload cannot drop by itself.
expect(order).toEqual(['clear', 'reload']);
});
});
+110
View File
@@ -0,0 +1,110 @@
import { browser } from '$app/environment';
import { clearAttachmentMetadataCache } from '$lib/components/editor/attachment-metadata';
import { CURSOR_STORAGE_PREFIX } from '$lib/collab/wsProvider.svelte';
/**
* What a real identity change does to this tab (BUG-3005, lead ruling after
* codex round 3): clear the persistent state a reload would NOT drop, then
* reload.
*
* WHY A RELOAD RATHER THAN MORE FENCES. Three review rounds each found another
* layer of surfaces holding the previous user's data — three stores, then
* fourteen route pages plus a connection and two caches, then the console
* subtree, share pages, the command palette and an open dialog. The population
* is "everything in the tab that ever held a fetch", and a per-surface fix
* cannot terminate: it is a list somebody has to keep complete forever. A
* reload drops every store, component, cache and in-flight request at once and
* nothing new has to be remembered.
*
* WHAT THE RELOAD DOES NOT DROP is exactly this function's other half.
* `localStorage`, `sessionStorage` and IndexedDB survive it, so those stay an
* enumeration — but a BOUNDED one, over storage keys rather than over every
* module in the app, and one that a grep for the storage APIs can check.
*
* The store-level fences stay too, and are not redundant with this: they cover
* the window between the identity changing and the page actually going away,
* during which in-flight responses are still settling into live stores.
*/
/** Must equal `RECENT_SEARCHES_KEY` in CommandPalette.svelte. Pinned by a test. */
export const RECENT_SEARCHES_KEY = 'pad-recent-searches';
/** Built by `workspace-route.ts`; the prefix is duplicated, pinned by a test. */
export const LAST_ROUTE_PREFIX = 'pad-last-route-';
/** Built inline by every page that uses `createScrollRestoration`. */
export const LAST_SCROLL_PREFIX = 'pad-last-scroll-';
export function clearPersistentIdentityState(): void {
if (!browser) return;
// The editor's attachment-metadata memo. In RAM, so the reload would get it
// anyway — cleared here so that this function is the one place the answer
// to "what does an identity change drop" lives.
clearAttachmentMetadataCache();
// The command palette's recent searches. Search terms carry private item
// names and project context, and the key has no user in it. The literal is
// duplicated from CommandPalette.svelte, which cannot export it — a guard
// test pins the two spellings together.
try {
localStorage.removeItem(RECENT_SEARCHES_KEY);
} catch {
// A browser with storage disabled throws on access. Nothing to clear.
}
// Route and scroll memory. `pad-last-route-<ws>` holds the last URL visited
// in a workspace — which can be a private item slug — and
// `pad-last-scroll-<ws>-<pathname>` embeds the path in its own KEY. Both are
// keyed by workspace with no user in them, so two accounts with access to
// the same workspace slug share them, and the second one is legible from
// the key list alone without reading a single value (codex round 4).
try {
for (const key of Object.keys(localStorage)) {
if (key.startsWith(LAST_ROUTE_PREFIX) || key.startsWith(LAST_SCROLL_PREFIX)) {
localStorage.removeItem(key);
}
}
} catch {
// As above.
}
// Collaborative-editing cursors, keyed by item id ALONE. B opening an item
// A edited in this tab would otherwise inherit A's op-log cursor and
// reconcile against a document history that is not theirs.
try {
for (const key of Object.keys(sessionStorage)) {
if (key.startsWith(CURSOR_STORAGE_PREFIX)) sessionStorage.removeItem(key);
}
} catch {
// As above.
}
}
/**
* Clear what survives a reload, then reload — the SWAP path only.
*
* A swap (one signed-in user to another) is the transition where nothing else
* navigates, so the reload is the whole mechanism. A SIGN-OUT does not come
* here: both sign-out sites navigate away by themselves, and a reload racing
* them aborts one of the two — which is exactly what it did, caught by
* `account-delete.spec.ts` rather than by any reasoning of mine. Sign-out
* calls `clearPersistentIdentityState` directly and lets its own hard
* navigation drop the rest.
*
* Separated from `clearPersistentIdentityState` so a test can drive the clears
* without navigating, and so the reload itself is one mockable call rather
* than a bare `location.reload()` buried in a layout.
*/
export function reloadForIdentityChange(): void {
if (!browser) return;
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();
}
}
@@ -0,0 +1,167 @@
import { describe, expect, it, vi, beforeEach } from 'vitest';
/**
* BUG-3005, codex round 1. The stores round 1 found still holding per-user
* state, plus the unfenced `loading` write in collectionStore.
*
* The four stores named on the item were not the population — the enumeration
* that produced them swept for per-user FIELDS, and these hold user-scoped
* state that does not look like one: an authorization-scoped statistics blob,
* a browser-tab title retired by a PATH stamp that an account swap does not
* move, and editor scalars that are consumed by GUARDS rather than displayed.
*/
const api = vi.hoisted(() => ({
collections: { list: vi.fn() },
items: { list: vi.fn(), listByCollection: vi.fn(), get: vi.fn() },
}));
vi.mock('$lib/api/client', () => ({ api }));
vi.mock('./localIndexPersistence', () => ({
hydrateCollections: vi.fn(async () => null),
persistCollections: vi.fn(async () => {}),
readDurableEpoch: vi.fn(async () => null),
}));
const auth = vi.hoisted(() => {
const listeners = new Set<() => void>();
let epoch = 0;
return {
userId: 'user-1',
get identityEpoch() { return epoch; },
identityFence() {
const captured = epoch;
return () => epoch === captured;
},
onIdentityChange(fn: () => void) {
listeners.add(fn);
return () => listeners.delete(fn);
},
fireIdentityChange() {
epoch++;
for (const fn of listeners) fn();
},
resetListeners() {
listeners.clear();
epoch = 0;
},
};
});
vi.mock('./auth.svelte', () => ({ authStore: auth }));
beforeEach(() => {
vi.resetModules();
auth.resetListeners();
api.collections.list.mockReset();
api.items.list.mockReset();
});
describe('collectionStore loading flag', () => {
it('is not cleared by a load that settles after the identity changed', async () => {
// `loading` is module state written after an await like any other. A
// settling as B's load is still in flight clears B's flag and flashes
// empty-state or error UI mid-load.
const { collectionStore } = await import('./collections.svelte');
let resolveA!: (v: unknown) => void;
api.items.list.mockReturnValueOnce(new Promise((r) => { resolveA = r; }));
const loadA = collectionStore.loadItems('ws');
expect(collectionStore.loading).toBe(true);
auth.fireIdentityChange();
// PRECONDITION: the identity reset owns the flag, so nothing is stuck.
expect(collectionStore.loading).toBe(false);
// B starts its own load, then A settles underneath it.
api.items.list.mockReturnValueOnce(new Promise(() => {}));
void collectionStore.loadItems('ws');
expect(collectionStore.loading).toBe(true);
resolveA([]);
await loadA;
expect(collectionStore.loading).toBe(true);
});
});
describe('adminStore across an identity change', () => {
it('drops the previous admin\'s statistics', async () => {
// `stats` is REALLY loaded first, through a stubbed fetch. Asserting
// null before and after would pass against a store with no clear at
// all — the failure mode this whole file exists to avoid.
const fetchMock = vi.fn(async () => ({
ok: true,
json: async () => ({ users: 42 }),
}));
vi.stubGlobal('fetch', fetchMock);
const { adminStore } = await import('./admin.svelte');
await adminStore.loadStats();
// PRECONDITION: the statistics are actually present.
expect(adminStore.stats).not.toBeNull();
expect(adminStore.loading).toBe(false);
auth.fireIdentityChange();
expect(adminStore.stats).toBeNull();
// FALSE, not true (codex round 2). The admin layout loads on MOUNT and a
// same-route admin-to-admin swap mounts nothing, so leaving this true
// pinned the console on "Loading admin data…" forever. The layout
// re-issues the load on the same signal; until then the honest state is
// "nothing loaded".
expect(adminStore.loading).toBe(false);
expect(adminStore.error).toBe('');
vi.unstubAllGlobals();
});
});
describe('titleStore across an identity change', () => {
it('drops the previous user\'s workspace and item title', async () => {
const { titleStore } = await import('./title.svelte');
titleStore.setPageTitle({ workspace: 'Acme', section: 'Ideas', item: 'IDEA-1' });
// PRECONDITION: the workspace part is set — it is the unstamped one, so
// it is the part a route change would NOT retire.
expect(titleStore.workspace).toBe('Acme');
auth.fireIdentityChange();
expect(titleStore.workspace).toBeUndefined();
expect(titleStore.section).toBeUndefined();
expect(titleStore.item).toBeUndefined();
});
});
describe('editorStore across an identity change', () => {
it('drops dirty and saveStatus, which are read by GUARDS', async () => {
const { editorStore } = await import('./editor.svelte');
editorStore.setDirty(true);
editorStore.setSaveStatus('saving');
editorStore.setMode('raw');
editorStore.setLastSaveTime(1234);
editorStore.setExternalChange(true);
// PRECONDITION: all five are set, so the assertion below is about the
// clear rather than about defaults.
expect(editorStore.dirty).toBe(true);
expect(editorStore.saveStatus).toBe('saving');
auth.fireIdentityChange();
expect(editorStore.dirty).toBe(false);
expect(editorStore.saveStatus).toBe('idle');
expect(editorStore.mode).toBe('edit');
expect(editorStore.lastSaveTime).toBe(0);
expect(editorStore.externalChange).toBe(false);
});
it('is WIDER than resetForDoc, which keeps saveStatus deliberately', async () => {
// The counterfactual for reusing `resetForDoc` here: it is a DOCUMENT
// change within one session and keeps `saveStatus`/`lastSaveTime` on
// purpose. An identity change ends the session, so it needs the wider
// clear and cannot borrow that one.
const { editorStore } = await import('./editor.svelte');
editorStore.setSaveStatus('saved');
editorStore.setLastSaveTime(99);
editorStore.resetForDoc();
expect(editorStore.saveStatus).toBe('saved');
expect(editorStore.lastSaveTime).toBe(99);
});
});
@@ -0,0 +1,188 @@
import { describe, expect, it, vi, beforeEach } from 'vitest';
/**
* BUG-3005, codex round 2. Four defects the round-1 fixes left or created.
*
* The sharpest is the single-flight slot: dropping a store's DATA on an
* identity change while leaving the previous identity's promise published made
* the reset actively harmful, because the next caller joins a request whose own
* fence then refuses to commit.
*/
const api = vi.hoisted(() => ({
collections: { list: vi.fn() },
items: { list: vi.fn(), listByCollection: vi.fn(), get: vi.fn(), starred: vi.fn() },
workspaces: { list: vi.fn(), get: vi.fn(), me: vi.fn(), create: vi.fn() },
}));
vi.mock('$lib/api/client', () => ({ api }));
vi.mock('./localIndexPersistence', () => ({
hydrateCollections: vi.fn(async () => null),
persistCollections: vi.fn(async () => {}),
readDurableEpoch: vi.fn(async () => null),
}));
const auth = vi.hoisted(() => {
const listeners = new Set<() => void>();
let epoch = 0;
let id = 'user-1';
return {
get userId() { return id; },
setUserId(next: string) { id = next; },
get identityEpoch() { return epoch; },
identityFence() {
const captured = epoch;
return () => epoch === captured;
},
onIdentityChange(fn: () => void) {
listeners.add(fn);
return () => listeners.delete(fn);
},
fireIdentityChange(nextId?: string) {
if (nextId !== undefined) id = nextId;
epoch++;
for (const fn of listeners) fn();
},
resetListeners() {
listeners.clear();
epoch = 0;
id = 'user-1';
},
};
});
vi.mock('./auth.svelte', () => ({ authStore: auth }));
beforeEach(() => {
vi.resetModules();
auth.resetListeners();
api.collections.list.mockReset();
api.items.starred.mockReset();
});
describe('collectionStore single-flight slot', () => {
it('does not leave B joining A\'s abandoned request', async () => {
// Without the invalidation: B's `ensureCollections` joins A's published
// promise, A's identity fence refuses to commit, and B ends with no
// collections AND no request of its own — a permanently empty sidebar,
// which is worse than the leak the reset was closing.
const { collectionStore } = await import('./collections.svelte');
let resolveA!: (v: unknown) => void;
api.collections.list.mockReturnValueOnce(new Promise((r) => { resolveA = r; }));
const loadA = collectionStore.loadCollections('ws');
// `loadCollections` reads the durable epoch before it fetches, so the
// request is one microtask away rather than synchronous.
await Promise.resolve();
await Promise.resolve();
// PRECONDITION: A's request is the published one.
expect(api.collections.list).toHaveBeenCalledTimes(1);
auth.fireIdentityChange();
// B asks for a list. It must ISSUE, not join.
api.collections.list.mockResolvedValueOnce([
{ id: 'c1', slug: 'tasks', name: 'Tasks', is_default: true, sort_order: 1 },
]);
await collectionStore.ensureCollections('ws');
expect(api.collections.list).toHaveBeenCalledTimes(2);
expect(collectionStore.collections).toHaveLength(1);
expect(collectionStore.collectionsAreFreshFor('ws')).toBe(true);
// And A settling afterwards still commits nothing.
resolveA([{ id: 'c9', slug: 'alphas', name: 'Alphas', is_default: true, sort_order: 1 }]);
await loadA;
expect(collectionStore.collections).toHaveLength(1);
expect(collectionStore.collections[0].slug).toBe('tasks');
});
});
describe('adminStore.loadStats', () => {
it('refuses a response that settles after the identity changed', async () => {
let resolveFetch!: (v: unknown) => void;
const fetchMock = vi.fn(() => new Promise((r) => { resolveFetch = r; }));
vi.stubGlobal('fetch', fetchMock);
const { adminStore } = await import('./admin.svelte');
const inflight = adminStore.loadStats();
// PRECONDITION: the request is actually out.
expect(fetchMock).toHaveBeenCalledTimes(1);
auth.fireIdentityChange();
resolveFetch({ ok: true, json: async () => ({ users: 42 }) });
await inflight;
expect(adminStore.stats).toBeNull();
vi.unstubAllGlobals();
});
});
describe('toastStore across an identity change', () => {
it('drops live toasts AND the history, which names items', async () => {
const { toastStore } = await import('./toast.svelte');
toastStore.show({ message: 'Archived TASK-12', type: 'success' });
// PRECONDITION: both surfaces hold it, or the assertion below is about
// arrays that were always empty.
expect(toastStore.toasts.length).toBeGreaterThan(0);
expect(toastStore.history.length).toBeGreaterThan(0);
auth.fireIdentityChange();
expect(toastStore.toasts).toHaveLength(0);
expect(toastStore.history).toHaveLength(0);
expect(toastStore.unreadCount).toBe(0);
});
});
describe('workspaceStore single-flight slot across A -> B -> A', () => {
it('refuses a list issued in the FIRST A session', async () => {
// Where the sequence bump inside `invalidate` is the only guard left,
// and the reason it is there rather than just clearing the slot.
//
// `loadAll` fences its commit on the captured USER ID, which is the
// comparison the epoch exists to replace: after A -> B -> A that id
// agrees, so the only thing standing between A's first-session list and
// B-then-A's store is `isLatest` — and `isLatest` is a NAVIGATION fence
// that no identity change moves on its own.
const { workspaceStore } = await import('./workspace.svelte');
let resolveA!: (v: unknown) => void;
api.workspaces.list.mockReturnValueOnce(new Promise((r) => { resolveA = r; }));
const loadA = workspaceStore.loadAll();
expect(api.workspaces.list).toHaveBeenCalledTimes(1);
auth.fireIdentityChange('user-b');
auth.fireIdentityChange('user-1');
// A's first-session response arrives last. The captured id matches the
// current one — this is the same user — and it must still be refused.
resolveA([{ id: 'w-alpha', slug: 'alphas-private', name: "Alpha's" }]);
await loadA;
expect(workspaceStore.workspaces).toHaveLength(0);
});
});
describe('openChildrenDialog across an identity change', () => {
it('abandons the queued mutation instead of letting B confirm it', async () => {
// The one place on this branch where the item's own bound ("nothing here
// is an authorization bypass") would stop being true: the dialog holds
// the FETCHED child list and a continuation that PERFORMS A's mutation
// when confirmed. It is mounted in the root layout, outside anything the
// workspace layout controls.
const { openChildrenDialog } = await import('./openChildrenDialog.svelte');
const details = { count: 2, children: [{ ref: 'TASK-9', title: "Alpha's child" }] } as never;
const first = openChildrenDialog.request('TASK-1', details);
const queued = openChildrenDialog.request('TASK-2', details);
// PRECONDITION: one is showing and one is queued behind it, so the test
// covers the queue and not just the active entry.
expect(openChildrenDialog.active).not.toBeNull();
auth.fireIdentityChange('user-b');
// FALSE, not a hanging promise: the producer awaiting this takes its
// cancel path and does not perform the write.
await expect(first).resolves.toBe(false);
await expect(queued).resolves.toBe(false);
expect(openChildrenDialog.active).toBeNull();
});
});
+52
View File
@@ -70,6 +70,7 @@ import {
} from './localIndexPersistence';
import { preserveProjectionMetadata, mergeEqualSeqProjection } from './itemRowMerge';
import { localSearch } from './localSearch.svelte';
import { authStore } from './auth.svelte';
import type { Item, ItemChangeRow, ItemIndexRow } from '$lib/types';
export type BootstrapState = 'cold' | 'loading' | 'ready' | 'error';
@@ -2293,8 +2294,59 @@ export const localIndex = {
persistWipe(priorUserId, ws).catch(() => undefined);
},
/**
* Drop EVERY workspace's state — the identity-change sweep (BUG-3005).
*
* `reset(ws)` is per-workspace and the sign-out path called it for the ONE
* workspace the layout happened to be looking at. Everything this store
* holds is per-user: rows the previous user could see, their cursor, their
* access epoch. A tab that visited two workspaces kept the other one.
*/
resetAll(): void {
for (const ws of [...workspaces.keys()]) {
localIndex.reset(ws);
}
// `reset` already drops the search index for each workspace it visits.
// This catches an index whose workspace state was already gone: the two
// maps are keyed independently and nothing keeps them in step.
localSearch.resetAll();
// THE DURABLE WIPE THIS INHERITS, and why it is kept (codex round 1).
// `reset` fire-and-forgets `persistWipe` for the workspace's last-known
// userId, so this drops the previous user's IndexedDB cache as well as
// their RAM. Not required for ISOLATION — the cache is namespaced per
// (user, workspace), so the next user could never read it — and it costs
// a returning user their warm cache.
//
// Kept because it is the behaviour sign-out already had (the workspace
// layout has called `reset` on sign-out since TASK-1360) and because
// leaving a signed-out user's item titles on the disk of a shared
// browser is the wrong default to adopt silently. What this DOES widen
// is the pre-existing race where a fast re-login's hydrate overlaps a
// wipe still in flight: it now applies to every workspace the tab
// visited rather than the one the layout named. Filed rather than fixed
// here — sequencing a fire-and-forget wipe against the next hydrate is
// a change to the persistence layer, not to this sweep.
},
/** Number of items currently held for a workspace. Test/debug aid. */
size(ws: string): number {
return workspaces.get(ws)?.items.size ?? 0;
},
};
// Drop every workspace's rows and search index when the signed-in user changes
// (BUG-3005).
//
// This store reset only on `bootstrap()` noticing a userId mismatch, which
// makes the invalidation depend on somebody calling bootstrap: a route that
// does not bootstrap the index on an identity change kept the previous user's
// rows in RAM, and `localSearch` kept a full-text index built from their item
// titles and bodies. The subscription makes it structural — the same reason
// `onIdentityChange` exists rather than a call at each sign-out site.
//
// The bootstrap mismatch check STAYS. It covers a different case: a first
// bootstrap for a workspace whose cached state predates this tab's identity
// signal, where no change has fired because nothing changed within this tab.
authStore.onIdentityChange(() => {
localIndex.resetAll();
});
@@ -0,0 +1,130 @@
import { describe, expect, it, vi, beforeEach } from 'vitest';
/**
* BUG-3005 — `localIndex` and `localSearch` held the previous user's rows
* across an SPA identity change.
*
* `localIndex` reset only when `bootstrap()` noticed a userId mismatch, which
* makes the invalidation depend on somebody calling bootstrap. A route that
* does not bootstrap the index on an identity change kept the rows in RAM —
* and `localSearch`, whose ONLY reset callers live inside `localIndex`, kept a
* full-text index built from the previous user's item titles and bodies.
*
* The per-workspace shape is the other half of the defect: `reset(ws)` names
* one slug, and a tab that visited two workspaces kept the one the sign-out
* path did not happen to be looking at.
*/
vi.mock('$lib/api/client', () => ({
api: { items: { index: vi.fn(), changes: vi.fn() } },
PadApiError: class extends Error {},
}));
// Real epoch, real listener set — a stub would let these pass against a store
// that subscribes to nothing.
const auth = vi.hoisted(() => {
const listeners = new Set<() => void>();
let epoch = 0;
return {
userId: 'user-1',
get identityEpoch() { return epoch; },
identityFence() {
const captured = epoch;
return () => epoch === captured;
},
onIdentityChange(fn: () => void) {
listeners.add(fn);
return () => listeners.delete(fn);
},
fireIdentityChange() {
epoch++;
for (const fn of listeners) fn();
},
resetListeners() {
listeners.clear();
epoch = 0;
},
};
});
vi.mock('./auth.svelte', () => ({ authStore: auth }));
function row(id: string, title: string) {
return {
id,
seq: 1,
title,
collection_slug: 'colors',
created_at: '2026-01-01T00:00:00Z',
updated_at: '2026-01-01T00:00:00Z',
} as never;
}
describe('localIndex / localSearch across an identity change', () => {
beforeEach(() => {
vi.resetModules();
auth.resetListeners();
});
it('drops EVERY workspace, not just the one a sign-out path names', async () => {
const { localIndex } = await import('./localIndex.svelte');
localIndex.upsert('ws-one', row('a', 'Alpha'));
localIndex.upsert('ws-two', row('b', 'Beta'));
// PRECONDITION: both workspaces hold rows, or "both were dropped" is a
// claim about state that was never there. This is the assertion the
// per-workspace reset would fail — it visits one slug.
expect(localIndex.size('ws-one')).toBe(1);
expect(localIndex.size('ws-two')).toBe(1);
auth.fireIdentityChange();
expect(localIndex.size('ws-one')).toBe(0);
expect(localIndex.size('ws-two')).toBe(0);
});
it('drops the SEARCH index, so the next user cannot read the last one\'s titles', async () => {
const { localIndex } = await import('./localIndex.svelte');
const { localSearch } = await import('./localSearch.svelte');
localIndex.upsert('ws-one', row('a', 'Alpha'));
localSearch.rebuild('ws-one', [row('a', 'Alpha')]);
// PRECONDITION: the title is findable, or the test cannot tell a dropped
// index from an index that never matched.
expect(localSearch.search('ws-one', 'Alpha').length).toBeGreaterThan(0);
auth.fireIdentityChange();
expect(localSearch.search('ws-one', 'Alpha')).toHaveLength(0);
});
it('drops an index whose workspace state is already gone', async () => {
// The two maps are keyed independently and nothing keeps them in step,
// so a sweep that only visits the workspaces `localIndex` still knows
// about would leave this index resident. That is why `resetAll` calls
// `localSearch.resetAll()` rather than relying on the per-workspace
// resets it just performed.
const { localIndex } = await import('./localIndex.svelte');
const { localSearch } = await import('./localSearch.svelte');
localSearch.rebuild('orphan-ws', [row('a', 'Alpha')]);
// PRECONDITION: localIndex holds nothing for this slug, so its own sweep
// will never visit it.
expect(localIndex.size('orphan-ws')).toBe(0);
expect(localSearch.search('orphan-ws', 'Alpha').length).toBeGreaterThan(0);
auth.fireIdentityChange();
expect(localSearch.search('orphan-ws', 'Alpha')).toHaveLength(0);
});
it('keeps the rows when the identity holds still', async () => {
// The counterfactual. A sweep that fired on anything else would empty
// the local-first cache during ordinary use, which is a worse defect
// than the leak: every read would fall back to the network.
const { localIndex } = await import('./localIndex.svelte');
localIndex.upsert('ws-one', row('a', 'Alpha'));
expect(localIndex.size('ws-one')).toBe(1);
});
});
+16
View File
@@ -499,6 +499,22 @@ export const localSearch = {
epochs.delete(ws);
},
/**
* Drop EVERY workspace's index (BUG-3005).
*
* `reset` is per-workspace and its callers name a slug, which is right for
* a 403 purge or a workspace deletion. An identity change is not scoped to
* a workspace: every index in this map was built from item titles and
* bodies the PREVIOUS user could see, and this map is keyed independently
* of `localIndex`'s — so an index whose workspace state is already gone
* would survive a sweep that only visited the workspaces localIndex still
* knows about.
*/
resetAll(): void {
indexes.clear();
epochs.clear();
},
/**
* Reactive per-workspace mutation epoch. Bumped on every successful
* `rebuild` / `upsert` / `remove`. Consumers should READ this inside
@@ -12,6 +12,7 @@
// resolves; queueing avoids dropping the second prompt on the floor.
import type { OpenChildrenDetails } from '$lib/items/openChildrenError';
import { authStore } from './auth.svelte';
interface PendingRequest {
parentRef: string;
@@ -59,11 +60,45 @@ function cancel(): void {
advanceQueue();
}
/**
* Abandon the active prompt and everything queued behind it (BUG-3005).
*
* Every pending request resolves FALSE — the same answer a cancel gives — so
* the producer awaiting it takes its cancel path and does not perform the
* mutation. Rejecting instead would surface an unhandled rejection at call
* sites that only ever expected a boolean.
*/
function abandonAll(): void {
const pending = active ? [active, ...queue] : [...queue];
queue.length = 0;
active = null;
for (const entry of pending) entry.resolve(false);
}
export const openChildrenDialog = {
get active(): PendingRequest | null {
return active;
},
request,
confirm,
cancel
cancel,
abandonAll
};
// This dialog is mounted in the ROOT layout, outside the workspace subtree the
// identity remount covers, and it holds two things that must not cross an
// identity change (BUG-3005, codex round 3):
//
// - `details`, the FETCHED child list from the 409 — B would read A's child
// titles and statuses out of an open dialog;
// - `resolve`, a continuation that PERFORMS A'S MUTATION when confirmed. That
// is a write under the wrong identity, not a display leak, and it is the
// one place on this branch where the item's own bound ("nothing here is an
// authorization bypass") would stop being true.
//
// Resolving false is what makes the second half safe: the producer sees a
// cancel and abandons the write, rather than being left with a promise that
// never settles.
authStore.onIdentityChange(() => {
openChildrenDialog.abandonAll();
});
+22
View File
@@ -74,6 +74,18 @@ export interface KeyedSingleFlight<K> {
*/
inFlightFor(key: K): Promise<void> | null;
/**
* Abandon the current run and the published slot (BUG-3005, codex round 2).
*
* A single-flight slot outlives the DATA its owner drops. Clearing a store
* on an identity change without this leaves the previous identity's promise
* published, so the next caller JOINS it, that run's own fence then refuses
* to commit, and the joiner is answered with nothing — no request issued and
* no data. Bumping the sequence is what makes the abandoned run lose
* `isLatest` as well, so it cannot commit after the slot is cleared either.
*/
invalidate(): void;
/**
* Issue a run. ALWAYS issues; never joins an existing one.
*
@@ -99,6 +111,16 @@ export function createKeyedSingleFlight<K>(options: SingleFlightOptions = {}): K
return null;
},
invalidate(): void {
// The bump is the load-bearing half: it is what strips `isLatest`
// from the run being abandoned. Clearing the slot alone would stop
// new joiners while still letting that run commit.
seq++;
inFlightKey = null;
inFlightPromise = null;
setLoading?.(false);
},
run(key: K, work: (run: SingleFlightRun) => Promise<void>): Promise<void> {
const mySeq = ++seq;
setLoading?.(true);
+31 -4
View File
@@ -1,4 +1,5 @@
import { api } from '$lib/api/client';
import { authStore } from './auth.svelte';
// Set of starred item IDs for the current user in the current workspace
let starredIds = $state<Set<string>>(new Set());
@@ -41,6 +42,14 @@ export const starredStore = {
async load(wsSlug: string) {
currentWs = wsSlug;
const seq = ++requestSeq;
// TWO FENCES, for two races (BUG-3005). `seq` is the navigation fence: a
// slower load for workspace A must not overwrite a faster one for B.
// The identity fence is a different question — this list is PER USER, so
// a response issued as A must not land in B's session even when the
// workspace never changed. Neither subsumes the other: a same-route
// account swap moves no workspace, and a workspace switch moves no
// identity.
const isSameIdentity = authStore.identityFence();
pendingToggles.clear();
// Clear stale state immediately to avoid showing a previous user/workspace's stars
starredIds = new Set();
@@ -48,11 +57,11 @@ export const starredStore = {
try {
const items = await api.items.starred(wsSlug, { include_terminal: true });
if (seq !== requestSeq) return;
if (seq !== requestSeq || !isSameIdentity()) return;
starredIds = applyPendingToggles(new Set(items.map(i => i.id)));
loaded = true;
} catch {
if (seq !== requestSeq) return;
if (seq !== requestSeq || !isSameIdentity()) return;
// Preserve any optimistic toggles even if the load failed
starredIds = applyPendingToggles(new Set());
loaded = true;
@@ -64,6 +73,7 @@ export const starredStore = {
// Drop rapid duplicate clicks while a toggle is in flight for this item
if (toggleInFlight.has(itemId)) return;
const isSameIdentity = authStore.identityFence();
const wasStarred = starredIds.has(itemId);
const nowStarred = !wasStarred;
@@ -87,8 +97,11 @@ export const starredStore = {
await api.items.star(wsSlug, itemSlug);
}
} catch {
// Revert on failure (only if still on the same workspace)
if (currentWs !== wsSlug) return;
// Revert on failure — only if this is still the same workspace AND
// the same signed-in user. Without the identity half, a star request
// that fails after an account swap writes A's item id into B's
// starred set (BUG-3005).
if (currentWs !== wsSlug || !isSameIdentity()) return;
pendingToggles.set(itemId, wasStarred);
const reverted = new Set(starredIds);
if (wasStarred) {
@@ -110,3 +123,17 @@ export const starredStore = {
toggleInFlight.clear();
}
};
// Drop this user's stars the moment the signed-in user changes (BUG-3005).
//
// `load()` clears before it fetches, which covers a workspace SWITCH — but it
// is keyed by workspace slug and a same-route account swap starts no new load,
// so A's starred ids stayed on screen for B until something else happened to
// trigger one. The fences above cover a settle that RACES the change; this
// covers the case where nothing is racing at all, which is the common one.
//
// Module scope, registered once, never unsubscribed: this store is a singleton
// and its lifetime is the tab's.
authStore.onIdentityChange(() => {
starredStore.clear();
});
@@ -0,0 +1,155 @@
import { describe, expect, it, vi, beforeEach } from 'vitest';
/**
* BUG-3005 — `starredStore` holds a PER-USER set and was only ever cleared
* inside `load()`, which is keyed by workspace slug.
*
* A same-route account swap starts no new load, so A's starred ids stayed
* rendered inside B's session with no request having been made for them. Two
* halves that fail independently: the identity-change reset (nothing racing),
* and the identity fence on a settle (something racing).
*/
const api = vi.hoisted(() => ({
items: {
starred: vi.fn(),
star: vi.fn(),
unstar: vi.fn(),
},
}));
vi.mock('$lib/api/client', () => ({ api }));
// A REAL epoch and fence, not stubs. A fence stub that always answered "still
// current" would let every assertion below pass against a store with no fence.
const auth = vi.hoisted(() => {
const listeners = new Set<() => void>();
let epoch = 0;
return {
userId: 'user-1',
get identityEpoch() { return epoch; },
identityFence() {
const captured = epoch;
return () => epoch === captured;
},
onIdentityChange(fn: () => void) {
listeners.add(fn);
return () => listeners.delete(fn);
},
/** What authStore.clear() or a sign-in as someone else fires. */
fireIdentityChange() {
epoch++;
for (const fn of listeners) fn();
},
resetListeners() {
listeners.clear();
epoch = 0;
},
};
});
vi.mock('./auth.svelte', () => ({ authStore: auth }));
describe('starredStore across an identity change', () => {
beforeEach(() => {
vi.resetModules();
auth.resetListeners();
api.items.starred.mockReset();
api.items.star.mockReset();
api.items.unstar.mockReset();
});
it('drops the previous user\'s stars when the identity changes with nothing in flight', async () => {
const { starredStore } = await import('./starred.svelte');
api.items.starred.mockResolvedValueOnce([{ id: 'item-a' }]);
await starredStore.load('ws');
// PRECONDITION: A's stars are actually loaded, or "they were dropped" is
// a claim about a set that was always empty.
expect(starredStore.isStarred('item-a')).toBe(true);
expect(starredStore.loaded).toBe(true);
auth.fireIdentityChange();
expect(starredStore.isStarred('item-a')).toBe(false);
expect(starredStore.ids.size).toBe(0);
expect(starredStore.loaded).toBe(false);
});
it('refuses a load that RESOLVES after the identity changed', async () => {
const { starredStore } = await import('./starred.svelte');
let resolve!: (v: unknown) => void;
api.items.starred.mockReturnValueOnce(new Promise((r) => { resolve = r; }));
const inflight = starredStore.load('ws');
auth.fireIdentityChange();
resolve([{ id: 'item-a' }]);
await inflight;
expect(starredStore.isStarred('item-a')).toBe(false);
expect(starredStore.loaded).toBe(false);
});
it('refuses a FAILED load that rejects after the identity changed', async () => {
// The catch arm commits too — it publishes an empty set and flips
// `loaded` — so a fence on the success path alone leaves the store
// claiming, inside B's session, that B's stars are loaded.
const { starredStore } = await import('./starred.svelte');
let reject!: (e: unknown) => void;
api.items.starred.mockReturnValueOnce(new Promise((_, r) => { reject = r; }));
const inflight = starredStore.load('ws');
auth.fireIdentityChange();
reject(new Error('network'));
await inflight;
expect(starredStore.loaded).toBe(false);
});
it('does not revert a failed UNSTAR into the NEXT user\'s set', async () => {
// Direction matters, and the first version of this test had it backwards.
// Reverting a failed STAR is a delete, which is invisible in an empty
// set — the test passed with no identity fence at all. Reverting a failed
// UNSTAR is an ADD, and that is the write that lands one user's starred
// item in another user's set.
//
// B also signs back into the SAME workspace before the revert fires,
// because the identity-change reset blanks `currentWs` and would
// otherwise be the guard doing the work. Both halves are needed for this
// to discriminate.
const { starredStore } = await import('./starred.svelte');
api.items.starred.mockResolvedValueOnce([{ id: 'item-a' }]);
await starredStore.load('ws');
expect(starredStore.isStarred('item-a')).toBe(true);
let reject!: (e: unknown) => void;
api.items.unstar.mockReturnValueOnce(new Promise((_, r) => { reject = r; }));
const toggling = starredStore.toggle('ws', 'item-a-slug', 'item-a');
// PRECONDITION: the optimistic REMOVE landed, so the revert is an add.
expect(starredStore.isStarred('item-a')).toBe(false);
auth.fireIdentityChange();
api.items.starred.mockResolvedValueOnce([]);
await starredStore.load('ws');
// PRECONDITION: B's set is loaded and empty, and the workspace fence can
// no longer discriminate.
expect(starredStore.loaded).toBe(true);
expect(starredStore.ids.size).toBe(0);
reject(new Error('nope'));
await toggling;
expect(starredStore.isStarred('item-a')).toBe(false);
});
it('still commits an ordinary load when the identity holds still', async () => {
// The counterfactual. A fence that refused everything would empty the
// starred set for every user permanently, which is worse than the leak.
const { starredStore } = await import('./starred.svelte');
api.items.starred.mockResolvedValueOnce([{ id: 'item-a' }]);
await starredStore.load('ws');
expect(starredStore.isStarred('item-a')).toBe(true);
expect(starredStore.loaded).toBe(true);
});
});
+10
View File
@@ -1,3 +1,4 @@
import { authStore } from './auth.svelte';
/**
* Title store — central source of truth for the browser-tab title.
*
@@ -128,3 +129,12 @@ export const titleStore = {
itemPath = undefined;
},
};
// The path STAMP is what normally retires a title part, and a same-route
// identity change moves no path — so A's workspace name and item ref stayed in
// the browser tab and the mobile context bar for B (BUG-3005, codex round 1).
// Cosmetic rather than a data leak of any size, and cheap enough that leaving
// one member of the class unfixed costs more than fixing it.
authStore.onIdentityChange(() => {
titleStore.clearPageTitle();
});
+25 -1
View File
@@ -1,3 +1,4 @@
import { authStore } from './auth.svelte';
export interface ToastAction {
label: string;
onAction: () => void;
@@ -116,6 +117,19 @@ function clearHistory(): void {
unreadCount = 0;
}
/**
* Drop live toasts, the history and the unread count (BUG-3005).
*
* Timers are cancelled rather than left to fire: a pending auto-dismiss for a
* toast that no longer exists would call `dismiss` on a missing id, and the
* map would keep the handle until then.
*/
function clearAll(): void {
for (const id of [...timers.keys()]) clearTimerFor(id);
toasts = [];
clearHistory();
}
export const toastStore = {
get toasts(): Toast[] {
return toasts;
@@ -129,5 +143,15 @@ export const toastStore = {
show,
dismiss,
markAllRead,
clearHistory
clearHistory,
clearAll
};
// The notification tray is a readable LOG of the previous user's actions
// (BUG-3005, the enumeration table). Toast text routinely names items —
// "Archived TASK-12" — so leaving the history in place lets B read what A just
// did. Live toasts go too: a toast fired for A's action has no meaning in B's
// session, and `dismissAll` also cancels their timers.
authStore.onIdentityChange(() => {
toastStore.clearAll();
});
+8
View File
@@ -580,4 +580,12 @@ authStore.onIdentityChange(() => {
current = null;
currentMembership = null;
membershipKnown = false;
// The SINGLE-FLIGHT SLOT, for the reason collectionStore's reset carries
// the same line (BUG-3005, codex round 2): dropping the list while leaving
// the previous identity's promise published lets the next caller JOIN it,
// that run's own fence then refuses to commit, and the joiner gets no data
// and no request. Not a defect this file was filed for — it is the same
// defect in the store BUG-2991 fixed, and leaving the identical one line
// away would be knowing about it and not saying so.
loadAllFlight.invalidate();
});
+45 -1
View File
@@ -5,6 +5,10 @@
import { page } from '$app/state';
import { workspaceStore } from '$lib/stores/workspace.svelte';
import { authStore } from '$lib/stores/auth.svelte';
import {
reloadForIdentityChange,
clearPersistentIdentityState,
} from '$lib/stores/identityReload.svelte';
import { uiStore } from '$lib/stores/ui.svelte';
import { titleStore } from '$lib/stores/title.svelte';
import { setAccessRevokedHandler, setRateLimitHandler } from '$lib/api/client';
@@ -105,8 +109,48 @@
// Deliberately outside the async body below — an `onMount` that returns
// a Promise has its resolved value ignored, so a cleanup returned from
// an async `onMount` is never called.
return authStore.onIdentityChange(() => {
return authStore.onIdentityChange((previousUserId) => {
// Re-armed for the pre-reload window: this latch is THIS component's
// state and the reload below may not have happened yet.
workspacesRequested = false;
// WHAT A REAL IDENTITY CHANGE DOES TO THIS TAB (BUG-3005, lead
// ruling after codex round 3): clear the persistent state a reload
// would not drop, then reload.
//
// Three review rounds each found another layer of surfaces holding
// the previous user's data, because the fix was per-surface and the
// population is "everything in the tab that ever held a fetch". A
// reload ends the enumeration: every store, component, cache and
// in-flight request goes at once, and no future surface has to be
// remembered. What it does NOT drop — localStorage, sessionStorage,
// IndexedDB — is still an enumeration, but over STORAGE KEYS, which
// is bounded and greppable.
// ONLY WHEN SOMEBODY WAS SIGNED IN BEFORE. A sign-IN from an
// unauthenticated tab changes the identity, but the state it would
// be dropping is anonymous — nobody's private data — and reloading
// there puts a full page load in the middle of the login flow.
if (!previousUserId) return;
// SIGN-OUT CLEARS BUT DOES NOT RELOAD. Both sign-out sites navigate
// away by themselves — account delete with `location.href`, console
// logout with a hard navigation for the same reason — and a reload
// racing them ABORTS one of the two. That is not a hypothesis: it
// is `account-delete.spec.ts:147` failing with
// `net::ERR_ABORTED; maybe frame was detached?`, which is how this
// branch learned it. The persistent clears still have to happen,
// because a hard navigation drops the tab's memory and not
// localStorage, sessionStorage or IndexedDB.
//
// A SWAP has no such navigation of its own, so there the reload IS
// the mechanism.
//
// The property is the same either way: every transition away from a
// real user drops that user's in-tab state.
if (!authStore.userId) {
clearPersistentIdentityState();
return;
}
reloadForIdentityChange();
});
});
@@ -229,6 +229,27 @@
}
});
// CLOSE THE STREAM, and let the reload do the rest (BUG-3005, lead ruling
// after codex round 3).
//
// A real identity change reloads this tab — see `reloadForIdentityChange`
// in the root layout for why the fix stopped being per-surface. So this
// listener no longer re-runs the layout's per-workspace work: the page is
// going away, and a reload does it properly.
//
// The SSE disconnect STAYS, and is the reason this listener still exists.
// `sseService.connect` is idempotent per workspace, so the EventSource A
// opened would otherwise keep delivering into this tab for the whole
// pre-reload window — a stream still authorized, because signing in as B
// REPLACES the cookie without destroying A's session row, which is exactly
// the case BUG-3007's server-side close cannot see (BUG-3011 tracks the
// server half). Closing it here bounds that window to zero rather than to
// however long the reload takes.
const stopIdentityWatch = authStore.onIdentityChange(() => {
sseService.disconnect();
});
onDestroy(stopIdentityWatch);
// Re-attempt WebMCP registration when the auth gate resolves. The
// workspace $effect above runs inside untrack() (so a workspace switch
// doesn't drag in the whole workspaces array as a dep), which means it
@@ -5,6 +5,7 @@
import { workspaceStore } from '$lib/stores/workspace.svelte';
import { collectionStore } from '$lib/stores/collections.svelte';
import { starredStore } from '$lib/stores/starred.svelte';
import { authStore } from '$lib/stores/auth.svelte';
import { createScrollRestoration } from '$lib/scroll/restore.svelte';
import ItemCard from '$lib/components/collections/ItemCard.svelte';
import PageHeader from '$lib/components/common/PageHeader.svelte';
@@ -50,21 +51,31 @@
workspaceStore.setCurrent(wsSlug);
});
// NO IDENTITY LISTENER HERE, deliberately (BUG-3005, lead ruling). This page
// had one, and with the tab reloading on a real identity change it became
// the third path doing the same reload — the layout's listener, this one,
// and the page's own mount effect. One mechanism; the fence below is what
// this page still needs, because it covers a response settling in the
// window before the reload takes the page away.
async function loadStarred(slug: string) {
loading = true;
const seq = ++loadSeq;
// The identity that ASKED. `seq` is a navigation/refresh fence and does
// not move on an account swap, so it cannot tell this apart.
const isSameIdentity = authStore.identityFence();
try {
const [starredItems, colls] = await Promise.all([
api.items.starred(slug, { include_terminal: includeTerminal }),
api.collections.list(slug)
]);
if (seq !== loadSeq) return;
if (seq !== loadSeq || !isSameIdentity()) return;
fetchedItems = starredItems;
collections = colls;
} catch {
if (seq !== loadSeq) return;
} finally {
if (seq === loadSeq) loading = false;
if (seq === loadSeq && isSameIdentity()) loading = false;
}
}
@@ -0,0 +1,150 @@
// Runs in the jsdom vitest project (filename ends `.svelte.test.ts`).
//
// BUG-3005, codex round 1 — the starred PAGE keeps its own copy of the
// previous user's items, and the store fix alone does not reach it.
//
// Two things make this page worse than an ordinary stale cache, and the second
// is caused by the store fix itself:
//
// - its load effect is keyed on the workspace slug and the terminal filter,
// so a same-route account swap starts no reload at all;
// - `items` falls back to UNFILTERED `fetchedItems` whenever
// `starredStore.loaded` is false — which is exactly what the store's
// identity reset sets it to. So the store's fix routes this page around
// its only filter and B sees A's starred items in full.
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { render, cleanup } from '@testing-library/svelte';
import { tick } from 'svelte';
const api = vi.hoisted(() => ({
auth: { session: vi.fn() },
items: { starred: vi.fn(), star: vi.fn(), unstar: vi.fn() },
collections: { list: vi.fn() },
workspaces: { get: vi.fn(), me: vi.fn(), list: vi.fn() },
}));
vi.mock('$lib/api/client', () => ({
api,
setAccessRevokedHandler: () => {},
setRateLimitHandler: () => {},
isPlanLimitError: () => false,
planLimitMessage: () => '',
}));
vi.mock('$app/navigation', () => ({
goto: vi.fn(),
beforeNavigate: () => {},
afterNavigate: () => {},
invalidateAll: vi.fn(),
pushState: vi.fn(),
replaceState: vi.fn(),
}));
import StarredPage from './+page.svelte';
import { authStore } from '$lib/stores/auth.svelte';
import { starredStore } from '$lib/stores/starred.svelte';
import { page } from '$app/state';
function sessionFor(id: string) {
return { authenticated: true, user: { id, email: `${id}@example.com` } };
}
const ITEM_A = {
id: 'item-a',
slug: 'alphas-secret',
title: "Alpha's secret item",
collection_slug: 'ideas',
status: 'open',
fields: {},
};
async function settle(): Promise<void> {
for (let i = 0; i < 10; i++) {
await Promise.resolve();
await tick();
}
}
describe('starred page across an identity change', () => {
beforeEach(async () => {
// 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();
page.params.workspace = 'ws';
page.params.username = 'alice';
// Establish an identity so later transitions actually fire — the first
// resolution is the baseline and notifies nobody.
api.auth.session.mockResolvedValue(sessionFor('user-a'));
await authStore.load();
});
afterEach(() => {
cleanup();
authStore.clear();
});
it('refuses a load issued as A that settles after the identity changed', async () => {
// The page's identity LISTENER is gone (the tab reloads instead), so
// 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.
//
// 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: 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();
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');
});
});
@@ -0,0 +1,158 @@
// Runs in the jsdom vitest project (filename ends `.svelte.test.ts`).
//
// BUG-3005, codex round 2 — the persistent workspace layout across an identity
// change. Two bindings, and neither is observable anywhere else (CONVE-19):
//
// A real identity change RELOADS the tab (lead ruling after round 3), so this
// layout no longer re-runs its per-workspace work and no longer remounts leaf
// pages — the reload does both, properly, and one mechanism is the point.
//
// What survives here is the SSE DISCONNECT, and it is not redundant with the
// reload. `sseService.connect` is idempotent per workspace, so the EventSource
// A opened keeps delivering into this tab for the whole pre-reload window —
// still authorized, because signing in as B replaces the cookie without
// destroying A's session row, which is precisely the case BUG-3007's
// server-side close cannot see. Closing it here bounds that window to zero.
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { render, cleanup } from '@testing-library/svelte';
import { createRawSnippet, tick } from 'svelte';
const api = vi.hoisted(() => ({
auth: { session: vi.fn() },
workspaces: { get: vi.fn(), me: vi.fn(), list: vi.fn() },
collections: { list: vi.fn() },
items: { starred: vi.fn(), list: vi.fn() },
dashboard: { get: vi.fn() },
tags: { list: vi.fn() },
}));
vi.mock('$lib/api/client', () => ({
api,
setAccessRevokedHandler: () => {},
setRateLimitHandler: () => {},
isPlanLimitError: () => false,
planLimitMessage: () => '',
PadApiError: class extends Error {},
}));
vi.mock('$app/navigation', () => ({
goto: vi.fn(),
beforeNavigate: () => {},
afterNavigate: () => {},
invalidateAll: vi.fn(),
pushState: vi.fn(),
replaceState: vi.fn(),
}));
const sse = vi.hoisted(() => ({
connect: vi.fn(),
disconnect: vi.fn(),
onItemEvent: vi.fn(() => () => {}),
onCollectionEvent: vi.fn(() => () => {}),
onWorkspaceEvent: vi.fn(() => () => {}),
onConnectionChange: vi.fn(() => () => {}),
}));
vi.mock('$lib/services/sse.svelte', () => ({ sseService: sse }));
const sync = vi.hoisted(() => ({
init: vi.fn(),
setWorkspace: vi.fn(),
onSync: vi.fn(() => () => {}),
markSynced: vi.fn(),
syncNow: vi.fn(),
destroy: vi.fn(),
}));
vi.mock('$lib/services/sync.svelte', () => ({ syncService: sync }));
vi.mock('$lib/webmcp/register', () => ({ registerWorkspaceTools: vi.fn(async () => null) }));
import Layout from './+layout.svelte';
import { authStore } from '$lib/stores/auth.svelte';
import { page } from '$app/state';
function sessionFor(id: string) {
return { authenticated: true, user: { id, email: `${id}@example.com` } };
}
/** Counts how many times the leaf-page snippet has been constructed. */
let mountCount = 0;
const childSnippet = createRawSnippet(() => ({
// Counted in `render`, not in the factory: the factory is invoked once per
// snippet value, so a remount would not move it and the test would be
// vacuous. `render` runs each time the block is (re)created.
render: () => {
mountCount++;
return `<div>leaf</div>`;
},
}));
async function settle(): Promise<void> {
for (let i = 0; i < 10; i++) {
await Promise.resolve();
await tick();
}
}
describe('workspace layout across an identity change', () => {
beforeEach(async () => {
vi.resetModules();
mountCount = 0;
sse.connect.mockClear();
sse.disconnect.mockClear();
api.auth.session.mockReset();
api.collections.list.mockResolvedValue([]);
api.items.starred.mockResolvedValue([]);
api.workspaces.get.mockResolvedValue({ id: 'w1', slug: 'ws', name: 'WS' });
api.workspaces.me.mockResolvedValue({ role: 'owner' });
api.dashboard.get.mockResolvedValue({ has_agent_activity: true });
api.tags.list.mockResolvedValue([]);
page.params.workspace = 'ws';
page.params.username = 'alice';
api.auth.session.mockResolvedValue(sessionFor('user-a'));
await authStore.load();
});
afterEach(() => {
cleanup();
authStore.clear();
});
it('disconnects the previous identity\'s SSE stream', async () => {
render(Layout, { props: { children: childSnippet } });
await settle();
// PRECONDITION: the layout connected for A. Without this, "it
// reconnected" is a claim about a stream that was never opened.
expect(sse.connect).toHaveBeenCalled();
const connectsBefore = sse.connect.mock.calls.length;
const disconnectsBefore = sse.disconnect.mock.calls.length;
api.auth.session.mockResolvedValue(sessionFor('user-b'));
await authStore.load();
await settle();
expect(sse.disconnect.mock.calls.length).toBeGreaterThan(disconnectsBefore);
// And does NOT reconnect: the tab is reloading, and a fresh connection
// opened milliseconds before teardown is work nobody needs.
expect(sse.connect.mock.calls.length).toBe(connectsBefore);
});
it('does NOT remount the leaf page — the reload owns that now', async () => {
// The `{#key}` remount this layout carried for one round is gone. It was
// a second mechanism doing the reload's job, and it gave the starred
// page three reload paths on one identity change. Pinned as an ABSENCE
// so re-adding it has to be a deliberate change rather than a quiet one.
render(Layout, { props: { children: childSnippet } });
await settle();
// PRECONDITION: mounted once for A, so "still 1" is a claim about a
// component that actually rendered.
expect(mountCount).toBe(1);
api.auth.session.mockResolvedValue(sessionFor('user-b'));
await authStore.load();
await settle();
expect(mountCount).toBe(1);
});
});
+8 -1
View File
@@ -42,7 +42,14 @@
async function logout() {
await api.auth.logout();
authStore.clear();
goto('/login');
// HARD navigation, not `goto` (BUG-3005). An SPA navigation to /login
// leaves every client-side singleton, component and cache exactly where
// it was — which is the original defect: the next person to sign in on
// this browser reads the previous user's data out of a tab that never
// went away. Account deletion in `console/settings/+page.svelte` has
// always done this, with a comment saying why; the two sign-out sites
// agree now.
window.location.href = '/login';
}
function closeMobileMenu() {
@@ -0,0 +1,44 @@
// Node-project test (no DOM): a SOURCE guard that the console logout leaves the
// page by a HARD navigation (BUG-3005).
//
// `goto('/login')` is an SPA navigation. It leaves every client-side singleton,
// component and cache exactly where it was — which is this bug's original
// defect: the next person to sign in on this browser reads the previous user's
// data out of a tab that never went away. Account deletion in
// `console/settings/+page.svelte` has always used `window.location.href` and
// says why in its own comment; this makes the two sign-out sites agree.
//
// It is a source guard because the alternative is rendering the console layout
// to observe an assignment to `window.location`, which jsdom does not perform.
// WHAT A SOURCE GUARD CANNOT DO: it checks spellings, not behaviour — a logout
// that hard-navigates somewhere useless would still pass.
import { describe, expect, it } from 'vitest';
import { readFileSync } from 'node:fs';
import { resolve } from 'node:path';
function logoutBody(file: string): string {
const src = readFileSync(resolve(__dirname, file), 'utf8').replace(/^[ \t]*\/\/.*$/gm, '');
const start = src.indexOf('async function logout()');
expect(start, `${file} no longer declares logout()`).toBeGreaterThan(-1);
return src.slice(start, src.indexOf('\n\t}', start));
}
describe('console sign-out', () => {
it('hard-navigates rather than using goto', () => {
const body = logoutBody('./+layout.svelte');
expect(body).toContain("window.location.href = '/login'");
expect(body).not.toContain('goto(');
});
it('still clears the auth store first', () => {
// Order matters: `clear()` is what fires the identity listeners that run
// the persistent clears, and a navigation started first can cut them off.
const body = logoutBody('./+layout.svelte');
expect(body.indexOf('authStore.clear()')).toBeLessThan(body.indexOf('window.location.href'));
});
it('account deletion still hard-navigates too — the site this one now matches', () => {
const src = readFileSync(resolve(__dirname, './settings/+page.svelte'), 'utf8');
expect(src).toContain("window.location.href = '/login'");
});
});
@@ -30,6 +30,12 @@ vi.mock('$lib/api/client', () => ({
// `goto` must not run: the unauthenticated branches call it, and jsdom has no
// navigation. Nothing here asserts on it.
const identityReload = vi.hoisted(() => ({
reloadForIdentityChange: vi.fn(),
clearPersistentIdentityState: vi.fn(),
}));
vi.mock('$lib/stores/identityReload.svelte', () => identityReload);
vi.mock('$app/navigation', () => ({
goto: vi.fn(),
beforeNavigate: () => {},
@@ -136,3 +142,90 @@ describe('BUG-2991: the root layout refetches workspaces when the user changes',
expect(workspaceStore.workspaces).toEqual([U2_WS]);
});
});
describe('BUG-3005: which identity transitions reload the tab', () => {
// The reload exists so one user's data cannot be visible to the next. Which
// transitions need it is therefore a question about whose data is being
// dropped, and an ANONYMOUS baseline holds nobody's private data — so a
// sign-IN does not reload. Reloading there also put a full page load in the
// middle of the login flow, racing its own navigation; the E2E suite caught
// that, which is why these three legs exist.
//
// EACH LEG ASSERTS AN EXACT CALL COUNT, and that is load-bearing rather than
// style: `afterEach` clears the store, so every test starts from an
// ESTABLISHED empty identity and its own setup performs a sign-in. Removing
// the guard therefore adds a reload during setup and all three legs fail —
// the swap and sign-out ones on the COUNT rather than on the absence.
it('reloads on a SWAP from one user to another', async () => {
render(Layout, { props: { children: childSnippet } });
await settle();
// PRECONDITION: u1 is established, so what follows is a change rather
// than a baseline.
expect(authStore.userId).toBe('u1');
expect(identityReload.reloadForIdentityChange).not.toHaveBeenCalled();
api.auth.session.mockResolvedValue(sessionFor('u2'));
await authStore.load();
await settle();
expect(identityReload.reloadForIdentityChange).toHaveBeenCalledTimes(1);
});
it('does not CLEAR-without-reloading on a swap — the reload path owns both', async () => {
// The counterfactual for the split: a swap must not take the sign-out
// branch, or it would clear the storage and then leave the tab standing
// with the previous user's page still mounted and nothing navigating
// away from it.
render(Layout, { props: { children: childSnippet } });
await settle();
api.auth.session.mockResolvedValue(sessionFor('u2'));
await authStore.load();
await settle();
expect(identityReload.clearPersistentIdentityState).not.toHaveBeenCalled();
});
it('CLEARS but does not reload on SIGN-OUT', async () => {
// Both sign-out sites navigate away by themselves — account delete with
// `location.href`, console logout with a hard navigation for the same
// reason — and a reload racing them ABORTS one of the two. That is not
// a hypothesis: `account-delete.spec.ts:147` failed with
// `net::ERR_ABORTED; maybe frame was detached?` and that is how this
// branch learned it.
//
// The clears still have to run: a hard navigation drops the tab's
// memory, not localStorage, sessionStorage or IndexedDB.
render(Layout, { props: { children: childSnippet } });
await settle();
expect(authStore.userId).toBe('u1');
authStore.clear();
await settle();
expect(identityReload.clearPersistentIdentityState).toHaveBeenCalledTimes(1);
expect(identityReload.reloadForIdentityChange).not.toHaveBeenCalled();
});
it('does NOT reload on a SIGN-IN from an unauthenticated tab', async () => {
// The leg the E2E failure is about. `collab-helpers.ts` signs in from
// inside a mounted page, and a reload there takes the page out from
// under whatever is driving it — in a test, and in the real login flow.
api.auth.session.mockResolvedValue(null);
render(Layout, { props: { children: childSnippet } });
await settle();
// PRECONDITION: the tab is unauthenticated and that baseline is
// established, so the sign-in below IS a transition and this leg is not
// passing because nothing fired.
expect(authStore.userId).toBe('');
api.auth.session.mockResolvedValue(sessionFor('u1'));
await authStore.load();
await settle();
expect(authStore.userId).toBe('u1');
expect(identityReload.reloadForIdentityChange).not.toHaveBeenCalled();
expect(identityReload.clearPersistentIdentityState).not.toHaveBeenCalled();
});
});