Merge pull request #811 from hildebrandttk/fix/files-stale-data-on-account-switch

fix(files): reset the account-scoped Files drive on every account switch

components/files/files-app.tsx

@@ -106,7 +106,11 @@ export function FilesApp({ linkSegments }: FilesAppProps = {}) {
   const isMobile = useIsMobile();
   const isEmbedded = useIsEmbedded();
   const [folderLayout, setFolderLayout] = useState<FolderLayout>(() => loadFilesSettings().folderLayout);
-  const hasFetched = useRef(false);
+  // The account the files client was last initialised for (`undefined` = never).
+  // Tracked so a Pro-shell account switch re-initialises instead of showing the
+  // previous account's files until a manual Home click.
+  const initedAccountRef = useRef<string | null | undefined>(undefined);
+  const filesBootstrapRef = useRef(false);
 
   // Sync folderLayout when settings change
   useEffect(() => {
@@ -153,11 +157,22 @@ export function FilesApp({ linkSegments }: FilesAppProps = {}) {
   // are surfaced as top-level folders at the root, so we *don't* auto-attach
   // to the active account - the user picks one explicitly.
   useEffect(() => {
-    if (!isAuthenticated || !client || hasFetched.current) return;
-    hasFetched.current = true;
+    if (!isAuthenticated || !client) return;
+    // Re-run when the ACTIVE account changes (Pro multi-account switch), not
+    // just once - otherwise the previous account's files linger until a manual
+    // Home click.
+    if (initedAccountRef.current === activeAccountId) return;
+    const switched = initedAccountRef.current !== undefined;
+    initedAccountRef.current = activeAccountId;
     if (isEmbedded) {
       useFileStore.getState().clearClient();
     } else {
+      if (switched) {
+        // Drop the previous account's attached view so its files don't linger,
+        // and let the bootstrap effect below re-list for the new account.
+        useFileStore.getState().clearClient();
+        filesBootstrapRef.current = false;
+      }
       initClient(client, activeAccountId);
     }
   }, [isAuthenticated, client, initClient, activeAccountId, isEmbedded]);
@@ -203,7 +218,6 @@ export function FilesApp({ linkSegments }: FilesAppProps = {}) {
   // race - the warm one following the deep link while the cold one, a tick
   // later, listed the root on top of it.
   const storeClient = useFileStore(s => s.client);
-  const filesBootstrapRef = useRef(false);
   useEffect(() => {
     if (filesBootstrapRef.current) return;
     if (!storeClient) return;

integration/tests/13-files-account-switch.spec.ts

@@ -0,0 +1,69 @@
+/**
+ * The Files drive is account-scoped: switching the active account must swap the
+ * drive to the new account's FileNodes and never leave the previous account's
+ * folders on screen.
+ *
+ * Regression for the stale-drive bug: the Files store is a global that outlives
+ * the FilesApp component, and it was the one account-scoped store not reset on a
+ * switch (account-state-manager cleared email/contacts/calendar/… but not
+ * files). So after switching accounts the old account's folders lingered until a
+ * manual navigation - both when the switch happened on the Files view and, more
+ * commonly, when it happened on the mail view before the drive was reopened.
+ *
+ * Two paths are exercised:
+ *   1. switch while the Files view is NOT mounted (mail view → switch → open
+ *      Files) - the reported normal-shell case, guarded by clearing the store
+ *      centrally on every switch.
+ *   2. switch while the Files view IS mounted (rail account switcher) - guarded
+ *      by FilesApp re-initialising on the active-account change.
+ */
+import { test, expect } from '@playwright/test';
+import { ACCOUNTS } from './helpers/config';
+import { JmapClient } from './helpers/jmap';
+import { login, addAccount, switchAccount, openFiles, openMailView, driveEntry } from './helpers/app';
+
+// Distinct top-level folder in each account's drive; presence/absence of these
+// names is how we tell which account's drive is on screen.
+const ALICE_DIR = 'alice-only-folder';
+const BOB_DIR = 'bob-only-folder';
+
+test.describe('Files drive is account-scoped across an account switch', () => {
+  test.beforeAll(async () => {
+    const alice = await JmapClient.connect(ACCOUNTS.alice.email, ACCOUNTS.alice.password);
+    const bob = await JmapClient.connect(ACCOUNTS.bob.email, ACCOUNTS.bob.password);
+    // Start each drive from a known-empty root so the only entries are ours.
+    await alice.resetFiles();
+    await bob.resetFiles();
+    await alice.createFileDirectory(ALICE_DIR);
+    await bob.createFileDirectory(BOB_DIR);
+  });
+
+  test('switching accounts swaps the drive with no stale folders', async ({ page }) => {
+    await login(page, ACCOUNTS.alice);
+    await addAccount(page, ACCOUNTS.bob); // active account becomes bob
+
+    // Bob's drive loads into the (global) Files store.
+    await openFiles(page);
+    await expect(driveEntry(page, BOB_DIR)).toBeVisible();
+    await expect(driveEntry(page, ALICE_DIR)).toHaveCount(0);
+
+    // Path 1 - switch happens OFF the Files view: go to mail, switch to alice
+    // there (FilesApp unmounted), then reopen Files. Without the central reset
+    // bob's folder would still be sitting in the store here.
+    await openMailView(page);
+    await switchAccount(page, ACCOUNTS.alice.email);
+    await openFiles(page);
+    await expect(driveEntry(page, ALICE_DIR)).toBeVisible();
+    await expect(driveEntry(page, BOB_DIR)).toHaveCount(0);
+
+    // Path 2 - switch happens ON the Files view via the rail account switcher.
+    await switchAccount(page, ACCOUNTS.bob.email);
+    await expect(driveEntry(page, BOB_DIR)).toBeVisible();
+    await expect(driveEntry(page, ALICE_DIR)).toHaveCount(0);
+
+    // And back again, to be sure the swap is symmetric and repeatable.
+    await switchAccount(page, ACCOUNTS.alice.email);
+    await expect(driveEntry(page, ALICE_DIR)).toBeVisible();
+    await expect(driveEntry(page, BOB_DIR)).toHaveCount(0);
+  });
+});

integration/tests/helpers/app.ts

@@ -137,6 +137,42 @@ export async function forceSync(page: Page): Promise<void> {
   await page.evaluate(() => document.dispatchEvent(new Event('visibilitychange')));
 }
 
+// ─── Files (drive) ─────────────────────────────────────────────────────────
+
+/** The primary navigation rail (there are two - mobile + desktop chrome). */
+function navRail(page: Page): Locator {
+  return page.locator('nav[aria-label="Navigation"]').first();
+}
+
+/**
+ * Open the Files drive via the navigation rail. A deep `page.goto('/files')`
+ * bounces to login in this SPA, so navigate in-app through the rail link
+ * (rendered for every route once files support is detected). Waits until the
+ * file browser toolbar is present.
+ */
+export async function openFiles(page: Page): Promise<void> {
+  await navRail(page).getByRole('link', { name: 'Files', exact: true }).click();
+  await page.locator('[role="toolbar"]').first().waitFor({ state: 'visible', timeout: 30000 });
+}
+
+/**
+ * Return to the mail view via the navigation rail's Mail link and wait until it
+ * has settled. Settling matters before touching the account switcher: while the
+ * mailbox is still loading the sidebar reflows, which leaves the switcher's
+ * popover items shifting/detaching under a click. Waiting for the Inbox row
+ * (and the switcher) pins the layout first.
+ */
+export async function openMailView(page: Page): Promise<void> {
+  await navRail(page).getByRole('link', { name: 'Mail', exact: true }).click();
+  await accountSwitcher(page).waitFor({ state: 'visible', timeout: 30000 });
+  await folderRow(page, { role: 'inbox' }).first().waitFor({ state: 'visible', timeout: 30000 });
+}
+
+/** A drive entry (folder/file) row, selected by its name via `data-resource`. */
+export function driveEntry(page: Page, name: string): Locator {
+  return page.locator(`[data-resource="${name}"]`);
+}
+
 // ─── Composer / drafts ────────────────────────────────────────────────────
 
 /** Open the composer via the keyboard shortcut and wait for it to render. */

integration/tests/helpers/jmap.ts

@@ -14,6 +14,7 @@ const CORE = 'urn:ietf:params:jmap:core';
 const MAIL = 'urn:ietf:params:jmap:mail';
 const PRINCIPALS = 'urn:ietf:params:jmap:principals';
 const SUBMISSION = 'urn:ietf:params:jmap:submission';
+const FILENODE = 'urn:ietf:params:jmap:filenode';
 
 /** Rights granted on a shared mailbox (JMAP ACL). */
 export const FULL_MAILBOX_RIGHTS = {
@@ -43,6 +44,10 @@ export class JmapClient {
   private authHeader: string;
   private apiUrl: string;
   accountId = '';
+  /** Account that holds this user's Files drive (FileNode). On Stalwart's
+   *  personal mailboxes this equals the mail account, but read it from the
+   *  session so the helper stays correct if that ever diverges. */
+  filesAccountId = '';
   /** Every account visible in this user's session (own + shared/group),
    *  keyed by accountId -> account name (its email address). */
   accounts: Record<string, string> = {};
@@ -64,6 +69,7 @@ export class JmapClient {
     const primary = session.primaryAccounts?.[MAIL];
     if (!primary) throw new Error(`No mail account for ${email} in JMAP session`);
     c.accountId = primary;
+    c.filesAccountId = session.primaryAccounts?.[FILENODE] ?? primary;
     c.accounts = Object.fromEntries(
       Object.entries(session.accounts ?? {}).map(([id, a]) => [id, (a as { name: string }).name]),
     );
@@ -295,4 +301,42 @@ export class JmapClient {
       await new Promise((r) => setTimeout(r, 500));
     }
   }
+
+  // ─── Files (JMAP FileNode) ────────────────────────────────────────────────
+
+  /** Ids of every FileNode in this account's drive. */
+  private async allFileNodeIds(): Promise<string[]> {
+    const r = await this.request(
+      [['FileNode/query', { accountId: this.filesAccountId, filter: {}, limit: 5000 }, '0']],
+      [CORE, FILENODE],
+    );
+    return r.methodResponses[0][1].ids as string[];
+  }
+
+  /** Wipe the account's drive so a test starts from a known-empty root. */
+  async resetFiles(): Promise<void> {
+    const ids = await this.allFileNodeIds();
+    if (!ids.length) return;
+    await this.request(
+      [['FileNode/set', { accountId: this.filesAccountId, destroy: ids }, '0']],
+      [CORE, FILENODE],
+    );
+  }
+
+  /**
+   * Create a top-level directory FileNode and return its id. A directory is a
+   * FileNode with no blob/type/size (server stores it with `file == null`),
+   * mirroring how the app's createFileDirectory seeds one.
+   */
+  async createFileDirectory(name: string, parentId: string | null = null): Promise<string> {
+    const props: Record<string, unknown> = { name };
+    if (parentId !== null) props.parentId = parentId;
+    const r = await this.request(
+      [['FileNode/set', { accountId: this.filesAccountId, create: { dir0: props } }, '0']],
+      [CORE, FILENODE],
+    );
+    const created = r.methodResponses[0][1].created?.dir0;
+    if (!created) throw new Error(`createFileDirectory failed: ${JSON.stringify(r.methodResponses[0][1])}`);
+    return created.id as string;
+  }
 }

lib/__tests__/account-state-manager.test.ts

@@ -12,6 +12,7 @@ import { useCalendarStore } from '@/stores/calendar-store';
 import { useFilterStore } from '@/stores/filter-store';
 import { useIdentityStore } from '@/stores/identity-store';
 import { useVacationStore } from '@/stores/vacation-store';
+import { useFileStore } from '@/stores/file-store';
 import { DEFAULT_SEARCH_FILTERS } from '@/lib/jmap/search-utils';
 import { makeEmail, makeMailbox } from './helpers/factories';
 
@@ -109,6 +110,33 @@ describe('clearAllStores', () => {
     expect(s.tagCounts).toEqual({});
     expect(s.searchFilters).toEqual(DEFAULT_SEARCH_FILTERS);
   });
+
+  it('resets the account-scoped Files drive (no stale resources across a switch)', () => {
+    // Simulate account A having opened the drive: client attached, a folder
+    // navigated into, and its listing loaded.
+    const fakeClient = {} as never;
+    useFileStore.getState().initClient(fakeClient, 'A');
+    useFileStore.setState({
+      resources: [{ id: 'r1', name: 'A-secret.txt' } as never],
+      pathStack: [{ id: null, name: '' }, { id: 'folderA', name: 'Folder A' }],
+      currentPath: '/Folder A',
+      currentParentId: 'folderA',
+      supportsFiles: true,
+      selectedResources: new Set(['r1']),
+    });
+
+    clearAllStores();
+
+    const f = useFileStore.getState();
+    expect(f.client).toBeNull();
+    expect(f.currentAccountId).toBeNull();
+    expect(f.resources).toEqual([]);
+    expect(f.selectedResources.size).toBe(0);
+    expect(f.currentParentId).toBeNull();
+    expect(f.currentPath).toBe('/');
+    expect(f.pathStack).toEqual([{ id: null, name: '' }]);
+    expect(f.supportsFiles).toBeNull();
+  });
 });
 
 describe('evictAccount / evictAll', () => {

lib/account-state-manager.ts

@@ -11,6 +11,7 @@ import { useFilterStore } from '@/stores/filter-store';
 import { DEFAULT_SEARCH_FILTERS } from '@/lib/jmap/search-utils';
 import { useIdentityStore } from '@/stores/identity-store';
 import { useVacationStore } from '@/stores/vacation-store';
+import { useFileStore } from '@/stores/file-store';
 
 // Minimal snapshot shapes - we only capture what we need
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -132,6 +133,14 @@ export function clearAllStores(): void {
   useVacationStore.getState().clearState();
   useCalendarStore.getState().clearState();
   useFilterStore.getState().clearState();
+  // The Files drive is account-scoped like every store above, but unlike them
+  // it lives in a global store that outlives the FilesApp component. If we
+  // don't reset it here, the previous account's client + resources linger in
+  // memory and resurface (stale) whenever the drive is opened after a switch -
+  // even when FilesApp wasn't mounted at switch time (the common case: switch
+  // from the mail view, then navigate to Files). Clearing centrally makes the
+  // reset happen on every switch path, not just the one the component observes.
+  useFileStore.getState().clearClient();
 }
 
 /** Evict cached state for one account */