refactor(ext): load org entries in the shared switcher (works in popup + vault tab)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W3b8DA2mXmLWkd4zDbHXHg

extension/src/popup/components/__tests__/item-list.test.ts

@@ -25,23 +25,27 @@ describe('popup/item-list in org context', () => {
     document.body.innerHTML = '<div id="app"></div>';
   });
 
-  // Task 3 — list loads via the context-aware message helper. In an org
-  // context the initial render must populate the list from `org_list_items`
-  // (the personal entries don't carry over across an org_switch).
-  it('loads org items (grant-filtered) when context is an org', async () => {
-    mockGetState.mockReturnValue({ orgContext: 'org-1', entries: [], searchQuery: '', selectedIndex: 0 });
-    // org_list_items returns OrgManifestEntry[] (dedicated org type; collection is
-    // REQUIRED, not the personal ManifestEntry's optional one) — Dev-A mirrors
-    // OrgManifestEntry into shared/types.ts; the list consumes it at integration.
-    send.mockResolvedValue({ ok: true, data: [
-      { id: 'a', type: 'login', title: 'db', tags: [], collection: 'prod-infra', modified: 1 },
-    ]});
+  // Task 3 (loading-design-v2) — the switcher loads org entries into state
+  // before navigating. renderItemList renders whatever is in state.entries and
+  // does NOT fire its own org_list_items load. This test asserts that: given
+  // pre-populated state.entries, the rows appear; and no org_list_items call
+  // is made by renderItemList itself.
+  it('renders org entries already in state.entries without re-fetching', () => {
+    mockGetState.mockReturnValue({
+      orgContext: 'org-1',
+      entries: [['a', {
+        id: 'a', type: 'login', title: 'db', tags: [], collection: 'prod-infra',
+        modified: 1, favorite: false, attachment_summaries: [],
+      }]],
+      searchQuery: '',
+      selectedIndex: 0,
+    });
+    const app = document.getElementById('app')!;
 
-    renderItemList(document.getElementById('app')!);
-    await Promise.resolve();
-    await Promise.resolve();
+    renderItemList(app);
 
-    expect(send).toHaveBeenCalledWith({ type: 'org_list_items' });
+    expect(app.textContent).toContain('db');
+    expect(send).not.toHaveBeenCalledWith({ type: 'org_list_items' });
   });
 
   // Task 3 — read-only this plan: no add affordance in org context. Dev-C's

extension/src/popup/components/__tests__/org-switcher.test.ts

@@ -12,10 +12,12 @@ vi.mock('../../../shared/state', () => ({
 }));
 
 import { renderOrgSwitcher } from '../org-switcher';
-import { sendMessage, navigate } from '../../../shared/state';
+import { sendMessage, navigate, getState, setState } from '../../../shared/state';
 
 const send = sendMessage as ReturnType<typeof vi.fn>;
 const nav = navigate as ReturnType<typeof vi.fn>;
+const mockGetState = getState as ReturnType<typeof vi.fn>;
+const mockSetState = setState as ReturnType<typeof vi.fn>;
 
 describe('popup/org-switcher', () => {
   let host: HTMLElement;
@@ -38,12 +40,51 @@ describe('popup/org-switcher', () => {
     const sel = host.querySelector('select') as HTMLSelectElement;
     sel.value = 'org-1';
     sel.dispatchEvent(new Event('change'));
+    // Two awaits: one for org_switch, one for the subsequent list load.
+    // navigate('list') fires after both resolve.
+    await Promise.resolve();
     await Promise.resolve();
 
     expect(send).toHaveBeenCalledWith({ type: 'org_switch', context: 'org-1' });
     expect(nav).toHaveBeenCalledWith('list', expect.anything());
   });
 
+  // loading-design-v2 — switching to an org loads entries into shared state via
+  // org_list_items, normalizes them (favorite:false, attachment_summaries:[]),
+  // and calls setState({ entries }) before navigating. Both popup renderItemList
+  // and vault renderListPane then render state.entries without a separate load.
+  it('loads org entries into state after switching to an org', async () => {
+    // First getState call: initial render needs 'personal' so the select shows
+    // Personal selected. Second call (inside messageForList after setState):
+    // needs 'org-1' so messageForList() returns { type: 'org_list_items' }.
+    mockGetState
+      .mockReturnValueOnce({ orgContext: 'personal', orgConfigs: [], orgOffline: false })
+      .mockReturnValueOnce({ orgContext: 'org-1', orgConfigs: [], orgOffline: false });
+
+    send.mockImplementation(async (req: { type: string }) => {
+      if (req.type === 'org_list_configs') return { ok: true, data: [{ orgId: 'org-1', displayName: 'Acme' }] };
+      if (req.type === 'org_switch') return { ok: true, data: { context: 'org-1', offline: false } };
+      if (req.type === 'org_list_items') return { ok: true, data: [
+        { id: 'x1', type: 'login', title: 'Org Login', tags: [], collection: 'eng', modified: 1 },
+      ]};
+      return { ok: true, data: [] };
+    });
+
+    await renderOrgSwitcher(host);
+    const sel = host.querySelector('select') as HTMLSelectElement;
+    sel.value = 'org-1';
+    sel.dispatchEvent(new Event('change'));
+    await Promise.resolve();
+    await Promise.resolve();
+
+    expect(send).toHaveBeenCalledWith({ type: 'org_list_items' });
+    expect(mockSetState).toHaveBeenCalledWith({
+      entries: [['x1', expect.objectContaining({
+        title: 'Org Login', favorite: false, attachment_summaries: [],
+      })]],
+    });
+  });
+
   // Task 5 — offline read-only banner.
   it('offline org_switch renders the writes-disabled banner', async () => {
     send.mockImplementation(async (req: { type: string }) =>

extension/src/popup/components/item-list.ts

@@ -3,7 +3,7 @@
 /// to the detail view.
 
 import { getState, setState, sendMessage, navigate, escapeHtml, openVaultTab } from '../../shared/state';
-import { currentContext, messageForList, messageForGet } from '../../shared/org-context';
+import { currentContext, messageForList, messageForGet, normalizeOrgEntries } from '../../shared/org-context';
 import { showToast } from '../../shared/toast';
 import {
   GLYPH_VAULT_TAB,
@@ -33,38 +33,6 @@ function typeIcon(t: ItemType): string {
   }
 }
 
-/// Normalize OrgManifestEntry[] (leaner org shape; no attachment_summaries /
-/// favorite / icon_hint / group) into the Array<[ItemId, ManifestEntry]> that
-/// buildRowsHtml and getFilteredEntries expect. Used by BOTH the sync handler
-/// and the context-aware initial load.
-function normalizeOrgEntries(data: OrgManifestEntry[]): Array<[ItemId, ManifestEntry]> {
-  return data.map(e => [e.id, {
-    id: e.id,
-    type: e.type,
-    title: e.title,
-    tags: e.tags,
-    modified: e.modified,
-    trashed_at: e.trashed_at,
-    favorite: false,
-    attachment_summaries: [],
-    // icon_hint, group: undefined (optional in ManifestEntry; omitted for org items)
-  }]);
-}
-
-/// Context-aware item list loader. Sends list_items or org_list_items depending
-/// on the current context, then normalizes the response into entries and calls
-/// setState. Called once per context by renderItemList (loop-safe guard).
-async function loadEntriesForContext(): Promise<void> {
-  const resp = await sendMessage(messageForList());
-  if (!resp.ok) {
-    setState({ error: resp.error ?? 'Failed to load items' });
-    return;
-  }
-  const entries: Array<[ItemId, ManifestEntry]> = currentContext() === 'personal'
-    ? (resp.data as { items: Array<[ItemId, ManifestEntry]> }).items
-    : normalizeOrgEntries(resp.data as OrgManifestEntry[]);
-  setState({ entries });
-}
 
 function buildRowsHtml(): string {
   const state = getState();
@@ -192,16 +160,6 @@ export function renderItemList(app: HTMLElement): void {
   wireRowClicks();
 
   document.addEventListener('keydown', handleListKeydown);
-
-  // Context-aware initial load (org only; personal uses manual sync).
-  // Loop-safe guard: setState({loadedContext}) triggers a re-render that calls
-  // renderItemList again, but the guard now matches so no second load fires.
-  // When loadEntriesForContext() later resolves and calls setState({entries}),
-  // loadedContext still matches so still no loop.
-  if (isOrg && getState().loadedContext !== currentContext()) {
-    setState({ loadedContext: currentContext() });
-    void loadEntriesForContext();
-  }
 }
 
 async function openItem(id: ItemId): Promise<void> {

extension/src/popup/components/org-switcher.ts

@@ -6,8 +6,9 @@
 // org renders a read-only banner; Dev-C's write UI reads `orgOffline` to
 // disable writes.
 
-import type { OrgConfigSummary } from '../../shared/types';
+import type { OrgConfigSummary, OrgManifestEntry, ManifestEntry, ItemId } from '../../shared/types';
 import { sendMessage, setState, navigate, getState } from '../../shared/state';
+import { messageForList, normalizeOrgEntries } from '../../shared/org-context';
 
 // Module-level teardown (one instance per browser context; popup and vault
 // each have their own module scope).
@@ -59,6 +60,19 @@ export async function renderOrgSwitcher(host: HTMLElement): Promise<void> {
       existing?.remove();
     }
 
+    // Load the new context's entries into shared state before navigating.
+    // messageForList() reads the just-set orgContext so it picks the right SW
+    // message (list_items for personal, org_list_items for org). Both popup's
+    // renderItemList and vault's renderListPane read state.entries, so updating
+    // it here makes both surfaces work correctly on switch.
+    const listResp = await sendMessage(messageForList());
+    if (listResp.ok) {
+      const entries: Array<[ItemId, ManifestEntry]> = data.context === 'personal'
+        ? (listResp.data as { items: Array<[ItemId, ManifestEntry]> }).items
+        : normalizeOrgEntries(listResp.data as OrgManifestEntry[]);
+      setState({ entries });
+    }
+
     navigate('list', {});
   };
 

extension/src/shared/org-context.ts

@@ -7,6 +7,7 @@
 
 import { getState } from './state';
 import type { Request } from './messages';
+import type { ItemId, ManifestEntry, OrgManifestEntry } from './types';
 
 export function currentContext(): 'personal' | string {
   return getState().orgContext ?? 'personal';
@@ -19,3 +20,11 @@ export function messageForList(): Request {
 export function messageForGet(id: string): Request {
   return currentContext() === 'personal' ? { type: 'get_item', id } : { type: 'org_get_item', id };
 }
+
+// Normalize OrgManifestEntry[] (leaner org shape; no attachment_summaries /
+// favorite / icon_hint / group) into the Array<[ItemId, ManifestEntry]> that
+// both list renderers (popup renderItemList + vault renderListPane) expect.
+// Org rows show no attachment indicator (PM-accepted).
+export function normalizeOrgEntries(entries: OrgManifestEntry[]): Array<[ItemId, ManifestEntry]> {
+  return entries.map(e => [e.id, { ...e, favorite: false, attachment_summaries: [] }]);
+}

extension/src/shared/popup-state.ts

@@ -57,11 +57,6 @@ export interface PopupState {
   orgOffline?: boolean;
   orgConfigs?: OrgConfigSummary[];
   orgCollections?: Collection[];
-  // Tracks which context's items were last loaded into `entries`. Used by
-  // renderItemList to fire a context-aware load exactly ONCE per context
-  // switch (loop-safe guard: setState({loadedContext}) re-renders, but the
-  // guard then matches so no second load fires).
-  loadedContext?: string;
   // Vault-tab-only fields. The popup surface leaves these at their defaults
   // (unlocked=false implicit via separate lock-screen view, drawer/panel false).
   // Kept on the shared shape so VaultState satisfies StateHost.getState()