test layout-utils, and stop setApp leaking one app's config to the next

47 tests, the first under any Workspace path. These functions carry a panel's identity now, so a regression in swapPanels is two agents exchanging names, not a cosmetic glitch. Writing them found one: setApp preserved config whenever appType was not null, so changing a panel from chat to terminal handed the terminal the chat's {agentName} to read as its own settings. The comment beside it already stated the opposite intent. Not reachable through the UI today — the picker only appears on an empty panel, so the only route out of an app is via null, which does clear it — but setApp is exported and its signature permits the direct swap. Now only a same-app set keeps the config. Also pins two things as expectations rather than folklore: a move drops zoom and fitContent (todo 5.3, to fail the day that is fixed), and a split redistributes sibling sizes evenly (todo 5.5). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-07 08:44:15 +00:00
parent ecc7fb90d8
commit 85452d1afa
2 changed files with 505 additions and 3 deletions
@@ -0,0 +1,499 @@
+import { describe, expect, test } from 'bun:test';
+import type { LayoutGroup, LayoutNode, LayoutPanel } from './types';
+import {
+  clampZoom,
+  collectPanelConfigs,
+  countPanels,
+  hasAnyApp,
+  movePanel,
+  pruneEmptyPanels,
+  removePanel,
+  setApp,
+  setPanelConfig,
+  setZoom,
+  splitPanel,
+  swapPanels,
+  updateSizes,
+} from './layout-utils';
+
+// These functions are the whole of the layout's behaviour, and since `e588524` they also carry a panel's
+// *identity* — the opaque `config` an app uses to remember what it is. `apps/Chat` stores an agent's name
+// there, so a regression in `swapPanels` here is two agents exchanging identities, not a cosmetic glitch.
+// Everything below is a pure function over a serialisable tree; there is nothing to mock.
+
+const panel = (id: string, appType: string | null = null, extra: Partial<LayoutPanel> = {}): LayoutPanel => ({
+  type: 'panel',
+  id,
+  appType,
+  ...extra,
+});
+
+const group = (id: string, direction: 'horizontal' | 'vertical', children: LayoutNode[]): LayoutGroup => ({
+  type: 'group',
+  id,
+  direction,
+  children: children.map((node) => ({ node, size: 100 / children.length })),
+});
+
+/** Find a panel by id anywhere in the tree — the assertion helper, deliberately not the module's own. */
+const find = (node: LayoutNode, id: string): LayoutPanel | undefined => {
+  if (node.type === 'panel') return node.id === id ? node : undefined;
+  for (const child of node.children) {
+    const hit = find(child.node, id);
+    if (hit) return hit;
+  }
+  return undefined;
+};
+
+const ids = (node: LayoutNode): string[] =>
+  node.type === 'panel' ? [node.id] : node.children.flatMap((c) => ids(c.node));
+
+describe('countPanels / hasAnyApp', () => {
+  test('counts leaves at any depth', () => {
+    expect(countPanels(panel('a'))).toBe(1);
+    expect(countPanels(group('g', 'horizontal', [panel('a'), group('h', 'vertical', [panel('b'), panel('c')])]))).toBe(
+      3,
+    );
+  });
+
+  test('hasAnyApp is false only when every panel is empty', () => {
+    expect(hasAnyApp(group('g', 'horizontal', [panel('a'), panel('b')]))).toBe(false);
+    expect(hasAnyApp(group('g', 'horizontal', [panel('a'), panel('b', 'x')]))).toBe(true);
+  });
+});
+
+describe('splitPanel', () => {
+  test('appends a sibling when the direction matches the parent, keeping the original node by reference', () => {
+    const a = panel('a', 'x');
+    const root = group('g', 'horizontal', [a, panel('b')]);
+
+    const next = splitPanel(root, 'a', 'horizontal') as LayoutGroup;
+
+    expect(next.id).toBe('g');
+    expect(next.children).toHaveLength(3);
+    // The original node object survives — this is what keeps the panel's React subtree mounted.
+    expect(next.children[0]!.node).toBe(a);
+    expect(next.children.map((c) => c.size)).toEqual([100 / 3, 100 / 3, 100 / 3]);
+  });
+
+  test('wraps in a new group when the direction differs', () => {
+    const root = group('g', 'horizontal', [panel('a', 'x'), panel('b')]);
+
+    const next = splitPanel(root, 'a', 'vertical') as LayoutGroup;
+    const wrapper = next.children[0]!.node as LayoutGroup;
+
+    expect(wrapper.type).toBe('group');
+    expect(wrapper.direction).toBe('vertical');
+    expect(wrapper.children[0]!.node).toEqual(panel('a', 'x'));
+    // The wrapper is a new node with a fresh id, which is why this case remounts the panel (todo §5.2).
+    expect(wrapper.id).not.toBe('a');
+  });
+
+  test('splitting a root panel replaces the root with a group', () => {
+    const next = splitPanel(panel('a', 'x'), 'a', 'horizontal') as LayoutGroup;
+
+    expect(next.type).toBe('group');
+    expect(countPanels(next)).toBe(2);
+  });
+
+  test('an unknown panel id leaves the tree untouched by reference', () => {
+    const root = group('g', 'horizontal', [panel('a'), panel('b')]);
+    expect(splitPanel(root, 'nope', 'horizontal')).toBe(root);
+  });
+
+  test('sizes are redistributed evenly, discarding tuned proportions', () => {
+    // Documenting current behaviour, not endorsing it — this is todo §5.5 "preserve sibling sizes".
+    const root: LayoutGroup = {
+      type: 'group',
+      id: 'g',
+      direction: 'horizontal',
+      children: [
+        { node: panel('a'), size: 80 },
+        { node: panel('b'), size: 20 },
+      ],
+    };
+
+    const next = splitPanel(root, 'a', 'horizontal') as LayoutGroup;
+    expect(next.children.every((c) => c.size === 100 / 3)).toBe(true);
+  });
+});
+
+describe('removePanel', () => {
+  test('refuses to remove the last panel', () => {
+    const root = panel('a', 'x');
+    expect(removePanel(root, 'a')).toBe(root);
+
+    const wrapped = group('g', 'horizontal', [panel('a', 'x')]);
+    expect(removePanel(wrapped, 'a')).toBe(wrapped);
+  });
+
+  test('collapses a two-child group to the survivor, which keeps its own id and config', () => {
+    const root = group('g', 'horizontal', [panel('a', 'x'), panel('b', 'y', { config: { agentName: 'backend' } })]);
+
+    const next = removePanel(root, 'a');
+
+    // The group is gone and the survivor is promoted — this is the remount in todo §5.2, row 4.
+    expect(next.type).toBe('panel');
+    expect((next as LayoutPanel).id).toBe('b');
+    expect((next as LayoutPanel).config).toEqual({ agentName: 'backend' });
+  });
+
+  test('renormalises sizes to 100 when three become two', () => {
+    const root = group('g', 'horizontal', [panel('a'), panel('b'), panel('c')]);
+
+    const next = removePanel(root, 'b') as LayoutGroup;
+
+    expect(ids(next)).toEqual(['a', 'c']);
+    expect(next.children.reduce((sum, c) => sum + c.size, 0)).toBeCloseTo(100);
+  });
+
+  test('removes from a nested group', () => {
+    const root = group('g', 'horizontal', [panel('a'), group('h', 'vertical', [panel('b'), panel('c')])]);
+
+    expect(ids(removePanel(root, 'c'))).toEqual(['a', 'b']);
+  });
+
+  test('unwraps a group left holding a single child', () => {
+    const root = group('g', 'horizontal', [
+      group('h', 'vertical', [group('k', 'horizontal', [panel('p1', 'x'), panel('p2', 'y')])]),
+      panel('b', 'z'),
+    ]);
+
+    const next = removePanel(root, 'p1') as LayoutGroup;
+
+    expect(ids(next)).toEqual(['p2', 'b']);
+    // Both `h` and `k` collapsed away rather than leaving one-child groups behind.
+    expect(next.children[0]!.node.type).toBe('panel');
+  });
+
+  test('a group emptied entirely becomes a fresh empty panel', () => {
+    const root = group('g', 'horizontal', [group('h', 'vertical', [panel('a', 'x')]), panel('b', 'y')]);
+
+    const next = removePanel(root, 'a') as LayoutGroup;
+
+    expect(countPanels(next)).toBe(2);
+    expect((next.children[0]!.node as LayoutPanel).appType).toBeNull();
+    expect(ids(next)[1]).toBe('b');
+  });
+});
+
+describe('setApp', () => {
+  test('sets the app type in place', () => {
+    const root = group('g', 'horizontal', [panel('a'), panel('b')]);
+
+    expect(find(setApp(root, 'a', 'officerdev/chat'), 'a')!.appType).toBe('officerdev/chat');
+  });
+
+  test('clearing the app drops the config with it', () => {
+    const root = group('g', 'horizontal', [panel('a', 'officerdev/chat', { config: { agentName: 'frontend' } })]);
+
+    const cleared = find(setApp(root, 'a', null), 'a')!;
+
+    expect(cleared.appType).toBeNull();
+    expect('config' in cleared).toBe(false);
+  });
+
+  test('changing to a DIFFERENT app drops the config too', () => {
+    // The config belongs to whichever app is running. A terminal must never be handed a chat panel's
+    // `{agentName}` and read it as its own settings. Only reachable through the UI as app → null → app
+    // today, but `setApp` is exported and the signature permits a direct swap.
+    const root = group('g', 'horizontal', [panel('a', 'officerdev/chat', { config: { agentName: 'frontend' } })]);
+
+    const changed = find(setApp(root, 'a', 'officerdev/terminal'), 'a')!;
+
+    expect(changed.appType).toBe('officerdev/terminal');
+    expect('config' in changed).toBe(false);
+  });
+
+  test('re-setting the SAME app keeps the config', () => {
+    const root = group('g', 'horizontal', [panel('a', 'officerdev/chat', { config: { agentName: 'frontend' } })]);
+
+    expect(find(setApp(root, 'a', 'officerdev/chat'), 'a')!.config).toEqual({ agentName: 'frontend' });
+  });
+
+  test('an unknown panel id leaves the tree untouched by reference', () => {
+    const root = group('g', 'horizontal', [panel('a'), panel('b')]);
+    expect(setApp(root, 'nope', 'x')).toBe(root);
+  });
+});
+
+describe('setPanelConfig / collectPanelConfigs', () => {
+  test('writes, then replaces wholesale rather than merging', () => {
+    const root = group('g', 'horizontal', [panel('a', 'officerdev/chat')]);
+
+    const named = setPanelConfig(root, 'a', { agentName: 'frontend' });
+    expect(find(named, 'a')!.config).toEqual({ agentName: 'frontend' });
+
+    const replaced = setPanelConfig(named, 'a', { other: 1 });
+    expect(find(replaced, 'a')!.config).toEqual({ other: 1 });
+  });
+
+  test('undefined and {} both remove the key, so an unconfigured panel adds nothing to the layout', () => {
+    const root = group('g', 'horizontal', [panel('a', 'x', { config: { agentName: 'frontend' } })]);
+
+    expect('config' in find(setPanelConfig(root, 'a', undefined), 'a')!).toBe(false);
+    expect('config' in find(setPanelConfig(root, 'a', {}), 'a')!).toBe(false);
+  });
+
+  test('setting a config never disturbs the app type or the panel id', () => {
+    const root = group('g', 'horizontal', [panel('a', 'officerdev/chat'), panel('b', 'officerdev/terminal')]);
+
+    const next = setPanelConfig(root, 'a', { agentName: 'frontend' });
+
+    expect(find(next, 'a')!.appType).toBe('officerdev/chat');
+    expect(find(next, 'b')).toEqual(panel('b', 'officerdev/terminal'));
+    expect(ids(next)).toEqual(['a', 'b']);
+  });
+
+  test('an unknown panel id leaves the tree untouched by reference', () => {
+    const root = group('g', 'horizontal', [panel('a'), panel('b')]);
+    expect(setPanelConfig(root, 'nope', { x: 1 })).toBe(root);
+  });
+
+  test('collects only configured panels, at any depth', () => {
+    const root = group('g', 'horizontal', [
+      panel('a', 'x', { config: { agentName: 'frontend' } }),
+      group('h', 'vertical', [panel('b'), panel('c', 'y', { config: { agentName: 'backend' } })]),
+    ]);
+
+    expect(collectPanelConfigs(root)).toEqual({
+      a: { agentName: 'frontend' },
+      c: { agentName: 'backend' },
+    });
+  });
+
+  test('collects nothing from a tree with no configs', () => {
+    expect(collectPanelConfigs(group('g', 'horizontal', [panel('a', 'x'), panel('b')]))).toEqual({});
+  });
+});
+
+describe('swapPanels', () => {
+  test('exchanges appType and config together, leaving ids where they are', () => {
+    const root = group('g', 'horizontal', [
+      panel('a', 'officerdev/chat', { config: { agentName: 'frontend' } }),
+      panel('b', 'officerdev/chat', { config: { agentName: 'backend' } }),
+    ]);
+
+    const next = swapPanels(root, 'a', 'b');
+
+    // Ids are positions in the tree; contents move between them.
+    expect(ids(next)).toEqual(['a', 'b']);
+    expect(find(next, 'a')!.config).toEqual({ agentName: 'backend' });
+    expect(find(next, 'b')!.config).toEqual({ agentName: 'frontend' });
+  });
+
+  test('a config never survives in the panel it came from', () => {
+    // The regression that would matter most: two agents both answering to the same name.
+    const root = group('g', 'horizontal', [
+      panel('a', 'officerdev/chat', { config: { agentName: 'frontend' } }),
+      panel('b', 'officerdev/terminal'),
+    ]);
+
+    const next = swapPanels(root, 'a', 'b');
+
+    expect('config' in find(next, 'a')!).toBe(false);
+    expect(find(next, 'a')!.appType).toBe('officerdev/terminal');
+    expect(find(next, 'b')!.config).toEqual({ agentName: 'frontend' });
+  });
+
+  test('swaps across different branches of the tree', () => {
+    const root = group('g', 'horizontal', [
+      panel('a', 'x', { config: { n: 1 } }),
+      group('h', 'vertical', [panel('b'), panel('c', 'y', { config: { n: 2 } })]),
+    ]);
+
+    const next = swapPanels(root, 'a', 'c');
+
+    expect(find(next, 'a')!.config).toEqual({ n: 2 });
+    expect(find(next, 'c')!.config).toEqual({ n: 1 });
+  });
+
+  test('an unknown id on either side leaves the tree untouched by reference', () => {
+    const root = group('g', 'horizontal', [panel('a', 'x'), panel('b')]);
+    expect(swapPanels(root, 'a', 'nope')).toBe(root);
+    expect(swapPanels(root, 'nope', 'b')).toBe(root);
+  });
+
+  test('does not mutate the input tree', () => {
+    const root = group('g', 'horizontal', [panel('a', 'x', { config: { n: 1 } }), panel('b', 'y')]);
+    const before = structuredClone(root);
+
+    swapPanels(root, 'a', 'b');
+
+    expect(root).toEqual(before);
+  });
+});
+
+describe('movePanel', () => {
+  test("carries the panel's config to its new position", () => {
+    // The drag path is dead code today (todo §5.3), but this is the invariant that disarmed it as a
+    // hazard: an agent's name travels with the panel rather than being reset to defaults.
+    const root = group('g', 'horizontal', [
+      panel('a', 'officerdev/chat', { config: { agentName: 'frontend' } }),
+      panel('b', 'officerdev/terminal'),
+      panel('c'),
+    ]);
+
+    const next = movePanel(root, 'a', 'c', 'right');
+
+    // Exactly one panel carries the config, and it is a different node id than before.
+    const configs = Object.entries(collectPanelConfigs(next));
+    expect(configs).toHaveLength(1);
+    expect(configs[0]![1]).toEqual({ agentName: 'frontend' });
+    expect(configs[0]![0]).not.toBe('a');
+  });
+
+  test('center position delegates to a swap', () => {
+    const root = group('g', 'horizontal', [
+      panel('a', 'x', { config: { n: 1 } }),
+      panel('b', 'y', { config: { n: 2 } }),
+    ]);
+
+    expect(movePanel(root, 'a', 'b', 'center')).toEqual(swapPanels(root, 'a', 'b'));
+  });
+
+  test('moving a panel onto itself is a no-op by reference', () => {
+    const root = group('g', 'horizontal', [panel('a', 'x'), panel('b')]);
+    expect(movePanel(root, 'a', 'a', 'right')).toBe(root);
+  });
+
+  test('an unknown source leaves the tree untouched by reference', () => {
+    const root = group('g', 'horizontal', [panel('a', 'x'), panel('b')]);
+    expect(movePanel(root, 'nope', 'a', 'right')).toBe(root);
+  });
+
+  test('wraps the target in a new group when the drop direction differs from its parent', () => {
+    const root = group('g', 'horizontal', [
+      panel('a', 'officerdev/chat', { config: { agentName: 'frontend' } }),
+      panel('b', 'x'),
+      panel('c', 'y'),
+    ]);
+
+    const next = movePanel(root, 'a', 'c', 'top') as LayoutGroup;
+    const wrapper = next.children[1]!.node as LayoutGroup;
+
+    expect(wrapper.type).toBe('group');
+    expect(wrapper.direction).toBe('vertical');
+    // The moved contents land above the target, config intact.
+    expect((wrapper.children[0]!.node as LayoutPanel).config).toEqual({ agentName: 'frontend' });
+    expect((wrapper.children[1]!.node as LayoutPanel).id).toBe('c');
+  });
+
+  test('wraps below the target when dropping bottom', () => {
+    const root = group('g', 'horizontal', [
+      panel('a', 'officerdev/chat', { config: { agentName: 'frontend' } }),
+      panel('b', 'x'),
+      panel('c', 'y'),
+    ]);
+
+    const wrapper = (movePanel(root, 'a', 'c', 'bottom') as LayoutGroup).children[1]!.node as LayoutGroup;
+
+    expect((wrapper.children[0]!.node as LayoutPanel).id).toBe('c');
+    expect((wrapper.children[1]!.node as LayoutPanel).config).toEqual({ agentName: 'frontend' });
+  });
+
+  test('inserts before the target when dropping left', () => {
+    const root = group('g', 'horizontal', [panel('a', 'x', { config: { n: 1 } }), panel('b'), panel('c')]);
+
+    const next = movePanel(root, 'a', 'c', 'left') as LayoutGroup;
+    const order = ids(next);
+
+    expect(order).toHaveLength(3);
+    // The moved panel sits immediately before `c`.
+    expect(order[order.indexOf('c') - 1]).toBe(Object.keys(collectPanelConfigs(next))[0]);
+  });
+
+  test('KNOWN GAP: a move drops zoom and fitContent', () => {
+    // `PanelContents` carries only `{appType, config}`. Recorded in todo §5.3 as a thing to fix before
+    // the drag UI is ever re-enabled; pinned here so the gap is a failing expectation the day it closes,
+    // not a silent behaviour change.
+    const root = group('g', 'horizontal', [
+      panel('a', 'x', { zoom: 1.3, fitContent: true, config: { n: 1 } }),
+      panel('b'),
+      panel('c'),
+    ]);
+
+    const next = movePanel(root, 'a', 'c', 'right');
+    const moved = Object.keys(collectPanelConfigs(next))[0]!;
+
+    expect(find(next, moved)!.zoom).toBeUndefined();
+    expect(find(next, moved)!.fitContent).toBeUndefined();
+  });
+});
+
+describe('setZoom / clampZoom', () => {
+  test('clamps to the declared range and snaps to one decimal', () => {
+    expect(clampZoom(0.1)).toBe(0.7);
+    expect(clampZoom(9)).toBe(1.6);
+    expect(clampZoom(1.24)).toBe(1.2);
+  });
+
+  test('stores nothing at 1, so an untouched panel adds no key', () => {
+    const root = group('g', 'horizontal', [panel('a', 'x', { zoom: 1.3 })]);
+
+    expect('zoom' in find(setZoom(root, 'a', 1), 'a')!).toBe(false);
+  });
+
+  test('a no-op zoom returns the tree by reference', () => {
+    const root = group('g', 'horizontal', [panel('a', 'x', { zoom: 1.3 })]);
+    expect(setZoom(root, 'a', 1.3)).toBe(root);
+    expect(setZoom(root, 'nope', 1.5)).toBe(root);
+  });
+
+  test('zoom does not disturb config', () => {
+    const root = group('g', 'horizontal', [panel('a', 'x', { config: { agentName: 'frontend' } })]);
+
+    expect(find(setZoom(root, 'a', 1.4), 'a')!.config).toEqual({ agentName: 'frontend' });
+  });
+});
+
+describe('updateSizes', () => {
+  test('applies sizes to the named group only', () => {
+    const root = group('g', 'horizontal', [panel('a'), group('h', 'vertical', [panel('b'), panel('c')])]);
+
+    const next = updateSizes(root, 'h', [70, 30]) as LayoutGroup;
+    const inner = next.children[1]!.node as LayoutGroup;
+
+    expect(inner.children.map((c) => c.size)).toEqual([70, 30]);
+    expect(next.children.map((c) => c.size)).toEqual([50, 50]);
+  });
+
+  test('a short sizes array leaves the remaining children alone', () => {
+    const root = group('g', 'horizontal', [panel('a'), panel('b')]);
+
+    expect((updateSizes(root, 'g', [70]) as LayoutGroup).children.map((c) => c.size)).toEqual([70, 50]);
+  });
+});
+
+describe('pruneEmptyPanels', () => {
+  test('drops empty panels and collapses what is left', () => {
+    const root = group('g', 'horizontal', [panel('a', 'x'), panel('b'), panel('c')]);
+
+    const next = pruneEmptyPanels(root)!;
+
+    expect(next.type).toBe('panel');
+    expect((next as LayoutPanel).id).toBe('a');
+  });
+
+  test('returns null when nothing has an app', () => {
+    expect(pruneEmptyPanels(group('g', 'horizontal', [panel('a'), panel('b')]))).toBeNull();
+    expect(pruneEmptyPanels(panel('a'))).toBeNull();
+  });
+
+  test('renormalises sizes of the survivors', () => {
+    const root = group('g', 'horizontal', [panel('a', 'x'), panel('b', 'y'), panel('c')]);
+
+    const next = pruneEmptyPanels(root) as LayoutGroup;
+
+    expect(next.children.reduce((sum, c) => sum + c.size, 0)).toBeCloseTo(100);
+  });
+
+  test('keeps a configured panel that has an app', () => {
+    const root = group('g', 'horizontal', [
+      panel('a', 'officerdev/chat', { config: { agentName: 'frontend' } }),
+      panel('b'),
+    ]);
+
+    expect((pruneEmptyPanels(root) as LayoutPanel).config).toEqual({ agentName: 'frontend' });
+  });
+});
@@ -236,9 +236,12 @@ export function swapPanels(root: LayoutNode, idA: string, idB: string): LayoutNo
 }

 export function setApp(root: LayoutNode, panelId: string, appType: string | null): LayoutNode {
-  // Clearing the app clears its config with it — the next app to occupy the panel would not understand
-  // the old one's settings, and leaving them would resurrect them if the same app came back later.
-  return setContents(root, panelId, appType === null ? { appType: null } : { appType, config: findPanelContents(root, panelId)?.config });
+  // Changing the app clears its config with it — config is owned by whichever app the panel is running,
+  // and the next occupant would read the previous one's settings as its own. Only a no-op set (the same
+  // app, e.g. re-picking what is already there) keeps them.
+  const current = findPanelContents(root, panelId);
+  if (!current) return root;
+  return setContents(root, panelId, { appType, config: appType === current.appType ? current.config : undefined });
 }

 /**