feat(dispatch): remove a chosen lane from Tiled Dispatch

docs/superpowers/plans/2026-07-28-dispatch-lane-removal.md

@@ -0,0 +1,290 @@
+# Dispatch Lane Removal Implementation Plan
+
+> **Status:** IMPLEMENTED and reviewed (PR #625).
+>
+> **Where the implementation differs from this plan:**
+>
+> - **The focus rule is not a plain clamp.** A lane removed BEFORE the focused
+>   one shifts indices down, so holding `focusedLane` constant would silently
+>   move focus to the next agent. It adjusts, then clamps. D5 described only the
+>   clamp half.
+> - **`closeSession` had to change signature** to `Promise<boolean>` so D2's
+>   "only splice if it actually closed" is expressible. Two orchestration call
+>   sites adapt locally to their `Promise<void>` contract.
+> - **`ratios` is not dropped wholesale.** Index 0 is the index-SIDEBAR
+>   fraction, not a lane boundary — only `ratios.slice(1)` are lane weights. D4
+>   was wrong to copy `setTiledLaneCount` here: a removal can preserve the
+>   sidebar width and drop just the removed lane's weight, which a count
+>   *increase* cannot (it would have to invent a weight for the new lane).
+> - **The destructive command's `when` tests liveness, not presence.** A lane
+>   can hold a set-but-dead id for one render; admitting on presence alone let
+>   the command run and do neither of the two things its title promises.
+>
+> **Known pre-existing bug this PR did NOT fix, deliberately:** an orchestrating
+> agent whose close is declined by the user is told the child closed.
+> `requestCloseConfirmation` resolves `false` rather than throwing, so
+> `closeOrchestrationAgent`/`closeOrchestrationRun` report success
+> unconditionally and `skippedSessionIds` stays empty. The `Promise<boolean>`
+> added here is the missing half of the fix, but threading it changes what a
+> cross-process caller is told and deserves its own review rather than a
+> ride-along. See the comment at the wrapper sites.
+
+**Goal:** Let a user remove a *specific* lane from Tiled Dispatch, instead of only being able to shrink the grid from the tail.
+
+---
+
+## The problem
+
+Tiled Dispatch's lane count is a single number, and shrinking it always drops the **tail**:
+
+```ts
+// actions/dispatch.ts — setTiledLaneCount
+const lanes = next < tiled.lanes.length
+  ? tiled.lanes.slice(0, next)          // <- always the last lanes
+  : buildAutoLanes(prev, next, tiled.lanes)
+```
+
+So with 7 lanes open and the agent in lane 3 finished, going 7 → 6 removes lane **7**. The user then has to re-select several lanes by hand to get back to the arrangement they wanted.
+
+**The obvious workaround does not work either.** Closing the agent in lane 3 does not shrink the grid: `clearTiledLaneSessions` sets that lane's `selectedSessionId` to undefined, and `buildAutoLanes`' auto-fill re-homes another agent into the now-empty lane. Count stays 7.
+
+**Net: there is currently no way to shrink the tiled grid at a position of the user's choosing.** That is the gap.
+
+---
+
+## Two commands, not one
+
+The product default is *remove the lane and close the agent*. But a command that sometimes destroys a session and sometimes does not is exactly the kind of thing that surprises someone at speed, so the destructive and non-destructive behaviours are separate commands with separate names.
+
+| | Destructive (the default) | Non-destructive |
+|---|---|---|
+| id | `close-agent-remove-lane` | `remove-tiled-lane` |
+| title | **Close Agent and Remove Lane** | **Remove Lane** |
+| agent | closed via `workspace.closeSession` | keeps running, stays in the index |
+| lane | spliced out, count −1 | spliced out, count −1 |
+
+### Why these names
+
+- **The destructive one leads with the destruction.** `Close` is already this repo's verb for ending a session (`Close Focused Session`, `Close Tab`, `Close Old Agents`), so a title that starts with it is legible at a glance. Putting `Remove Lane` first would bury the irreversible half.
+- **`Remove Agent` was rejected outright.** It reads non-destructive and is not — the worst possible name for the default.
+- **`Kill Lane` was rejected**: `kill` is reserved in this catalog for buried sessions (`Kill Buried Session…`).
+- **The safe command gets the shorter title**, because it is the one a user runs casually.
+- Both are imperative one-shot verbs per `docs/command-style.md` rule 4, and neither takes further input, so neither carries an ellipsis (rule 8).
+
+---
+
+## Design decisions
+
+### D1 — One state action, two callers
+
+`removeTiledLane(laneIndex)` lands in `actions/dispatch.ts` beside `setTiledLaneCount`, which it mirrors. The destructive command is that action **plus** a `closeSession` call; it is not a second code path through the lane state. Two lane-splicing implementations would drift.
+
+### D2 — Order: close first, then splice
+
+`closeSession` is async and runs its own confirmation dialog for irreversible closes. Splicing the lane first would leave the grid already shrunk while the user is still deciding, and a cancelled confirm would leave the layout changed with the agent alive — the worst of both. So: await the close, and only splice if it actually happened.
+
+**This means `closeSession` must report whether it closed.** It currently returns `Promise<void>`. Widening it to return a boolean is in scope; the alternative — re-reading state to infer whether the session survived — is a guess.
+
+### D3 — Refuse at the minimum lane count
+
+`when` requires `lanes.length > MIN_DISPATCH_TILES`. Removing the last lane would leave a tiled layout with nothing in it; the command for that is **Dispatch Mode** (exit tiled), and offering a lane-removal that silently becomes a mode-exit would be two different actions wearing one name.
+
+### D4 — Reset `ratios`, matching `setTiledLaneCount`
+
+Stored lane-boundary ratios are positional. Removing a lane invalidates them, so `ratios: undefined` and let the layout recompute — exactly what `setTiledLaneCount` already does on any count change.
+
+### D5 — Clamp `focusedLane`
+
+Removing the focused lane leaves `focusedLane` pointing past the end when it was the last one. Clamp to `lanes.length - 1`, the same clamp `setTiledLaneCount` applies.
+
+### D6 — Lane 0 is a real lane, and removing it has a visible consequence
+
+Lane 0 is not the index sidebar — the index is a separate column. Lane 0 is an ordinary agent lane that simply has no mini-list of its own, because it is selected from the full index (`TiledDispatchLayout.tsx`: `{laneIndex > 0 && <DispatchMiniList …>}`).
+
+So removing lane 0 promotes lane 1 into position 0, and that lane **loses its own selector**. Mechanically fine, and no special-casing is warranted — but the command descriptions should not pretend the grid is homogeneous.
+
+### D7 — Surface is `dispatch`
+
+Both commands are meaningless outside Tiled Dispatch. `surface: 'dispatch'` per `docs/command-style.md` rules 10–11.
+
+---
+
+## Files
+
+| File | Change |
+|---|---|
+| `src/renderer/src/workspace/hook/actions/dispatch.ts` | new `removeTiledLane(laneIndex)`; add to the returned object and its type |
+| `src/renderer/src/workspace/hook/actions/pane.ts` | `closeSession` returns `Promise<boolean>` |
+| `src/renderer/src/workspace/hook/index.ts` | expose `removeTiledLane` |
+| `src/renderer/src/features/workspace/commands/layoutCommands.ts` | the two commands |
+| `src/renderer/src/features/command-palette/catalog.test.ts` | snapshot + counts 102 → 104 |
+| `src/renderer/src/workspace/dispatch/tiledLaneRemoval.test.ts` | **new** — unit tests for the splice/clamp logic |
+
+---
+
+## Tasks
+
+### Task 1: `removeTiledLane` state action
+
+- [ ] **Step 1:** In `actions/dispatch.ts`, beside `setTiledLaneCount`:
+
+```ts
+/**
+ * Remove ONE lane by index, shrinking the grid by one.
+ *
+ * WHY this exists next to setTiledLaneCount rather than being expressible
+ * through it: that action only takes a COUNT, and shrinking by count always
+ * drops the tail (`lanes.slice(0, next)`). With seven lanes open and the
+ * finished agent in lane three, 7 -> 6 removes lane seven and leaves the user
+ * re-selecting the rest by hand.
+ *
+ * Closing the agent instead does not shrink anything either: the lane empties
+ * and buildAutoLanes' auto-fill re-homes another agent into it. So before this
+ * action there was no way at all to shrink the tiled grid at a chosen position.
+ */
+const removeTiledLane = useCallback(
+  (laneIndex: number) => {
+    setState(prev => {
+      const tiled = prev.dispatchMode?.tiled
+      if (!tiled) return prev
+      // Refuse below the floor. Emptying the layout is Dispatch Mode's job;
+      // a lane-removal that silently becomes a mode-exit is two actions
+      // sharing one name.
+      if (tiled.lanes.length <= MIN_DISPATCH_TILES) return prev
+      if (laneIndex < 0 || laneIndex >= tiled.lanes.length) return prev
+      const lanes = tiled.lanes.filter((_, i) => i !== laneIndex)
+      return {
+        ...prev,
+        dispatchMode: {
+          ...prev.dispatchMode!,
+          tiled: {
+            lanes,
+            // Same clamp setTiledLaneCount applies: removing the last lane
+            // leaves focusedLane past the end.
+            focusedLane: Math.min(tiled.focusedLane, lanes.length - 1),
+            // Ratios are positional, so removing a lane invalidates them.
+            ratios: undefined,
+          },
+        },
+      }
+    })
+  },
+  [setState],
+)
+```
+
+- [ ] **Step 2:** Add `removeTiledLane: (laneIndex: number) => void` to the hook's return type and the returned object; expose it in `workspace/hook/index.ts`.
+
+- [ ] **Step 3:** `npx tsc -b --pretty false` → exit 0.
+
+- [ ] **Step 4:** Commit.
+
+### Task 2: `closeSession` reports whether it closed
+
+- [ ] **Step 1:** Widen `closeSession` in `actions/pane.ts` from `Promise<void>` to `Promise<boolean>` — `true` when the session was closed, `false` when it did not exist or the user cancelled the confirmation. Update the declared signature and every `return` in that function.
+
+- [ ] **Step 2:** Existing callers ignore the value, so no call-site changes are required. Verify with `tsc`.
+
+- [ ] **Step 3:** Commit.
+
+### Task 3: The two commands
+
+- [ ] **Step 1:** In `layoutCommands.ts`, after `tiled-dispatch`:
+
+```ts
+{
+  id: 'remove-tiled-lane',
+  category: 'layout-dispatch',
+  surface: 'dispatch',
+  title: 'Remove Lane',
+  description: '**What it does:** Removes the **focused lane** from Tiled Dispatch and shrinks the grid by one. The agent keeps running and stays in the index.\n\n**Use when:** You are done watching one agent but want to keep the others exactly where they are.\n\n**Notes:** Changing the tile count instead always drops the LAST lane. Removing the leftmost lane promotes the next one into its place, where it is selected from the full index rather than its own compact selector.',
+  keywords: ['remove', 'lane', 'tile', 'tiled dispatch', 'shrink', 'close lane'],
+  when: ({ workspace }) => {
+    const tiled = workspace.state.dispatchMode?.tiled
+    return Boolean(tiled && tiled.lanes.length > MIN_DISPATCH_TILES)
+  },
+  run: ({ workspace }) => {
+    const tiled = workspace.state.dispatchMode?.tiled
+    if (!tiled) return
+    workspace.removeTiledLane(tiled.focusedLane)
+  },
+},
+{
+  id: 'close-agent-remove-lane',
+  category: 'layout-dispatch',
+  surface: 'dispatch',
+  title: 'Close Agent and Remove Lane',
+  description: '**What it does:** Closes the agent in the **focused lane**, then removes that lane and shrinks the grid by one.\n\n**Use when:** An agent has finished and you want it gone along with its slot.\n\n**Notes:** This ends the session. Use **Remove Lane** to reclaim the slot while leaving the agent running. Irreversible closes still confirm first, and cancelling leaves the grid untouched.',
+  keywords: ['close', 'agent', 'lane', 'tile', 'tiled dispatch', 'shrink', 'done'],
+  when: ({ workspace }) => {
+    const tiled = workspace.state.dispatchMode?.tiled
+    if (!tiled || tiled.lanes.length <= MIN_DISPATCH_TILES) return false
+    return Boolean(tiled.lanes[tiled.focusedLane]?.selectedSessionId)
+  },
+  run: async ({ workspace }) => {
+    const tiled = workspace.state.dispatchMode?.tiled
+    if (!tiled) return
+    const laneIndex = tiled.focusedLane
+    const sessionId = tiled.lanes[laneIndex]?.selectedSessionId
+    if (!sessionId) return
+    // Close FIRST. closeSession runs its own confirmation for irreversible
+    // closes; splicing before it resolves would shrink the grid while the
+    // user was still deciding, and a cancelled confirm would leave the
+    // layout changed with the agent still alive.
+    const closed = await workspace.closeSession(sessionId)
+    if (closed) workspace.removeTiledLane(laneIndex)
+  },
+},
+```
+
+- [ ] **Step 2:** Import `MIN_DISPATCH_TILES` from `tiledDispatchSelectors`.
+
+- [ ] **Step 3:** Update `catalog.test.ts` — ordered snapshot (both ids after `tiled-dispatch`), `toHaveLength(104)`, the two test names, and **both** arithmetic assertions. Note the second one: its subtracted term is the count of approved additions and its expected value is the pre-governance baseline of 102, so raise the **subtrahend** to 7, never the right-hand side.
+
+- [ ] **Step 4:** `npm run check:keybindings` → OK.
+
+- [ ] **Step 5:** Commit.
+
+### Task 4: Tests
+
+Tests are welcome in this repo (`docs/testing/standard.md`); the splice/clamp logic is pure state and worth pinning.
+
+- [ ] **Step 1:** Extract the reducer body into a pure exported helper in `tiledDispatchSelectors.ts` so it can be tested without a hook:
+
+```ts
+export function removeLaneFromTiled(
+  tiled: TiledDispatchState,
+  laneIndex: number,
+): TiledDispatchState | null   // null = refused
+```
+
+Have `removeTiledLane` call it.
+
+- [ ] **Step 2:** `tiledLaneRemoval.test.ts` covering:
+  - removing a middle lane keeps the lanes either side, in order
+  - removing the focused lane clamps `focusedLane` into range
+  - removing a lane before the focused one keeps the same lane focused
+  - refuses at `MIN_DISPATCH_TILES`
+  - refuses an out-of-range index
+  - always clears `ratios`
+
+- [ ] **Step 3:** `NODE_ENV=test npx vitest run` → all green.
+
+- [ ] **Step 4:** Commit.
+
+### Task 5: Full gate
+
+- [ ] `npx tsc -b --pretty false` → exit 0
+- [ ] `NODE_ENV=test npx vitest run` → green
+- [ ] `npm run check:keybindings` → OK
+- [ ] `npm run test:contract` → satisfied
+
+---
+
+## Self-review
+
+**Covers the reported problem:** yes — the user can now remove lane 3 specifically, in both the keep-the-agent and close-the-agent flavours.
+
+**Type consistency:** `removeTiledLane(laneIndex: number) => void` produced in Task 1, consumed in Task 3. `closeSession` widened in Task 2, consumed in Task 3. `removeLaneFromTiled` produced in Task 4 Step 1, consumed by Task 1's action.
+
+**Known limitation, recorded not fixed:** both commands act on the **focused** lane only. Removing an arbitrary lane by pointer — a small "×" on each lane header — is the natural mouse-first follow-up and is deliberately out of scope here.

src/renderer/src/features/command-palette/catalog.test.ts

@@ -9,9 +9,9 @@ import type { CommandDef } from '@renderer/features/command-palette/types'
 // Phase 0 of the command-governance plan (docs/superpowers/plans/
 // 2026-07-23-command-surface-audit.md): CHARACTERIZE THE CURRENT CATALOG.
 //
-// This file pinned the exact 102-id before-state, and now pins the 102-id
-// after-state: five durable preferences retired to Settings, five approved
-// additions. Keeping ONE snapshot that moved — rather
+// This file pinned the exact 102-id before-state, and now pins the 104-id
+// after-state: five durable preferences retired to Settings, seven approved
+// additions (102 - 5 + 7 = 104). Keeping ONE snapshot that moved — rather
 // than a "baseline" file and an "after" file — is what makes the plan's
 // headline count an assertion anyone can check against running code instead of
 // prose.
@@ -67,10 +67,13 @@ const BASELINE_COMMAND_IDS: readonly string[] = [
   'clear-composer',
   'undo-clear-composer',
   'send-composer',
-  // layoutCommands (8, was 9: toggle-status-mode retired)
+  // layoutCommands (10, was 9: toggle-status-mode retired, two lane-removal
+  // commands added)
   'dispatch-mode',
   'global-dispatch',
   'tiled-dispatch',
+  'remove-tiled-lane',
+  'close-agent-remove-lane',
   'normalize-layout',
   'hard-normalize-layout',
   'rotate-layout',
@@ -173,21 +176,22 @@ const NAVIGATION_COMMAND_GROUP: readonly string[] = [
 const ids = (): string[] => builtInCommandCatalog.map(c => c.id)
 
 describe('built-in command catalog — baseline characterization', () => {
-  it('contains exactly the 102 governed commands in registration order', () => {
+  it('contains exactly the 104 governed commands in registration order', () => {
     // Order matters: this is the palette's empty-query browse order.
     expect(ids()).toEqual([...BASELINE_COMMAND_IDS])
   })
 
-  it('has exactly 102 commands', () => {
+  it('has exactly 104 commands', () => {
     // Stated separately from the order assertion because this number is the
     // thing that moves, and a bare count failure is a clearer signal than a
     // 99-line array diff.
     //
     // 102 baseline → 98 after governance (5 retirements, 1 addition) → 99 with
-    // `open-keyboard-shortcuts` → 102 with the three composer commands. Each
+    // `open-keyboard-shortcuts` → 102 with the three composer commands → 104
+    // with the two lane-removal commands. Each
     // step of that arithmetic was a deliberate edit to this line, which is the
     // entire point of pinning it.
-    expect(builtInCommandCatalog).toHaveLength(102)
+    expect(builtInCommandCatalog).toHaveLength(104)
   })
 
   it('reports no structural defects', () => {
@@ -204,7 +208,7 @@ describe('built-in command catalog — baseline characterization', () => {
 })
 
 describe('generated per-provider split commands', () => {
-  // The arithmetic is 98 literal ids + 4 generated = 102. If a provider
+  // The arithmetic is 100 literal ids + 4 generated = 104. If a provider
   // is ever added to AGENT_PROVIDER_KINDS, this invariant is what tells the
   // author that the catalog count moved for a legitimate reason, and forces the
   // baseline snapshot above to be updated deliberately.
@@ -218,10 +222,10 @@ describe('generated per-provider split commands', () => {
   })
 
   it('accounts for the difference between literal and total command count', () => {
-    // 102 total - 4 generated = 98 literal `id:` fields across the command
+    // 104 total - 4 generated = 100 literal `id:` fields across the command
     // modules. At the original baseline this read 102 - 4 = 98; it moved down by
-    // the five retirements, then back up by the five additions.
-    expect(builtInCommandCatalog.length - nonDefaultProviders.length * 2).toBe(98)
+    // the five retirements, then back up by the seven additions.
+    expect(builtInCommandCatalog.length - nonDefaultProviders.length * 2).toBe(100)
   })
 
   it('emits both directions for every non-default provider', () => {
@@ -320,7 +324,7 @@ describe('governance targets', () => {
   })
 
   it('lands on the arithmetic the plan predicted', () => {
-    // 102 baseline - 5 retirements + 5 additions = 102, checked against the
+    // 102 baseline - 5 retirements + 7 additions = 104, checked against the
     // real catalog rather than trusted as prose.
     //
     // The subtracted term is the count of APPROVED ADDITIONS and the expected
@@ -332,9 +336,10 @@ describe('governance targets', () => {
     // Additions so far: `open-command-palette` (governance: the palette could
     // not be rebound because it had no command id), `open-keyboard-shortcuts`,
     // and the three composer commands (`clear-composer`,
-    // `undo-clear-composer`, `send-composer`).
-    expect(builtInCommandCatalog.length + RETIRED_COMMAND_IDS.length - 5).toBe(102)
-    expect(builtInCommandCatalog).toHaveLength(102)
+    // `undo-clear-composer`, `send-composer`) and the two lane-removal
+    // commands (`remove-tiled-lane`, `close-agent-remove-lane`).
+    expect(builtInCommandCatalog.length + RETIRED_COMMAND_IDS.length - 7).toBe(102)
+    expect(builtInCommandCatalog).toHaveLength(104)
   })
 })