Prevent replaying split pane startup commands (#737)

This commit is contained in:
Brennan Benson 2026-04-16 20:36:37 -07:00 committed by GitHub
parent a3dd4e8fb3
commit 31d402baa5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 118 additions and 7 deletions

View File

@ -0,0 +1,75 @@
import { describe, expect, it, vi } from 'vitest'
import { splitPaneWithOneShotStartup } from './use-terminal-pane-lifecycle'
describe('splitPaneWithOneShotStartup', () => {
it('only exposes startup to the intentional split and clears it afterwards', () => {
const deps: { startup?: { command: string; env?: Record<string, string> } | null } = {
startup: null
}
const seenStartupValues: (typeof deps.startup)[] = []
const createdPane = splitPaneWithOneShotStartup(
deps,
{ command: 'orca setup', env: { ORCA_ROLE: 'setup' } },
() => {
seenStartupValues.push(deps.startup ?? null)
return { id: 2 }
}
)
expect(createdPane).toEqual({ id: 2 })
expect(seenStartupValues).toEqual([{ command: 'orca setup', env: { ORCA_ROLE: 'setup' } }])
expect(deps.startup).toBeNull()
})
it('isolates startup payloads across sequential calls (setup then issue)', () => {
const deps: { startup?: { command: string; env?: Record<string, string> } | null } = {
startup: null
}
const seenStartupValues: (typeof deps.startup)[] = []
splitPaneWithOneShotStartup(
deps,
{ command: 'orca setup', env: { ORCA_ROLE: 'setup' } },
() => {
seenStartupValues.push(deps.startup ?? null)
return { id: 2 }
}
)
expect(deps.startup).toBeNull()
splitPaneWithOneShotStartup(deps, { command: 'orca issue' }, () => {
seenStartupValues.push(deps.startup ?? null)
return { id: 3 }
})
expect(seenStartupValues).toEqual([
{ command: 'orca setup', env: { ORCA_ROLE: 'setup' } },
{ command: 'orca issue' }
])
expect(deps.startup).toBeNull()
const userSplitObservedStartup = ((splitPane: () => { id: number }) => {
splitPane()
return deps.startup ?? null
})(() => ({ id: 4 }))
expect(userSplitObservedStartup).toBeNull()
expect(deps.startup).toBeNull()
})
it('clears startup even when splitPane throws', () => {
const deps: { startup?: { command: string } | null } = { startup: null }
const splitPane = vi.fn(() => {
throw new Error('split failed')
})
expect(() => splitPaneWithOneShotStartup(deps, { command: 'orca setup' }, splitPane)).toThrow(
'split failed'
)
expect(splitPane).toHaveBeenCalledTimes(1)
expect(deps.startup).toBeNull()
})
})

View File

@ -77,6 +77,34 @@ type UseTerminalPaneLifecycleDeps = {
setRenamingPaneId: React.Dispatch<React.SetStateAction<number | null>>
}
type SplitStartupPayload = { command: string; env?: Record<string, string> }
type SplitWithStartupDeps = {
startup?: SplitStartupPayload | null
}
/** Scopes `deps.startup` to a single call of `splitPane()`, clearing it in `finally` so later splits do not replay the payload. */
export function splitPaneWithOneShotStartup<TPane>(
deps: SplitWithStartupDeps,
startup: SplitStartupPayload,
splitPane: () => TPane
): TPane {
// Why: the startup payload is only for the pane created by this split.
// Pane creation fans out through onPaneCreated using a spread copy of `deps`,
// so connectPanePty cannot clear the caller's original object for us.
// Reset the shared field in finally so later user-driven splits never replay
// setup/issue commands, even if splitPane throws during creation.
// Relies on manager.splitPane → onPaneCreated → connectPanePty reading
// `deps.startup` synchronously before returning; if that chain ever becomes
// async, this helper must switch to awaiting the split before clearing.
deps.startup = startup
try {
return splitPane()
} finally {
deps.startup = null
}
}
export function useTerminalPaneLifecycle({
tabId,
worktreeId,
@ -421,9 +449,11 @@ export function useTerminalPaneLifecycle({
}
// Why: setup split creates a right-side pane for the setup script so the
// main (left) terminal stays immediately usable. We inject the setup command
// into ptyDeps.startup right before splitting — connectPanePty (called from
// onPaneCreated) reads it synchronously and clears it, so only the new pane
// gets the command. The initial pane already consumed startup=null above.
// into ptyDeps.startup right before splitting and clear it immediately after
// — connectPanePty receives a spread copy (`{...ptyDeps}`), so mutations
// inside connectPanePty don't propagate back to ptyDeps. Without clearing
// here, any later user-initiated split (e.g. Cmd+D) would re-run the setup
// command in the newly created pane.
let issueAutomationAnchorPaneId: number | null = null
// Why: capture the main shell pane *before* any splits mutate the pane list.
// Both the setup and issue-command paths need to restore focus back to this
@ -433,8 +463,11 @@ export function useTerminalPaneLifecycle({
if (setupSplit) {
if (initialPane) {
ptyDeps.startup = { command: setupSplit.command, env: setupSplit.env }
const setupPane = manager.splitPane(initialPane.id, 'vertical')
const setupPane = splitPaneWithOneShotStartup(
ptyDeps,
{ command: setupSplit.command, env: setupSplit.env },
() => manager.splitPane(initialPane.id, 'vertical')
)
issueAutomationAnchorPaneId = setupPane?.id ?? null
// Restore focus to the main (left) pane so the user's terminal
// receives keyboard input — the setup pane runs unattended.
@ -455,8 +488,11 @@ export function useTerminalPaneLifecycle({
manager.getActivePane() ??
manager.getPanes()[0]
if (targetPane) {
ptyDeps.startup = { command: issueCommandSplit.command, env: issueCommandSplit.env }
manager.splitPane(targetPane.id, 'vertical')
splitPaneWithOneShotStartup(
ptyDeps,
{ command: issueCommandSplit.command, env: issueCommandSplit.env },
() => manager.splitPane(targetPane.id, 'vertical')
)
// Why: if setup already claimed the right half, nest issue automation
// inside that automation area instead of splitting the main shell again.
// This preserves the primary terminal as the dominant pane while setup