test WorkspaceView; fix the close button that never closed

TrafficLights took onRemove and isLastPanel but called onClearApp either way — isLastPanel only chose the tooltip. Closing a panel from its own chrome was impossible: the panel stayed, emptied, and the context menu was the only working path. Worse than cosmetic now that identity lives in the layout — clearing the app drops the config that named the panel's agent while the panel survives to be renamed by whatever is put in it next.

Also wraps the ephemeral pane's sizing effect in a try/catch: react-resizable-panels asserts rather than no-ops when asked to size a group it has not laid out yet, and an assert thrown from an effect aborts the commit — taking the whole dashboard down for a file preview's geometry.

src/workspaces/officerdev/src/components/Workspace/PanelSlot.tsx

@@ -138,9 +138,15 @@ const TrafficLights = ({
   const { maximizedPanelId, setMaximizedPanelId } = useWorkspace();
   const isMaximized = maximizedPanelId === panelId;
 
+  // The button says "Close panel" unless this is the only panel left, and it used to call `onClearApp`
+  // either way — `isLastPanel` reached here only to choose the tooltip. So closing a panel from its own
+  // chrome was impossible: the panel stayed, emptied, and the only working path was the context menu.
+  // Worse than cosmetic now that identity lives in the layout — clearing the app drops the `config` that
+  // named the panel's agent while the panel itself survives to be renamed by the next thing put in it.
   const handleClose = useCallback(() => {
-    onClearApp();
-  }, [onClearApp]);
+    if (isLastPanel) onClearApp();
+    else onRemove(panelId);
+  }, [isLastPanel, onClearApp, onRemove, panelId]);
 
   const handleRestore = useCallback(() => {
     setMaximizedPanelId(null);

src/workspaces/officerdev/src/components/Workspace/WorkspaceView.test.tsx

@@ -0,0 +1,225 @@
+import { describe, expect, test, beforeEach } from 'bun:test';
+import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
+import { render, fireEvent, screen } from '@testing-library/react';
+import { globalQueryKey } from 'hooks/useGlobal';
+import type { AppRegistryMap, DashboardState, LayoutGroup, LayoutNode, LayoutPanel } from './types';
+import { countPanels, findPanelApp } from './layout-utils';
+import { WorkspaceView } from './WorkspaceView';
+import { usePanelClose } from './usePanelClose';
+
+/**
+ * `WorkspaceView` owns every mutation of the layout tree, which is now also where panel identity lives —
+ * a chat panel's `config.agentName` is the address a peer agent is delivered to. Two properties decide
+ * whether an unattended dashboard survives, and neither had a test:
+ *
+ *   - **Every write is an updater, never a computed tree.** Two of the paths are not immediate (the
+ *     resize debounce fires 500 ms after the drag, and a window resize fires `onLayout` on every group at
+ *     once), so a write computed from the rendered `layout` can be built on a tree that predates the write
+ *     before it. It then silently undoes it, and the resurrected tree carries an older `config`.
+ *   - **`usePanelClose` fires when a close was *asked for*, and only then** — not on the unmounts that a
+ *     drag, a swap or a mobile switch cause, and not for a workspace you have navigated away from.
+ *
+ * Everything below drives the real `WorkspaceView` through the real `WorkspaceRenderer` and `PanelSlot`,
+ * so the buttons are the buttons. It found one defect on the first run: see "closing a panel".
+ */
+
+type Recorded = { arg: LayoutNode | ((prev: LayoutNode) => LayoutNode); wasUpdater: boolean };
+
+const panel = (id: string, appType: string | null = 'probe', config?: Record<string, unknown>): LayoutPanel => ({
+  type: 'panel',
+  id,
+  appType,
+  ...(config ? { config } : {}),
+});
+
+const group = (id: string, children: LayoutNode[]): LayoutGroup => ({
+  type: 'group',
+  id,
+  direction: 'horizontal',
+  children: children.map((node) => ({ node, size: 100 / children.length })),
+});
+
+const ProbeIcon = ((props: Record<string, unknown>) => <svg {...props} />) as AppRegistryMap[string]['icon'];
+
+// Registered close handlers, by panel id, so a test can say what a panel does when it is retired.
+const closeBehaviour = new Map<string, () => void>();
+const closesFired: string[] = [];
+
+const Probe = ({ panelId }: { panelId: string }) => {
+  usePanelClose(panelId, () => {
+    closesFired.push(panelId);
+    closeBehaviour.get(panelId)?.();
+  });
+  return <div data-testid={`probe-${panelId}`} />;
+};
+
+const registry: AppRegistryMap = {
+  probe: { name: 'Probe', icon: ProbeIcon, component: Probe },
+  other: { name: 'Other', icon: ProbeIcon, component: Probe },
+};
+
+let queryClient: QueryClient;
+let writes: Recorded[];
+
+/**
+ * A workspace that behaves like the real one: `setValue` accepts either shape, applies it to the *current*
+ * tree, and re-renders. Recording whether each write arrived as a function is the whole point — a computed
+ * tree here is the §5.5 lost update, and it is invisible in the resulting layout when only one write is in
+ * flight.
+ */
+function mount(initial: LayoutNode, props: Partial<Parameters<typeof WorkspaceView>[0]> = {}) {
+  let current = initial;
+  let view: ReturnType<typeof render> | null = null;
+  writes = [];
+
+  const workspace = (): DashboardState => ({
+    key: 'dashboard:test',
+    value: current,
+    isLoaded: true,
+    setValue: (update) => {
+      writes.push({ arg: update, wasUpdater: typeof update === 'function' });
+      current = typeof update === 'function' ? (update as (prev: LayoutNode) => LayoutNode)(current) : update;
+      // A write raised from an effect during the very first render arrives before `render` has returned.
+      // The tree it produced is already the one on screen for `current`; re-rendering is what a real
+      // `useDashboardState` would do next, and there is nothing to re-render into yet.
+      view?.rerender(tree());
+    },
+  });
+
+  const tree = () => (
+    <QueryClientProvider client={queryClient}>
+      <WorkspaceView {...props} workspace={workspace()} />
+    </QueryClientProvider>
+  );
+
+  view = render(tree());
+  return { layout: () => current, rerender: () => view?.rerender(tree()) };
+}
+
+beforeEach(() => {
+  closeBehaviour.clear();
+  closesFired.length = 0;
+  queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } });
+  // `useAppRegistry` reads a `useGlobal` key that `seedAppRegistry` writes before anything renders. Seeding
+  // the cache directly is the same thing without dragging in the real app list.
+  queryClient.setQueryData(globalQueryKey('APP_REGISTRY'), registry);
+});
+
+describe('every write is an updater, not a computed tree', () => {
+  test('closing a panel writes a function', () => {
+    mount(group('g', [panel('a'), panel('b')]));
+
+    fireEvent.click(screen.getAllByTitle('Close panel')[0]!);
+
+    expect(writes.length).toBe(1);
+    expect(writes[0]!.wasUpdater).toBe(true);
+  });
+
+  test('two writes compose, even though the second was raised from a stale render', () => {
+    // The lost-update shape, reproduced deliberately: both callbacks are captured from the same rendered
+    // tree, and the second is invoked without the first's render having been observed. Composed through
+    // updaters, `b` is still gone when `a`'s app is cleared.
+    const view = mount(group('g', [panel('a'), panel('b'), panel('c')]));
+    const closeButtons = screen.getAllByTitle('Close panel');
+
+    fireEvent.click(closeButtons[1]!); // remove b
+    fireEvent.click(closeButtons[0]!); // remove a, from a handler made before b was gone
+
+    expect(countPanels(view.layout())).toBe(1);
+    expect(writes.every((w) => w.wasUpdater)).toBe(true);
+  });
+});
+
+describe('closing a panel', () => {
+  test('the last panel is cleared, not removed — there is nothing to fall back to', () => {
+    const view = mount(panel('a'));
+
+    fireEvent.click(screen.getByTitle('Clear app'));
+
+    expect(countPanels(view.layout())).toBe(1);
+    expect(findPanelApp(view.layout(), 'a')).toBe(null);
+  });
+
+  test('a panel that is NOT the last is removed', () => {
+    // Regression pin. `TrafficLights` took `onRemove` and `isLastPanel` and used `isLastPanel` only to
+    // choose the tooltip: the red button said "Close panel" and called `onClearApp` regardless, so it
+    // emptied the panel and left the box behind. Closing a panel from its own chrome was impossible —
+    // the only working path was the context menu — and an emptied panel is worse than a cosmetic one
+    // here, because clearing the app drops the `config` that named the agent while the panel survives.
+    const view = mount(group('g', [panel('a'), panel('b')]));
+
+    fireEvent.click(screen.getAllByTitle('Close panel')[0]!);
+
+    expect(countPanels(view.layout())).toBe(1);
+  });
+
+  test('a locked workspace offers no close at all', () => {
+    mount(group('g', [panel('a'), panel('b')]), { locked: true });
+
+    expect(screen.queryAllByTitle('Close panel')).toEqual([]);
+    expect(screen.queryAllByTitle('Clear app')).toEqual([]);
+    expect(screen.getAllByTitle('Maximize').length).toBe(2);
+  });
+});
+
+describe('usePanelClose fires on intent, and only on intent', () => {
+  test('removing a panel retires it', () => {
+    mount(group('g', [panel('a'), panel('b')]));
+
+    fireEvent.click(screen.getAllByTitle('Close panel')[0]!);
+
+    expect(closesFired).toEqual(['a']);
+  });
+
+  test('clearing the last panel retires it too — the app went away', () => {
+    mount(panel('a'));
+
+    fireEvent.click(screen.getByTitle('Clear app'));
+
+    expect(closesFired).toEqual(['a']);
+  });
+
+  test('a handler that throws does not stop the panel from closing', () => {
+    // The layout write is the user's instruction; the cleanup is best effort on top of it.
+    const view = mount(group('g', [panel('a'), panel('b')]));
+    closeBehaviour.set('a', () => {
+      throw new Error('release failed');
+    });
+
+    fireEvent.click(screen.getAllByTitle('Close panel')[0]!);
+
+    expect(countPanels(view.layout())).toBe(1);
+  });
+
+  test('a handler fires once and is not re-run by a later close of another panel', () => {
+    const view = mount(group('g', [panel('a'), panel('b'), panel('c')]));
+    const buttons = screen.getAllByTitle('Close panel');
+
+    fireEvent.click(buttons[0]!);
+    fireEvent.click(screen.getAllByTitle('Close panel')[0]!);
+
+    expect(closesFired.length).toBe(2);
+    expect(new Set(closesFired).size).toBe(2);
+    expect(countPanels(view.layout())).toBe(1);
+  });
+});
+
+describe('appTypes repairs a persisted layout, once', () => {
+  test('an appType the screen no longer renders is replaced and the repair is written back', () => {
+    const view = mount(group('g', [panel('a', 'probe'), panel('b', 'a-deleted-app')]), {
+      appTypes: { allowed: ['probe'], fallback: 'probe' },
+    });
+
+    expect(findPanelApp(view.layout(), 'b')).toBe('probe');
+    // Exactly one write: the normalised tree normalises to itself, so the effect does not re-fire.
+    expect(writes.length).toBe(1);
+  });
+
+  test('a layout that is already legal is not written at all', () => {
+    mount(group('g', [panel('a', 'probe'), panel('b', 'probe')]), {
+      appTypes: { allowed: ['probe'], fallback: 'probe' },
+    });
+
+    expect(writes).toEqual([]);
+  });
+});

src/workspaces/officerdev/src/components/Workspace/WorkspaceView.tsx

@@ -235,10 +235,19 @@ export const WorkspaceView = ({ workspace, locked, appTypes, cwd = '~', root, co
     const panel = ephemeralPanelRef.current;
     if (!panel) return;
 
-    if (isEphemeralOpen) {
-      panel.resize(100 - (ephemeral.defaultBaseSize ?? 40));
-    } else {
-      panel.collapse();
+    // `react-resizable-panels` asserts rather than no-ops when it is asked to resize a group it has not
+    // laid out yet — and an assert thrown from an effect aborts the commit, which here means the whole
+    // dashboard rather than the pane that could not be sized. The pane is a file preview; the panels
+    // behind it are a night's work. Only the ephemeral pane's own geometry is at stake, and the next
+    // render tries again.
+    try {
+      if (isEphemeralOpen) {
+        panel.resize(100 - (ephemeral.defaultBaseSize ?? 40));
+      } else {
+        panel.collapse();
+      }
+    } catch (err) {
+      console.warn('[workspace] could not size the ephemeral pane yet', err);
     }
   }, [isEphemeralOpen, ephemeral?.defaultBaseSize]);