From 9d6fa2f215f2851075c01c7d4b921d27bb4f4bd1 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Thu, 10 Sep 2026 15:58:43 -0700 Subject: [PATCH 1/6] docs(dispatch): plan restoring the first Grid Dispatch lane's selector strip Refs #850 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01J8PfoJxJ1cvJ9akXsWSVZB --- ...26-09-10-grid-dispatch-first-lane-strip.md | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 docs/superpowers/plans/2026-09-10-grid-dispatch-first-lane-strip.md diff --git a/docs/superpowers/plans/2026-09-10-grid-dispatch-first-lane-strip.md b/docs/superpowers/plans/2026-09-10-grid-dispatch-first-lane-strip.md new file mode 100644 index 000000000..905425765 --- /dev/null +++ b/docs/superpowers/plans/2026-09-10-grid-dispatch-first-lane-strip.md @@ -0,0 +1,90 @@ +# Grid Dispatch First-Lane Strip Plan + +**Issue:** #850 (regression from #687 / #681) +**Branch:** `fix/grid-dispatch-first-lane-strip`, based on `origin/main` @ `48ea6912` + +**Goal:** Every Grid Dispatch lane, including the first (leftmost) lane of each +row, gets its own compact index strip (`DispatchMiniList`), so swapping the +agent in any lane is one click on a selector that belongs to that lane. + +## Why the first lane lost its selector + +Before Grid Dispatch rows (parent of `524671b2`), the full index list beside the +lanes always wrote lane 0 (`setTiledLaneSession(0, …)`). The first lane skipping +the strip was correct then: the index *was* lane 0's selector, and a strip would +have been a second selector for the same lane. + +`524671b2` (#687) gave every row its own index and retargeted that index to +`focusedLaneInRow ?? start`, meaning whichever lane of the row is focused. It kept +the `column > 0` strip gate and its comment. From then on the first lane had no +selector of its own. Changing it takes two steps (click into the lane to move +focus, then pick from the index), while every other lane takes one click on its +strip. + +The fix keeps the index's follow-focus behavior, which the user chose on purpose, +and gives the first lane its strip back. It does not revert the index to lane-0-only. + +## Changes + +All under `src/renderer/src/workspace/dispatch/`. + +1. **`TiledDispatchLayout.tsx`** + - Render `DispatchMiniList` for every lane: drop the `column > 0 &&` gate on + the strip. Keep the gate on `LaneBoundary`, since the first lane has no left + neighbour to resize against. + - Replace the "The row's FIRST lane has no strip" WHY comment with the reason + every lane has one: the index follows focus, so it cannot be any single + lane's dedicated selector. + - Empty-lane hint: + - Drop the `column === 0` "Pick an agent from the index" branch, because + every lane now has a strip. + - Read `gridRow.projectTabIds` instead of the legacy `gridRow.projectTabId`. + `normalizeGridShape` folds the legacy field into `projectTabIds` on read + and nothing writes it again, so the old read was always undefined. A bound + row therefore got the unbound copy promising "⌥↓ for the top of the index", + and ⌥↓ takes the top of the *global* index, not the bound row's. + `02011066` split the copy to avoid exactly that promise. +2. **`DispatchMiniList.tsx`** + - Prop type `Pick` becomes + `'projectTabIds'`. That is the field `rowScopedRows` actually reads. Runtime + behavior is already right because the full row object is passed; only the + declared contract is stale. + +## Tests (`gridDispatchLayout.renderer.test.tsx`) + +Written first, and each one must fail on `origin/main` before the fix: + +- **Every lane has a strip.** A 2×2 grid renders 4 strips; a `[3, 1]` grid + renders 4 strips. These replace the two tests that pinned "first lane has no + strip". +- **The first lane's strip selects into the first lane, not the focused lane.** + With focus on lane 1 of row 0, clicking lane 0's strip calls + `selectTiledLaneSession(0, …)` and `setTiledFocusedLane(0)`. This is the + one-click navigation the issue is about. +- **A project-bound row's empty lane does not promise the top of the index.** A + focused empty lane in a row with `projectTabIds` shows + "Pick an agent from the strip, or press ⌥↓". An unbound row keeps the + "…for the top of the index" copy. + +The `DispatchMiniList` mock gains an `onSelect` click hook and the `DispatchEmpty` +mock renders its hint, so these contracts are observable. Also update the stale +"once each row's first lane lost its strip" comment on the row-targeting test. +That test's claim still holds. + +## Out of scope + +- Classic single-pane Dispatch (`DispatchLayout` without `tiled`). Its index is + still its only pane's selector. +- Index click semantics, keybindings, `DispatchAgentList`. + +## Verification + +On Node 24 (`.nvmrc`; Node 25 breaks happy-dom's `localStorage`): + +- `NODE_ENV=test npx vitest run --project renderer src/renderer/src/workspace/dispatch/` + shows RED before the fix and GREEN after. +- `npm run typecheck` (vitest and electron-vite do not type-check). +- One full `npm test` before opening the PR. + +The app is not launched. The layout contract is proven at the rendered-layout +level, which is where the #673 post-mortem says a layout fix has to be proven. From a93980faa70bfabd41a04615d1113777881ca37e Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Thu, 10 Sep 2026 16:31:01 -0700 Subject: [PATCH 2/6] fix(dispatch): give every Grid Dispatch lane its own selector strip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since #687 a row's index fills whichever lane of the row is focused, but the first lane of each row still skipped its compact index strip on the old grounds that the index was its selector. That left the leftmost lane as the only one that could not be swapped in one click: the user had to click into it to move focus, then pick from the index. Every lane, single-lane rows included, now carries a strip addressed to that lane, and the index keeps following focus. A strip that appeared only once a row gained a second lane would shift the existing lane's content sideways mid-rearrangement, so the rule has no exceptions. The empty-lane hint loses its first-lane branch and a dead branch keyed on the legacy projectTabId, which normalizeGridShape folds away on read. That branch already fell through to the correct copy, because ⌥↓ has walked the focused row's own list since 29ecd829, so "the top of the index" holds in project-bound rows too. DispatchMiniList's prop type now names projectTabIds, the field it actually filters on. Refs #850 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01J8PfoJxJ1cvJ9akXsWSVZB --- .../workspace/dispatch/DispatchMiniList.tsx | 21 +++- .../dispatch/TiledDispatchLayout.tsx | 86 ++++++++------ .../gridDispatchLayout.renderer.test.tsx | 105 +++++++++++++----- 3 files changed, 141 insertions(+), 71 deletions(-) diff --git a/src/renderer/src/workspace/dispatch/DispatchMiniList.tsx b/src/renderer/src/workspace/dispatch/DispatchMiniList.tsx index c2a008967..cfaeab12b 100644 --- a/src/renderer/src/workspace/dispatch/DispatchMiniList.tsx +++ b/src/renderer/src/workspace/dispatch/DispatchMiniList.tsx @@ -12,12 +12,17 @@ import { dispatchRowTitle, } from '@renderer/workspace/dispatch/DispatchAgentList' -// The Tiled Dispatch lane selector, deliberately stripped to NOTHING but the +// The Grid Dispatch lane selector, deliberately stripped to NOTHING but the // index chips ([A1], [A2], ★1 …) — no titles, no activity dots, no badges, // no section headers, no tab separators. Rationale (user's call): the rich -// context already lives one lane over in the full pinned index; repeating any -// of it here just wastes the narrow column. To identify what a chip refers -// to, glance back at the index (or hover the chip for its tooltip). +// context already lives in the row's full index at the row's left edge; +// repeating any of it here just wastes the narrow column. To identify what a +// chip refers to, glance back at that index (or hover the chip for its +// tooltip). +// +// Every lane carries one, the row's first lane included (#850). The full index +// fills whichever lane of its row is focused, so the strip is the only +// selector addressed to one specific lane. // // Each chip is visually identical to the full list's index cell — same width // and same activity background — because it's painted with the SAME @@ -28,8 +33,12 @@ import { type Props = { rows: DispatchAgentRow[] /** The grid row this strip belongs to — supplies its project binding and - * child density, so two strips over the same workspace legitimately differ. */ - gridRow?: Pick + * child density, so two strips over the same workspace legitimately differ. + * `projectTabIds`, not the legacy `projectTabId`: that is the field + * rowScopedRows filters on, and normalizeGridShape folds the legacy one + * away on read. Naming the legacy field here type-checked (every key is + * optional) while promising a binding the strip never read. */ + gridRow?: Pick selectedSessionId?: SessionId focused: boolean onSelect: (row: DispatchAgentRow) => void diff --git a/src/renderer/src/workspace/dispatch/TiledDispatchLayout.tsx b/src/renderer/src/workspace/dispatch/TiledDispatchLayout.tsx index 0a4b2db9b..d0517ff0c 100644 --- a/src/renderer/src/workspace/dispatch/TiledDispatchLayout.tsx +++ b/src/renderer/src/workspace/dispatch/TiledDispatchLayout.tsx @@ -358,33 +358,44 @@ function GridRowView({ workspace={workspace} /> )} - {/* The row's FIRST lane has no strip: the row's own index list - sits directly beside it and is its selector. That pairing is - the point of giving every row its own index — a strip there - would be a second selector for the same lane, six inches from - the first, and it costs 46px of the widest lane in the row. - Every OTHER lane needs its own, because the index is already - spoken for. */} - {column > 0 && ( -
- { - void workspace.selectTiledLaneSession(laneIndex, row.sessionId) - workspace.setTiledFocusedLane(laneIndex) - }} - // Without this the strip's "+N more" renders as a button and - // does nothing — an affordance that promises an action it - // cannot perform, in the exact case the cap exists for. - onToggleExpandedParent={sessionId => - workspace.toggleDispatchRowExpandedParent(rowIndex, sessionId) - } - /> -
- )} + {/* EVERY lane gets its own strip, the row's first lane included + (#850). The row's index fills whichever lane of the row is + FOCUSED (selectIntoRow above), so it is no single lane's + selector. Before #687 the index always wrote lane 0, which is + why the first lane used to skip the strip. Once the index + began following focus, that lane was left without a one-click + selector of its own: changing it took two gestures, click into + the lane and then pick from the index, while every other lane + took one. + + Single-lane rows get a strip too, although their index can + only ever fill that one lane. A strip that appeared only when + a second lane was added would shove the existing lane's content + 46px sideways at the moment the user is rearranging the row. + One rule for every lane also keeps the chip column in the same + place in every lane, which is what makes it scannable. */} +
+ { + void workspace.selectTiledLaneSession(laneIndex, row.sessionId) + workspace.setTiledFocusedLane(laneIndex) + }} + // Without this the strip's "+N more" renders as a button and + // does nothing — an affordance that promises an action it + // cannot perform, in the exact case the cap exists for. + onToggleExpandedParent={sessionId => + workspace.toggleDispatchRowExpandedParent(rowIndex, sessionId) + } + /> +
{ @@ -412,16 +423,21 @@ function GridRowView({ // Advertising it in an unfocused lane would tell the user // to press something that yanks the agent they are working // with and leaves this lane untouched. - // The FIRST lane of a row has no strip (its index list is - // its selector), so naming one there points the user at - // something that is not on screen. + // + // One copy for every lane. Every lane has a strip (#850), + // and ⌥↓ walks the focused ROW's own filtered list + // (tiledRowScopedRows in useKeybinds, since 29ecd829), so + // "the top of the index" is true in a project-bound row too. + // An older branch withheld that phrase from bound rows + // because ⌥↓ used to walk the global list. It keyed off the + // legacy `projectTabId`, which normalizeGridShape folds into + // `projectTabIds` on read, so it was already dead and showed + // this copy anyway. Do not re-add a binding branch unless ⌥↓ + // stops being row-scoped: "fixing" that read to + // `projectTabIds` would withhold a promise that is true. hint={ focused && !lane?.selectedSessionId - ? column === 0 - ? 'Pick an agent from the index, or press ⌥↓' - : gridRow.projectTabId - ? 'Pick an agent from the strip, or press ⌥↓' - : 'Pick an agent from the strip, or press ⌥↓ for the top of the index' + ? 'Pick an agent from the strip, or press ⌥↓ for the top of the index' : undefined } /> diff --git a/src/renderer/src/workspace/dispatch/gridDispatchLayout.renderer.test.tsx b/src/renderer/src/workspace/dispatch/gridDispatchLayout.renderer.test.tsx index 983e6a147..abe1af575 100644 --- a/src/renderer/src/workspace/dispatch/gridDispatchLayout.renderer.test.tsx +++ b/src/renderer/src/workspace/dispatch/gridDispatchLayout.renderer.test.tsx @@ -60,15 +60,34 @@ vi.mock('@renderer/workspace/dispatch/DispatchAgentList', () => ({
{message}
), })) +// The strip is mocked down to what the LAYOUT owns about it: which lane's +// selection it was handed (the only way to tell one lane's strip from +// another's, since the fixture's lanes hold distinct agents), whether its +// expand control is wired, and where the layout's own `onSelect` closure sends +// a pick. Chip rendering is DispatchColorFlags.renderer.test.tsx's job. vi.mock('@renderer/workspace/dispatch/DispatchMiniList', () => ({ - DispatchMiniList: ({ onToggleExpandedParent }: { + DispatchMiniList: ({ rows, selectedSessionId, onSelect, onToggleExpandedParent }: { + rows: { sessionId: string }[] + selectedSessionId?: string + onSelect: (row: { sessionId: string }) => void onToggleExpandedParent?: (id: string) => void - }) => ( -
- ), + }) => { + // Stands in for clicking a chip. It picks an agent OTHER than the lane's + // current one, so the click is a real swap and a layout that ignored it + // could not pass by leaving the lane as it was. + const pick = rows.find(row => row.sessionId !== selectedSessionId) + return ( +
{ + if (pick) onSelect(pick) + }} + /> + ) + }, })) vi.mock('@providers/registry.renderer', () => ({ getRendererProvider: () => ({ @@ -84,6 +103,7 @@ const FIXTURE = JSON.parse( function renderGrid(tiled: TiledDispatchState) { const selectTiledLaneSession = vi.fn().mockResolvedValue(undefined) + const setTiledFocusedLane = vi.fn() const state: WorkspaceState = { ...FIXTURE.state, dispatchMode: { ...FIXTURE.state.dispatchMode!, scope: 'global', tiled }, @@ -96,7 +116,7 @@ function renderGrid(tiled: TiledDispatchState) { focusDispatchSession: vi.fn(), focusSessionInTab: vi.fn(), selectGridRelatedSession: vi.fn(), - setTiledFocusedLane: vi.fn(), + setTiledFocusedLane, selectTiledLaneSession, setDispatchRowHeights: vi.fn(), setDispatchRowIndexFraction: vi.fn(), @@ -114,6 +134,7 @@ function renderGrid(tiled: TiledDispatchState) { />, ), selectTiledLaneSession, + setTiledFocusedLane, } } @@ -140,30 +161,53 @@ describe('Grid Dispatch layout', () => { expect(getAllByTestId('row-index')).toHaveLength(2) }) - it('leaves each row s FIRST lane to that row s index list', () => { - // The row's own index sits directly beside its first lane and is that - // lane's selector — the pairing that giving every row an index exists for. - // A strip there would be a second selector for the same lane, inches from - // the first, eating 46px of the row's widest lane. - const { getAllByTestId } = renderGrid({ - lanes: laneIds.map(id => ({ selectedSessionId: id })), - rows: [{ length: 2 }, { length: 2 }], - focusedLane: 0, - }) + it('gives every lane its own strip, the first lane of each row included', () => { + // #850. A row's index fills whichever lane of the row is FOCUSED, so it is + // no single lane's selector. Any lane without a strip, which used to be + // each row's first lane, could only be changed in two gestures: click into + // it, then pick from the index. A single-lane row is included on purpose. + // If it had no strip, adding a second lane would slide a strip into the + // existing lane and shift its content by 46px. + const cases: { rows: TiledDispatchState['rows']; strips: number }[] = [ + { rows: [{ length: 2 }, { length: 2 }], strips: 4 }, + { rows: [{ length: 3 }, { length: 1 }], strips: 4 }, + ] + for (const { rows, strips } of cases) { + const { getAllByTestId } = renderGrid({ + lanes: laneIds.map(id => ({ selectedSessionId: id })), + rows, + focusedLane: 0, + }) - // 2 rows x 2 lanes, minus the first lane of each row. - expect(getAllByTestId('lane-strip')).toHaveLength(2) + expect(getAllByTestId('lane-strip')).toHaveLength(strips) + cleanup() + } }) - it('gives a strip to every lane the index does not already select', () => { - const { getAllByTestId } = renderGrid({ + it('swaps the first lane from its own strip while another lane has focus', () => { + // The navigation #850 restores. With focus in lane 1, the row's index would + // fill lane 1, so lane 0's strip is the only one-click way to change lane 0. + // It has to write lane 0 (not the focused lane) through the waking path, + // and move focus there, because picking an agent for a lane means working + // in it next. Selecting by its current agent is what identifies lane 0's + // strip: the fixture's lanes hold distinct agents. + const { getAllByTestId, selectTiledLaneSession, setTiledFocusedLane } = renderGrid({ lanes: laneIds.map(id => ({ selectedSessionId: id })), - rows: [{ length: 3 }, { length: 1 }], - focusedLane: 0, + rows: [{ length: 2 }, { length: 2 }], + focusedLane: 1, }) - // Row 0 keeps strips on lanes 2 and 3; row 1's single lane has none. - expect(getAllByTestId('lane-strip')).toHaveLength(2) + const firstLaneStrip = getAllByTestId('lane-strip') + .find(strip => strip.getAttribute('data-selected') === laneIds[0]) + expect(firstLaneStrip).toBeDefined() + const picked = firstLaneStrip!.getAttribute('data-pick') + expect(picked).toBeTruthy() + firstLaneStrip!.click() + + expect(selectTiledLaneSession).toHaveBeenCalledTimes(1) + expect(selectTiledLaneSession).toHaveBeenCalledWith(0, picked) + expect(setTiledFocusedLane).toHaveBeenCalledTimes(1) + expect(setTiledFocusedLane).toHaveBeenCalledWith(0) }) it('wires the strip s expand control instead of shipping a dead button', () => { @@ -234,10 +278,11 @@ describe('Grid Dispatch layout', () => { }) it('sends a row s index click into THAT row, not the focused one', () => { - // Load-bearing once each row's first lane lost its strip: with focus in - // row 0, clicking row 1's index must target row 1's first lane (flat 2). - // The earlier version of this test clicked row 0's index while focus was - // already in row 0, so it could not tell `focusedLaneInRow` from `start`. + // With focus in row 0, clicking row 1's index must target row 1's first + // lane (flat 2). Clicking a row's index means "I am working in this row + // now", so it must never reach across into the row that happens to have + // focus. The earlier version of this test clicked row 0's index while focus + // was already in row 0, so it could not tell `focusedLaneInRow` from `start`. const { getAllByTestId, selectTiledLaneSession } = renderGrid({ lanes: laneIds.map(id => ({ selectedSessionId: id })), rows: [{ length: 2 }, { length: 2 }], From 0ef2b17e693bd5435c06813d14a3d1d3b5985789 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Thu, 10 Sep 2026 16:31:02 -0700 Subject: [PATCH 3/6] =?UTF-8?q?docs(dispatch):=20amend=20the=20first-lane?= =?UTF-8?q?=20strip=20plan=20for=20the=20row-scoped=20=E2=8C=A5=E2=86=93?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first draft planned to fix the hint's legacy projectTabId read so bound rows would stop promising "the top of the index". That premise predates 29ecd829, which made ⌥↓ row-scoped and the promise true, so the plan now records deleting the dead branch instead and drops the hint test. Refs #850 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01J8PfoJxJ1cvJ9akXsWSVZB --- ...26-09-10-grid-dispatch-first-lane-strip.md | 34 ++++++++++++------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/docs/superpowers/plans/2026-09-10-grid-dispatch-first-lane-strip.md b/docs/superpowers/plans/2026-09-10-grid-dispatch-first-lane-strip.md index 905425765..5f0586447 100644 --- a/docs/superpowers/plans/2026-09-10-grid-dispatch-first-lane-strip.md +++ b/docs/superpowers/plans/2026-09-10-grid-dispatch-first-lane-strip.md @@ -38,12 +38,21 @@ All under `src/renderer/src/workspace/dispatch/`. - Empty-lane hint: - Drop the `column === 0` "Pick an agent from the index" branch, because every lane now has a strip. - - Read `gridRow.projectTabIds` instead of the legacy `gridRow.projectTabId`. - `normalizeGridShape` folds the legacy field into `projectTabIds` on read - and nothing writes it again, so the old read was always undefined. A bound - row therefore got the unbound copy promising "⌥↓ for the top of the index", - and ⌥↓ takes the top of the *global* index, not the bound row's. - `02011066` split the copy to avoid exactly that promise. + - Delete the dead `gridRow.projectTabId` branch, so every row shows + "Pick an agent from the strip, or press ⌥↓ for the top of the index". + + > **Amended during implementation (2026-09-10).** The first draft of this + > plan said to *fix* that read to `projectTabIds`, on the premise that ⌥↓ + > takes the top of the global index and so "the top of the index" is false + > in a bound row. That premise is out of date. `524671b2` split the copy + > while ⌥↓ still walked the global list (`dispatchRows(workspace)` at that + > commit). `29ecd829` then made ⌥↓ walk the focused row's own filtered list + > (`tiledRowScopedRows` in `useKeybinds.ts`), which makes "the top of the + > index" true in a bound row too. The stale read happened to show that copy + > anyway, because `normalizeGridShape` folds `projectTabId` away. Fixing the + > read would have withheld a true promise, so the branch is deleted. Bound + > rows see no change. The hint is copy, so no test pins it; a WHY comment at + > the hint says not to re-add a binding branch while ⌥↓ is row-scoped. 2. **`DispatchMiniList.tsx`** - Prop type `Pick` becomes `'projectTabIds'`. That is the field `rowScopedRows` actually reads. Runtime @@ -61,16 +70,15 @@ Written first, and each one must fail on `origin/main` before the fix: With focus on lane 1 of row 0, clicking lane 0's strip calls `selectTiledLaneSession(0, …)` and `setTiledFocusedLane(0)`. This is the one-click navigation the issue is about. -- **A project-bound row's empty lane does not promise the top of the index.** A - focused empty lane in a row with `projectTabIds` shows - "Pick an agent from the strip, or press ⌥↓". An unbound row keeps the - "…for the top of the index" copy. - -The `DispatchMiniList` mock gains an `onSelect` click hook and the `DispatchEmpty` -mock renders its hint, so these contracts are observable. Also update the stale +The `DispatchMiniList` mock reports its lane's selection and forwards a click to +the layout's real `onSelect` closure, so the tests can tell lane 0's strip from +the others and observe which lane the layout writes. Also update the stale "once each row's first lane lost its strip" comment on the row-targeting test. That test's claim still holds. +(The first draft also listed a bound-row hint test. It was dropped with the +premise it relied on; see the amendment under Changes.) + ## Out of scope - Classic single-pane Dispatch (`DispatchLayout` without `tiled`). Its index is From 19dbccd16c569146920a3721752eed23061e0cf4 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Thu, 10 Sep 2026 17:12:21 -0700 Subject: [PATCH 4/6] test(dispatch): pin each Grid Dispatch strip's lane, row binding and expand target Review of #859 showed the strip tests could not tell right wiring from wrong. The only strip clicked sat in flat lane 0, where laneIndex, column and start are all 0. Nothing checked which row's binding a strip was handed, and the expand test passed for any handler, a no-op included. The strip-click test is now a table over lanes 0, 2 and 3 with focus elsewhere. New tests check that each strip gets its own row's binding and that expand addresses its own row. Another pins that a row's index fills the focused lane of that row, which is the premise that makes every lane need a strip. Each was checked by mutating the layout: column, start, grid.rows[0], a row-0 expand and a bare start in selectIntoRow all fail. Strip lookup now requires exactly one match. Refs #850 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01J8PfoJxJ1cvJ9akXsWSVZB --- .../gridDispatchLayout.renderer.test.tsx | 162 +++++++++++++----- 1 file changed, 121 insertions(+), 41 deletions(-) diff --git a/src/renderer/src/workspace/dispatch/gridDispatchLayout.renderer.test.tsx b/src/renderer/src/workspace/dispatch/gridDispatchLayout.renderer.test.tsx index abe1af575..453d7d8ee 100644 --- a/src/renderer/src/workspace/dispatch/gridDispatchLayout.renderer.test.tsx +++ b/src/renderer/src/workspace/dispatch/gridDispatchLayout.renderer.test.tsx @@ -60,14 +60,18 @@ vi.mock('@renderer/workspace/dispatch/DispatchAgentList', () => ({
{message}
), })) -// The strip is mocked down to what the LAYOUT owns about it: which lane's -// selection it was handed (the only way to tell one lane's strip from -// another's, since the fixture's lanes hold distinct agents), whether its -// expand control is wired, and where the layout's own `onSelect` closure sends -// a pick. Chip rendering is DispatchColorFlags.renderer.test.tsx's job. +// The strip is mocked down to what the LAYOUT owns about it: +// - which lane's selection it was handed (the only way to tell one lane's +// strip from another's, since the fixture's lanes hold distinct agents); +// - which grid row's binding it was handed; +// - where the layout's own `onSelect` and `onToggleExpandedParent` closures +// send a pick or an expand. +// Chip rendering is DispatchColorFlags.renderer.test.tsx's job, and the row +// filtering the real strip applies is rowScopedRows.test.ts's. vi.mock('@renderer/workspace/dispatch/DispatchMiniList', () => ({ - DispatchMiniList: ({ rows, selectedSessionId, onSelect, onToggleExpandedParent }: { + DispatchMiniList: ({ rows, gridRow, selectedSessionId, onSelect, onToggleExpandedParent }: { rows: { sessionId: string }[] + gridRow?: { projectTabIds?: string[] } selectedSessionId?: string onSelect: (row: { sessionId: string }) => void onToggleExpandedParent?: (id: string) => void @@ -81,11 +85,22 @@ vi.mock('@renderer/workspace/dispatch/DispatchMiniList', () => ({ data-testid="lane-strip" data-selected={selectedSessionId ?? ''} data-pick={pick?.sessionId ?? ''} - data-can-expand={onToggleExpandedParent ? 'true' : 'false'} + data-project={(gridRow?.projectTabIds ?? []).join(',')} onClick={() => { if (pick) onSelect(pick) }} - /> + > + {/* Stands in for the strip's "+N more" control. It stops propagation + so that expanding never doubles as a chip pick. */} +
) }, })) @@ -104,6 +119,7 @@ const FIXTURE = JSON.parse( function renderGrid(tiled: TiledDispatchState) { const selectTiledLaneSession = vi.fn().mockResolvedValue(undefined) const setTiledFocusedLane = vi.fn() + const toggleDispatchRowExpandedParent = vi.fn() const state: WorkspaceState = { ...FIXTURE.state, dispatchMode: { ...FIXTURE.state.dispatchMode!, scope: 'global', tiled }, @@ -122,7 +138,7 @@ function renderGrid(tiled: TiledDispatchState) { setDispatchRowIndexFraction: vi.fn(), setDispatchLaneWeights: vi.fn(), setDispatchRowCapChildren: vi.fn(), - toggleDispatchRowExpandedParent: vi.fn(), + toggleDispatchRowExpandedParent, } as unknown as Workspace return { ...render( @@ -135,9 +151,20 @@ function renderGrid(tiled: TiledDispatchState) { ), selectTiledLaneSession, setTiledFocusedLane, + toggleDispatchRowExpandedParent, } } +/** The strip handed `sessionId` as its lane's selection. Fails loudly unless + * exactly one strip matches: identifying a lane by its agent relies on the + * fixture's lanes holding distinct agents, and a silent first match would let + * a changed fixture click the wrong lane's strip and still pass. */ +function stripSelecting(strips: HTMLElement[], sessionId: string): HTMLElement { + const matches = strips.filter(strip => strip.getAttribute('data-selected') === sessionId) + expect(matches).toHaveLength(1) + return matches[0]! +} + const laneIds = FIXTURE.state.dispatchMode!.tiled!.lanes.map( lane => lane.selectedSessionId, ) as SessionId[] @@ -184,45 +211,61 @@ describe('Grid Dispatch layout', () => { } }) - it('swaps the first lane from its own strip while another lane has focus', () => { - // The navigation #850 restores. With focus in lane 1, the row's index would - // fill lane 1, so lane 0's strip is the only one-click way to change lane 0. - // It has to write lane 0 (not the focused lane) through the waking path, - // and move focus there, because picking an agent for a lane means working - // in it next. Selecting by its current agent is what identifies lane 0's - // strip: the fixture's lanes hold distinct agents. - const { getAllByTestId, selectTiledLaneSession, setTiledFocusedLane } = renderGrid({ - lanes: laneIds.map(id => ({ selectedSessionId: id })), - rows: [{ length: 2 }, { length: 2 }], - focusedLane: 1, - }) + it('writes each strip s own lane and focuses it, wherever focus is', () => { + // The navigation #850 restores: a strip is the selector addressed to ONE + // lane, so a pick must land in that lane (through the waking path) and move + // focus there, never into whichever lane happens to be focused. The first + // case is the #850 gesture itself. With focus in lane 1, the row's index + // would fill lane 1, so lane 0's strip is the only one-click way to change + // lane 0. + // + // Why three cases: in flat lane 0, `laneIndex`, `column` and `start` are + // all 0, so a strip wired to the wrong one would pass there. Lane 2 (row 1, + // column 0, focus in the other row) catches `column`. Lane 3 (row 1, + // column 1, focus in the same row) catches `start` and `grid.focusedLane`. + const cases = [ + { lane: 0, focusedLane: 1 }, + { lane: 2, focusedLane: 1 }, + { lane: 3, focusedLane: 2 }, + ] + for (const { lane, focusedLane } of cases) { + const { getAllByTestId, selectTiledLaneSession, setTiledFocusedLane } = renderGrid({ + lanes: laneIds.map(id => ({ selectedSessionId: id })), + rows: [{ length: 2 }, { length: 2 }], + focusedLane, + }) - const firstLaneStrip = getAllByTestId('lane-strip') - .find(strip => strip.getAttribute('data-selected') === laneIds[0]) - expect(firstLaneStrip).toBeDefined() - const picked = firstLaneStrip!.getAttribute('data-pick') - expect(picked).toBeTruthy() - firstLaneStrip!.click() + const strip = stripSelecting(getAllByTestId('lane-strip'), laneIds[lane]!) + const picked = strip.getAttribute('data-pick') + expect(picked).toBeTruthy() + strip.click() - expect(selectTiledLaneSession).toHaveBeenCalledTimes(1) - expect(selectTiledLaneSession).toHaveBeenCalledWith(0, picked) - expect(setTiledFocusedLane).toHaveBeenCalledTimes(1) - expect(setTiledFocusedLane).toHaveBeenCalledWith(0) + expect(selectTiledLaneSession.mock.calls).toEqual([[lane, picked]]) + expect(setTiledFocusedLane.mock.calls).toEqual([[lane]]) + cleanup() + } }) - it('wires the strip s expand control instead of shipping a dead button', () => { - // The strip renders "+N more" as a real button and optional-calls this - // handler. Omitting it shipped an affordance that promises an action and - // silently does nothing — in the exact case the cap exists for. - const { getAllByTestId } = renderGrid({ + it('expands a capped parent in the strip s own row', () => { + // The strip's "+N more" expands a parent in ITS row only (expandedParents + // is per-row state). Before, this test checked only that a handler was + // present, so a no-op handler, or one hardcoded to row 0, still passed and + // shipped a button that expands the wrong row or nothing at all. DOM order + // is row-major: two strips in row 0, then two in row 1. + const { getAllByTestId, toggleDispatchRowExpandedParent } = renderGrid({ lanes: laneIds.map(id => ({ selectedSessionId: id })), rows: [{ length: 2 }, { length: 2 }], focusedLane: 0, }) - for (const strip of getAllByTestId('lane-strip')) { - expect(strip.getAttribute('data-can-expand')).toBe('true') - } + for (const expand of getAllByTestId('lane-strip-expand')) expand.click() + + expect(toggleDispatchRowExpandedParent.mock.calls).toEqual([ + [0, 'parent-under-cap'], + [0, 'parent-under-cap'], + [1, 'parent-under-cap'], + [1, 'parent-under-cap'], + ]) }) it('renders uneven rows without evening them out', () => { @@ -255,6 +298,23 @@ describe('Grid Dispatch layout', () => { expect(indexes[1]!.getAttribute('data-project')).toBe('') }) + it('passes each strip its own row s project binding', () => { + // A strip lists what its ROW offers (rowScopedRows filters by the row's + // binding). If a strip were handed another row's descriptor, or none, it + // would offer agents its row's index excludes, and a pick from it would + // place a project-A agent in a row bound to project B. Row-major DOM order: + // row 0's two strips first. + const boundTab = FIXTURE.state.activeTabId + const { getAllByTestId } = renderGrid({ + lanes: laneIds.map(id => ({ selectedSessionId: id })), + rows: [{ length: 2, projectTabIds: [boundTab] }, { length: 2 }], + focusedLane: 0, + }) + + expect(getAllByTestId('lane-strip').map(strip => strip.getAttribute('data-project'))) + .toEqual([boundTab, boundTab, '', '']) + }) + it('selects through the waking path, never the raw lane writer', () => { // #690: rehydrate deliberately does not respawn detached sessions, so a // hibernated agent placed straight into a lane renders fine and then @@ -281,8 +341,9 @@ describe('Grid Dispatch layout', () => { // With focus in row 0, clicking row 1's index must target row 1's first // lane (flat 2). Clicking a row's index means "I am working in this row // now", so it must never reach across into the row that happens to have - // focus. The earlier version of this test clicked row 0's index while focus - // was already in row 0, so it could not tell `focusedLaneInRow` from `start`. + // focus. This case cannot tell `focusedLaneInRow ?? start` from a bare + // `start` (focus is outside the clicked row, so both give 2); the next test + // is the one that pins follow-focus. const { getAllByTestId, selectTiledLaneSession } = renderGrid({ lanes: laneIds.map(id => ({ selectedSessionId: id })), rows: [{ length: 2 }, { length: 2 }], @@ -295,6 +356,25 @@ describe('Grid Dispatch layout', () => { expect(selectTiledLaneSession.mock.calls[0]![0]).toBe(2) }) + it('fills the focused lane from a row s index when focus is already in that row', () => { + // The premise of #850. The index follows focus within its row, so it is no + // single lane's selector, and every lane needs its own strip for that + // reason. If the index ever goes back to always filling the row's first + // lane, this fails. At that point the first lane's strip is a duplicate + // selector and #850's reasoning should be revisited, not silently kept. + const { getAllByTestId, selectTiledLaneSession, setTiledFocusedLane } = renderGrid({ + lanes: laneIds.map(id => ({ selectedSessionId: id })), + rows: [{ length: 2 }, { length: 2 }], + focusedLane: 1, + }) + + getAllByTestId('row-index')[0]!.click() + + expect(selectTiledLaneSession).toHaveBeenCalledTimes(1) + expect(selectTiledLaneSession.mock.calls[0]![0]).toBe(1) + expect(setTiledFocusedLane.mock.calls).toEqual([[1]]) + }) + it('gives a row bound to two projects both their index sections', () => { // The feature in one assertion, and the reason it is cheap: // buildDispatchGroups already groups by tab, so a two-project row renders From 318ad66fac201e850c4ba6f5e40e491408902652 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Thu, 10 Sep 2026 17:12:21 -0700 Subject: [PATCH 5/6] docs(dispatch): correct the first-lane strip regression history The comment, plan and PR said #687 kept the first lane strip-less. It did the opposite. 524671b2 (#687) made the index follow focus and gave every lane a strip for that reason. 1eb9a472 (#691, a lane-wake fix) then removed the first lane's strip as a side change, on the grounds that the index was its selector, which had already stopped being true. Both PR reviewers caught it with git show. The record now points at the change that actually regressed, and the plan records the review and its resolutions. Refs #850 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01J8PfoJxJ1cvJ9akXsWSVZB --- ...26-09-10-grid-dispatch-first-lane-strip.md | 58 +++++++++++++++++-- .../dispatch/TiledDispatchLayout.tsx | 20 +++++-- 2 files changed, 66 insertions(+), 12 deletions(-) diff --git a/docs/superpowers/plans/2026-09-10-grid-dispatch-first-lane-strip.md b/docs/superpowers/plans/2026-09-10-grid-dispatch-first-lane-strip.md index 5f0586447..a3088bc04 100644 --- a/docs/superpowers/plans/2026-09-10-grid-dispatch-first-lane-strip.md +++ b/docs/superpowers/plans/2026-09-10-grid-dispatch-first-lane-strip.md @@ -1,6 +1,6 @@ # Grid Dispatch First-Lane Strip Plan -**Issue:** #850 (regression from #687 / #681) +**Issue:** #850 (regression from #691's side change; see the corrected history below) **Branch:** `fix/grid-dispatch-first-lane-strip`, based on `origin/main` @ `48ea6912` **Goal:** Every Grid Dispatch lane, including the first (leftmost) lane of each @@ -15,11 +15,22 @@ the strip was correct then: the index *was* lane 0's selector, and a strip would have been a second selector for the same lane. `524671b2` (#687) gave every row its own index and retargeted that index to -`focusedLaneInRow ?? start`, meaning whichever lane of the row is focused. It kept -the `column > 0` strip gate and its comment. From then on the first lane had no -selector of its own. Changing it takes two steps (click into the lane to move -focus, then pick from the index), while every other lane takes one click on its -strip. +`focusedLaneInRow ?? start`, meaning whichever lane of the row is focused. It +also gave **every** lane a strip, first lane included, for exactly that reason +("the index belongs to the row, not to its first lane"). + +`1eb9a472` (#691, a fix for waking hibernated agents placed into lanes) then +removed the first lane's strip as a side change ("Also: …"), arguing that the +row's index "IS its selector". That was already false, because the index +followed focus. From then on the first lane had no selector of its own. +Changing it takes two steps (click into the lane to move focus, then pick from +the index), while every other lane takes one click on its strip. + +> **Corrected after review (2026-09-10).** The first version of this section +> said `524671b2` "kept the `column > 0` strip gate". Both PR reviewers found +> that wrong with `git show`, and I confirmed it: `524671b2` removed the gate +> and `1eb9a472` restored it. The fix is unchanged; the attribution matters +> because it tells the next investigator which change to read. The fix keeps the index's follow-focus behavior, which the user chose on purpose, and gives the first lane its strip back. It does not revert the index to lane-0-only. @@ -96,3 +107,38 @@ On Node 24 (`.nvmrc`; Node 25 breaks happy-dom's `localStorage`): The app is not launched. The layout contract is proven at the rendered-layout level, which is where the #673 post-mortem says a layout fix has to be proven. + +## Review (2026-09-10) + +Two orchestrated reviewers ran, both on Codex: one on correctness and +behavior, one on tests, comments and conventions. A Claude reviewer was +attempted twice and never received its prompt (known issue #854: long +`create_agent` prompts fail for Claude children), so the second seat moved to +Codex. Verdicts: "Yes" and "With fixes". No Critical findings. + +Resolved on this branch: + +- **Wrong regression history** (both reviewers). Corrected in the layout's WHY + comment, this plan, the PR body and #850, as described above. +- **Strip-click test only exercised flat lane 0**, where `laneIndex`, `column` + and `start` are all 0. It is now a table over lanes 0, 2 and 3 with focus + elsewhere. Proven by mutation: `column` and `start` each fail it. +- **Strip `gridRow` wiring was untested.** New test: each strip receives its own + row's binding. Proven by mutation (`grid.rows[0]` fails it). +- **Expand test proved only that a handler was present.** It now asserts + `toggleDispatchRowExpandedParent(rowIndex, parentId)` per strip. Proven by + mutation (row hardcoded to 0 fails it). +- **Row-index comment overclaimed** what its case distinguishes. Reworded, and + a new test pins that the index fills the focused lane of its own row. That is + the premise of #850, and it is proven by mutation (a bare `start` fails it). +- **Ambiguous strip lookup.** `stripSelecting` asserts exactly one strip matches. +- **Empty-lane hint promised a pick in a row that offers no agents** (a + pre-existing issue in the code this PR rewrote). The hint is now gated on + `rowScopedRows(rows, gridRow)` containing an agent, the same filter the strip + and ⌥↓ use. The test was RED before the change. + +Not changed, and recorded as a known limitation: the fixed 46px strip combined +with the percentage-only `LANE_MIN_FRACTION` (0.08 of the lane region) can leave +a lane dragged to its minimum with no visible content. That already applied to +every other lane; this PR extends it to the first lane. The fix is a +minimum-content-width rule for lanes and belongs in its own change. diff --git a/src/renderer/src/workspace/dispatch/TiledDispatchLayout.tsx b/src/renderer/src/workspace/dispatch/TiledDispatchLayout.tsx index d0517ff0c..610c36c30 100644 --- a/src/renderer/src/workspace/dispatch/TiledDispatchLayout.tsx +++ b/src/renderer/src/workspace/dispatch/TiledDispatchLayout.tsx @@ -361,12 +361,20 @@ function GridRowView({ {/* EVERY lane gets its own strip, the row's first lane included (#850). The row's index fills whichever lane of the row is FOCUSED (selectIntoRow above), so it is no single lane's - selector. Before #687 the index always wrote lane 0, which is - why the first lane used to skip the strip. Once the index - began following focus, that lane was left without a one-click - selector of its own: changing it took two gestures, click into - the lane and then pick from the index, while every other lane - took one. + selector. + + History, because this rule has flipped twice. Before Grid + Dispatch, one sidebar index always wrote lane 0, so lane 0 + correctly had no strip. #687 (524671b2) made the index + per-row and follow focus, and gave every lane a strip for + exactly that reason. #691 (1eb9a472, a lane-wake fix) then + removed the first lane's strip as a side change, arguing that + the index "IS its selector". That was already false, because + the index followed focus, and it left the first lane as the + only one that took two gestures to change (click into it, then + pick from the index). Re-removing this strip is only right if + the index stops following focus; the renderer test "fills the + focused lane from a row's index…" fails first if it does. Single-lane rows get a strip too, although their index can only ever fill that one lane. A strip that appeared only when From 6ee33cc0cc415b80b195b9d378fd0ae4866e600f Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Thu, 10 Sep 2026 17:12:39 -0700 Subject: [PATCH 6/6] fix(dispatch): withhold the empty-lane hint when the row offers no agents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A focused empty lane advertised "pick from the strip, or press ⌥↓" even in a row bound to projects that currently have no agents. There the strip is empty and ⌥↓ returns without doing anything, so both halves of the hint promised a pick that could not happen. The hint is now gated on the row offering at least one agent. That is checked through rowScopedRows, the same filter the strip and ⌥↓ use, so the hint and the gestures it names cannot drift apart. The test was RED before the change, and it has an unbound contrast case so "never show a hint" cannot pass it. Pre-existing, raised in review of #859 because the PR rewrote this hint. Refs #850 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01J8PfoJxJ1cvJ9akXsWSVZB --- .../dispatch/TiledDispatchLayout.tsx | 20 +++++++++++- .../gridDispatchLayout.renderer.test.tsx | 32 +++++++++++++++++-- 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/src/renderer/src/workspace/dispatch/TiledDispatchLayout.tsx b/src/renderer/src/workspace/dispatch/TiledDispatchLayout.tsx index 610c36c30..e1b9670ea 100644 --- a/src/renderer/src/workspace/dispatch/TiledDispatchLayout.tsx +++ b/src/renderer/src/workspace/dispatch/TiledDispatchLayout.tsx @@ -25,6 +25,7 @@ import { DispatchEmpty, } from '@renderer/workspace/dispatch/DispatchAgentList' import { DispatchMiniList } from '@renderer/workspace/dispatch/DispatchMiniList' +import { rowScopedRows } from '@renderer/workspace/dispatch/rowScopedRows' import type { DispatchGridRow, SessionId, TabId } from '@renderer/workspace/types' type Props = { @@ -254,6 +255,16 @@ function GridRowView({ const indexFraction = clampIndexFraction(gridRow.indexFraction ?? DEFAULT_INDEX_FRACTION) const focusedLaneInRow = grid.focusedLane >= start && grid.focusedLane < end ? grid.focusedLane : null + // Whether this row's strips and ⌥↓ have anything to pick from. It asks the + // SAME filter both of them use (rowScopedRows: project binding + child cap; + // ⌥↓ reaches it through tiledRowScopedRows in useKeybinds), so the empty-lane + // hint below can never promise a pick the row does not actually offer. A + // hand-rolled "any row in a bound tab" check here would be a second + // definition of "what this row offers", and would drift from the first. + const rowOffersAgents = useMemo( + () => rowScopedRows(rows, gridRow).some(item => item.kind === 'agent'), + [rows, gridRow], + ) const rowRef = useRef(null) const laneRegionRef = useRef(null) @@ -443,8 +454,15 @@ function GridRowView({ // this copy anyway. Do not re-add a binding branch unless ⌥↓ // stops being row-scoped: "fixing" that read to // `projectTabIds` would withhold a promise that is true. + // + // And none at all when the row offers no agents (a row + // bound to projects that currently have none). Its strip is + // empty and ⌥↓ returns without doing anything, so either + // half of the hint would promise a pick that cannot happen. + // The bare "Empty lane" stays; the row's index is where the + // user sees why. hint={ - focused && !lane?.selectedSessionId + focused && !lane?.selectedSessionId && rowOffersAgents ? 'Pick an agent from the strip, or press ⌥↓ for the top of the index' : undefined } diff --git a/src/renderer/src/workspace/dispatch/gridDispatchLayout.renderer.test.tsx b/src/renderer/src/workspace/dispatch/gridDispatchLayout.renderer.test.tsx index 453d7d8ee..e93b482be 100644 --- a/src/renderer/src/workspace/dispatch/gridDispatchLayout.renderer.test.tsx +++ b/src/renderer/src/workspace/dispatch/gridDispatchLayout.renderer.test.tsx @@ -56,8 +56,11 @@ vi.mock('@renderer/workspace/dispatch/DispatchAgentList', () => ({ /> ) }, - DispatchEmpty: ({ message }: { message: string }) => ( -
{message}
+ // The hint is exposed as data, not asserted as copy: the layout decides + // WHETHER an empty lane may advertise a pick, and that decision is the + // behavior worth pinning. The wording is a design choice. + DispatchEmpty: ({ message, hint }: { message: string; hint?: string }) => ( +
{message}
), })) // The strip is mocked down to what the LAYOUT owns about it: @@ -375,6 +378,31 @@ describe('Grid Dispatch layout', () => { expect(setTiledFocusedLane.mock.calls).toEqual([[1]]) }) + it('withholds the empty-lane hint when the row offers no agents', () => { + // The hint advertises two gestures: pick from the strip, or press ⌥↓. Both + // act on the ROW's offered set (rowScopedRows; ⌥↓ via tiledRowScopedRows). + // In a row whose projects currently have no agents, the strip is empty and + // ⌥↓ returns without doing anything, so the hint would promise a pick that + // cannot happen. The contrasting unbound row pins the other half, so the fix + // cannot be "never show a hint". The tab id is deliberately one no session + // belongs to. + const cases = [ + { rows: [{ length: 2 }], hinted: true }, + { rows: [{ length: 2, projectTabIds: ['tab-with-no-agents'] }], hinted: false }, + ] + for (const { rows, hinted } of cases) { + const { getByTestId } = renderGrid({ + // Lane 1 empty AND focused: the only lane the hint may appear in. + lanes: [{ selectedSessionId: laneIds[0] }, {}], + rows, + focusedLane: 1, + }) + + expect(getByTestId('lane-empty').getAttribute('data-hint') !== '').toBe(hinted) + cleanup() + } + }) + it('gives a row bound to two projects both their index sections', () => { // The feature in one assertion, and the reason it is cheap: // buildDispatchGroups already groups by tab, so a two-project row renders