From 04371a9d99e8e2567cb369d1a785b041d1bd07eb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Padez?= <andre.padez@gmail.com>
Date: Fri, 7 Aug 2026 09:49:41 +0000
Subject: [PATCH] pin why a layout diff cannot answer "what closed"
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The panel-close signal (§5.1) was going to be a before/after diff of the layout tree — the todo
document says so. It cannot be. `movePanel` inserts through `newPanelFrom`, which mints a fresh
`uid()`, so a dragged panel's id is gone from the new tree while its app is still on screen; and
`swapPanels` exchanges `{appType, config}` between two ids that both stay put, so a swap reads as
two closes and two opens. Everything downstream of a close signal is destructive — a pty killed, a
session released — so a mechanism that fires on a rearrangement is worse than none.

Two tests, no production change. The signal has to be raised where the intent is known, at
`WorkspaceView`'s `handleRemove`/`handleSetApp` call sites.
---
 .../components/Workspace/layout-utils.test.ts | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/src/workspaces/officerdev/src/components/Workspace/layout-utils.test.ts b/src/workspaces/officerdev/src/components/Workspace/layout-utils.test.ts
index c3194760..09b4c8dd 100644
--- a/src/workspaces/officerdev/src/components/Workspace/layout-utils.test.ts
+++ b/src/workspaces/officerdev/src/components/Workspace/layout-utils.test.ts
@@ -497,3 +497,33 @@ describe('pruneEmptyPanels', () => {
     expect((pruneEmptyPanels(root) as LayoutPanel).config).toEqual({ agentName: 'frontend' });
   });
 });
+
+// A panel id is a *position in the tree*, not an app instance. These two tests are why the panel-close
+// signal (§5.1) is raised at the mutator call sites rather than by diffing the layout before and after,
+// which is what the todo originally proposed: on a diff, both of these read as "that panel closed", and
+// everything downstream of that signal is destructive — a pty killed, a session released.
+describe('panel ids do not survive a rearrangement', () => {
+  test('a move mints a new id for the panel it moves', () => {
+    const three = group('g', 'horizontal', [panel('p1', 'x'), panel('p2', 'y'), panel('p3', 'z')]);
+
+    const next = movePanel(three, 'p3', 'p1', 'bottom');
+
+    // `p3`'s app is still on screen. Its id is not: `insertPanel` builds a fresh node via `newPanelFrom`.
+    expect(ids(next)).not.toContain('p3');
+    expect(find(next, 'p1')).toBeDefined();
+    const moved = ids(next).filter((id) => id !== 'p1' && id !== 'p2');
+    expect(moved).toHaveLength(1);
+    expect(find(next, moved[0]!)!.appType).toBe('z');
+  });
+
+  test('a swap leaves both ids in place and exchanges what is in them', () => {
+    const two = group('g', 'horizontal', [panel('p1', 'x'), panel('p2', 'y')]);
+
+    const next = swapPanels(two, 'p1', 'p2');
+
+    // Every id survives, and every app survives — but each id now names a different app than before.
+    expect(ids(next)).toEqual(['p1', 'p2']);
+    expect(find(next, 'p1')!.appType).toBe('y');
+    expect(find(next, 'p2')!.appType).toBe('x');
+  });
+});