Fix multiline quick command execution (#2779)
This commit is contained in:
parent
5287a581a6
commit
71b3998cd7
|
|
@ -8,6 +8,9 @@ export type PtyConnectionDeps = {
|
|||
cwd?: string
|
||||
startup?: {
|
||||
command: string
|
||||
/** Renderer-delivered startup input. Used when terminal paste semantics
|
||||
* matter, such as multiline quick commands. */
|
||||
delivery?: 'terminal-paste'
|
||||
env?: Record<string, string>
|
||||
/** Telemetry payload for `agent_started`. Forwarded to `pty:spawn`
|
||||
* so main fires the event only after the spawn succeeds. */
|
||||
|
|
|
|||
|
|
@ -517,6 +517,60 @@ describe('connectPanePty', () => {
|
|||
)
|
||||
})
|
||||
|
||||
it('delivers terminal-paste startup commands through xterm before submitting', async () => {
|
||||
const pendingTimeouts: (() => void)[] = []
|
||||
const originalSetTimeout = globalThis.setTimeout
|
||||
globalThis.setTimeout = vi.fn((fn: () => void) => {
|
||||
pendingTimeouts.push(fn)
|
||||
return 999 as unknown as ReturnType<typeof setTimeout>
|
||||
}) as unknown as typeof setTimeout
|
||||
|
||||
try {
|
||||
const { connectPanePty } = await import('./pty-connection')
|
||||
|
||||
const capturedDataCallback: { current: ((data: string) => void) | null } = { current: null }
|
||||
const transport = createMockTransport()
|
||||
transport.connect.mockImplementation(
|
||||
async ({ callbacks }: { callbacks: ConnectCallbacks }) => {
|
||||
capturedDataCallback.current = callbacks.onData ?? null
|
||||
return 'pty-local-paste'
|
||||
}
|
||||
)
|
||||
transportFactoryQueue.push(transport)
|
||||
|
||||
mockStoreState = {
|
||||
...mockStoreState,
|
||||
tabsByWorktree: { 'wt-1': [{ id: 'tab-1', ptyId: null }] },
|
||||
repos: [{ id: 'repo1', connectionId: null }]
|
||||
}
|
||||
|
||||
const pane = createPane(1)
|
||||
pane.terminal.modes.bracketedPasteMode = true
|
||||
pane.terminal.write.mockImplementation((_data: string, callback?: () => void) => {
|
||||
callback?.()
|
||||
})
|
||||
const manager = createManager(1)
|
||||
const command = 'cd packages\nbun run build\ncd ..'
|
||||
const deps = createDeps({ startup: { command, delivery: 'terminal-paste' } })
|
||||
|
||||
connectPanePty(pane as never, manager as never, deps as never)
|
||||
expect(createdTransportOptions[0]?.command).toBeUndefined()
|
||||
expect(capturedDataCallback.current).not.toBeNull()
|
||||
|
||||
capturedDataCallback.current?.('user@host $ ')
|
||||
for (const fn of pendingTimeouts) {
|
||||
fn()
|
||||
}
|
||||
await flushAsyncTicks()
|
||||
|
||||
expect(pane.terminal.paste).toHaveBeenCalledWith(command)
|
||||
expect(transport.sendInput).toHaveBeenCalledWith('\r')
|
||||
expect(transport.sendInput).not.toHaveBeenCalledWith(`${command}\r`)
|
||||
} finally {
|
||||
globalThis.setTimeout = originalSetTimeout
|
||||
}
|
||||
})
|
||||
|
||||
it('infers interrupts only from the focused terminal key target', async () => {
|
||||
const { connectPanePty } = await import('./pty-connection')
|
||||
const transport = createMockTransport()
|
||||
|
|
|
|||
|
|
@ -49,7 +49,8 @@ import {
|
|||
import { createAgentCompletionCoordinator } from './agent-completion-coordinator'
|
||||
import {
|
||||
markTerminalBracketedPasteInterrupted,
|
||||
observeTerminalBracketedPasteModeOutput
|
||||
observeTerminalBracketedPasteModeOutput,
|
||||
pasteTerminalText
|
||||
} from './terminal-bracketed-paste'
|
||||
|
||||
const pendingSpawnByPaneKey = new Map<string, Promise<string | null>>()
|
||||
|
|
@ -818,10 +819,11 @@ export function connectPanePty(
|
|||
: null) ?? (tab?.ptyId ? getRemoteRuntimePtyEnvironmentId(tab.ptyId) : null)
|
||||
const activeRuntimeEnvironmentId = state.settings?.activeRuntimeEnvironmentId?.trim() || null
|
||||
const runtimeEnvironmentId = remoteRuntimeOwnerForTransport ?? activeRuntimeEnvironmentId
|
||||
const shouldDeliverStartupViaTerminalPaste = paneStartup?.delivery === 'terminal-paste'
|
||||
const transportOptions = {
|
||||
cwd: deps.cwd,
|
||||
env: paneEnv,
|
||||
command: paneStartup?.command,
|
||||
command: shouldDeliverStartupViaTerminalPaste ? undefined : paneStartup?.command,
|
||||
connectionId,
|
||||
worktreeId: deps.worktreeId,
|
||||
// Why: closes the SIGKILL race documented in INVESTIGATION.md by letting
|
||||
|
|
@ -1101,12 +1103,11 @@ export function connectPanePty(
|
|||
}
|
||||
}
|
||||
|
||||
// Why: for local connections (connectionId === null) the local PTY provider
|
||||
// already writes the startup command via writeStartupCommandWhenShellReady,
|
||||
// which is shell-ready-aware and reliable. Re-sending it here would cause
|
||||
// the command to appear twice in the terminal. For SSH connections the relay
|
||||
// has no equivalent mechanism, so the renderer must inject it via sendInput.
|
||||
let pendingStartupCommand = connectionId ? (paneStartup?.command ?? null) : null
|
||||
// Why: for ordinary local startup commands, the local PTY provider already
|
||||
// writes via the shell-ready barrier. terminal-paste startup commands must
|
||||
// stay renderer-delivered so xterm can apply bracketed-paste semantics.
|
||||
let pendingStartupCommand =
|
||||
shouldDeliverStartupViaTerminalPaste || connectionId ? (paneStartup?.command ?? null) : null
|
||||
|
||||
const startFreshSpawn = (): void => {
|
||||
// Why: pre-signal the main process so its cooperation gate suppresses
|
||||
|
|
@ -1210,11 +1211,27 @@ export function connectPanePty(
|
|||
}
|
||||
startupInjectTimer = setTimeout(() => {
|
||||
startupInjectTimer = null
|
||||
if (!pendingStartupCommand || disposed) {
|
||||
return
|
||||
}
|
||||
transport.sendInput(`${pendingStartupCommand}\r`)
|
||||
pendingStartupCommand = null
|
||||
void (async () => {
|
||||
const command = pendingStartupCommand
|
||||
if (!command || disposed) {
|
||||
return
|
||||
}
|
||||
if (shouldDeliverStartupViaTerminalPaste) {
|
||||
await waitForTerminalOutputParsed(pane.terminal)
|
||||
}
|
||||
if (pendingStartupCommand !== command || disposed) {
|
||||
return
|
||||
}
|
||||
if (shouldDeliverStartupViaTerminalPaste) {
|
||||
// Why: multiline quick commands must be delivered as terminal paste
|
||||
// before Enter, otherwise foreground commands can read later lines.
|
||||
pasteTerminalText(pane.terminal, command)
|
||||
transport.sendInput('\r')
|
||||
} else {
|
||||
transport.sendInput(`${command}\r`)
|
||||
}
|
||||
pendingStartupCommand = null
|
||||
})()
|
||||
}, 50)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,10 +1,21 @@
|
|||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { sendTerminalQuickCommandToPane } from './terminal-quick-command-dispatch'
|
||||
|
||||
function createPane() {
|
||||
return {
|
||||
terminal: {
|
||||
focus: vi.fn(),
|
||||
modes: { bracketedPasteMode: true },
|
||||
options: { ignoreBracketedPasteMode: false },
|
||||
paste: vi.fn()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
describe('sendTerminalQuickCommandToPane', () => {
|
||||
it('writes the formatted command to the PTY transport and refocuses the terminal', () => {
|
||||
const sendInput = vi.fn(() => true)
|
||||
const focus = vi.fn()
|
||||
const pane = createPane()
|
||||
|
||||
const sent = sendTerminalQuickCommandToPane({
|
||||
command: {
|
||||
|
|
@ -13,18 +24,19 @@ describe('sendTerminalQuickCommandToPane', () => {
|
|||
command: 'git status',
|
||||
appendEnter: true
|
||||
},
|
||||
pane: { terminal: { focus } },
|
||||
pane,
|
||||
transport: { sendInput }
|
||||
})
|
||||
|
||||
expect(sent).toBe(true)
|
||||
expect(sendInput).toHaveBeenCalledWith('git status\r')
|
||||
expect(focus).toHaveBeenCalledOnce()
|
||||
expect(pane.terminal.paste).not.toHaveBeenCalled()
|
||||
expect(pane.terminal.focus).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('does not focus the terminal when no connected transport accepts input', () => {
|
||||
const sendInput = vi.fn(() => false)
|
||||
const focus = vi.fn()
|
||||
const pane = createPane()
|
||||
|
||||
const sent = sendTerminalQuickCommandToPane({
|
||||
command: {
|
||||
|
|
@ -33,12 +45,57 @@ describe('sendTerminalQuickCommandToPane', () => {
|
|||
command: 'npm test',
|
||||
appendEnter: false
|
||||
},
|
||||
pane: { terminal: { focus } },
|
||||
pane,
|
||||
transport: { sendInput }
|
||||
})
|
||||
|
||||
expect(sent).toBe(false)
|
||||
expect(sendInput).toHaveBeenCalledWith('npm test')
|
||||
expect(focus).not.toHaveBeenCalled()
|
||||
expect(pane.terminal.paste).not.toHaveBeenCalled()
|
||||
expect(pane.terminal.focus).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('pastes multiline commands before appending enter', () => {
|
||||
const sendInput = vi.fn(() => true)
|
||||
const pane = createPane()
|
||||
const commandText = 'cd packages\nbun run build\ncd ..'
|
||||
|
||||
const sent = sendTerminalQuickCommandToPane({
|
||||
command: {
|
||||
id: 'build',
|
||||
label: 'Build',
|
||||
command: commandText,
|
||||
appendEnter: true
|
||||
},
|
||||
pane,
|
||||
transport: { sendInput }
|
||||
})
|
||||
|
||||
expect(sent).toBe(true)
|
||||
expect(pane.terminal.paste).toHaveBeenCalledWith(commandText)
|
||||
expect(sendInput).toHaveBeenCalledWith('\r')
|
||||
expect(pane.terminal.focus).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('pastes multiline insert-only commands without submitting', () => {
|
||||
const sendInput = vi.fn(() => true)
|
||||
const pane = createPane()
|
||||
const commandText = 'echo one\necho two'
|
||||
|
||||
const sent = sendTerminalQuickCommandToPane({
|
||||
command: {
|
||||
id: 'insert',
|
||||
label: 'Insert',
|
||||
command: commandText,
|
||||
appendEnter: false
|
||||
},
|
||||
pane,
|
||||
transport: { sendInput }
|
||||
})
|
||||
|
||||
expect(sent).toBe(true)
|
||||
expect(pane.terminal.paste).toHaveBeenCalledWith(commandText)
|
||||
expect(sendInput).not.toHaveBeenCalled()
|
||||
expect(pane.terminal.focus).toHaveBeenCalledOnce()
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,9 +1,17 @@
|
|||
import type { TerminalQuickCommand } from '../../../../shared/types'
|
||||
import { buildTerminalQuickCommandInput } from '../../../../shared/terminal-quick-commands'
|
||||
import { pasteTerminalText } from './terminal-bracketed-paste'
|
||||
|
||||
type QuickCommandPane = {
|
||||
terminal: {
|
||||
focus: () => void
|
||||
modes: {
|
||||
bracketedPasteMode: boolean
|
||||
}
|
||||
options: {
|
||||
ignoreBracketedPasteMode?: boolean
|
||||
}
|
||||
paste: (text: string) => void
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -11,6 +19,8 @@ type QuickCommandTransport = {
|
|||
sendInput: (data: string) => boolean
|
||||
}
|
||||
|
||||
const LINE_BREAK_RE = /[\r\n]/
|
||||
|
||||
export function sendTerminalQuickCommandToPane({
|
||||
command,
|
||||
pane,
|
||||
|
|
@ -24,6 +34,17 @@ export function sendTerminalQuickCommandToPane({
|
|||
return false
|
||||
}
|
||||
|
||||
if (LINE_BREAK_RE.test(command.command)) {
|
||||
// Why: sending multiline quick commands as raw queued PTY input lets a
|
||||
// foreground command consume later lines before the shell sees them.
|
||||
pasteTerminalText(pane.terminal, command.command)
|
||||
const sent = command.appendEnter ? transport.sendInput('\r') : true
|
||||
if (sent) {
|
||||
pane.terminal.focus()
|
||||
}
|
||||
return sent
|
||||
}
|
||||
|
||||
const sent = transport.sendInput(buildTerminalQuickCommandInput(command))
|
||||
if (sent) {
|
||||
pane.terminal.focus()
|
||||
|
|
|
|||
|
|
@ -0,0 +1,80 @@
|
|||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { runQuickCommandInNewTab } from './run-quick-command-in-new-tab'
|
||||
|
||||
type MockStoreState = {
|
||||
createTab: ReturnType<typeof vi.fn>
|
||||
queueTabStartupCommand: ReturnType<typeof vi.fn>
|
||||
setActiveTabType: ReturnType<typeof vi.fn>
|
||||
setTabBarOrder: ReturnType<typeof vi.fn>
|
||||
setRecentQuickCommandForGroup: ReturnType<typeof vi.fn>
|
||||
tabsByWorktree: Record<string, { id: string }[]>
|
||||
openFiles: { id: string; worktreeId: string }[]
|
||||
browserTabsByWorktree: Record<string, { id: string }[]>
|
||||
tabBarOrderByWorktree: Record<string, string[]>
|
||||
}
|
||||
|
||||
let mockState: MockStoreState
|
||||
|
||||
vi.mock('@/store', () => ({
|
||||
useAppStore: {
|
||||
getState: () => mockState
|
||||
}
|
||||
}))
|
||||
|
||||
function createStoreState(): MockStoreState {
|
||||
const state: MockStoreState = {
|
||||
createTab: vi.fn(() => ({ id: 'tab-new' })),
|
||||
queueTabStartupCommand: vi.fn(),
|
||||
setActiveTabType: vi.fn(),
|
||||
setTabBarOrder: vi.fn(),
|
||||
setRecentQuickCommandForGroup: vi.fn(),
|
||||
tabsByWorktree: { 'wt-1': [{ id: 'tab-existing' }, { id: 'tab-new' }] },
|
||||
openFiles: [],
|
||||
browserTabsByWorktree: {},
|
||||
tabBarOrderByWorktree: {}
|
||||
}
|
||||
return state
|
||||
}
|
||||
|
||||
describe('runQuickCommandInNewTab', () => {
|
||||
beforeEach(() => {
|
||||
mockState = createStoreState()
|
||||
})
|
||||
|
||||
it('queues multiline quick commands for terminal-paste delivery', () => {
|
||||
const result = runQuickCommandInNewTab({
|
||||
command: {
|
||||
id: 'build',
|
||||
label: 'Build',
|
||||
command: 'cd packages\nbun run build\ncd ..',
|
||||
appendEnter: true
|
||||
},
|
||||
worktreeId: 'wt-1',
|
||||
groupId: 'group-1'
|
||||
})
|
||||
|
||||
expect(result).toEqual({ tabId: 'tab-new' })
|
||||
expect(mockState.queueTabStartupCommand).toHaveBeenCalledWith('tab-new', {
|
||||
command: 'cd packages\nbun run build\ncd ..',
|
||||
delivery: 'terminal-paste'
|
||||
})
|
||||
expect(mockState.setRecentQuickCommandForGroup).toHaveBeenCalledWith('group-1', 'build')
|
||||
})
|
||||
|
||||
it('keeps single-line quick commands on the standard startup path', () => {
|
||||
runQuickCommandInNewTab({
|
||||
command: {
|
||||
id: 'status',
|
||||
label: 'Status',
|
||||
command: 'git status',
|
||||
appendEnter: true
|
||||
},
|
||||
worktreeId: 'wt-1',
|
||||
groupId: 'group-1'
|
||||
})
|
||||
|
||||
expect(mockState.queueTabStartupCommand).toHaveBeenCalledWith('tab-new', {
|
||||
command: 'git status'
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
@ -2,6 +2,8 @@ import { useAppStore } from '@/store'
|
|||
import { reconcileTabOrder } from '@/components/tab-bar/reconcile-order'
|
||||
import type { TerminalQuickCommand } from '../../../shared/types'
|
||||
|
||||
const LINE_BREAK_RE = /[\r\n]/
|
||||
|
||||
export type RunQuickCommandInNewTabArgs = {
|
||||
command: TerminalQuickCommand
|
||||
worktreeId: string
|
||||
|
|
@ -34,7 +36,8 @@ export function runQuickCommandInNewTab({
|
|||
const tab = store.createTab(worktreeId, groupId)
|
||||
|
||||
store.queueTabStartupCommand(tab.id, {
|
||||
command: command.command
|
||||
command: command.command,
|
||||
...(LINE_BREAK_RE.test(command.command) ? { delivery: 'terminal-paste' as const } : {})
|
||||
})
|
||||
|
||||
// Why: match `+` button's createNewTerminalTab — without this, a worktree
|
||||
|
|
|
|||
|
|
@ -193,6 +193,9 @@ export type TerminalSlice = {
|
|||
string,
|
||||
{
|
||||
command: string
|
||||
/** Renderer-delivered startup input. Used by multiline quick commands so
|
||||
* xterm can use bracketed paste before the submit Enter is sent. */
|
||||
delivery?: 'terminal-paste'
|
||||
env?: Record<string, string>
|
||||
/** Telemetry metadata for the `agent_started` event. Threaded all the
|
||||
* way to the `pty:spawn` IPC handler in main so the event fires only
|
||||
|
|
@ -308,6 +311,7 @@ export type TerminalSlice = {
|
|||
tabId: string,
|
||||
startup: {
|
||||
command: string
|
||||
delivery?: 'terminal-paste'
|
||||
env?: Record<string, string>
|
||||
telemetry?: AgentStartedTelemetry
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue