From b52aa846b8ff3e3f8835dcedba21e2dd85a2cb1c Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 12 Sep 2026 10:51:12 -0700 Subject: [PATCH 1/6] docs(workspace): plan project-tab reuse on New Tab and Merge Project Tabs Refs #913 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_016vtjk6o4nukQp7Vfbt5TDc --- .../plans/2026-09-12-merge-project-tabs.md | 127 ++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 docs/superpowers/plans/2026-09-12-merge-project-tabs.md diff --git a/docs/superpowers/plans/2026-09-12-merge-project-tabs.md b/docs/superpowers/plans/2026-09-12-merge-project-tabs.md new file mode 100644 index 000000000..a7786d061 --- /dev/null +++ b/docs/superpowers/plans/2026-09-12-merge-project-tabs.md @@ -0,0 +1,127 @@ +# Reuse an open project tab on New Tab, and Merge Project Tabs + +Status: planned; implementation follows on this branch. + +Feature Issue: [agent-code#913](https://github.com/Juliusolsson05/agent-code/issues/913). +Branch: `feat/merge-project-tabs`. Worktree: `.worktrees/merge-project-tabs`. +Base: `origin/main` at `c6adfe83` (2026-09-12). This plan is the first commit. + +## Outcome + +Two related changes that share one rule, "which tabs hold this directory": + +1. **Reuse on open.** When the folder typed into the New Tab path picker is + already held by an open tab, the picker says so and its primary action + becomes "go to tab"; "new tab anyway" stays available for the deliberate + second-tab case. A folder nobody holds behaves exactly as today. +2. **Merge Project Tabs.** A command that folds chosen tabs into a target + tab without touching any process: source grid panes and detached agents + become Dispatch agents of the target; buried records and Dispatch row + project filters are re-pointed; tiled-tab, Spotlight and Reader references + to removed tabs are cleared; the emptied tabs are removed. The modal lists + what moves before anything happens. + +## Why the duplicates exist + +`Tab` carries no directory. The tab title is the basename of the folder chosen +at creation, and the only durable link between a tab and a directory is the +`cwd` of the sessions it holds. The operator capability `projects.open` +already treats "an existing tab holds a session with exactly this cwd" as +"this project is open" and reuses it unless `createDuplicate` is explicit. +The path picker predates that decision and always creates. Window adoption +appends a closed window's tabs without root dedupe, so it can leave a +duplicate too. Three `agent-code` tabs on 2026-09-11 were created on Sept 3, +7 and 10 through the picker. + +## Design + +### The shared rule + +`findTabsHoldingDirectory(state, cwd)` in `workspace/queries.ts`: tabs (in +tab order) for which `resolveTabSessions` yields at least one session whose +`cwd` equals the directory exactly. `projects.open` switches to it; the path +picker surface uses it to decide the "already open" affordance. Exact match +only: a worktree is a different directory on purpose, and the merge command +is the tool for folding worktree tabs together. + +### Reuse on open + +`PathPickerSurface` passes `openTabsForPath(expandedPath)` into the modal. +The modal already tracks `resolvedPath`; when that path is held by a tab, it +shows "Already open as A · agent-code" and renders "go to tab" as the +primary action (activate, no spawn) with "new tab anyway" as the secondary +(the existing `onAccept`). Nothing changes for resume rows: resuming a past +conversation still lands in a new tab, which is a separate decision recorded +as out of scope below. + +### The merge planner + +`mergeProjectTabs(state, tileTabs, { targetTabId, sourceTabIds, now })` in +`workspace/mergeProjectTabs.ts`, a pure function returning either a refusal +(`target_is_source`, `unknown_tab`, `nothing_to_merge`) or the next state +plus a summary of what moved: + +- `tabs`: sources removed, order otherwise preserved. +- `activeTabId`: re-pointed to the target when it named a source. +- `detachedSessions`: every source grid leaf gains a `DetachedSessionRecord` + under the target (`surface: 'dispatch'`, `detachedAt: now`); records that + already pointed at a source are re-pointed. `projectTabTitle`/`projectTabIndex` + reflect the target in the new tab array. +- `buried`: `sourceTabId` re-pointed with the target's title and index. +- `dispatchMode.tiled.rows[].projectTabIds` (and the legacy `projectTabId`): + sources replaced by the target, deduplicated. Lanes are session-keyed and + untouched. +- `tileTabs`: sources removed, focus moved, then `sanitizeTileTabsState` + (which exits tiled tabs below two). +- Spotlight and Reader state that named a source tab are cleared by the hook + action, because the pane they zoomed is no longer in a grid. + +WHY grid panes go to Dispatch rather than into the target's grid: `Tab.root` +must stay a tile tree the user built; attaching every source pane would turn +one tab into a wall of panes, and `buildDispatchGroups` lists detached +sessions under their tab already, so nothing is hidden. The user attaches +what they want afterwards. + +WHY there is no undo: no process is killed and no session leaves the +workspace, so the reverse operation is "detach/attach as you like"; an undo +entry would have to snapshot layout state that the undo stack was not built +to hold. The confirmation lists what moves instead. + +The invariant the tests pin: `collectOwnedSessionIds` is identical before +and after a merge. A merge that lost a session would have it deleted by the +next autosave. + +### Command and modal + +- `merge-project-tabs` in `tabCommands.ts`, category `layout-dispatch`, + surface `app`, visible with two or more tabs, panel-state badge like + Reorder Tabs. Opens `mergeProjectTabsOpen` on the UI shell. +- `MergeProjectTabsModal`: target selector (default: the active tab), a + checkbox list of the other tabs labelled `A · title` with the directories + each holds and its session count, sources pre-ticked when they share a + directory with the target, a summary line ("2 tabs, 9 agents move to + Dispatch under E · agent-code"), Cancel / Merge. The surface calls the + hook action `mergeTabs(target, sources)` which applies the planner and + toasts the summary. + +## Tests + +- `mergeProjectTabs.test.ts` (unit): the three-tab fixture with detached, + buried, tiled Dispatch rows bound to a source, tiled tabs containing a + source, and a spotlight on a source; asserts every re-pointing above, the + ownership invariant, the target grid untouched, and the refusals. +- `queries.test.ts` or a new `findTabsHoldingDirectory` case: exact match, + worktree excluded, tab order. +- `PathPickerModal.renderer.test.tsx`: with a holder, the primary action + activates and does not accept; "new tab anyway" accepts; without a holder + the primary action accepts as before. +- Command catalog baseline: 121 commands, 24 approved additions. +- `tsc -b` on both projects, `npm run test:contract`, touched suites green. + +## Out of scope + +- Resume flows (path picker rows and the Conversations picker) still create + a new tab; folding a resumed conversation into an existing tab needs the + detached-agent resume path and is a separate change. +- Window adoption dedupe: the merge command is the cure there. +- Attaching merged panes into the target's grid. From 038df389e98ad73b5ecb7532280442e1b246cb4f Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 12 Sep 2026 10:57:30 -0700 Subject: [PATCH 2/6] feat(workspace): reuse an open project tab from the New Tab picker A Tab carries no directory; the only durable link to a folder is the cwd of the sessions it holds, and the operator's projects.open has treated "a tab holds a session with exactly this cwd" as "already open" since it shipped. The keyboard path predated that rule and minted a fresh tab every time, which is how one repository ended up as three tabs. The picker now shares the rule: Enter and the primary action go to the holding tab, "new tab anyway" keeps the deliberate second-tab case, and the decision is made from the path about to be opened rather than the debounced hint. Refs #913 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_016vtjk6o4nukQp7Vfbt5TDc --- .../surfaces/PathPickerSurface.tsx | 26 ++++++- .../ui/PathPickerModal.renderer.test.tsx | 54 +++++++++++++++ .../path-picker/ui/PathPickerModal.tsx | 69 ++++++++++++++++--- src/renderer/src/workspace/control/agents.ts | 6 +- .../src/workspace/queries.directory.test.ts | 38 ++++++++++ src/renderer/src/workspace/queries.ts | 22 ++++++ 6 files changed, 204 insertions(+), 11 deletions(-) create mode 100644 src/renderer/src/workspace/queries.directory.test.ts diff --git a/src/renderer/src/features/path-picker/surfaces/PathPickerSurface.tsx b/src/renderer/src/features/path-picker/surfaces/PathPickerSurface.tsx index 2ab09b888..93e626e80 100644 --- a/src/renderer/src/features/path-picker/surfaces/PathPickerSurface.tsx +++ b/src/renderer/src/features/path-picker/surfaces/PathPickerSurface.tsx @@ -3,7 +3,8 @@ import type { AgentProviderKind } from '@shared/types/providerKind' import { PathPickerModal } from '@renderer/features/path-picker/ui/PathPickerModal' import { useAppStore } from '@renderer/app-state/hooks' import { useWorkspaceContext } from '@renderer/workspace/WorkspaceContext' -import { resolveTabSessions } from '@renderer/workspace/queries' +import { findTabsHoldingDirectory, resolveTabSessions } from '@renderer/workspace/queries' +import { tabIndexLabel } from '@renderer/workspace/tile-tree/paneLabelFormat' // Registry wrapper (#494): owns the store + workspace wiring App.tsx // used to inline for the new-tab / resume path picker. Always mounted @@ -82,6 +83,27 @@ export function PathPickerSurface() { [closePathPicker, workspace], ) + // The same rule the operator's projects.open applies (#913): a tab holds a + // directory when one of its sessions runs there. Labelled the way Dispatch + // names tabs so "Already open as E · agent-code" points at something the + // user can find in the tab bar. + const openTabsForPath = useCallback( + (expandedPath: string) => + findTabsHoldingDirectory(workspace.state, expandedPath).map(tab => ({ + tabId: tab.id, + label: `${tabIndexLabel(workspace.state.tabs.indexOf(tab))} · ${tab.title}`, + })), + [workspace.state], + ) + + const onActivateTab = useCallback( + (tabId: string) => { + workspace.activateTab(tabId) + closePathPicker() + }, + [closePathPicker, workspace], + ) + return ( ) } diff --git a/src/renderer/src/features/path-picker/ui/PathPickerModal.renderer.test.tsx b/src/renderer/src/features/path-picker/ui/PathPickerModal.renderer.test.tsx index 59966613f..3bb0bb5c7 100644 --- a/src/renderer/src/features/path-picker/ui/PathPickerModal.renderer.test.tsx +++ b/src/renderer/src/features/path-picker/ui/PathPickerModal.renderer.test.tsx @@ -108,3 +108,57 @@ describe('PathPickerModal resume target coherence', () => { expect(screen.queryByRole('alert')).not.toBeInTheDocument() }) }) + +describe('PathPickerModal reuse of an open tab (#913)', () => { + it('goes to the tab that already holds the folder, and only creates on an explicit new-tab choice', async () => { + installApi(vi.fn(async () => response([]))) + const onAccept = vi.fn() + const onActivateTab = vi.fn() + render( + (path === '/repo' ? [{ tabId: 'tab-e', label: 'E · repo' }] : [])} + onActivateTab={onActivateTab} + />, + ) + + // The hint follows the debounced resolution of the typed path. + await screen.findByText('Already open as E · repo.') + expect(screen.queryByRole('button', { name: 'new session' })).not.toBeInTheDocument() + + // Enter (the input's submit) and the primary button both go to the tab: + // this is the default that stops ⌘T from minting duplicate tabs. + fireEvent.click(screen.getByRole('button', { name: 'go to tab' })) + await waitFor(() => expect(onActivateTab).toHaveBeenCalledWith('tab-e')) + expect(onAccept).not.toHaveBeenCalled() + + fireEvent.click(screen.getByRole('button', { name: 'new tab anyway' })) + await waitFor(() => expect(onAccept).toHaveBeenCalledWith('/repo', 'claude')) + }) + + it('creates as before when no tab holds the folder', async () => { + installApi(vi.fn(async () => response([]))) + const onAccept = vi.fn() + const onActivateTab = vi.fn() + render( + []} + onActivateTab={onActivateTab} + />, + ) + + fireEvent.click(await screen.findByRole('button', { name: 'new session' })) + await waitFor(() => expect(onAccept).toHaveBeenCalledWith('/repo', 'claude')) + expect(onActivateTab).not.toHaveBeenCalled() + expect(screen.queryByText(/Already open as/)).not.toBeInTheDocument() + }) +}) diff --git a/src/renderer/src/features/path-picker/ui/PathPickerModal.tsx b/src/renderer/src/features/path-picker/ui/PathPickerModal.tsx index 93379a741..1b951d4eb 100644 --- a/src/renderer/src/features/path-picker/ui/PathPickerModal.tsx +++ b/src/renderer/src/features/path-picker/ui/PathPickerModal.tsx @@ -48,6 +48,14 @@ type Props = { sessionId: string, provider: AgentProvider, ) => void | Promise + /** + * Tabs that already hold a session in `expandedPath` (#913). When any + * exist, Enter and the primary button go to the first one instead of + * creating a duplicate tab; "new tab anyway" keeps the deliberate case. + * Absent means the caller has no workspace to consult (tests, embedding). + */ + openTabsForPath?: (expandedPath: string) => Array<{ tabId: string; label: string }> + onActivateTab?: (tabId: string) => void } export function PathPickerModal({ @@ -56,6 +64,8 @@ export function PathPickerModal({ onCancel, onAccept, onResume, + openTabsForPath, + onActivateTab, }: Props) { const [value, setValue] = useState(defaultValue) const [error, setError] = useState(null) @@ -179,7 +189,14 @@ export function PathPickerModal({ return () => clearTimeout(t) }, [value, open, provider]) - const submit = async () => { + // Decided at submit time from the freshly expanded path, never from the + // debounced `resolvedPath` the hint below renders: the user can press Enter + // before the debounce settles, and the choice must follow the path that is + // actually about to be opened. + const holdersOf = (expandedPath: string) => + (onActivateTab && openTabsForPath ? openTabsForPath(expandedPath) : []) + + const submit = async (options: { forceNewTab?: boolean } = {}) => { if (busy) return setBusy(true) setError(null) @@ -191,6 +208,13 @@ export function PathPickerModal({ // user input. const result = await window.api.expandCwd(value) if (result.ok) { + const holders = holdersOf(result.path) + if (holders.length > 0 && !options.forceNewTab) { + // The folder is already on screen: go there instead of minting the + // duplicate tab that ⌘T used to create every time (#913). + onActivateTab!(holders[0]!.tabId) + return + } await onAccept(result.path, provider) return } @@ -241,6 +265,9 @@ export function PathPickerModal({ } } + // Display only; `submit` re-derives from the path it is about to open. + const alreadyOpenAs = resolvedPath && !pendingCreatePath ? holdersOf(resolvedPath) : [] + return ( )} + {alreadyOpenAs.length > 0 && ( +
+ Already open as {alreadyOpenAs.map(tab => tab.label).join(', ')}. +
+ )} +
- + {alreadyOpenAs.length > 0 ? ( + <> + + + + ) : ( + + )}
diff --git a/src/renderer/src/workspace/control/agents.ts b/src/renderer/src/workspace/control/agents.ts index 1bdf923d7..425496ecf 100644 --- a/src/renderer/src/workspace/control/agents.ts +++ b/src/renderer/src/workspace/control/agents.ts @@ -3,7 +3,7 @@ import { startControlTask } from './startTask' import { ControlError, defineCapability, pageInput, pageSchema, paginate } from '@control-sdk' import { useAppStore } from '@renderer/app-state/store' import { hasAppInteractionOwner } from '@renderer/lib/interaction-ownership' -import { resolveTabSessions } from '@renderer/workspace/queries' +import { findTabsHoldingDirectory, resolveTabSessions } from '@renderer/workspace/queries' import { observeWorkspace, workspaceObservationSchema } from '@renderer/workspace/control' import type { Workspace } from '@renderer/workspace/hook' import { AGENT_PROVIDER_RUNTIMES } from '@shared/types/providerKind' @@ -229,7 +229,9 @@ export function agentControlCapabilities(getWorkspace: () => Workspace) { handler: async ({ cwd, provider: kind, createDuplicate }) => { requireUi() const state = useAppStore.getState().workspaceState - const matches = state.tabs.filter(tab => resolveTabSessions(state, tab.id).some(id => state.sessions[id]?.cwd === cwd)) + // One rule with the path picker (#913): a tab holds a directory when one + // of its sessions runs there. + const matches = findTabsHoldingDirectory(state, cwd) if (!createDuplicate && matches.length > 1) throw new ControlError('ambiguous_owner', 'Several project tabs use this directory; select a tab ID') if (!createDuplicate && matches.length === 1) { const tab = matches[0] diff --git a/src/renderer/src/workspace/queries.directory.test.ts b/src/renderer/src/workspace/queries.directory.test.ts new file mode 100644 index 000000000..a77139440 --- /dev/null +++ b/src/renderer/src/workspace/queries.directory.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, it } from 'vitest' + +import { findTabsHoldingDirectory } from '@renderer/workspace/queries' +import type { WorkspaceState } from '@renderer/workspace/types' + +// The rule ⌘T and the operator's projects.open now share (#913). +const state: WorkspaceState = { + tabs: [ + { id: 'tab-a', title: 'agent-code', focusedSessionId: 'a', root: { type: 'leaf', sessionId: 'a' } }, + { id: 'tab-b', title: 'startup', focusedSessionId: 'b', root: { type: 'leaf', sessionId: 'b' } }, + { id: 'tab-c', title: 'agent-code', focusedSessionId: 'c', root: { type: 'leaf', sessionId: 'c' } }, + ], + activeTabId: 'tab-a', + dispatchMode: null, + sessions: { + a: { cwd: '/dev/agent-code', kind: 'claude' }, + b: { cwd: '/dev/startup', kind: 'codex' }, + c: { cwd: '/dev/agent-code/.worktrees/grok', kind: 'claude' }, + parked: { cwd: '/dev/agent-code', kind: 'codex' }, + }, + detachedSessions: { + parked: { sessionId: 'parked', surface: 'dispatch', projectTabId: 'tab-c', projectTabTitle: 'agent-code', projectTabIndex: 2, detachedAt: 1 }, + }, + buried: [], + pinnedSessionIds: [], +} + +describe('findTabsHoldingDirectory', () => { + it('matches the exact directory through grid and detached sessions, in tab order', () => { + expect(findTabsHoldingDirectory(state, '/dev/agent-code').map(tab => tab.id)).toEqual(['tab-a', 'tab-c']) + expect(findTabsHoldingDirectory(state, '/dev/startup').map(tab => tab.id)).toEqual(['tab-b']) + }) + + it('treats a worktree as a different directory', () => { + expect(findTabsHoldingDirectory(state, '/dev/agent-code/.worktrees/grok').map(tab => tab.id)).toEqual(['tab-c']) + expect(findTabsHoldingDirectory(state, '/dev/agent-code/.worktrees/other')).toEqual([]) + }) +}) diff --git a/src/renderer/src/workspace/queries.ts b/src/renderer/src/workspace/queries.ts index 38143da1a..aac3ce517 100644 --- a/src/renderer/src/workspace/queries.ts +++ b/src/renderer/src/workspace/queries.ts @@ -124,3 +124,25 @@ export function resolveAllSessions(state: WorkspaceState): SessionId[] { export function isDetached(state: WorkspaceState, sessionId: SessionId): boolean { return state.detachedSessions[sessionId] !== undefined } + +/** + * Tabs that currently hold a session whose working directory is exactly + * `cwd`, in tab order. + * + * WHY this is the definition of "this project is already open" (#913): a + * `Tab` carries no directory of its own; its title is the basename chosen at + * creation and the only durable link to a folder is the cwd of the sessions + * it holds. The operator capability `projects.open` has used this rule since + * it shipped; the path picker now shares it so ⌘T stops minting a fresh tab + * for a folder that is already on screen. Exact match on purpose: a worktree + * is a different directory, and Merge Project Tabs is the tool for folding + * worktree tabs together. + */ +export function findTabsHoldingDirectory( + state: WorkspaceState, + cwd: string, +): WorkspaceState['tabs'] { + return state.tabs.filter(tab => + resolveTabSessions(state, tab.id).some(id => state.sessions[id]?.cwd === cwd), + ) +} From 132cc0e1065f6711108b5d3e8b225c8477220154 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 12 Sep 2026 10:57:30 -0700 Subject: [PATCH 3/6] feat(workspace): add Merge Project Tabs Folds chosen tabs into a target without touching any process. closeTab kills what it owns and detach refuses a tab's last pane, so neither could express this; a pure planner re-points detached, buried and Dispatch row references, turns source grid panes into Dispatch agents of the target, removes the emptied tabs, and the hook applies it through functional updates with tiled-tab, Spotlight and Reader references cleared. The invariant the tests pin is that the owned-session set is identical before and after, because a session that lost its owner would be deleted by the next autosave. Refs #913 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_016vtjk6o4nukQp7Vfbt5TDc --- src/renderer/src/app-state/types.ts | 2 + src/renderer/src/app-state/uiShell/slice.ts | 5 + src/renderer/src/app-state/uiShell/types.ts | 4 + src/renderer/src/app/surfaces/registry.tsx | 4 + .../features/command-palette/catalog.test.ts | 33 ++-- .../src/features/command-palette/types.ts | 5 + .../command-palette/ui/CommandPalette.tsx | 9 + .../workspace/commands/tabCommands.ts | 17 ++ .../surfaces/MergeProjectTabsSurface.tsx | 43 ++++ .../workspace/ui/MergeProjectTabsModal.tsx | 157 +++++++++++++++ .../src/workspace/hook/actions/tab.ts | 48 +++++ src/renderer/src/workspace/hook/index.ts | 1 + .../src/workspace/mergeProjectTabs.test.ts | 119 +++++++++++ .../src/workspace/mergeProjectTabs.ts | 185 ++++++++++++++++++ 14 files changed, 617 insertions(+), 15 deletions(-) create mode 100644 src/renderer/src/features/workspace/surfaces/MergeProjectTabsSurface.tsx create mode 100644 src/renderer/src/features/workspace/ui/MergeProjectTabsModal.tsx create mode 100644 src/renderer/src/workspace/mergeProjectTabs.test.ts create mode 100644 src/renderer/src/workspace/mergeProjectTabs.ts diff --git a/src/renderer/src/app-state/types.ts b/src/renderer/src/app-state/types.ts index 90506c66f..69cef02e4 100644 --- a/src/renderer/src/app-state/types.ts +++ b/src/renderer/src/app-state/types.ts @@ -48,6 +48,8 @@ export type UiShellSlice = UiShellState & { closeTileTabsModal: () => void openReorderTabs: () => void closeReorderTabs: () => void + openMergeProjectTabs: () => void + closeMergeProjectTabs: () => void openPinAgents: () => void closePinAgents: () => void openSettingsPage: () => void diff --git a/src/renderer/src/app-state/uiShell/slice.ts b/src/renderer/src/app-state/uiShell/slice.ts index 9a6e642da..934ec1345 100644 --- a/src/renderer/src/app-state/uiShell/slice.ts +++ b/src/renderer/src/app-state/uiShell/slice.ts @@ -18,6 +18,7 @@ export const createUiShellSlice: StateCreator< tileTabsModalOpen: false, tileTabsInitialSelectedIds: [], reorderTabsOpen: false, + mergeProjectTabsOpen: false, pinAgentsOpen: false, settingsPageOpen: false, agentTitlePromptSessionId: null, @@ -120,6 +121,10 @@ export const createUiShellSlice: StateCreator< set({ reorderTabsOpen: true }, false, 'uiShell/openReorderTabs'), closeReorderTabs: () => set({ reorderTabsOpen: false }, false, 'uiShell/closeReorderTabs'), + openMergeProjectTabs: () => + set({ mergeProjectTabsOpen: true }, false, 'uiShell/openMergeProjectTabs'), + closeMergeProjectTabs: () => + set({ mergeProjectTabsOpen: false }, false, 'uiShell/closeMergeProjectTabs'), openPinAgents: () => set({ pinAgentsOpen: true }, false, 'uiShell/openPinAgents'), diff --git a/src/renderer/src/app-state/uiShell/types.ts b/src/renderer/src/app-state/uiShell/types.ts index a12d2a5f3..9e32ad2a9 100644 --- a/src/renderer/src/app-state/uiShell/types.ts +++ b/src/renderer/src/app-state/uiShell/types.ts @@ -72,6 +72,10 @@ export type UiShellState = { * autosave from recording half-finished reorder attempts that the * user later cancels with Escape. */ reorderTabsOpen: boolean + /** When true, the Merge Project Tabs modal is open (#913). Same rationale + * as reorderTabsOpen: the target/source draft is modal-local; the merge + * lands on WorkspaceState only when the user confirms. */ + mergeProjectTabsOpen: boolean /** When true, the Pin Agents multi-select modal is open. * * Same rationale for living on uiShell as reorderTabsOpen above: diff --git a/src/renderer/src/app/surfaces/registry.tsx b/src/renderer/src/app/surfaces/registry.tsx index b1172dcdb..0e8bd14a2 100644 --- a/src/renderer/src/app/surfaces/registry.tsx +++ b/src/renderer/src/app/surfaces/registry.tsx @@ -18,6 +18,7 @@ import { ReorderTabsSurface } from '@renderer/features/workspace/surfaces/Reorde import { PinAgentsSurface } from '@renderer/features/dispatch-pin/surfaces/PinAgentsSurface' import { BuryPanePromptSurface } from '@renderer/features/workspace/surfaces/BuryPanePromptSurface' import { RootManagementConfirmSurface } from '@renderer/features/workspace/surfaces/RootManagementConfirmSurface' +import { MergeProjectTabsSurface } from '@renderer/features/workspace/surfaces/MergeProjectTabsSurface' import { CloseConfirmationSurface } from '@renderer/features/workspace/surfaces/CloseConfirmationSurface' import { ViewPromptsSurface } from '@renderer/features/workspace/surfaces/ViewPromptsSurface' import { ConversationsSurface } from '@renderer/features/conversations/surfaces/ConversationsSurface' @@ -99,6 +100,9 @@ export const modalSurfaces: SurfaceEntry[] = [ // closes the palette first; it must paint over every established modal so // the warning is never hidden behind the surface it is warning about. { id: 'root-management-confirm', Component: RootManagementConfirmSurface }, + // Appended per the contract above; opened only from a command that closes + // the palette first (#913). + { id: 'merge-project-tabs', Component: MergeProjectTabsSurface }, ] /** diff --git a/src/renderer/src/features/command-palette/catalog.test.ts b/src/renderer/src/features/command-palette/catalog.test.ts index db05a9964..126d0c8ab 100644 --- a/src/renderer/src/features/command-palette/catalog.test.ts +++ b/src/renderer/src/features/command-palette/catalog.test.ts @@ -14,8 +14,8 @@ import type { CommandDef } from '@renderer/features/command-palette/types' // row commands (#681), 113 with New Window (#688), 114 with Clear // Agent Composer (#683), 115 with API Key Vault (#831), 116 with // Remove Cybersecurity Block (#848), 117 with New Agent In… (#852), -// 119 with TLDR preview and TLDR MCP (#888), and 120 with Root Agent Code -// Management (#906). +// 119 with TLDR preview and TLDR MCP (#888), 120 with Root Agent Code +// Management (#906), and 121 with Merge Project Tabs (#913). // 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 @@ -33,12 +33,13 @@ import type { CommandDef } from '@renderer/features/command-palette/types' /** The exact ordered catalog after governance (was 102 at main @ 670f4c2d). */ const BASELINE_COMMAND_IDS: readonly string[] = [ - // tabCommands (6) + // tabCommands (7) 'new-tab', 'close-tab', 'next-tab', 'prev-tab', 'reorder-tabs', + 'merge-project-tabs', 'resume-session', // windowCommands (1) 'new-window', @@ -200,12 +201,12 @@ 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 120 governed commands in registration order', () => { + it('contains exactly the 121 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 120 commands', () => { + it('has exactly 121 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. @@ -217,11 +218,12 @@ describe('built-in command catalog — baseline characterization', () => { // Window → 114 with Clear Agent Composer (#683) → 115 with API Key // Vault (#831) → 116 with Remove Cybersecurity Block (#848) → 117 with // New Agent In… (#852) → 119 with TLDR preview and TLDR MCP (#888) → 120 - // with Root Agent Code Management (#906). + // with Root Agent Code Management (#906) → 121 with Merge Project Tabs + // (#913). // Each step of that arithmetic was a deliberate edit to this line, which is the entire point of pinning it. (The two test // titles above had drifted to "115" while this line said 116; they now // track it again.) - expect(builtInCommandCatalog).toHaveLength(120) + expect(builtInCommandCatalog).toHaveLength(121) }) it('reports no structural defects', () => { @@ -255,12 +257,12 @@ describe('generated per-provider split commands', () => { }) it('accounts for the difference between literal and total command count', () => { - // 120 total - 4 generated = 116 literal `id:` fields across the command + // 121 total - 4 generated = 117 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 nine additions, Grid Dispatch's // six row commands, New Window, and the later single additions recorded in - // the count test above (through Root Agent Code Management, #906). - expect(builtInCommandCatalog.length - nonDefaultProviders.length * 2).toBe(116) + // the count test above (through Merge Project Tabs, #913). + expect(builtInCommandCatalog.length - nonDefaultProviders.length * 2).toBe(117) }) it('emits both directions for every non-default provider', () => { @@ -359,7 +361,7 @@ describe('governance targets', () => { }) it('lands on the arithmetic the plan predicted', () => { - // 102 baseline - 5 retirements + 23 additions = 120, checked against the + // 102 baseline - 5 retirements + 24 additions = 121, checked against the // real catalog rather than trusted as prose. // // The subtracted term is the count of APPROVED ADDITIONS and the expected @@ -379,10 +381,11 @@ describe('governance targets', () => { // `dispatch-focus-row-down`, `new-window` (#688), and // `clear-agent-composer` (#683), `api-key-vault` (#831), // `remove-cybersecurity-block` (#848), `new-agent-in` (#852), - // `tldr-preview` and `enable-tldr-mcp` (#888), and - // `enable-root-agent-code-management` (#906). - expect(builtInCommandCatalog.length + RETIRED_COMMAND_IDS.length - 23).toBe(102) - expect(builtInCommandCatalog).toHaveLength(120) + // `tldr-preview` and `enable-tldr-mcp` (#888), + // `enable-root-agent-code-management` (#906), and + // `merge-project-tabs` (#913). + expect(builtInCommandCatalog.length + RETIRED_COMMAND_IDS.length - 24).toBe(102) + expect(builtInCommandCatalog).toHaveLength(121) }) }) diff --git a/src/renderer/src/features/command-palette/types.ts b/src/renderer/src/features/command-palette/types.ts index 627109227..c04481c7b 100644 --- a/src/renderer/src/features/command-palette/types.ts +++ b/src/renderer/src/features/command-palette/types.ts @@ -189,6 +189,8 @@ export type CommandContext = { openNewTabPicker: () => void openTileTabs: () => void openReorderTabs: () => void + /** Open the Merge Project Tabs modal (#913); the modal performs the merge. */ + openMergeProjectTabs: () => void openSettings: () => void /** Open the command palette. Exists so ⌘⇧P has a command to name instead * of a hard-coded callback that nothing could rebind or collision-check. */ @@ -285,6 +287,7 @@ export type CommandContext = { closeBulkProviderSwitch: () => void closeConversations: () => void closeReorderTabs: () => void + closeMergeProjectTabs: () => void closePinAgents: () => void closePathPicker: () => void closePalette: () => void @@ -337,6 +340,8 @@ export type CommandContext = { remotePanelOpen: boolean /** The Reorder Tabs modal is on screen. */ reorderTabsOpen: boolean + /** The Merge Project Tabs modal is on screen. */ + mergeProjectTabsOpen: boolean /** The Pin Agents modal is on screen. */ pinAgentsOpen: boolean /** The path picker (New Tab / Resume) is on screen. */ diff --git a/src/renderer/src/features/command-palette/ui/CommandPalette.tsx b/src/renderer/src/features/command-palette/ui/CommandPalette.tsx index 54b96c354..740a49acd 100644 --- a/src/renderer/src/features/command-palette/ui/CommandPalette.tsx +++ b/src/renderer/src/features/command-palette/ui/CommandPalette.tsx @@ -266,6 +266,7 @@ function OpenCommandPalette({ ) }, [openTileTabsModal, workspace.activeTab, workspace.tileTabs]) const onReorderTabsRequest = useAppStore(state => state.openReorderTabs) + const openMergeProjectTabs = useAppStore(state => state.openMergeProjectTabs) const onSettingsRequest = useAppStore(state => state.openSettingsPage) const openPaletteAction = useAppStore(state => state.openCommandPalette) const openViewPrompts = useAppStore(state => state.openViewPrompts) @@ -287,6 +288,7 @@ function OpenCommandPalette({ const closeBulkProviderSwitch = useAppStore(state => state.closeBulkProviderSwitch) const closeConversations = useAppStore(state => state.closeConversations) const closeReorderTabs = useAppStore(state => state.closeReorderTabs) + const closeMergeProjectTabs = useAppStore(state => state.closeMergeProjectTabs) const closePinAgents = useAppStore(state => state.closePinAgents) const closePathPicker = useAppStore(state => state.closePathPicker) const openUsageModal = useAppStore(state => state.openUsageModal) @@ -356,6 +358,7 @@ function OpenCommandPalette({ const conversationsOpen = useAppStore(state => state.conversationsOpen) const remotePanelOpen = useAppStore(state => state.remotePanelOpen) const reorderTabsOpen = useAppStore(state => state.reorderTabsOpen) + const mergeProjectTabsOpen = useAppStore(state => state.mergeProjectTabsOpen) const pinAgentsOpen = useAppStore(state => state.pinAgentsOpen) const pathPickerOpen = useAppStore(state => state.pathPickerOpen) const gitBarOpen = useAppStore(state => state.gitBarOpen) @@ -568,6 +571,7 @@ function OpenCommandPalette({ openNewTabPicker: onNewTabRequest, openTileTabs: onTileTabsRequest, openReorderTabs: onReorderTabsRequest, + openMergeProjectTabs, openSettings: onSettingsRequest, // Reachable through the gateway (keybinding, native menu, programmatic) // but never rendered as a palette row — see @@ -593,6 +597,7 @@ function OpenCommandPalette({ closeBulkProviderSwitch, closeConversations, closeReorderTabs, + closeMergeProjectTabs, closePinAgents, closePathPicker, openUsageModal, @@ -650,6 +655,7 @@ function OpenCommandPalette({ conversationsOpen, remotePanelOpen, reorderTabsOpen, + mergeProjectTabsOpen, pinAgentsOpen, pathPickerOpen, gitBarOpen, @@ -685,6 +691,7 @@ function OpenCommandPalette({ onNewTabRequest, onTileTabsRequest, onReorderTabsRequest, + openMergeProjectTabs, onSettingsRequest, openViewPrompts, openConversations, @@ -703,6 +710,7 @@ function OpenCommandPalette({ closeBulkProviderSwitch, closeConversations, closeReorderTabs, + closeMergeProjectTabs, closePinAgents, closePathPicker, openUsageModal, @@ -758,6 +766,7 @@ function OpenCommandPalette({ conversationsOpen, remotePanelOpen, reorderTabsOpen, + mergeProjectTabsOpen, pinAgentsOpen, pathPickerOpen, gitBarOpen, diff --git a/src/renderer/src/features/workspace/commands/tabCommands.ts b/src/renderer/src/features/workspace/commands/tabCommands.ts index 6c39761b4..6a42b2753 100644 --- a/src/renderer/src/features/workspace/commands/tabCommands.ts +++ b/src/renderer/src/features/workspace/commands/tabCommands.ts @@ -57,6 +57,23 @@ export const tabCommands: CommandDef[] = [ ui.openReorderTabs() }, }, + { + id: 'merge-project-tabs', + category: 'layout-dispatch', + surface: 'app', + title: 'Merge Project Tabs', + description: '**What it does:** Folds other tabs into one tab. Their agents move to the target\'s Dispatch list; nothing restarts.\n\n**Use when:** The same folder ended up open in several tabs, or worktree tabs belong together.\n\n**Notes:** Buried panes and Dispatch row filters follow the target. No agent is closed, so anything can be re-arranged afterwards; the dialog lists what moves before you confirm.', + keywords: ['merge tabs', 'combine tabs', 'duplicate tab', 'same project', 'fold tabs', 'dispatch', 'worktree'], + when: ({ workspace }) => workspace.state.tabs.length > 1, + getState: ({ flags }) => panel(flags.mergeProjectTabsOpen), + run: ({ ui, flags }) => { + if (flags.mergeProjectTabsOpen) { + ui.closeMergeProjectTabs() + return + } + ui.openMergeProjectTabs() + }, + }, { id: 'resume-session', category: 'session', diff --git a/src/renderer/src/features/workspace/surfaces/MergeProjectTabsSurface.tsx b/src/renderer/src/features/workspace/surfaces/MergeProjectTabsSurface.tsx new file mode 100644 index 000000000..0ed69e592 --- /dev/null +++ b/src/renderer/src/features/workspace/surfaces/MergeProjectTabsSurface.tsx @@ -0,0 +1,43 @@ +import { useMemo } from 'react' + +import { useAppStore } from '@renderer/app-state/hooks' +import { MergeProjectTabsModal } from '@renderer/features/workspace/ui/MergeProjectTabsModal' +import type { MergeTabOption } from '@renderer/features/workspace/ui/MergeProjectTabsModal' +import { resolveTabSessions } from '@renderer/workspace/queries' +import { cwdBasename } from '@renderer/workspace/sessionDisplayTitle' +import { tabIndexLabel } from '@renderer/workspace/tile-tree/paneLabelFormat' +import { useWorkspaceContext } from '@renderer/workspace/WorkspaceContext' + +// The surface owns only the modal chrome; the merge itself is the workspace +// action, so autosave records the result exactly like any other layout change. +export function MergeProjectTabsSurface() { + const workspace = useWorkspaceContext() + const open = useAppStore(state => state.mergeProjectTabsOpen) + const close = useAppStore(state => state.closeMergeProjectTabs) + const { state } = workspace + + const tabs = useMemo(() => state.tabs.map((tab, index) => { + const sessionIds = resolveTabSessions(state, tab.id) + const cwds = [...new Set(sessionIds.flatMap(id => state.sessions[id]?.cwd ?? []))] + return { + id: tab.id, + label: `${tabIndexLabel(index)} · ${tab.title}`, + cwds, + directories: [...new Set(cwds.map(cwdBasename))], + sessionCount: sessionIds.length, + } + }), [state]) + + return ( + { + workspace.mergeTabs(targetId, sourceIds) + close() + }} + /> + ) +} diff --git a/src/renderer/src/features/workspace/ui/MergeProjectTabsModal.tsx b/src/renderer/src/features/workspace/ui/MergeProjectTabsModal.tsx new file mode 100644 index 000000000..0dea131f1 --- /dev/null +++ b/src/renderer/src/features/workspace/ui/MergeProjectTabsModal.tsx @@ -0,0 +1,157 @@ +import { useEffect, useMemo, useRef, useState } from 'react' + +import { Button } from '@renderer/components/ui/button' +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from '@renderer/components/ui/dialog' +import type { TabId } from '@renderer/workspace/types' + +export type MergeTabOption = { + id: TabId + /** `A · agent-code`, the Dispatch vocabulary. */ + label: string + /** Exact working directories the tab's sessions run in; drives the default selection. */ + cwds: string[] + /** Directory basenames for display. */ + directories: string[] + sessionCount: number +} + +type Props = { + open: boolean + tabs: MergeTabOption[] + initialTargetId: TabId + onCancel: () => void + onConfirm: (targetId: TabId, sourceIds: TabId[]) => void +} + +/** + * Pick the tab to keep, then the tabs to fold into it (#913). + * + * WHY sources default to "shares a working directory with the target": that + * is the duplicate-tab case the command exists for, and it is decided on the + * exact cwd rather than the title, so two unrelated projects that happen to + * share a folder name are not pre-ticked. Worktree tabs share no cwd with the + * main checkout and are therefore a deliberate tick, never a default. + */ +export function MergeProjectTabsModal({ open, tabs, initialTargetId, onCancel, onConfirm }: Props) { + const [targetId, setTargetId] = useState(initialTargetId) + const [sources, setSources] = useState([]) + const wasOpenRef = useRef(false) + + const target = useMemo(() => tabs.find(tab => tab.id === targetId) ?? null, [tabs, targetId]) + + const defaultSourcesFor = (id: TabId): TabId[] => { + const chosen = tabs.find(tab => tab.id === id) + if (!chosen) return [] + const cwds = new Set(chosen.cwds) + return tabs.filter(tab => tab.id !== id && tab.cwds.some(cwd => cwds.has(cwd))).map(tab => tab.id) + } + + useEffect(() => { + if (open && !wasOpenRef.current) { + setTargetId(initialTargetId) + setSources(defaultSourcesFor(initialTargetId)) + } + wasOpenRef.current = open + // The default computation reads `tabs`, which the surface rebuilds per + // render; re-running on every tab change would reset a half-made choice. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [open, initialTargetId]) + + // A tab that closed while the dialog is open silently leaves the draft. + useEffect(() => { + if (!open) return + const valid = new Set(tabs.map(tab => tab.id)) + setSources(prev => { + const next = prev.filter(id => valid.has(id)) + return next.length === prev.length ? prev : next + }) + if (!valid.has(targetId) && tabs[0]) setTargetId(tabs[0].id) + }, [open, tabs, targetId]) + + const sourceSet = useMemo(() => new Set(sources), [sources]) + const movedCount = tabs.filter(tab => sourceSet.has(tab.id)).reduce((sum, tab) => sum + tab.sessionCount, 0) + + return ( + { if (!nextOpen) onCancel() }}> + + + Merge Project Tabs + + Keep one tab; the agents of the merged tabs move to its Dispatch list. Nothing restarts and no agent is closed. + + + +
+ + +
+ +
Merge into it
+
+ {tabs.filter(tab => tab.id !== targetId).map(tab => { + const checked = sourceSet.has(tab.id) + return ( + + ) + })} +
+ +
+ {sources.length === 0 || !target + ? 'Tick at least one tab to merge.' + : `${sources.length} tab${sources.length === 1 ? '' : 's'}, ${movedCount} agent${movedCount === 1 ? '' : 's'} move to ${target.label}.`} +
+ + + + + +
+
+ ) +} diff --git a/src/renderer/src/workspace/hook/actions/tab.ts b/src/renderer/src/workspace/hook/actions/tab.ts index 5f9792527..cacaaccbf 100644 --- a/src/renderer/src/workspace/hook/actions/tab.ts +++ b/src/renderer/src/workspace/hook/actions/tab.ts @@ -10,6 +10,9 @@ import { requestCloseConfirmation } from '@renderer/workspace/closeConfirmationB import { clearLiveEntryWindowSession } from '@renderer/session-runtime/liveEntryWindow' import { clearTiledLaneSessions } from '@renderer/workspace/dispatch/tiledDispatchSelectors' import { sanitizeTileTabsState, titleFromCwd } from '@renderer/workspace/layout/helpers' +import { mergeProjectTabs, retargetTileTabsAfterMerge } from '@renderer/workspace/mergeProjectTabs' +import type { MergeProjectTabsResult } from '@renderer/workspace/mergeProjectTabs' +import { tabIndexLabel } from '@renderer/workspace/tile-tree/paneLabelFormat' import type { WorkspaceSetReaderMode, @@ -45,6 +48,8 @@ export function useTabActions( activateTab: (tabId: TabId) => void activateTabByIndex: (index: number) => void reorderTabs: (tabIds: TabId[]) => void + /** Fold `sourceTabIds` into `targetTabId` without touching any process (#913). */ + mergeTabs: (targetTabId: TabId, sourceTabIds: TabId[]) => MergeProjectTabsResult nextTab: () => void prevTab: () => void } { @@ -294,6 +299,48 @@ export function useTabActions( [refs.stateRef, setSpotlight, setState, setTileTabs], ) + const mergeTabs = useCallback( + (targetTabId: TabId, sourceTabIds: TabId[]): MergeProjectTabsResult => { + // Plan against the live snapshot for validation and the toast, then + // apply the SAME plan through functional updates so a workspace + // mutation racing the modal cannot be overwritten by a stale value. + // No kill, no spawn, no runtime change: every session keeps its + // process and its runtime; only tab affinity moves. + const now = Date.now() + const planned = mergeProjectTabs(refs.stateRef.current, { targetTabId, sourceTabIds, now }) + if (!planned.ok) { + showToast( + planned.reason === 'target_is_source' + ? 'Merge cancelled — the target tab cannot be one of the merged tabs.' + : planned.reason === 'unknown_tab' + ? 'Merge cancelled — a tab changed while the dialog was open. Try again.' + : 'Nothing to merge.', + ) + return planned + } + setState(prev => { + const applied = mergeProjectTabs(prev, { targetTabId, sourceTabIds, now }) + return applied.ok ? applied.state : prev + }) + setTileTabs(prev => retargetTileTabsAfterMerge(prev, sourceTabIds, targetTabId)) + // Spotlight and Reader zoom a GRID pane of a tab; the pane they named + // is now a Dispatch agent of another tab, so the takeover has nothing + // to frame. + const removed = new Set(sourceTabIds) + setSpotlight(prev => (prev && removed.has(prev.tabId) ? null : prev)) + setReaderMode(prev => (prev && removed.has(prev.tabId) ? null : prev)) + const { summary } = planned + const moved = summary.detachedFromGrid.length + summary.repointedDetached.length + showToast( + `Merged ${summary.removedTabIds.length} tab${summary.removedTabIds.length === 1 ? '' : 's'} into ` + + `${tabIndexLabel(summary.targetIndex)} · ${summary.targetTitle} — ` + + `${moved} agent${moved === 1 ? '' : 's'} now in its Dispatch list`, + ) + return planned + }, + [refs.stateRef, setReaderMode, setSpotlight, setState, setTileTabs, showToast], + ) + const reorderTabs = useCallback( (tabIds: TabId[]) => { setState(prev => { @@ -363,6 +410,7 @@ export function useTabActions( activateTab, activateTabByIndex, reorderTabs, + mergeTabs, nextTab, prevTab, } diff --git a/src/renderer/src/workspace/hook/index.ts b/src/renderer/src/workspace/hook/index.ts index e7b4447ff..c0d0e6644 100644 --- a/src/renderer/src/workspace/hook/index.ts +++ b/src/renderer/src/workspace/hook/index.ts @@ -969,6 +969,7 @@ export function useWorkspace( activateTab: tabActions.activateTab, activateTabByIndex: tabActions.activateTabByIndex, reorderTabs: tabActions.reorderTabs, + mergeTabs: tabActions.mergeTabs, nextTab: tabActions.nextTab, prevTab: tabActions.prevTab, resizeFocused, diff --git a/src/renderer/src/workspace/mergeProjectTabs.test.ts b/src/renderer/src/workspace/mergeProjectTabs.test.ts new file mode 100644 index 000000000..c519a1083 --- /dev/null +++ b/src/renderer/src/workspace/mergeProjectTabs.test.ts @@ -0,0 +1,119 @@ +import { describe, expect, it } from 'vitest' + +import { mergeProjectTabs, retargetTileTabsAfterMerge } from '@renderer/workspace/mergeProjectTabs' +import { collectOwnedSessionIds } from '@renderer/workspace/sessionOwnership' +import { resolveTabSessions } from '@renderer/workspace/queries' +import type { SessionMeta, TileTabsState, WorkspaceState } from '@renderer/workspace/types' + +// The workspace that motivated #913, reduced: three tabs for one repository +// (two of them holding worktree agents) plus an unrelated project, with every +// kind of tab-keyed reference a merge has to carry. +function meta(cwd: string): SessionMeta { + return { cwd, kind: 'claude' } +} + +function fixture(): WorkspaceState { + return { + tabs: [ + { id: 'tab-b', title: 'agent-code', focusedSessionId: 'b-audit', root: { type: 'leaf', sessionId: 'b-audit' } }, + { id: 'tab-startup', title: 'startup', focusedSessionId: 'pitch', root: { type: 'leaf', sessionId: 'pitch' } }, + { id: 'tab-e', title: 'agent-code', focusedSessionId: 'e-root', root: { + type: 'split', direction: 'vertical', ratio: 0.5, + a: { type: 'leaf', sessionId: 'e-root' }, + b: { type: 'leaf', sessionId: 'e-grok' }, + } }, + { id: 'tab-g', title: 'agent-code', focusedSessionId: 'g-review', root: { type: 'leaf', sessionId: 'g-review' } }, + ], + activeTabId: 'tab-g', + dispatchMode: { + tiled: { + lanes: [{ selectedSessionId: 'g-review' }, { selectedSessionId: 'e-tldr' }], + rows: [ + { length: 1, projectTabIds: ['tab-g', 'tab-e'] }, + { length: 1, projectTabId: 'tab-b' }, + ], + focusedLane: 0, + }, + } as unknown as WorkspaceState['dispatchMode'], + sessions: { + 'b-audit': meta('/dev/agent-code'), + 'b-verify': meta('/dev/agent-code/.worktrees/opencode-terminal-headless'), + pitch: meta('/dev/startup'), + 'e-root': meta('/dev/agent-code'), + 'e-grok': meta('/dev/agent-code/.worktrees/grok-package-wiring'), + 'e-tldr': meta('/dev/agent-code'), + 'g-review': meta('/dev/agent-code'), + 'g-buried': meta('/dev/agent-code'), + }, + detachedSessions: { + 'b-verify': { sessionId: 'b-verify', surface: 'dispatch', projectTabId: 'tab-b', projectTabTitle: 'agent-code', projectTabIndex: 0, detachedAt: 10 }, + 'e-tldr': { sessionId: 'e-tldr', surface: 'dispatch', projectTabId: 'tab-e', projectTabTitle: 'agent-code', projectTabIndex: 2, detachedAt: 20 }, + }, + buried: [{ + id: 'g-buried', sessionId: 'g-buried', sessionMeta: meta('/dev/agent-code'), buriedAt: 30, + sourceTabId: 'tab-g', sourceTabTitle: 'agent-code', sourceTabIndex: 3, + }], + pinnedSessionIds: ['e-grok'], + } +} + +describe('mergeProjectTabs', () => { + it('moves every source session under the target without losing ownership of any session', () => { + const before = fixture() + const result = mergeProjectTabs(before, { targetTabId: 'tab-e', sourceTabIds: ['tab-b', 'tab-g'], now: 1000 }) + if (!result.ok) throw new Error(result.reason) + const { state, summary } = result + + // The load-bearing invariant: a session that lost its owner would be + // deleted by the next autosave (see collectOwnedSessionIds). + expect(collectOwnedSessionIds(state)).toEqual(collectOwnedSessionIds(before)) + expect(state.tabs.map(tab => tab.id)).toEqual(['tab-startup', 'tab-e']) + expect(state.tabs[1]).toBe(before.tabs[2]) // the target's tree is untouched + expect(state.activeTabId).toBe('tab-e') + + // Grid panes of the removed tabs are now Dispatch agents of the target; + // detached records that pointed at a removed tab follow it with the + // target's title and NEW index. + expect(resolveTabSessions(state, 'tab-e')).toEqual(['e-root', 'e-grok', 'b-verify', 'e-tldr', 'b-audit', 'g-review']) + expect(state.detachedSessions['b-audit']).toEqual({ + sessionId: 'b-audit', surface: 'dispatch', projectTabId: 'tab-e', projectTabTitle: 'agent-code', projectTabIndex: 1, detachedAt: 1000, + }) + expect(state.detachedSessions['b-verify']).toMatchObject({ projectTabId: 'tab-e', projectTabIndex: 1, detachedAt: 10 }) + expect(state.buried[0]).toMatchObject({ sourceTabId: 'tab-e', sourceTabTitle: 'agent-code', sourceTabIndex: 1 }) + expect(state.pinnedSessionIds).toEqual(['e-grok']) + // Row filters that named a removed tab name the target once; the legacy + // single binding is folded into the array; lanes are untouched. + const rows = (state.dispatchMode as { tiled: { rows: unknown[]; lanes: unknown[] } }).tiled + expect(rows.rows).toEqual([{ length: 1, projectTabIds: ['tab-e'] }, { length: 1, projectTabIds: ['tab-e'] }]) + expect(rows.lanes).toEqual([{ selectedSessionId: 'g-review' }, { selectedSessionId: 'e-tldr' }]) + + expect(summary).toEqual({ + targetTabId: 'tab-e', targetTitle: 'agent-code', targetIndex: 1, removedTabIds: ['tab-b', 'tab-g'], + detachedFromGrid: ['b-audit', 'g-review'], repointedDetached: ['b-verify'], repointedBuried: ['g-buried'], + }) + expect(resolveTabSessions(state, 'tab-startup')).toEqual(['pitch']) + }) + + it('refuses a target among the sources, an unknown tab, and an empty selection', () => { + const state = fixture() + expect(mergeProjectTabs(state, { targetTabId: 'tab-e', sourceTabIds: ['tab-e', 'tab-b'], now: 1 })).toEqual({ ok: false, reason: 'target_is_source' }) + expect(mergeProjectTabs(state, { targetTabId: 'tab-e', sourceTabIds: ['tab-nope'], now: 1 })).toEqual({ ok: false, reason: 'unknown_tab' }) + expect(mergeProjectTabs(state, { targetTabId: 'tab-nope', sourceTabIds: ['tab-b'], now: 1 })).toEqual({ ok: false, reason: 'unknown_tab' }) + expect(mergeProjectTabs(state, { targetTabId: 'tab-e', sourceTabIds: [], now: 1 })).toEqual({ ok: false, reason: 'nothing_to_merge' }) + }) +}) + +describe('retargetTileTabsAfterMerge', () => { + const tiled: TileTabsState = { tabIds: ['tab-b', 'tab-e', 'tab-g'], focusedTabId: 'tab-g', direction: 'vertical', ratios: [0.2, 0.5, 0.3] } + + it('drops merged tabs, moves focus to the target and keeps the surviving ratios aligned', () => { + expect(retargetTileTabsAfterMerge(tiled, ['tab-g'], 'tab-e')).toMatchObject({ tabIds: ['tab-b', 'tab-e'], focusedTabId: 'tab-e' }) + const ratios = retargetTileTabsAfterMerge(tiled, ['tab-g'], 'tab-e')!.ratios + expect(ratios[0]! / ratios[1]!).toBeCloseTo(0.4) + }) + + it('exits tiled tabs when fewer than two remain, and leaves an absent layout absent', () => { + expect(retargetTileTabsAfterMerge(tiled, ['tab-b', 'tab-g'], 'tab-e')).toBeNull() + expect(retargetTileTabsAfterMerge(null, ['tab-b'], 'tab-e')).toBeNull() + }) +}) diff --git a/src/renderer/src/workspace/mergeProjectTabs.ts b/src/renderer/src/workspace/mergeProjectTabs.ts new file mode 100644 index 000000000..354b3e3d5 --- /dev/null +++ b/src/renderer/src/workspace/mergeProjectTabs.ts @@ -0,0 +1,185 @@ +import type { + DetachedSessionRecord, + SessionId, + TabId, + TileTabsState, + WorkspaceState, +} from '@renderer/workspace/types' +import { collectLeaves } from '@renderer/workspace/tile-tree/treeOps' +import { sanitizeTileTabsState } from '@renderer/workspace/layout/helpers' + +// Merge Project Tabs (#913): fold source tabs into a target tab WITHOUT +// touching any process. +// +// WHY a pure planner over WorkspaceState instead of a sequence of existing +// actions (detach, close tab): `closeTab` kills every session it owns, which +// is the one thing a merge must never do, and `detachSession` refuses the last +// grid pane of a tab because a tab cannot exist without a tile tree. A merge +// removes the tab itself, so that guard does not apply; the leaf simply +// becomes a detached record of the target. Doing the whole re-pointing in one +// pure function makes the invariant testable in isolation: the owned-session +// set (`collectOwnedSessionIds`) is identical before and after, so no session +// can be orphaned and then deleted by the next autosave. +// +// WHY source grid panes go to Dispatch rather than into the target's grid: +// `Tab.root` is a tile tree the user built. Attaching every source pane would +// turn one tab into a wall of splits, and `buildDispatchGroups` already lists +// detached sessions under their tab, so nothing is hidden. The user attaches +// what they want afterwards. + +export type MergeProjectTabsInput = { + targetTabId: TabId + sourceTabIds: readonly TabId[] + /** Stamp for the new detached records; injected so tests are deterministic. */ + now: number +} + +export type MergeProjectTabsSummary = { + targetTabId: TabId + targetTitle: string + /** Index of the target in the merged tab array (the letter Dispatch shows). */ + targetIndex: number + removedTabIds: TabId[] + /** Source grid panes that became detached records of the target. */ + detachedFromGrid: SessionId[] + /** Source detached records re-pointed at the target. */ + repointedDetached: SessionId[] + /** Buried records re-pointed at the target. */ + repointedBuried: SessionId[] +} + +export type MergeProjectTabsResult = + | { ok: true; state: WorkspaceState; summary: MergeProjectTabsSummary } + | { ok: false; reason: 'unknown_tab' | 'target_is_source' | 'nothing_to_merge' } + +export function mergeProjectTabs( + state: WorkspaceState, + input: MergeProjectTabsInput, +): MergeProjectTabsResult { + const sources = [...new Set(input.sourceTabIds)] + if (sources.length === 0) return { ok: false, reason: 'nothing_to_merge' } + if (sources.includes(input.targetTabId)) return { ok: false, reason: 'target_is_source' } + const known = new Set(state.tabs.map(tab => tab.id)) + if (!known.has(input.targetTabId) || sources.some(id => !known.has(id))) { + return { ok: false, reason: 'unknown_tab' } + } + const sourceSet = new Set(sources) + + const tabs = state.tabs.filter(tab => !sourceSet.has(tab.id)) + const targetIndex = tabs.findIndex(tab => tab.id === input.targetTabId) + const target = tabs[targetIndex]! + const affinity = { + projectTabId: target.id, + projectTabTitle: target.title, + projectTabIndex: targetIndex, + } + + const detachedSessions: Record = {} + const repointedDetached: SessionId[] = [] + for (const [sessionId, record] of Object.entries(state.detachedSessions)) { + if (sourceSet.has(record.projectTabId)) { + detachedSessions[sessionId] = { ...record, ...affinity } + repointedDetached.push(sessionId) + } else { + detachedSessions[sessionId] = record + } + } + const detachedFromGrid: SessionId[] = [] + for (const tab of state.tabs) { + if (!sourceSet.has(tab.id)) continue + for (const sessionId of collectLeaves(tab.root)) { + // A leaf with no SessionMeta is already an orphan the ownership rules + // would drop; carrying it as a detached record would only resurrect it. + if (!state.sessions[sessionId]) continue + detachedSessions[sessionId] = { + sessionId, + surface: 'dispatch', + ...affinity, + detachedAt: input.now, + } + detachedFromGrid.push(sessionId) + } + } + + const repointedBuried: SessionId[] = [] + const buried = state.buried.map(record => { + if (!sourceSet.has(record.sourceTabId)) return record + repointedBuried.push(record.sessionId) + return { + ...record, + sourceTabId: target.id, + sourceTabTitle: target.title, + sourceTabIndex: targetIndex, + } + }) + + // Row project filters name tabs; a filter that named a source now names the + // target once. Lanes are session-keyed and need nothing. + const tiledRows = state.dispatchMode?.tiled?.rows + const dispatchMode = state.dispatchMode?.tiled && tiledRows + ? { + ...state.dispatchMode, + tiled: { + ...state.dispatchMode.tiled, + rows: tiledRows.map(row => { + const bound = row.projectTabIds ?? (row.projectTabId ? [row.projectTabId] : undefined) + if (!bound || !bound.some(id => sourceSet.has(id))) return row + const { projectTabId: _legacy, ...rest } = row + const projectTabIds = [...new Set(bound.map(id => (sourceSet.has(id) ? target.id : id)))] + return { ...rest, projectTabIds } + }), + }, + } + : state.dispatchMode + + return { + ok: true, + state: { + ...state, + tabs, + activeTabId: sourceSet.has(state.activeTabId) ? target.id : state.activeTabId, + detachedSessions, + buried, + dispatchMode, + }, + summary: { + targetTabId: target.id, + targetTitle: target.title, + targetIndex, + removedTabIds: sources, + detachedFromGrid, + repointedDetached, + repointedBuried, + }, + } +} + +/** + * The tiled-tabs half of a merge, shaped as a functional update so the hook + * can apply it through `setTileTabs(prev => ...)` against the live value + * rather than a render-time snapshot. Sources leave the tiled set; focus moves + * to the target when it was on a source and the target is tiled, else to the + * first remaining tab; `sanitizeTileTabsState` exits tiled tabs below two. + */ +export function retargetTileTabsAfterMerge( + tileTabs: TileTabsState | null, + sourceTabIds: readonly TabId[], + targetTabId: TabId, +): TileTabsState | null { + if (!tileTabs) return null + const sourceSet = new Set(sourceTabIds) + const kept = tileTabs.tabIds + .map((id, index) => ({ id, ratio: tileTabs.ratios[index] })) + .filter(item => !sourceSet.has(item.id)) + const tabIds = kept.map(item => item.id) + const ratios = kept.map(item => item.ratio).filter((ratio): ratio is number => typeof ratio === 'number') + const focusedTabId = sourceSet.has(tileTabs.focusedTabId) + ? (tabIds.includes(targetTabId) ? targetTabId : (tabIds[0] ?? tileTabs.focusedTabId)) + : tileTabs.focusedTabId + return sanitizeTileTabsState({ + ...tileTabs, + tabIds, + focusedTabId, + ratios: ratios.length === tabIds.length ? ratios : [], + }) +} From 953ca2ae2710ec4bada51af3e60d75afd4a5d1fa Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 12 Sep 2026 10:57:41 -0700 Subject: [PATCH 4/6] docs(workspace): record the Merge Project Tabs pull request in the plan Refs #913 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_016vtjk6o4nukQp7Vfbt5TDc --- docs/superpowers/plans/2026-09-12-merge-project-tabs.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/superpowers/plans/2026-09-12-merge-project-tabs.md b/docs/superpowers/plans/2026-09-12-merge-project-tabs.md index a7786d061..961116130 100644 --- a/docs/superpowers/plans/2026-09-12-merge-project-tabs.md +++ b/docs/superpowers/plans/2026-09-12-merge-project-tabs.md @@ -1,6 +1,7 @@ # Reuse an open project tab on New Tab, and Merge Project Tabs -Status: planned; implementation follows on this branch. +Status: implemented, tests green, PR open for review. +PR: [agent-code#914](https://github.com/Juliusolsson05/agent-code/pull/914). Merge requires explicit approval. Feature Issue: [agent-code#913](https://github.com/Juliusolsson05/agent-code/issues/913). Branch: `feat/merge-project-tabs`. Worktree: `.worktrees/merge-project-tabs`. From e5f85358b6ae927b559df90a3eb81de48810ada2 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 12 Sep 2026 11:54:44 -0700 Subject: [PATCH 5/6] fix(tabs): keep the active tab on first open of Merge Project Tabs; prefer the current tab in the picker Addresses the first orchestrated review of #914 (REQUEST CHANGES). Blocking B1: the modal draft was seeded by an effect on open that raced the "tab closed while open" prune in the same commit; on the first open per app run (surfaces mount before the workspace loads, active id '') the prune won with the first tab, so the dialog kept tab A with A still ticked, or kept an unrelated first tab with the active tab's duplicates ticked. The draft now lives in a child mounted inside the dialog content, so it initialises from the live active tab on every open and needs no seeding effect; the prune is one functional update over the whole draft so a fallback target can never stay ticked. New renderer test mounts before the workspace loads. Non-blocking findings folded in: - picker prefers the current tab among holders (Enter no longer jumps from G to B), says "stay here" when the holder is the current tab, Shift+Enter is the keyboard path for "new tab anyway", buttons force the action their label names so a click before the debounce cannot switch tabs under a "new session" label; the picker test now presses Enter for real - mergeTabs plans inside the updater and gates every side effect on the applied result instead of toasting success for a refused merge - planner refreshes projectTabIndex/sourceTabIndex of surviving records (indices shift when tabs are removed), uses the ownership hasSessionMeta test, counts a pane that already had a detached record once - a tiled source hands its slot to an untiled target so the kept tab stays on screen - WHY comments: no undo, hibernation after restart, what is deliberately left alone; command description states the restart consequence Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_016vtjk6o4nukQp7Vfbt5TDc --- .../plans/2026-09-12-merge-project-tabs.md | 2 +- .../surfaces/PathPickerSurface.tsx | 4 +- .../src/features/path-picker/ui/PathInput.tsx | 9 +- .../ui/PathPickerModal.renderer.test.tsx | 70 +++++- .../path-picker/ui/PathPickerModal.tsx | 76 ++++-- .../workspace/commands/tabCommands.ts | 2 +- .../MergeProjectTabsModal.renderer.test.tsx | 82 +++++++ .../workspace/ui/MergeProjectTabsModal.tsx | 223 ++++++++++-------- .../src/workspace/hook/actions/tab.ts | 41 ++-- .../src/workspace/mergeProjectTabs.test.ts | 32 +++ .../src/workspace/mergeProjectTabs.ts | 97 ++++++-- src/renderer/src/workspace/queries.ts | 6 + .../src/workspace/sessionOwnership.ts | 2 +- 13 files changed, 482 insertions(+), 164 deletions(-) create mode 100644 src/renderer/src/features/workspace/ui/MergeProjectTabsModal.renderer.test.tsx diff --git a/docs/superpowers/plans/2026-09-12-merge-project-tabs.md b/docs/superpowers/plans/2026-09-12-merge-project-tabs.md index 961116130..6efe15e45 100644 --- a/docs/superpowers/plans/2026-09-12-merge-project-tabs.md +++ b/docs/superpowers/plans/2026-09-12-merge-project-tabs.md @@ -1,6 +1,6 @@ # Reuse an open project tab on New Tab, and Merge Project Tabs -Status: implemented, tests green, PR open for review. +Status: implemented, tests green, PR #914 open; first orchestrated review (REQUEST CHANGES, B1 modal seeding race) addressed, re-review pending. PR: [agent-code#914](https://github.com/Juliusolsson05/agent-code/pull/914). Merge requires explicit approval. Feature Issue: [agent-code#913](https://github.com/Juliusolsson05/agent-code/issues/913). diff --git a/src/renderer/src/features/path-picker/surfaces/PathPickerSurface.tsx b/src/renderer/src/features/path-picker/surfaces/PathPickerSurface.tsx index 93e626e80..a9383b269 100644 --- a/src/renderer/src/features/path-picker/surfaces/PathPickerSurface.tsx +++ b/src/renderer/src/features/path-picker/surfaces/PathPickerSurface.tsx @@ -86,12 +86,14 @@ export function PathPickerSurface() { // The same rule the operator's projects.open applies (#913): a tab holds a // directory when one of its sessions runs there. Labelled the way Dispatch // names tabs so "Already open as E · agent-code" points at something the - // user can find in the tab bar. + // user can find in the tab bar. `current` lets the picker prefer the tab + // the user is standing in, which ⌘T pre-fills for and Enter must not leave. const openTabsForPath = useCallback( (expandedPath: string) => findTabsHoldingDirectory(workspace.state, expandedPath).map(tab => ({ tabId: tab.id, label: `${tabIndexLabel(workspace.state.tabs.indexOf(tab))} · ${tab.title}`, + current: tab.id === workspace.state.activeTabId, })), [workspace.state], ) diff --git a/src/renderer/src/features/path-picker/ui/PathInput.tsx b/src/renderer/src/features/path-picker/ui/PathInput.tsx index 04d615333..515404fd2 100644 --- a/src/renderer/src/features/path-picker/ui/PathInput.tsx +++ b/src/renderer/src/features/path-picker/ui/PathInput.tsx @@ -18,7 +18,10 @@ import type { CSSProperties } from 'react' // - Render a dropdown of suggestions below the input with keyboard // (ArrowUp/Down) and mouse navigation. // - Tab = apply the highlighted suggestion (like bash tab-complete). -// - Enter = submit the current value (caller validates + acts). +// - Enter = submit the current value (caller validates + acts). The +// modifier state rides along so the caller can offer a second submit +// (the picker's Shift+Enter = "new tab anyway", #913) without this +// component knowing what either one means. // - Escape = cancel (if no suggestions open, delegates to onCancel). // // What this component deliberately DOESN'T do: @@ -37,7 +40,7 @@ export type DirEntry = { type Props = { value: string onChange: (next: string) => void - onSubmit: () => void + onSubmit: (modifiers: { shift: boolean }) => void onCancel: () => void placeholder?: string /** If true (default), the dropdown only lists directories. */ @@ -190,7 +193,7 @@ export function PathInput({ } if (e.key === 'Enter') { e.preventDefault() - onSubmit() + onSubmit({ shift: e.shiftKey }) return } if (e.key === 'Escape') { diff --git a/src/renderer/src/features/path-picker/ui/PathPickerModal.renderer.test.tsx b/src/renderer/src/features/path-picker/ui/PathPickerModal.renderer.test.tsx index 3bb0bb5c7..0aa56723f 100644 --- a/src/renderer/src/features/path-picker/ui/PathPickerModal.renderer.test.tsx +++ b/src/renderer/src/features/path-picker/ui/PathPickerModal.renderer.test.tsx @@ -110,6 +110,8 @@ describe('PathPickerModal resume target coherence', () => { }) describe('PathPickerModal reuse of an open tab (#913)', () => { + const pathInput = () => screen.getByPlaceholderText('/path/to/project or ~/…') + it('goes to the tab that already holds the folder, and only creates on an explicit new-tab choice', async () => { installApi(vi.fn(async () => response([]))) const onAccept = vi.fn() @@ -121,7 +123,7 @@ describe('PathPickerModal reuse of an open tab (#913)', () => { onCancel={vi.fn()} onAccept={onAccept} onResume={vi.fn()} - openTabsForPath={path => (path === '/repo' ? [{ tabId: 'tab-e', label: 'E · repo' }] : [])} + openTabsForPath={path => (path === '/repo' ? [{ tabId: 'tab-e', label: 'E · repo', current: false }] : [])} onActivateTab={onActivateTab} />, ) @@ -130,16 +132,51 @@ describe('PathPickerModal reuse of an open tab (#913)', () => { await screen.findByText('Already open as E · repo.') expect(screen.queryByRole('button', { name: 'new session' })).not.toBeInTheDocument() - // Enter (the input's submit) and the primary button both go to the tab: - // this is the default that stops ⌘T from minting duplicate tabs. - fireEvent.click(screen.getByRole('button', { name: 'go to tab' })) + // Enter on the input goes to the tab: this is the default that stops ⌘T + // from minting duplicate tabs. + fireEvent.keyDown(pathInput(), { key: 'Enter' }) await waitFor(() => expect(onActivateTab).toHaveBeenCalledWith('tab-e')) expect(onAccept).not.toHaveBeenCalled() + fireEvent.click(screen.getByRole('button', { name: 'go to tab' })) + expect(onActivateTab).toHaveBeenCalledTimes(2) + fireEvent.click(screen.getByRole('button', { name: 'new tab anyway' })) await waitFor(() => expect(onAccept).toHaveBeenCalledWith('/repo', 'claude')) }) + it('prefers the current tab over an earlier holder, and Shift+Enter still opens a new tab', async () => { + installApi(vi.fn(async () => response([]))) + const onAccept = vi.fn() + const onActivateTab = vi.fn() + render( + [ + { tabId: 'tab-b', label: 'B · repo', current: false }, + { tabId: 'tab-g', label: 'G · repo', current: true }, + ]} + onActivateTab={onActivateTab} + />, + ) + + // ⌘T pre-fills the active tab's folder, so Enter on the default must + // stay in G rather than jump to B just because B comes first. + await screen.findByText('Already open in this tab (G · repo), and as B · repo.') + expect(screen.getByRole('button', { name: 'stay here' })).toBeInTheDocument() + fireEvent.keyDown(pathInput(), { key: 'Enter' }) + await waitFor(() => expect(onActivateTab).toHaveBeenCalledWith('tab-g')) + expect(onActivateTab).not.toHaveBeenCalledWith('tab-b') + + // The keyboard path for a deliberate duplicate. + fireEvent.keyDown(pathInput(), { key: 'Enter', shiftKey: true }) + await waitFor(() => expect(onAccept).toHaveBeenCalledWith('/repo', 'claude')) + }) + it('creates as before when no tab holds the folder', async () => { installApi(vi.fn(async () => response([]))) const onAccept = vi.fn() @@ -159,6 +196,29 @@ describe('PathPickerModal reuse of an open tab (#913)', () => { fireEvent.click(await screen.findByRole('button', { name: 'new session' })) await waitFor(() => expect(onAccept).toHaveBeenCalledWith('/repo', 'claude')) expect(onActivateTab).not.toHaveBeenCalled() - expect(screen.queryByText(/Already open as/)).not.toBeInTheDocument() + expect(screen.queryByText(/Already open/)).not.toBeInTheDocument() + }) + + it('a button does what its label says even when the hint has not caught up yet', async () => { + installApi(vi.fn(async () => response([]))) + const onAccept = vi.fn() + const onActivateTab = vi.fn() + render( + [{ tabId: 'tab-e', label: 'E · repo', current: false }]} + onActivateTab={onActivateTab} + />, + ) + + // Clicked before the 150 ms debounce has resolved the path: the button + // still reads "new session", so it must create, not switch tabs. + fireEvent.click(screen.getByRole('button', { name: 'new session' })) + await waitFor(() => expect(onAccept).toHaveBeenCalledWith('/repo', 'claude')) + expect(onActivateTab).not.toHaveBeenCalled() }) }) diff --git a/src/renderer/src/features/path-picker/ui/PathPickerModal.tsx b/src/renderer/src/features/path-picker/ui/PathPickerModal.tsx index 1b951d4eb..430aaecfb 100644 --- a/src/renderer/src/features/path-picker/ui/PathPickerModal.tsx +++ b/src/renderer/src/features/path-picker/ui/PathPickerModal.tsx @@ -49,15 +49,34 @@ type Props = { provider: AgentProvider, ) => void | Promise /** - * Tabs that already hold a session in `expandedPath` (#913). When any - * exist, Enter and the primary button go to the first one instead of - * creating a duplicate tab; "new tab anyway" keeps the deliberate case. - * Absent means the caller has no workspace to consult (tests, embedding). + * Tabs that already hold a session in `expandedPath` (#913), in tab order, + * with `current` marking the active tab. When any exist, Enter and the + * primary button go to one of them instead of creating a duplicate tab; + * "new tab anyway" and Shift+Enter keep the deliberate case. Absent means + * the caller has no workspace to consult (tests, embedding). */ - openTabsForPath?: (expandedPath: string) => Array<{ tabId: string; label: string }> + openTabsForPath?: (expandedPath: string) => OpenTabHolder[] onActivateTab?: (tabId: string) => void } +export type OpenTabHolder = { tabId: string; label: string; current: boolean } + +/** + * Which holder Enter goes to when several tabs hold the folder. + * + * WHY the current tab wins: ⌘T pre-fills the active tab's folder, so with + * tabs B, E and G all on the same repository and G active, "first in tab + * order" would jump the user from G to B for pressing Enter on the default. + * Staying put is the only answer that never surprises. Otherwise the first + * holder in tab order is taken, which is a guess the operator capability + * `projects.open` deliberately refuses to make (`ambiguous_owner`): an + * operator has no "current tab" and no hint on screen, while the user here + * sees every holder named and can still pick "new tab anyway". + */ +function preferredHolder(holders: OpenTabHolder[]): OpenTabHolder | null { + return holders.find(holder => holder.current) ?? holders[0] ?? null +} + export function PathPickerModal({ open, defaultValue = '', @@ -192,8 +211,10 @@ export function PathPickerModal({ // Decided at submit time from the freshly expanded path, never from the // debounced `resolvedPath` the hint below renders: the user can press Enter // before the debounce settles, and the choice must follow the path that is - // actually about to be opened. - const holdersOf = (expandedPath: string) => + // actually about to be opened. The buttons are the one exception — see the + // footer: a button does what its label says, and the label is what the + // debounced hint knew. + const holdersOf = (expandedPath: string): OpenTabHolder[] => (onActivateTab && openTabsForPath ? openTabsForPath(expandedPath) : []) const submit = async (options: { forceNewTab?: boolean } = {}) => { @@ -208,11 +229,14 @@ export function PathPickerModal({ // user input. const result = await window.api.expandCwd(value) if (result.ok) { - const holders = holdersOf(result.path) - if (holders.length > 0 && !options.forceNewTab) { + const holder = options.forceNewTab ? null : preferredHolder(holdersOf(result.path)) + if (holder) { // The folder is already on screen: go there instead of minting the - // duplicate tab that ⌘T used to create every time (#913). - onActivateTab!(holders[0]!.tabId) + // duplicate tab that ⌘T used to create every time (#913). When the + // holder is the current tab this is a stay-put that only closes + // the picker; the primary button says so ("stay here"). The + // provider toggle is irrelevant on this path — nothing is spawned. + onActivateTab!(holder.tabId) return } await onAccept(result.path, provider) @@ -267,6 +291,8 @@ export function PathPickerModal({ // Display only; `submit` re-derives from the path it is about to open. const alreadyOpenAs = resolvedPath && !pendingCreatePath ? holdersOf(resolvedPath) : [] + const preferred = preferredHolder(alreadyOpenAs) + const otherHolders = alreadyOpenAs.filter(holder => holder !== preferred) return ( void submit()} + onSubmit={({ shift }) => void submit({ forceNewTab: shift })} onCancel={onCancel} placeholder="/path/to/project or ~/…" directoriesOnly @@ -354,7 +380,9 @@ export function PathPickerModal({ ) : ( - tab completes · ↑↓ to browse · enter to open · esc to cancel + {preferred + ? 'tab completes · ↑↓ to browse · enter to go there · ⇧enter for a new tab anyway · esc to cancel' + : 'tab completes · ↑↓ to browse · enter to open · esc to cancel'} )} @@ -375,9 +403,12 @@ export function PathPickerModal({ )} - {alreadyOpenAs.length > 0 && ( + {preferred && (
- Already open as {alreadyOpenAs.map(tab => tab.label).join(', ')}. + {preferred.current + ? `Already open in this tab (${preferred.label})` + : `Already open as ${preferred.label}`} + {otherHolders.length > 0 ? `, and as ${otherHolders.map(tab => tab.label).join(', ')}` : ''}.
)} @@ -390,7 +421,14 @@ export function PathPickerModal({ > cancel - {alreadyOpenAs.length > 0 ? ( + {/* WHY every button forces the action its label names: the labels + follow the debounced hint, and a click can land before the hint + has caught up with the typed path. Letting `submit` re-decide + would then switch tabs under a button that said "new session". + Enter is the only submit that decides at submit time, because + Enter carries no label to honour and reuse is the safer default + for a keypress that outran the hint. */} + {preferred ? ( <> ) : ( - - - -
+ + + + + ) } diff --git a/src/renderer/src/workspace/hook/actions/tab.ts b/src/renderer/src/workspace/hook/actions/tab.ts index cacaaccbf..4a57326dd 100644 --- a/src/renderer/src/workspace/hook/actions/tab.ts +++ b/src/renderer/src/workspace/hook/actions/tab.ts @@ -301,27 +301,38 @@ export function useTabActions( const mergeTabs = useCallback( (targetTabId: TabId, sourceTabIds: TabId[]): MergeProjectTabsResult => { - // Plan against the live snapshot for validation and the toast, then - // apply the SAME plan through functional updates so a workspace - // mutation racing the modal cannot be overwritten by a stale value. + // The plan is made INSIDE the updater against `prev`, never against + // `refs.stateRef`, which is refreshed on render and can lag a workspace + // mutation that landed between the modal's last paint and this click. + // Reading the result back out of the updater is only sound because + // `setState` is the zustand store setter, which applies the updater + // synchronously (pane.ts relies on the same property). Every side + // effect below is gated on that result: a refused merge must leave + // tiled tabs, Spotlight, Reader and the toast exactly as they were, + // otherwise the user is told "merged" while the tabs are still there. // No kill, no spawn, no runtime change: every session keeps its // process and its runtime; only tab affinity moves. const now = Date.now() - const planned = mergeProjectTabs(refs.stateRef.current, { targetTabId, sourceTabIds, now }) - if (!planned.ok) { + // A holder object rather than a `let`: TypeScript keeps a `let` + // narrowed to its initial value across the updater call (it cannot see + // the closure assign), which would type the merged branch as `never`. + const applied: { result: MergeProjectTabsResult | null } = { result: null } + setState(prev => { + const planned = mergeProjectTabs(prev, { targetTabId, sourceTabIds, now }) + applied.result = planned + return planned.ok ? planned.state : prev + }) + const result = applied.result ?? { ok: false as const, reason: 'nothing_to_merge' as const } + if (!result.ok) { showToast( - planned.reason === 'target_is_source' + result.reason === 'target_is_source' ? 'Merge cancelled — the target tab cannot be one of the merged tabs.' - : planned.reason === 'unknown_tab' + : result.reason === 'unknown_tab' ? 'Merge cancelled — a tab changed while the dialog was open. Try again.' : 'Nothing to merge.', ) - return planned + return result } - setState(prev => { - const applied = mergeProjectTabs(prev, { targetTabId, sourceTabIds, now }) - return applied.ok ? applied.state : prev - }) setTileTabs(prev => retargetTileTabsAfterMerge(prev, sourceTabIds, targetTabId)) // Spotlight and Reader zoom a GRID pane of a tab; the pane they named // is now a Dispatch agent of another tab, so the takeover has nothing @@ -329,16 +340,16 @@ export function useTabActions( const removed = new Set(sourceTabIds) setSpotlight(prev => (prev && removed.has(prev.tabId) ? null : prev)) setReaderMode(prev => (prev && removed.has(prev.tabId) ? null : prev)) - const { summary } = planned + const { summary } = result const moved = summary.detachedFromGrid.length + summary.repointedDetached.length showToast( `Merged ${summary.removedTabIds.length} tab${summary.removedTabIds.length === 1 ? '' : 's'} into ` + `${tabIndexLabel(summary.targetIndex)} · ${summary.targetTitle} — ` + `${moved} agent${moved === 1 ? '' : 's'} now in its Dispatch list`, ) - return planned + return result }, - [refs.stateRef, setReaderMode, setSpotlight, setState, setTileTabs, showToast], + [setReaderMode, setSpotlight, setState, setTileTabs, showToast], ) const reorderTabs = useCallback( diff --git a/src/renderer/src/workspace/mergeProjectTabs.test.ts b/src/renderer/src/workspace/mergeProjectTabs.test.ts index c519a1083..5b6bbb122 100644 --- a/src/renderer/src/workspace/mergeProjectTabs.test.ts +++ b/src/renderer/src/workspace/mergeProjectTabs.test.ts @@ -79,6 +79,9 @@ describe('mergeProjectTabs', () => { sessionId: 'b-audit', surface: 'dispatch', projectTabId: 'tab-e', projectTabTitle: 'agent-code', projectTabIndex: 1, detachedAt: 1000, }) expect(state.detachedSessions['b-verify']).toMatchObject({ projectTabId: 'tab-e', projectTabIndex: 1, detachedAt: 10 }) + // The target's OWN records moved from letter C to B when tab B left, and + // must not keep the old letter next to the ones they were just joined by. + expect(state.detachedSessions['e-tldr']).toMatchObject({ projectTabId: 'tab-e', projectTabIndex: 1, detachedAt: 20 }) expect(state.buried[0]).toMatchObject({ sourceTabId: 'tab-e', sourceTabTitle: 'agent-code', sourceTabIndex: 1 }) expect(state.pinnedSessionIds).toEqual(['e-grok']) // Row filters that named a removed tab name the target once; the legacy @@ -94,6 +97,28 @@ describe('mergeProjectTabs', () => { expect(resolveTabSessions(state, 'tab-startup')).toEqual(['pitch']) }) + it('skips a source pane with no metadata and counts a pane that already had a detached record once', () => { + const before = fixture() + // `phantom` is a leaf the ownership rules already treat as absent; + // `g-review` is both a grid pane of G and, by a broken earlier save, a + // detached record of G. + before.tabs[3]!.root = { + type: 'split', direction: 'horizontal', ratio: 0.5, + a: { type: 'leaf', sessionId: 'g-review' }, + b: { type: 'leaf', sessionId: 'phantom' }, + } + before.detachedSessions['g-review'] = { + sessionId: 'g-review', surface: 'dispatch', projectTabId: 'tab-g', projectTabTitle: 'agent-code', projectTabIndex: 3, detachedAt: 40, + } + const result = mergeProjectTabs(before, { targetTabId: 'tab-e', sourceTabIds: ['tab-g'], now: 1000 }) + if (!result.ok) throw new Error(result.reason) + expect(collectOwnedSessionIds(result.state)).toEqual(collectOwnedSessionIds(before)) + expect(result.state.detachedSessions['phantom']).toBeUndefined() + expect(result.state.detachedSessions['g-review']).toMatchObject({ projectTabId: 'tab-e', projectTabIndex: 2, detachedAt: 40 }) + expect(result.summary.detachedFromGrid).toEqual([]) + expect(result.summary.repointedDetached).toEqual(['g-review']) + }) + it('refuses a target among the sources, an unknown tab, and an empty selection', () => { const state = fixture() expect(mergeProjectTabs(state, { targetTabId: 'tab-e', sourceTabIds: ['tab-e', 'tab-b'], now: 1 })).toEqual({ ok: false, reason: 'target_is_source' }) @@ -112,6 +137,13 @@ describe('retargetTileTabsAfterMerge', () => { expect(ratios[0]! / ratios[1]!).toBeCloseTo(0.4) }) + it('gives a tiled source\'s slot to a target that was not tiled, so the kept tab stays on screen', () => { + const twoTiled: TileTabsState = { tabIds: ['tab-b', 'tab-g'], focusedTabId: 'tab-g', direction: 'vertical', ratios: [0.3, 0.7] } + expect(retargetTileTabsAfterMerge(twoTiled, ['tab-g'], 'tab-e')).toMatchObject({ tabIds: ['tab-b', 'tab-e'], focusedTabId: 'tab-e', ratios: [0.3, 0.7] }) + // Two tiled sources into an untiled target: one slot, the other leaves. + expect(retargetTileTabsAfterMerge(tiled, ['tab-b', 'tab-g'], 'tab-startup')).toMatchObject({ tabIds: ['tab-startup', 'tab-e'], focusedTabId: 'tab-startup' }) + }) + it('exits tiled tabs when fewer than two remain, and leaves an absent layout absent', () => { expect(retargetTileTabsAfterMerge(tiled, ['tab-b', 'tab-g'], 'tab-e')).toBeNull() expect(retargetTileTabsAfterMerge(null, ['tab-b'], 'tab-e')).toBeNull() diff --git a/src/renderer/src/workspace/mergeProjectTabs.ts b/src/renderer/src/workspace/mergeProjectTabs.ts index 354b3e3d5..210489890 100644 --- a/src/renderer/src/workspace/mergeProjectTabs.ts +++ b/src/renderer/src/workspace/mergeProjectTabs.ts @@ -7,6 +7,7 @@ import type { } from '@renderer/workspace/types' import { collectLeaves } from '@renderer/workspace/tile-tree/treeOps' import { sanitizeTileTabsState } from '@renderer/workspace/layout/helpers' +import { hasSessionMeta } from '@renderer/workspace/sessionOwnership' // Merge Project Tabs (#913): fold source tabs into a target tab WITHOUT // touching any process. @@ -26,6 +27,27 @@ import { sanitizeTileTabsState } from '@renderer/workspace/layout/helpers' // turn one tab into a wall of splits, and `buildDispatchGroups` already lists // detached sessions under their tab, so nothing is hidden. The user attaches // what they want afterwards. +// +// WHY there is no undo entry: undo-close exists to bring back sessions that +// were KILLED, by re-spawning them from a captured SessionMeta. A merge kills +// nothing, so there is nothing to re-spawn; reversing it would mean rebuilding +// the removed tabs' tile trees from a snapshot while the sessions inside them +// have kept running and may since have been attached, buried or closed. Every +// moved agent stays reachable in the target's Dispatch list, so the manual +// reverse (new tab, attach) is always available and never lossy. +// +// A consequence worth knowing before relying on "nothing restarts": it is true +// for the running app only. Grid panes are the only sessions rehydrate spawns +// at launch (`collectLiveProcessIds`); a merged pane is now a detached record, +// so after the next launch it is hibernated like every other Dispatch agent +// and wakes on its first use instead of being live from the start. +// +// What is deliberately left alone, because every session survives and these +// are keyed by session, not tab: Dispatch lanes, lane focus, pins, expanded +// parents, `gridRelatedSelections` (only read for grid panes, and +// `detachSessionToDispatch` leaves them too), and pane-close undo entries +// anchored on a source pane (they resolve as stale, exactly as after a +// detach). export type MergeProjectTabsInput = { targetTabId: TabId @@ -73,6 +95,14 @@ export function mergeProjectTabs( projectTabTitle: target.title, projectTabIndex: targetIndex, } + // Removing tabs shifts the index of every tab after them, and the records + // of SURVIVING tabs carry that index as a snapshot (`projectTabIndex`, + // `sourceTabIndex`). `buildDispatchGroups` recomputes it at render, but the + // agent-status model reads the raw value, and a merge that leaves the + // target's own records one letter behind the ones it just received would + // show two letters for one tab. `closeTab` leaves these stale and relies on + // the recompute; here the whole state is in hand, so refreshing is free. + const indexOf = new Map(tabs.map((tab, index) => [tab.id, index] as const)) const detachedSessions: Record = {} const repointedDetached: SessionId[] = [] @@ -80,9 +110,12 @@ export function mergeProjectTabs( if (sourceSet.has(record.projectTabId)) { detachedSessions[sessionId] = { ...record, ...affinity } repointedDetached.push(sessionId) - } else { - detachedSessions[sessionId] = record + continue } + const index = indexOf.get(record.projectTabId) + detachedSessions[sessionId] = index === undefined || index === record.projectTabIndex + ? record + : { ...record, projectTabIndex: index } } const detachedFromGrid: SessionId[] = [] for (const tab of state.tabs) { @@ -90,7 +123,20 @@ export function mergeProjectTabs( for (const sessionId of collectLeaves(tab.root)) { // A leaf with no SessionMeta is already an orphan the ownership rules // would drop; carrying it as a detached record would only resurrect it. - if (!state.sessions[sessionId]) continue + // Same own-property test as ownership, so the two never disagree about + // what counts as metadata. + if (!hasSessionMeta(state.sessions, sessionId)) continue + // A pane that already has a detached record is a pre-existing + // ownership violation (a session cannot be both a grid leaf and + // detached). The merge ends it: the pane is no longer a leaf, so the + // record becomes its only owner. Keep the record's own stamp and count + // the session once, under whichever list already claimed it. + const existing = detachedSessions[sessionId] + if (existing) { + detachedSessions[sessionId] = { ...existing, ...affinity } + if (!repointedDetached.includes(sessionId)) repointedDetached.push(sessionId) + continue + } detachedSessions[sessionId] = { sessionId, surface: 'dispatch', @@ -103,14 +149,19 @@ export function mergeProjectTabs( const repointedBuried: SessionId[] = [] const buried = state.buried.map(record => { - if (!sourceSet.has(record.sourceTabId)) return record - repointedBuried.push(record.sessionId) - return { - ...record, - sourceTabId: target.id, - sourceTabTitle: target.title, - sourceTabIndex: targetIndex, + if (sourceSet.has(record.sourceTabId)) { + repointedBuried.push(record.sessionId) + return { + ...record, + sourceTabId: target.id, + sourceTabTitle: target.title, + sourceTabIndex: targetIndex, + } } + const index = indexOf.get(record.sourceTabId) + return index === undefined || index === record.sourceTabIndex + ? record + : { ...record, sourceTabIndex: index } }) // Row project filters name tabs; a filter that named a source now names the @@ -157,9 +208,17 @@ export function mergeProjectTabs( /** * The tiled-tabs half of a merge, shaped as a functional update so the hook * can apply it through `setTileTabs(prev => ...)` against the live value - * rather than a render-time snapshot. Sources leave the tiled set; focus moves - * to the target when it was on a source and the target is tiled, else to the - * first remaining tab; `sanitizeTileTabsState` exits tiled tabs below two. + * rather than a render-time snapshot. + * + * WHY a tiled source is REPLACED by the target rather than dropped when the + * target is not itself tiled: the user was looking at that source's tile, and + * after the merge its agents belong to the target. Dropping the slot would + * leave `activeTabId` on the target while `MainSurface` keeps rendering the + * tiled set (it renders tiled tabs whenever the layout is set), so the tab the + * user just kept would be the one tab not on screen. Only the first tiled + * source takes the slot; the rest leave, and `sanitizeTileTabsState` exits + * tiled tabs below two. Focus follows the same rule, so a focus that sat on a + * source always lands on the target, which is tiled in either branch. */ export function retargetTileTabsAfterMerge( tileTabs: TileTabsState | null, @@ -168,14 +227,18 @@ export function retargetTileTabsAfterMerge( ): TileTabsState | null { if (!tileTabs) return null const sourceSet = new Set(sourceTabIds) + let slotTaken = tileTabs.tabIds.includes(targetTabId) const kept = tileTabs.tabIds .map((id, index) => ({ id, ratio: tileTabs.ratios[index] })) - .filter(item => !sourceSet.has(item.id)) + .flatMap(item => { + if (!sourceSet.has(item.id)) return [item] + if (slotTaken) return [] + slotTaken = true + return [{ id: targetTabId, ratio: item.ratio }] + }) const tabIds = kept.map(item => item.id) const ratios = kept.map(item => item.ratio).filter((ratio): ratio is number => typeof ratio === 'number') - const focusedTabId = sourceSet.has(tileTabs.focusedTabId) - ? (tabIds.includes(targetTabId) ? targetTabId : (tabIds[0] ?? tileTabs.focusedTabId)) - : tileTabs.focusedTabId + const focusedTabId = sourceSet.has(tileTabs.focusedTabId) ? targetTabId : tileTabs.focusedTabId return sanitizeTileTabsState({ ...tileTabs, tabIds, diff --git a/src/renderer/src/workspace/queries.ts b/src/renderer/src/workspace/queries.ts index aac3ce517..4d50b040e 100644 --- a/src/renderer/src/workspace/queries.ts +++ b/src/renderer/src/workspace/queries.ts @@ -137,6 +137,12 @@ export function isDetached(state: WorkspaceState, sessionId: SessionId): boolean * for a folder that is already on screen. Exact match on purpose: a worktree * is a different directory, and Merge Project Tabs is the tool for folding * worktree tabs together. + * + * The comparison is on the cwd string as stored. `expandCwd` resolves `~` + * and trailing slashes but not symlinks, so a session spawned through + * `projects.open` with a symlinked spelling of the same folder is a different + * directory here — consistent with how `projects.open` itself has always + * matched, and cheaper than a realpath round-trip per keystroke in the picker. */ export function findTabsHoldingDirectory( state: WorkspaceState, diff --git a/src/renderer/src/workspace/sessionOwnership.ts b/src/renderer/src/workspace/sessionOwnership.ts index 9ab9a0d82..8c3402703 100644 --- a/src/renderer/src/workspace/sessionOwnership.ts +++ b/src/renderer/src/workspace/sessionOwnership.ts @@ -96,7 +96,7 @@ export type PrunedSessionOwnership = { * key holding `undefined` has to read as "no metadata" on this side too or the * two halves disagree about what a pane is. */ -function hasSessionMeta( +export function hasSessionMeta( sessions: Record, id: SessionId, ): boolean { From f4f0dcc32efbd40cd1b7ab1f3f8d658cf6005c6e Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 12 Sep 2026 12:07:07 -0700 Subject: [PATCH 6/6] fix(tabs): make "stay here" a pure no-op and assert the buried index refresh Re-review of #914 (APPROVE WITH COMMENTS) follow-ups: the picker surface no longer activates the tab the user is already in, because activateTab clears Spotlight and "stay here" must not leave it; the planner test now carries a buried record of the surviving target and asserts its letter moves too; the plan doc's catalog count matches the merged baseline. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_016vtjk6o4nukQp7Vfbt5TDc --- docs/superpowers/plans/2026-09-12-merge-project-tabs.md | 2 +- .../src/features/path-picker/surfaces/PathPickerSurface.tsx | 5 ++++- src/renderer/src/workspace/mergeProjectTabs.test.ts | 6 ++++++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/docs/superpowers/plans/2026-09-12-merge-project-tabs.md b/docs/superpowers/plans/2026-09-12-merge-project-tabs.md index 6efe15e45..67a4de455 100644 --- a/docs/superpowers/plans/2026-09-12-merge-project-tabs.md +++ b/docs/superpowers/plans/2026-09-12-merge-project-tabs.md @@ -116,7 +116,7 @@ next autosave. - `PathPickerModal.renderer.test.tsx`: with a holder, the primary action activates and does not accept; "new tab anyway" accepts; without a holder the primary action accepts as before. -- Command catalog baseline: 121 commands, 24 approved additions. +- Command catalog baseline: 122 commands, 25 approved additions. - `tsc -b` on both projects, `npm run test:contract`, touched suites green. ## Out of scope diff --git a/src/renderer/src/features/path-picker/surfaces/PathPickerSurface.tsx b/src/renderer/src/features/path-picker/surfaces/PathPickerSurface.tsx index a9383b269..89efc99b1 100644 --- a/src/renderer/src/features/path-picker/surfaces/PathPickerSurface.tsx +++ b/src/renderer/src/features/path-picker/surfaces/PathPickerSurface.tsx @@ -98,9 +98,12 @@ export function PathPickerSurface() { [workspace.state], ) + // "stay here" must be exactly that: `activateTab` also clears Spotlight, + // so activating the tab the user is already in would drop them out of a + // Spotlight they opened ⌘T from. Only a different tab is activated. const onActivateTab = useCallback( (tabId: string) => { - workspace.activateTab(tabId) + if (tabId !== workspace.state.activeTabId) workspace.activateTab(tabId) closePathPicker() }, [closePathPicker, workspace], diff --git a/src/renderer/src/workspace/mergeProjectTabs.test.ts b/src/renderer/src/workspace/mergeProjectTabs.test.ts index 5b6bbb122..d37eb1782 100644 --- a/src/renderer/src/workspace/mergeProjectTabs.test.ts +++ b/src/renderer/src/workspace/mergeProjectTabs.test.ts @@ -44,6 +44,7 @@ function fixture(): WorkspaceState { 'e-tldr': meta('/dev/agent-code'), 'g-review': meta('/dev/agent-code'), 'g-buried': meta('/dev/agent-code'), + 'e-buried': meta('/dev/agent-code'), }, detachedSessions: { 'b-verify': { sessionId: 'b-verify', surface: 'dispatch', projectTabId: 'tab-b', projectTabTitle: 'agent-code', projectTabIndex: 0, detachedAt: 10 }, @@ -52,6 +53,9 @@ function fixture(): WorkspaceState { buried: [{ id: 'g-buried', sessionId: 'g-buried', sessionMeta: meta('/dev/agent-code'), buriedAt: 30, sourceTabId: 'tab-g', sourceTabTitle: 'agent-code', sourceTabIndex: 3, + }, { + id: 'e-buried', sessionId: 'e-buried', sessionMeta: meta('/dev/agent-code'), buriedAt: 35, + sourceTabId: 'tab-e', sourceTabTitle: 'agent-code', sourceTabIndex: 2, }], pinnedSessionIds: ['e-grok'], } @@ -83,6 +87,8 @@ describe('mergeProjectTabs', () => { // must not keep the old letter next to the ones they were just joined by. expect(state.detachedSessions['e-tldr']).toMatchObject({ projectTabId: 'tab-e', projectTabIndex: 1, detachedAt: 20 }) expect(state.buried[0]).toMatchObject({ sourceTabId: 'tab-e', sourceTabTitle: 'agent-code', sourceTabIndex: 1 }) + // A buried record of the surviving target moves letter with it too. + expect(state.buried[1]).toMatchObject({ id: 'e-buried', sourceTabId: 'tab-e', sourceTabIndex: 1, buriedAt: 35 }) expect(state.pinnedSessionIds).toEqual(['e-grok']) // Row filters that named a removed tab name the target once; the legacy // single binding is folded into the array; lanes are untouched.