mirror of
https://github.com/stablyai/orca.git
synced 2026-09-24 00:02:24 +00:00
refactor(floating-workspace): launch the default agent through the shared launcher
The floating workspace titlebar agent button drove tab startup itself: it built its own `buildAgentStartupPlan`, created the tab, queued the startup command and rebuilt the tab-bar order by hand. That is a second copy of what `launchAgentInNewTab` already does for every other "start an agent here" button, so a launch-point change had two places to land. The button now calls `launchAgentInNewTab` and keeps only its own placement: selecting the tab inside the floating panel's unified group and focusing it. `launchAgentInNewTab` gains an optional `activate` so a caller that places the tab itself can keep the new terminal out of the global selection. The floating panel needs this — activating would move the main window's active tab to a tab it does not show — and it matches the other floating tab creators, which already pass `activate: false` to `createTab` and select via `activateTab`. Two behaviours change, both fixes: - tab-bar order now goes through `persistAgentLaunchTabOrder`, which reconciles editor and browser tabs. The hand-rolled loop rebuilt order from terminal tabs only, dropping the floating workspace's markdown and browser tabs. - the startup plan now carries the resolved Windows shell, so argument quoting matches the shell the PTY actually gets.
This commit is contained in:
+58
-69
@@ -1,6 +1,8 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type * as ReactModule from 'react'
|
||||
import { toast } from 'sonner'
|
||||
import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants'
|
||||
import { resolveStructuredNativeChatSupport } from '../../../../shared/structured-native-chat-launch-route'
|
||||
import { FloatingTerminalWindowControls } from './FloatingTerminalWindowControls'
|
||||
|
||||
type ReactElementLike = {
|
||||
@@ -19,7 +21,7 @@ const mocks = vi.hoisted(() => ({
|
||||
setTabBarOrder: vi.fn(),
|
||||
queueTabStartupCommand: vi.fn(),
|
||||
focusTerminalTabSurface: vi.fn(),
|
||||
buildAgentStartupPlan: vi.fn()
|
||||
launchAgentInNewTab: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('react', async () => {
|
||||
@@ -41,8 +43,8 @@ vi.mock('@/lib/focus-terminal-tab-surface', () => ({
|
||||
focusTerminalTabSurface: mocks.focusTerminalTabSurface
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/tui-agent-startup', () => ({
|
||||
buildAgentStartupPlan: mocks.buildAgentStartupPlan
|
||||
vi.mock('@/lib/launch-agent-in-new-tab', () => ({
|
||||
launchAgentInNewTab: mocks.launchAgentInNewTab
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/agent-catalog', () => ({
|
||||
@@ -52,23 +54,10 @@ vi.mock('@/lib/agent-catalog', () => ({
|
||||
}
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/new-workspace', () => ({
|
||||
CLIENT_PLATFORM: 'darwin'
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/telemetry', () => ({
|
||||
tuiAgentToAgentKind: () => 'claude'
|
||||
}))
|
||||
|
||||
vi.mock('../../../../shared/tui-agent-selection', () => ({
|
||||
isTuiAgentEnabled: () => true
|
||||
}))
|
||||
|
||||
vi.mock('../../../../shared/tui-agent-launch-defaults', () => ({
|
||||
resolveTuiAgentLaunchArgs: () => [],
|
||||
resolveTuiAgentLaunchEnv: () => ({})
|
||||
}))
|
||||
|
||||
vi.mock('@/i18n/i18n', () => ({
|
||||
translate: (_key: string, fallback: string, vars?: Record<string, string>) =>
|
||||
vars ? fallback.replace(/\{\{(\w+)\}\}/g, (_match, name: string) => vars[name] ?? '') : fallback
|
||||
@@ -148,18 +137,10 @@ beforeEach(() => {
|
||||
for (const mock of Object.values(mocks)) {
|
||||
mock.mockReset()
|
||||
}
|
||||
mocks.createTab.mockImplementation(() => {
|
||||
const tab = { id: NEW_AGENT_TAB_ID }
|
||||
const state = storeBox.state as { tabsByWorktree: Record<string, { id: string }[]> }
|
||||
const existing = state.tabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? []
|
||||
state.tabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] = [...existing, tab]
|
||||
return tab
|
||||
})
|
||||
mocks.buildAgentStartupPlan.mockReturnValue({
|
||||
launchCommand: 'claude',
|
||||
launchConfig: {},
|
||||
env: undefined,
|
||||
startupCommandDelivery: undefined
|
||||
mocks.launchAgentInNewTab.mockReturnValue({
|
||||
surface: { kind: 'local-terminal', tabId: NEW_AGENT_TAB_ID },
|
||||
startupPlan: { launchCommand: 'claude', launchConfig: {} },
|
||||
pasteDraftAfterLaunch: false
|
||||
})
|
||||
storeBox.state = {
|
||||
settings: {
|
||||
@@ -183,45 +164,35 @@ afterEach(() => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
function clickLaunch(): void {
|
||||
const element = FloatingTerminalWindowControls({
|
||||
maximized: false,
|
||||
onToggleMaximized: vi.fn(),
|
||||
onMinimize: vi.fn()
|
||||
})
|
||||
findOnClickByAriaLabel(element, 'Open Claude in floating workspace')()
|
||||
}
|
||||
|
||||
describe('FloatingTerminalWindowControls default-agent launch', () => {
|
||||
it('activates the new agent tab so the floating panel selects and focuses it', () => {
|
||||
;(
|
||||
storeBox.state as {
|
||||
settings: Record<string, unknown>
|
||||
}
|
||||
).settings.nativeChatSessionOptions = {
|
||||
claude: { model: 'opus', valuesByModel: { opus: { effort: 'high' } } }
|
||||
}
|
||||
const element = FloatingTerminalWindowControls({
|
||||
maximized: false,
|
||||
onToggleMaximized: vi.fn(),
|
||||
onMinimize: vi.fn()
|
||||
it('launches through the shared agent launcher instead of driving tab startup itself', () => {
|
||||
clickLaunch()
|
||||
|
||||
expect(mocks.launchAgentInNewTab).toHaveBeenCalledExactlyOnceWith({
|
||||
agent: 'claude',
|
||||
worktreeId: FLOATING_TERMINAL_WORKTREE_ID,
|
||||
launchSource: 'shortcut',
|
||||
activate: false
|
||||
})
|
||||
// Why: the whole point of the migration. The shared launcher owns the startup plan and the
|
||||
// tab it lands in, so this button must not reach past it into the tab store.
|
||||
expect(mocks.createTab).not.toHaveBeenCalled()
|
||||
expect(mocks.queueTabStartupCommand).not.toHaveBeenCalled()
|
||||
expect(mocks.setTabBarOrder).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
const launch = findOnClickByAriaLabel(element, 'Open Claude in floating workspace')
|
||||
launch()
|
||||
it('activates the launched terminal tab so the floating panel selects and focuses it', () => {
|
||||
clickLaunch()
|
||||
|
||||
expect(mocks.buildAgentStartupPlan.mock.calls[0]?.[0]).not.toHaveProperty('sessionOptions')
|
||||
|
||||
expect(mocks.createTab).toHaveBeenCalledWith(
|
||||
FLOATING_TERMINAL_WORKTREE_ID,
|
||||
undefined,
|
||||
undefined,
|
||||
{ activate: false }
|
||||
)
|
||||
// Why: TerminalPane consumes any pending startup command on first render, so
|
||||
// the launch command must be queued before activation can mount the surface -
|
||||
// otherwise the new tab can come up as a bare shell.
|
||||
expect(mocks.queueTabStartupCommand).toHaveBeenCalledWith(
|
||||
NEW_AGENT_TAB_ID,
|
||||
expect.objectContaining({
|
||||
command: 'claude',
|
||||
launchAgent: 'claude'
|
||||
})
|
||||
)
|
||||
expect(mocks.queueTabStartupCommand.mock.invocationCallOrder[0]).toBeLessThan(
|
||||
mocks.activateTab.mock.invocationCallOrder[0]
|
||||
)
|
||||
// Why: the floating panel renders its visible tab from the unified group's
|
||||
// activeTabId, which only activateTab writes. setActiveTabForWorktree updates
|
||||
// the complementary legacy per-worktree map. Without activateTab the new agent
|
||||
@@ -232,11 +203,29 @@ describe('FloatingTerminalWindowControls default-agent launch', () => {
|
||||
)
|
||||
expect(mocks.activateTab).toHaveBeenCalledWith(NEW_AGENT_TAB_ID)
|
||||
expect(mocks.focusTerminalTabSurface).toHaveBeenCalledWith(NEW_AGENT_TAB_ID)
|
||||
// Why: createTab appends the new tab to the worktree; the order reconciliation
|
||||
// must keep the pre-existing tab and place the new agent tab last.
|
||||
expect(mocks.setTabBarOrder).toHaveBeenCalledWith(FLOATING_TERMINAL_WORKTREE_ID, [
|
||||
EXISTING_TAB_ID,
|
||||
NEW_AGENT_TAB_ID
|
||||
])
|
||||
})
|
||||
|
||||
it('reports a launch the shared launcher could not plan', () => {
|
||||
mocks.launchAgentInNewTab.mockReturnValue(null)
|
||||
|
||||
clickLaunch()
|
||||
|
||||
expect(toast.error).toHaveBeenCalledWith('Could not build launch command for Claude.')
|
||||
expect(mocks.activateTab).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
// Why: a floating window has nowhere to keep a structured session, so the launch must resolve a
|
||||
// terminal. Pinned against the shared resolver the launcher routes on, not a restatement here.
|
||||
it('keeps the floating workspace off the structured route', () => {
|
||||
// `claude` is a structured-session provider on a local host, so `floating-workspace` is the
|
||||
// only blocker that can produce this result — any other answer means the kind stopped deciding.
|
||||
expect(
|
||||
resolveStructuredNativeChatSupport({
|
||||
agent: 'claude',
|
||||
executionHostId: 'local',
|
||||
hostCapabilities: null,
|
||||
workspaceKind: 'floating'
|
||||
})
|
||||
).toEqual({ supported: false, blocker: 'floating-workspace' })
|
||||
})
|
||||
})
|
||||
|
||||
@@ -5,19 +5,13 @@ import { Button } from '@/components/ui/button'
|
||||
import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
|
||||
import { getAgentCatalog, AgentIcon } from '@/lib/agent-catalog'
|
||||
import { focusTerminalTabSurface } from '@/lib/focus-terminal-tab-surface'
|
||||
import { CLIENT_PLATFORM } from '@/lib/new-workspace'
|
||||
import { buildAgentStartupPlan } from '@/lib/tui-agent-startup'
|
||||
import { tuiAgentToAgentKind } from '@/lib/telemetry'
|
||||
import { launchAgentInNewTab } from '@/lib/launch-agent-in-new-tab'
|
||||
import { useAppStore } from '@/store'
|
||||
import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants'
|
||||
import {
|
||||
DEFAULT_DISABLED_TUI_AGENTS,
|
||||
isTuiAgentEnabled
|
||||
} from '../../../../shared/tui-agent-selection'
|
||||
import {
|
||||
resolveTuiAgentLaunchArgs,
|
||||
resolveTuiAgentLaunchEnv
|
||||
} from '../../../../shared/tui-agent-launch-defaults'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
import { useOptionalShortcutLabel } from '@/hooks/useShortcutLabel'
|
||||
|
||||
@@ -44,7 +38,6 @@ export function FloatingTerminalWindowControls({
|
||||
onMinimize
|
||||
}: FloatingTerminalWindowControlsProps): React.JSX.Element {
|
||||
const defaultTuiAgent = useAppStore((s) => s.settings?.defaultTuiAgent ?? null)
|
||||
const createTab = useAppStore((s) => s.createTab)
|
||||
const setActiveTabForWorktree = useAppStore((s) => s.setActiveTabForWorktree)
|
||||
const activateTab = useAppStore((s) => s.activateTab)
|
||||
const maximizeShortcutLabel = useOptionalShortcutLabel('floatingWorkspace.maximize')
|
||||
@@ -71,17 +64,18 @@ export function FloatingTerminalWindowControls({
|
||||
if (!defaultAgent) {
|
||||
return
|
||||
}
|
||||
const state = useAppStore.getState()
|
||||
const startupPlan = buildAgentStartupPlan({
|
||||
// Why: the shared launcher owns the startup plan, the route (floating always resolves a
|
||||
// terminal) and the tab-bar order, so this button stays one more caller of it rather than a
|
||||
// second copy of new-agent-tab startup.
|
||||
const result = launchAgentInNewTab({
|
||||
agent: defaultAgent,
|
||||
prompt: '',
|
||||
cmdOverrides: state.settings?.agentCmdOverrides ?? {},
|
||||
agentArgs: resolveTuiAgentLaunchArgs(defaultAgent, state.settings?.agentDefaultArgs),
|
||||
agentEnv: resolveTuiAgentLaunchEnv(defaultAgent, state.settings?.agentDefaultEnv),
|
||||
platform: CLIENT_PLATFORM,
|
||||
allowEmptyPromptLaunch: true
|
||||
worktreeId: FLOATING_TERMINAL_WORKTREE_ID,
|
||||
launchSource: 'shortcut',
|
||||
// Why: the floating panel must not move the main window's selection; it selects in its own
|
||||
// group below, matching the other floating tab creators.
|
||||
activate: false
|
||||
})
|
||||
if (!startupPlan) {
|
||||
if (!result) {
|
||||
toast.error(
|
||||
translate(
|
||||
'auto.components.floating.terminal.FloatingTerminalWindowControls.82da3701e7',
|
||||
@@ -91,41 +85,17 @@ export function FloatingTerminalWindowControls({
|
||||
)
|
||||
return
|
||||
}
|
||||
const tab = createTab(FLOATING_TERMINAL_WORKTREE_ID, undefined, undefined, { activate: false })
|
||||
state.queueTabStartupCommand(tab.id, {
|
||||
command: startupPlan.launchCommand,
|
||||
...(startupPlan.env ? { env: startupPlan.env } : {}),
|
||||
launchConfig: startupPlan.launchConfig,
|
||||
launchAgent: defaultAgent,
|
||||
...(startupPlan.startupCommandDelivery
|
||||
? { startupCommandDelivery: startupPlan.startupCommandDelivery }
|
||||
: {}),
|
||||
telemetry: {
|
||||
agent_kind: tuiAgentToAgentKind(defaultAgent),
|
||||
launch_source: 'shortcut',
|
||||
request_kind: 'new'
|
||||
}
|
||||
})
|
||||
if (result.surface.kind !== 'local-terminal') {
|
||||
return
|
||||
}
|
||||
// Why: the floating panel renders its visible tab from the unified group's
|
||||
// activeTabId. setActiveTabForWorktree only writes activeTabIdByWorktree, so
|
||||
// the new agent tab would be appended but never selected/focused. activateTab
|
||||
// selects it within the group, matching the empty-state tab creators.
|
||||
setActiveTabForWorktree(FLOATING_TERMINAL_WORKTREE_ID, tab.id)
|
||||
activateTab(tab.id)
|
||||
const fresh = useAppStore.getState()
|
||||
const currentTabs = fresh.tabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? []
|
||||
const stored = fresh.tabBarOrderByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? []
|
||||
const validIds = new Set(currentTabs.map((entry) => entry.id))
|
||||
const order = stored.filter((id) => validIds.has(id) && id !== tab.id)
|
||||
for (const entry of currentTabs) {
|
||||
if (entry.id !== tab.id && !order.includes(entry.id)) {
|
||||
order.push(entry.id)
|
||||
}
|
||||
}
|
||||
order.push(tab.id)
|
||||
fresh.setTabBarOrder(FLOATING_TERMINAL_WORKTREE_ID, order)
|
||||
focusTerminalTabSurface(tab.id)
|
||||
}, [activateTab, createTab, defaultAgent, defaultAgentLabel, setActiveTabForWorktree])
|
||||
setActiveTabForWorktree(FLOATING_TERMINAL_WORKTREE_ID, result.surface.tabId)
|
||||
activateTab(result.surface.tabId)
|
||||
focusTerminalTabSurface(result.surface.tabId)
|
||||
}, [activateTab, defaultAgent, defaultAgentLabel, setActiveTabForWorktree])
|
||||
|
||||
return (
|
||||
<div className="flex items-center gap-1 px-2" data-floating-terminal-no-drag>
|
||||
|
||||
@@ -0,0 +1,87 @@
|
||||
// Caller-owned placement coverage for launchAgentInNewTab, split from
|
||||
// launch-agent-in-new-tab.test.ts to keep both files within the lines budget.
|
||||
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const mockCreateTab = vi.fn()
|
||||
|
||||
const store = {
|
||||
settings: {
|
||||
agentCmdOverrides: {},
|
||||
agentDefaultArgs: {},
|
||||
agentDefaultEnv: {},
|
||||
activeRuntimeEnvironmentId: null
|
||||
},
|
||||
repos: [],
|
||||
allWorktrees: vi.fn(() => []),
|
||||
tabsByWorktree: { 'wt-1': [{ id: 'tab-1' }] },
|
||||
openFiles: [],
|
||||
browserTabsByWorktree: {},
|
||||
tabBarOrderByWorktree: {},
|
||||
createTab: mockCreateTab,
|
||||
queueTabInitialCwd: vi.fn(),
|
||||
queueTabStartupCommand: vi.fn(),
|
||||
setActiveTabType: vi.fn(),
|
||||
setTabBarOrder: vi.fn()
|
||||
}
|
||||
|
||||
vi.mock('@/store', () => ({
|
||||
useAppStore: { getState: () => store }
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/new-workspace', () => ({ CLIENT_PLATFORM: 'darwin' }))
|
||||
|
||||
vi.mock('@/lib/connection-context', () => ({
|
||||
getConnectionIdFromState: () => null
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/native-chat-transcript-readability', () => ({
|
||||
isNativeChatTranscriptLocalReadable: () => true
|
||||
}))
|
||||
|
||||
vi.mock('@/runtime/web-runtime-session', () => ({
|
||||
isWebRuntimeSessionActive: () => false
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/worktree-runtime-owner', () => ({
|
||||
getExecutionHostIdForWorktree: () => 'local',
|
||||
getRuntimeEnvironmentIdForWorktree: () => null
|
||||
}))
|
||||
|
||||
vi.mock('@/components/tab-bar/reconcile-order', () => ({
|
||||
reconcileTabOrder: (_stored: unknown, terminalIds: string[]) => terminalIds
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/telemetry', () => ({
|
||||
track: vi.fn(),
|
||||
tuiAgentToAgentKind: (agent: string) => agent
|
||||
}))
|
||||
|
||||
vi.mock('@/components/native-chat/native-chat-session-option-cache', () => ({
|
||||
seedNativeChatAppliedSessionOptions: vi.fn()
|
||||
}))
|
||||
|
||||
describe('launchAgentInNewTab terminal tab activation', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
mockCreateTab.mockReturnValue({ id: 'tab-1' })
|
||||
})
|
||||
|
||||
it('takes the global selection by default', async () => {
|
||||
const { launchAgentInNewTab } = await import('./launch-agent-in-new-tab')
|
||||
|
||||
launchAgentInNewTab({ agent: 'codex', worktreeId: 'wt-1' })
|
||||
|
||||
expect(mockCreateTab.mock.calls[0]?.[3]).not.toHaveProperty('activate')
|
||||
})
|
||||
|
||||
it('leaves the global selection alone when the caller places the tab itself', async () => {
|
||||
const { launchAgentInNewTab } = await import('./launch-agent-in-new-tab')
|
||||
|
||||
launchAgentInNewTab({ agent: 'codex', worktreeId: 'wt-1', activate: false })
|
||||
|
||||
// Why: the floating workspace selects within its own group; activating here would move the
|
||||
// main window's active tab to a tab it does not show.
|
||||
expect(mockCreateTab.mock.calls[0]?.[3]).toHaveProperty('activate', false)
|
||||
})
|
||||
})
|
||||
@@ -56,6 +56,13 @@ export type LaunchAgentInNewTabArgs = {
|
||||
launchPlatform?: NodeJS.Platform
|
||||
/** Called after the prompt is actually delivered to the agent input path. */
|
||||
onPromptDelivered?: () => void
|
||||
/**
|
||||
* Whether the new terminal tab takes the global selection. The floating workspace passes `false`
|
||||
* and selects within its own group instead, so launching there does not move the main window's
|
||||
* active tab. Terminal surface only — the structured and host-published routes own their own
|
||||
* activation.
|
||||
*/
|
||||
activate?: boolean
|
||||
/** Keeps a preflighted route authoritative across workspace creation. */
|
||||
agentSessionLaunchPlan?: AgentSessionLaunchPlan
|
||||
/** Lets a workspace reveal itself before the selected surface opens. */
|
||||
@@ -111,7 +118,8 @@ function launchAgentInNewTabInternal(args: LaunchAgentInNewTabArgs): LaunchAgent
|
||||
launchPlatform,
|
||||
onPromptDelivered,
|
||||
agentSessionLaunchPlan,
|
||||
beforeSurfaceOpen
|
||||
beforeSurfaceOpen,
|
||||
activate
|
||||
} = args
|
||||
const store = useAppStore.getState()
|
||||
const worktree = store.allWorktrees?.().find((entry: { id: string }) => entry.id === worktreeId)
|
||||
@@ -260,6 +268,7 @@ function launchAgentInNewTabInternal(args: LaunchAgentInNewTabArgs): LaunchAgent
|
||||
const tab = store.createTab(worktreeId, groupId, undefined, {
|
||||
launchAgent: agent,
|
||||
quickCommandLabel,
|
||||
...(activate === false ? { activate: false } : {}),
|
||||
...initialViewModeProps
|
||||
})
|
||||
seedNativeChatAppliedSessionOptions(tab.id, agent, startupPlan.sessionOptions)
|
||||
|
||||
Reference in New Issue
Block a user