starred/orca
1
0
Fork 0
mirror of https://github.com/stablyai/orca.git synced 2026-10-19 16:03:26 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(sidebar): keep split-mode setup panes silent, tighten surfaceOwner

Browse Source 
Review catch: with setupScriptLaunchMode split-vertical/horizontal the Setup
terminal goes through splitTerminal, whose reveal payload had no surfaceOwner,
so a background create still scrolled the sidebar in that configuration.

Also narrow surfaceOwner to `false` so "surface it" can only be expressed by
omitting the key, and fold the repeated conditional spreads into ownerSurfacing.
This commit is contained in:
Neil
2026-07-30 02:05:48 -07:00
parent aa273e44f2
commit ed28a01251
5 changed files with 200 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/main/runtime/orca-runtime.test.ts
+166
View File
@@ -36582,6 +36582,172 @@ describe('OrcaRuntimeService', () => {
)
})
it('does not surface the new workspace when a background create splits its setup pane', async () => {
const runtimeStore = {
...store,
getSettings: () => ({
...store.getSettings(),
setupScriptLaunchMode: 'split-horizontal' as const
})
}
const runtime = new OrcaRuntimeService(runtimeStore as never)
const activateWorktree = vi.fn()
const revealTerminalSession = vi.fn().mockResolvedValue({ tabId: 'tab-bg-setup-split' })
const spawn = vi
.fn()
.mockResolvedValueOnce({ id: 'pty-bg-split-main' })
.mockResolvedValueOnce({ id: 'pty-bg-split-setup' })
runtime.setPtyController({
spawn,
write: () => true,
kill: () => true,
getForegroundProcess: async () => null
})
runtime.setNotifier({
worktreesChanged: vi.fn(),
reposChanged: vi.fn(),
activateWorktree,
createTerminal: vi.fn(),
revealTerminalSession,
splitTerminal: vi.fn(),
renameTerminal: vi.fn(),
focusTerminal: vi.fn(),
closeTerminal: vi.fn(),
sleepWorktree: vi.fn(),
terminalFitOverrideChanged: vi.fn(),
terminalDriverChanged: vi.fn()
})
runtime.attachWindow(1)
computeWorktreePathMock.mockReturnValue('/tmp/workspaces/runtime-bg-split-setup')
ensurePathWithinWorkspaceMock.mockReturnValue('/tmp/workspaces/runtime-bg-split-setup')
vi.mocked(getEffectiveHooks).mockReturnValue({ scripts: { setup: 'pnpm install' } })
vi.mocked(shouldRunSetupForCreate).mockReturnValue(true)
vi.mocked(createSetupRunnerScript).mockReturnValue({
runnerScriptPath: '/tmp/repo/.git/orca/setup-runner.sh',
envVars: {
ORCA_ROOT_PATH: '/tmp/repo',
ORCA_WORKTREE_PATH: '/tmp/workspaces/runtime-bg-split-setup'
}
})
vi.mocked(listWorktrees).mockResolvedValue([
{
path: '/tmp/workspaces/runtime-bg-split-setup',
head: 'def',
branch: 'runtime-bg-split-setup',
isBare: false,
isMainWorktree: false
}
])
const result = await runtime.createManagedWorktree({
repoSelector: 'id:repo-1',
name: 'runtime-bg-split-setup',
setupDecision: 'run'
})
// Split launch modes reach the renderer through splitTerminal, not
// createTerminal — that path must suppress owner surfacing too.
expect(activateWorktree).not.toHaveBeenCalled()
await vi.waitFor(() => expect(spawn).toHaveBeenCalledTimes(2))
const mainEnv = (spawn.mock.calls[0]![0] as { env?: Record<string, string> }).env ?? {}
await vi.waitFor(() =>
expect(revealTerminalSession).toHaveBeenLastCalledWith(
result.worktree.id,
expect.objectContaining({
ptyId: 'pty-bg-split-setup',
tabId: mainEnv.ORCA_TAB_ID,
activate: false,
surfaceOwner: false,
splitDirection: 'horizontal'
})
)
)
})
it('still surfaces the new workspace when an activating create splits its setup pane', async () => {
const runtimeStore = {
...store,
getSettings: () => ({
...store.getSettings(),
setupScriptLaunchMode: 'split-horizontal' as const
})
}
const runtime = new OrcaRuntimeService(runtimeStore as never)
const revealTerminalSession = vi.fn().mockResolvedValue({ tabId: 'tab-active-setup-split' })
const spawn = vi
.fn()
.mockResolvedValueOnce({ id: 'pty-active-split-main' })
.mockResolvedValueOnce({ id: 'pty-active-split-setup' })
runtime.setPtyController({
spawn,
write: () => true,
kill: () => true,
getForegroundProcess: async () => null
})
runtime.setNotifier({
worktreesChanged: vi.fn(),
reposChanged: vi.fn(),
activateWorktree: vi.fn(),
createTerminal: vi.fn(),
revealTerminalSession,
splitTerminal: vi.fn(),
renameTerminal: vi.fn(),
focusTerminal: vi.fn(),
closeTerminal: vi.fn(),
sleepWorktree: vi.fn(),
terminalFitOverrideChanged: vi.fn(),
terminalDriverChanged: vi.fn()
})
runtime.attachWindow(1)
computeWorktreePathMock.mockReturnValue('/tmp/workspaces/runtime-active-split-setup')
ensurePathWithinWorkspaceMock.mockReturnValue('/tmp/workspaces/runtime-active-split-setup')
vi.mocked(getEffectiveHooks).mockReturnValue({ scripts: { setup: 'pnpm install' } })
vi.mocked(shouldRunSetupForCreate).mockReturnValue(true)
vi.mocked(createSetupRunnerScript).mockReturnValue({
runnerScriptPath: '/tmp/repo/.git/orca/setup-runner.sh',
envVars: {
ORCA_ROOT_PATH: '/tmp/repo',
ORCA_WORKTREE_PATH: '/tmp/workspaces/runtime-active-split-setup'
}
})
vi.mocked(listWorktrees).mockResolvedValue([
{
path: '/tmp/workspaces/runtime-active-split-setup',
head: 'def',
branch: 'runtime-active-split-setup',
isBare: false,
isMainWorktree: false
}
])
const result = await runtime.createManagedWorktree({
repoSelector: 'id:repo-1',
name: 'runtime-active-split-setup',
activate: true,
setupDecision: 'run',
createdWithAgent: 'claude',
startup: { command: 'claude' }
})
// No-regression guard: an explicit --activate still reveals the workspace.
await vi.waitFor(() => expect(spawn).toHaveBeenCalledTimes(2))
await vi.waitFor(() =>
expect(revealTerminalSession).toHaveBeenLastCalledWith(
result.worktree.id,
expect.objectContaining({
ptyId: 'pty-active-split-setup',
splitDirection: 'horizontal'
})
)
)
const splitRevealPayload = revealTerminalSession.mock.lastCall?.[1] as
| { surfaceOwner?: boolean }
| undefined
expect(splitRevealPayload).not.toHaveProperty('surfaceOwner')
})
it('does not warn when setup is explicitly skipped for CLI-created worktrees', async () => {
const metaById: Record<string, WorktreeMeta> = {}
const runtimeStore = {
src/main/runtime/orca-runtime.ts
+29 -15
View File
@@ -1274,7 +1274,7 @@ type TerminalCreateOptions = {
// Why: `false` adopts the terminal without pointing the user at it — no
// sidebar reveal, no tab focus. Distinct from 'background' presentation,
// which skips renderer adoption entirely.
surfaceOwner?: boolean
surfaceOwner?: false
tabId?: string
leafId?: string
sessionId?: string
@@ -1658,6 +1658,12 @@ function createTerminalRevealWarning(handle: string, error?: unknown): string {
].join(' ')
}
// Why: an absent `surfaceOwner` means "default", so surfacing callers must omit
// the key rather than send `true`.
function ownerSurfacing(shouldSurface: boolean): { surfaceOwner?: false } {
return shouldSurface ? {} : { surfaceOwner: false }
}
function resolveTerminalPresentation(opts: {
presentation?: RuntimeTerminalPresentation
focus?: boolean
@@ -1706,7 +1712,7 @@ type RuntimeNotifier = {
viewMode?: 'terminal' | 'chat'
activate?: boolean
presentation?: RuntimeTerminalPresentation
surfaceOwner?: boolean
surfaceOwner?: false
tabId?: string
leafId?: string
splitFromLeafId?: string
@@ -19948,7 +19954,7 @@ export class OrcaRuntimeService {
worktreeSelector: string,
worktreeId: string,
defaultTabs: CreateWorktreeResult['defaultTabs'] | undefined,
surfaceOwner?: boolean
surfacing: { surfaceOwner?: false } = {}
): Promise<string[]> {
if (!defaultTabs || defaultTabs.tabs.length === 0 || !this.ptyController?.spawn) {
return []
@@ -19960,7 +19966,7 @@ export class OrcaRuntimeService {
const terminal = await this.createTerminal(worktreeSelector, {
...(template.title ? { title: template.title } : {}),
...(command && defaultTabs.runCommands ? { command } : {}),
...(surfaceOwner === false ? { surfaceOwner: false } : {})
...surfacing
})
handles.push(terminal.handle)
if (template.color && terminal.tabId) {
@@ -19993,12 +19999,12 @@ export class OrcaRuntimeService {
wrappedSetupCommand?: string
// Why: a workspace provisioned in the background must not pull the sidebar
// to itself; the user never asked to look at these tabs.
surfaceOwner?: boolean
surfaceOwner?: false
}): Promise<{ setupSpawned: boolean; setupTerminalHandle: string | null }> {
if (!this.ptyController?.spawn) {
return { setupSpawned: false, setupTerminalHandle: null }
}
const silent = args.surfaceOwner === false ? ({ surfaceOwner: false } as const) : {}
const surfacing = ownerSurfacing(args.surfaceOwner !== false)
let setupSpawned = false
let setupTerminalHandle: string | null = null
try {
@@ -20006,7 +20012,7 @@ export class OrcaRuntimeService {
args.worktreeSelector,
args.worktreeId,
args.defaultTabs,
args.surfaceOwner
surfacing
)
let primaryTerminalHandle = args.primaryTerminalHandle ?? defaultTabHandles[0] ?? null
const setupLaunchMode =
@@ -20016,7 +20022,7 @@ export class OrcaRuntimeService {
>
).setupScriptLaunchMode ?? 'new-tab'
if (!args.hasStartupTerminal && !primaryTerminalHandle) {
const terminal = await this.createTerminal(args.worktreeSelector, { ...silent })
const terminal = await this.createTerminal(args.worktreeSelector, surfacing)
primaryTerminalHandle = terminal.handle
}
if (args.setup) {
@@ -20042,13 +20048,14 @@ export class OrcaRuntimeService {
direction: setupLaunchMode === 'split-horizontal' ? 'horizontal' : 'vertical',
command: setupCommand,
env: setupEnv,
activate: false
activate: false,
...surfacing
})
: this.createTerminal(args.worktreeSelector, {
title: 'Setup',
command: setupCommand,
env: setupEnv,
...silent
...surfacing
}))
setupTerminalHandle = setupTerminal.handle
setupSpawned = true
@@ -20328,7 +20335,7 @@ export class OrcaRuntimeService {
...(effectiveStartup.viewMode ? { viewMode: effectiveStartup.viewMode } : {}),
startupCommandDelivery: effectiveStartup.startupCommandDelivery,
telemetry: effectiveStartup.telemetry,
...(shouldActivate ? {} : { surfaceOwner: false })
...ownerSurfacing(shouldActivate)
})
if (effectiveDraftPaste) {
this.pasteStartupDraftWhenReady(terminal.handle, effectiveDraftPaste)
@@ -21069,7 +21076,7 @@ export class OrcaRuntimeService {
...(sequencedStartup.viewMode ? { viewMode: sequencedStartup.viewMode } : {}),
startupCommandDelivery: sequencedStartup.startupCommandDelivery,
telemetry: sequencedStartup.telemetry,
...(shouldActivate ? {} : { surfaceOwner: false })
...ownerSurfacing(shouldActivate)
})
if (effectiveDraftPaste) {
this.pasteStartupDraftWhenReady(terminal.handle, effectiveDraftPaste)
@@ -21421,7 +21428,7 @@ export class OrcaRuntimeService {
...(sequencedStartup.viewMode ? { viewMode: sequencedStartup.viewMode } : {}),
startupCommandDelivery: sequencedStartup.startupCommandDelivery,
telemetry: sequencedStartup.telemetry,
...(shouldActivate ? {} : { surfaceOwner: false })
...ownerSurfacing(shouldActivate)
})
if (args.startupDraftPaste) {
this.pasteStartupDraftWhenReady(terminal.handle, args.startupDraftPaste)
@@ -23966,7 +23973,7 @@ export class OrcaRuntimeService {
...(launchOpts.viewMode ? { viewMode: launchOpts.viewMode } : {}),
activate: presentation === 'focused',
...(presentation ? { presentation } : {}),
...(opts.surfaceOwner === false ? { surfaceOwner: false } : {}),
...ownerSurfacing(opts.surfaceOwner !== false),
tabId,
leafId
})
@@ -24052,7 +24059,7 @@ export class OrcaRuntimeService {
title: launchOpts.title,
activate: presentation === 'focused',
...(presentation ? { presentation } : {}),
...(opts.surfaceOwner === false ? { surfaceOwner: false } : {})
...ownerSurfacing(opts.surfaceOwner !== false)
})
})
@@ -25192,6 +25199,9 @@ export class OrcaRuntimeService {
env?: Record<string, string>
envToDelete?: string[]
activate?: boolean
// Why: same split as createTerminal — adopt the pane without revealing its
// workspace, for splits the user never asked to see.
surfaceOwner?: false
telemetrySource?: TerminalPaneSplitSource
} = {}
): Promise<RuntimeTerminalSplit> {
@@ -25229,6 +25239,9 @@ export class OrcaRuntimeService {
env?: Record<string, string>
envToDelete?: string[]
activate?: boolean
// Why: same split as createTerminal — adopt the pane without revealing its
// workspace, for splits the user never asked to see.
surfaceOwner?: false
telemetrySource?: TerminalPaneSplitSource
} = {}
): Promise<RuntimeTerminalSplit> {
@@ -25279,6 +25292,7 @@ export class OrcaRuntimeService {
ptyId: result.id,
title: null,
activate: opts.activate !== false,
...ownerSurfacing(opts.surfaceOwner !== false),
tabId: parentTabId,
leafId,
splitFromLeafId: parsedPaneKey.leafId,
src/preload/api-types.ts
+1 -1
View File
@@ -3157,7 +3157,7 @@ export type PreloadApi = {
activate?: boolean
focus?: boolean
presentation?: RuntimeTerminalPresentation
surfaceOwner?: boolean
surfaceOwner?: false
tabId?: string
leafId?: string
splitFromLeafId?: string
src/preload/index.ts
+2 -2
View File
@@ -3722,7 +3722,7 @@ const api = {
activate?: boolean
focus?: boolean
presentation?: RuntimeTerminalPresentation
surfaceOwner?: boolean
surfaceOwner?: false
tabId?: string
leafId?: string
splitFromLeafId?: string
@@ -3748,7 +3748,7 @@ const api = {
activate?: boolean
focus?: boolean
presentation?: RuntimeTerminalPresentation
surfaceOwner?: boolean
surfaceOwner?: false
tabId?: string
leafId?: string
splitFromLeafId?: string
src/shared/runtime-types.ts
+2 -1
View File
@@ -626,8 +626,9 @@ type RuntimeTerminalCreateBaseRequestPayload = {
* Why: adopting a terminal is separate from pointing the user at it. `false`
* keeps the tab silent — no sidebar reveal, no tab focus — for terminals the
* user never asked to see (e.g. a workspace created in the background).
* Absent means "surface it", so this is a suppression switch, never `true`.
*/
surfaceOwner?: boolean
surfaceOwner?: false
}
export type RuntimeTerminalCreateRequestPayload =