Fix stale mobile terminal input locks (#4506)
* Fix stale mobile terminal input locks * Label desktop terminal RPC sends
This commit is contained in:
parent
b3481d3652
commit
9325b0cbb0
|
|
@ -85,7 +85,8 @@ export const TERMINAL_HANDLERS: Record<string, CommandHandler> = {
|
|||
terminal: await getTerminalHandle(flags, cwd, client),
|
||||
text: getOptionalStringFlag(flags, 'text'),
|
||||
enter: flags.get('enter') === true,
|
||||
interrupt: flags.get('interrupt') === true
|
||||
interrupt: flags.get('interrupt') === true,
|
||||
client: { id: 'orca-cli', type: 'desktop' }
|
||||
})
|
||||
printResult(result, json, formatTerminalSend)
|
||||
},
|
||||
|
|
|
|||
|
|
@ -127,21 +127,27 @@ function createTerminalOutputBatcher(onFlush: (data: string) => void): {
|
|||
}
|
||||
}
|
||||
|
||||
function isTerminalInputLockedForClient(
|
||||
async function reclaimTerminalInputForClient(
|
||||
runtime: OrcaRuntimeService,
|
||||
ptyId: string,
|
||||
client: TerminalViewportClient | undefined
|
||||
): boolean {
|
||||
): Promise<boolean> {
|
||||
if (client?.type === 'mobile') {
|
||||
return false
|
||||
return true
|
||||
}
|
||||
// Why: pre-refactor mobile builds did not send client metadata. Desktop
|
||||
// callers we control now identify as desktop, so keep legacy mobile input
|
||||
// working without opening the new desktop path.
|
||||
if (!client) {
|
||||
return false
|
||||
return true
|
||||
}
|
||||
return runtime.getDriver(ptyId).kind === 'mobile'
|
||||
if (runtime.getDriver(ptyId).kind !== 'mobile') {
|
||||
return true
|
||||
}
|
||||
// Why: a live desktop typing into a remotely driven terminal is an explicit
|
||||
// take-back. Otherwise a stale mobile socket can black-hole input until
|
||||
// heartbeat cleanup, or forever behind a proxy that keeps it warm.
|
||||
return runtime.reclaimTerminalForDesktop(ptyId)
|
||||
}
|
||||
|
||||
function resolveMobileFloorClientId(
|
||||
|
|
@ -535,7 +541,10 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [
|
|||
handler: async (params, { runtime }) => {
|
||||
const leaf = runtime.resolveLeafForHandle(params.terminal)
|
||||
const driver = leaf?.ptyId ? runtime.getDriver(leaf.ptyId) : null
|
||||
if (leaf?.ptyId && isTerminalInputLockedForClient(runtime, leaf.ptyId, params.client)) {
|
||||
if (
|
||||
leaf?.ptyId &&
|
||||
!(await reclaimTerminalInputForClient(runtime, leaf.ptyId, params.client))
|
||||
) {
|
||||
return {
|
||||
send: {
|
||||
handle: params.terminal,
|
||||
|
|
@ -790,12 +799,17 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [
|
|||
if (!text) {
|
||||
return
|
||||
}
|
||||
if (isTerminalInputLockedForClient(runtime, stream.ptyId, stream.client)) {
|
||||
return
|
||||
}
|
||||
void runtime
|
||||
.sendTerminal(stream.terminal, { text, enter: false, interrupt: false })
|
||||
.then(async () => {
|
||||
void reclaimTerminalInputForClient(runtime, stream.ptyId, stream.client)
|
||||
.then((canSend) => {
|
||||
if (!canSend) {
|
||||
return null
|
||||
}
|
||||
return runtime.sendTerminal(stream.terminal, { text, enter: false, interrupt: false })
|
||||
})
|
||||
.then(async (result) => {
|
||||
if (!result) {
|
||||
return
|
||||
}
|
||||
if (stream.isMobile && stream.client?.id) {
|
||||
await runtime.mobileTookFloor(stream.ptyId, stream.client.id)
|
||||
}
|
||||
|
|
@ -1189,12 +1203,21 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [
|
|||
if (!text) {
|
||||
return
|
||||
}
|
||||
if (isTerminalInputLockedForClient(runtime, ptyId, params.client)) {
|
||||
return
|
||||
}
|
||||
void runtime
|
||||
.sendTerminal(params.terminal, { text, enter: false, interrupt: false })
|
||||
.then(async () => {
|
||||
void reclaimTerminalInputForClient(runtime, ptyId, params.client)
|
||||
.then((canSend) => {
|
||||
if (!canSend) {
|
||||
return null
|
||||
}
|
||||
return runtime.sendTerminal(params.terminal, {
|
||||
text,
|
||||
enter: false,
|
||||
interrupt: false
|
||||
})
|
||||
})
|
||||
.then(async (result) => {
|
||||
if (!result) {
|
||||
return
|
||||
}
|
||||
if (isMobile && clientId) {
|
||||
await runtime.mobileTookFloor(ptyId, clientId)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -288,7 +288,7 @@ describe('terminal multiplex RPC', () => {
|
|||
await dispatchPromise
|
||||
})
|
||||
|
||||
it('drops desktop multiplex input while a mobile client owns the terminal floor', async () => {
|
||||
it('reclaims desktop multiplex input while a mobile client owns the terminal floor', async () => {
|
||||
const messages: string[] = []
|
||||
const handlers = new Map<
|
||||
number,
|
||||
|
|
@ -312,6 +312,7 @@ describe('terminal multiplex RPC', () => {
|
|||
rows: 20
|
||||
}),
|
||||
getDriver: vi.fn().mockReturnValue({ kind: 'mobile', clientId: 'phone-1' }),
|
||||
reclaimTerminalForDesktop: vi.fn().mockResolvedValue(true),
|
||||
registerSubscriptionCleanup: vi.fn((id: string, cleanup: () => void) => {
|
||||
cleanups.set(id, cleanup)
|
||||
}),
|
||||
|
|
@ -383,7 +384,14 @@ describe('terminal multiplex RPC', () => {
|
|||
)!
|
||||
)
|
||||
|
||||
expect(runtime.sendTerminal).not.toHaveBeenCalled()
|
||||
await vi.waitFor(() => expect(runtime.reclaimTerminalForDesktop).toHaveBeenCalledWith('pty-1'))
|
||||
await vi.waitFor(() =>
|
||||
expect(runtime.sendTerminal).toHaveBeenCalledWith('terminal-1', {
|
||||
text: 'typed while locked',
|
||||
enter: false,
|
||||
interrupt: false
|
||||
})
|
||||
)
|
||||
cleanups.get('terminal-multiplex:conn-locked')?.()
|
||||
await dispatchPromise
|
||||
})
|
||||
|
|
|
|||
|
|
@ -36,11 +36,16 @@ describe('terminal send RPC', () => {
|
|||
expect(runtime.isTerminalRunningAgent).toHaveBeenCalledWith('terminal-1')
|
||||
})
|
||||
|
||||
it('drops desktop input while a mobile client owns the terminal floor', async () => {
|
||||
it('reclaims the terminal for desktop input while a mobile client owns the floor', async () => {
|
||||
const runtime = stubRuntime({
|
||||
resolveLeafForHandle: vi.fn().mockReturnValue({ ptyId: 'pty-1' }),
|
||||
getDriver: vi.fn().mockReturnValue({ kind: 'mobile', clientId: 'mobile-1' }),
|
||||
sendTerminal: vi.fn(),
|
||||
reclaimTerminalForDesktop: vi.fn().mockResolvedValue(true),
|
||||
sendTerminal: vi.fn().mockResolvedValue({
|
||||
handle: 'terminal-1',
|
||||
accepted: true,
|
||||
bytesWritten: 1
|
||||
}),
|
||||
mobileTookFloor: vi.fn()
|
||||
})
|
||||
const dispatcher = new RpcDispatcher({ runtime, methods: TERMINAL_METHODS })
|
||||
|
|
@ -57,14 +62,13 @@ describe('terminal send RPC', () => {
|
|||
if (!response.ok) {
|
||||
throw new Error(response.error.message)
|
||||
}
|
||||
expect(response.result).toEqual({
|
||||
send: {
|
||||
handle: 'terminal-1',
|
||||
accepted: false,
|
||||
bytesWritten: 0
|
||||
}
|
||||
expect(response.result).toMatchObject({ send: { accepted: true, bytesWritten: 1 } })
|
||||
expect(runtime.reclaimTerminalForDesktop).toHaveBeenCalledWith('pty-1')
|
||||
expect(runtime.sendTerminal).toHaveBeenCalledWith('terminal-1', {
|
||||
text: 'x',
|
||||
enter: false,
|
||||
interrupt: false
|
||||
})
|
||||
expect(runtime.sendTerminal).not.toHaveBeenCalled()
|
||||
expect(runtime.mobileTookFloor).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -475,6 +475,9 @@ describe('connectPanePty', () => {
|
|||
dispatch: vi.fn().mockResolvedValue({ delivered: true }),
|
||||
playSound: vi.fn().mockResolvedValue({ played: true })
|
||||
},
|
||||
runtime: {
|
||||
restoreTerminalFit: vi.fn().mockResolvedValue({ restored: true })
|
||||
},
|
||||
agentStatus: {
|
||||
inferInterrupt: vi.fn().mockResolvedValue(false)
|
||||
}
|
||||
|
|
@ -1943,7 +1946,7 @@ describe('connectPanePty', () => {
|
|||
expect(window.api.agentStatus.inferInterrupt).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does not infer interrupts when mobile presence lock blocks terminal input', async () => {
|
||||
it('reclaims and replays terminal input when mobile presence lock is active', async () => {
|
||||
const { connectPanePty } = await import('./pty-connection')
|
||||
const { setDriverForPty } = await import('@/lib/pane-manager/mobile-driver-state')
|
||||
|
||||
|
|
@ -1988,15 +1991,56 @@ describe('connectPanePty', () => {
|
|||
}
|
||||
terminalTarget.dispatch(keyEvent({ key: 'c', ctrlKey: true }))
|
||||
;(onDataHandler as unknown as (data: string) => void)('\x03')
|
||||
vi.advanceTimersByTime(500)
|
||||
;(onDataHandler as unknown as (data: string) => void)('x')
|
||||
await flushAsyncTicks()
|
||||
|
||||
expect(transport.sendInput).not.toHaveBeenCalled()
|
||||
expect(window.api.runtime.restoreTerminalFit).toHaveBeenCalledWith(ptyId)
|
||||
expect(window.api.runtime.restoreTerminalFit).toHaveBeenCalledTimes(1)
|
||||
expect(transport.sendInput).toHaveBeenCalledWith('\x03')
|
||||
expect(transport.sendInput).toHaveBeenCalledWith('x')
|
||||
expect(window.api.agentStatus.inferInterrupt).not.toHaveBeenCalled()
|
||||
} finally {
|
||||
setDriverForPty(ptyId, { kind: 'idle' })
|
||||
}
|
||||
})
|
||||
|
||||
it('lets remote locked terminal input reach the runtime transport for reclaim', async () => {
|
||||
const { connectPanePty } = await import('./pty-connection')
|
||||
const { setDriverForPty } = await import('@/lib/pane-manager/mobile-driver-state')
|
||||
|
||||
const ptyId = 'remote:env-1@@terminal-1'
|
||||
setDriverForPty(ptyId, { kind: 'mobile', clientId: 'phone-1' })
|
||||
try {
|
||||
const transport = createMockTransport(ptyId)
|
||||
transportFactoryQueue.push(transport)
|
||||
mockStoreState = {
|
||||
...mockStoreState,
|
||||
tabsByWorktree: { 'wt-1': [{ id: 'tab-1', ptyId }] },
|
||||
ptyIdsByTabId: { 'tab-1': [ptyId] }
|
||||
}
|
||||
|
||||
const pane = createPane(1)
|
||||
let onDataHandler: ((data: string) => void) | null = null
|
||||
pane.terminal.onData = vi.fn(((handler: (data: string) => void) => {
|
||||
onDataHandler = handler
|
||||
return { dispose: vi.fn() }
|
||||
}) as typeof pane.terminal.onData)
|
||||
|
||||
connectPanePty(pane as never, createManager(1) as never, createDeps() as never)
|
||||
|
||||
if (!onDataHandler) {
|
||||
throw new Error('expected onData handler to be registered')
|
||||
}
|
||||
;(onDataHandler as unknown as (data: string) => void)('x')
|
||||
await flushAsyncTicks()
|
||||
|
||||
expect(window.api.runtime.restoreTerminalFit).not.toHaveBeenCalled()
|
||||
expect(transport.sendInput).toHaveBeenCalledWith('x')
|
||||
} finally {
|
||||
setDriverForPty(ptyId, { kind: 'idle' })
|
||||
}
|
||||
})
|
||||
|
||||
it('does not infer interrupts when the transport rejects terminal input', async () => {
|
||||
const { connectPanePty } = await import('./pty-connection')
|
||||
|
||||
|
|
|
|||
|
|
@ -16,7 +16,7 @@ import { shouldSeedCacheTimerOnInitialTitle } from './cache-timer-seeding'
|
|||
import type { PtyConnectionDeps } from './pty-connection-types'
|
||||
import { safeFit } from '@/lib/pane-manager/pane-tree-ops'
|
||||
import { getFitOverrideForPty, bindPanePtyId } from '@/lib/pane-manager/mobile-fit-overrides'
|
||||
import { isPtyLocked } from '@/lib/pane-manager/mobile-driver-state'
|
||||
import { isPtyLocked, setDriverForPty } from '@/lib/pane-manager/mobile-driver-state'
|
||||
import { isPaneReplaying, replayIntoTerminal } from './replay-guard'
|
||||
import { terminalOutputPrefersDomRenderer } from '@/lib/pane-manager/terminal-complex-script'
|
||||
import {
|
||||
|
|
@ -121,6 +121,37 @@ function isCodexStartupCommand(command: string): boolean {
|
|||
return executable === 'codex' || executable?.startsWith('codex-') === true
|
||||
}
|
||||
|
||||
const pendingDesktopInputReclaims = new Map<string, Promise<boolean>>()
|
||||
|
||||
async function reclaimLockedPtyForDesktopInput(ptyId: string): Promise<boolean> {
|
||||
if (isRemoteRuntimePtyId(ptyId)) {
|
||||
return true
|
||||
}
|
||||
const pending = pendingDesktopInputReclaims.get(ptyId)
|
||||
if (pending) {
|
||||
return pending
|
||||
}
|
||||
const reclaim = reclaimLocalLockedPtyForDesktopInput(ptyId).finally(() => {
|
||||
pendingDesktopInputReclaims.delete(ptyId)
|
||||
})
|
||||
pendingDesktopInputReclaims.set(ptyId, reclaim)
|
||||
return reclaim
|
||||
}
|
||||
|
||||
async function reclaimLocalLockedPtyForDesktopInput(ptyId: string): Promise<boolean> {
|
||||
try {
|
||||
const restored = (await window.api.runtime.restoreTerminalFit(ptyId)).restored === true
|
||||
if (restored) {
|
||||
// Why: driver-change IPC is asynchronous; unblocking locally avoids
|
||||
// dropping rapid follow-up keystrokes during the same desktop take-back.
|
||||
setDriverForPty(ptyId, { kind: 'desktop' })
|
||||
}
|
||||
return restored || !isPtyLocked(ptyId)
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
function shouldKeepHiddenStartupRendererQueriesLive(
|
||||
startup: PtyConnectionDeps['startup']
|
||||
): boolean {
|
||||
|
|
@ -1202,15 +1233,24 @@ export function connectPanePty(
|
|||
clearPendingTerminalInputIntent()
|
||||
return
|
||||
}
|
||||
// Why: presence-lock input drop. While mobile is the driver for this
|
||||
// PTY, desktop keystrokes must not reach the shell — any input would
|
||||
// race the mobile session and is also dimensionally wrong (PTY is at
|
||||
// phone fit). Renderer-side guard belongs here so we don't even mark
|
||||
// the pane as "interacted" (no unread clear, no take-floor cascade).
|
||||
// The pty:write IPC has a defense-in-depth twin. See
|
||||
// docs/mobile-presence-lock.md.
|
||||
if (currentPtyId && isPtyLocked(currentPtyId)) {
|
||||
clearPendingTerminalInputIntent()
|
||||
// Why: typing into a mobile-driven terminal is a desktop take-back.
|
||||
// Local PTYs must restore first so main's IPC defense accepts the
|
||||
// replay; remote PTYs let the runtime RPC reclaim at its own seam.
|
||||
void reclaimLockedPtyForDesktopInput(currentPtyId).then((reclaimed) => {
|
||||
if (!reclaimed || transport.getPtyId() !== currentPtyId) {
|
||||
return
|
||||
}
|
||||
if (transport.sendInput(data)) {
|
||||
markTerminalInputSent()
|
||||
observeAcceptedTerminalInput(data)
|
||||
observeSentTerminalInputIntent(data)
|
||||
deps.clearTerminalTabUnread(deps.tabId)
|
||||
deps.clearTerminalPaneUnread(cacheKey)
|
||||
deps.clearWorktreeUnread(deps.worktreeId)
|
||||
}
|
||||
})
|
||||
return
|
||||
}
|
||||
// Why: a real keystroke into the terminal is the unambiguous "user is
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
/* eslint-disable max-lines -- Why: these note-send routing cases share one mocked app store and RPC harness. */
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
getActiveAgentNoteTarget,
|
||||
|
|
@ -210,7 +211,12 @@ describe('active agent note send', () => {
|
|||
expect(testState.callRuntimeRpc).toHaveBeenCalledWith(
|
||||
{ kind: 'local' },
|
||||
'terminal.send',
|
||||
{ terminal: 'term-1', text: 'File: src/app.ts', enter: true },
|
||||
{
|
||||
terminal: 'term-1',
|
||||
text: 'File: src/app.ts',
|
||||
enter: true,
|
||||
client: { id: 'orca-desktop', type: 'desktop' }
|
||||
},
|
||||
{ timeoutMs: 15000 }
|
||||
)
|
||||
})
|
||||
|
|
|
|||
|
|
@ -158,7 +158,12 @@ export async function sendNotesToActiveAgentSession({
|
|||
const { send } = await callRuntimeRpc<{ send: RuntimeTerminalSend }>(
|
||||
runtimeTarget,
|
||||
'terminal.send',
|
||||
{ terminal: terminal.handle, text: trimmedPrompt, enter: true },
|
||||
{
|
||||
terminal: terminal.handle,
|
||||
text: trimmedPrompt,
|
||||
enter: true,
|
||||
client: { id: 'orca-desktop', type: 'desktop' }
|
||||
},
|
||||
{ timeoutMs: ACTIVE_AGENT_SEND_RPC_TIMEOUT_MS }
|
||||
)
|
||||
return send.accepted ? { status: 'sent' } : { status: 'not-writable' }
|
||||
|
|
|
|||
|
|
@ -51,7 +51,11 @@ describe('runtime terminal owner routing', () => {
|
|||
expect(runtimeCall).toHaveBeenCalledWith({
|
||||
selector: 'env-1',
|
||||
method: 'terminal.send',
|
||||
params: { terminal: 'terminal-1', text: 'x' },
|
||||
params: {
|
||||
terminal: 'terminal-1',
|
||||
text: 'x',
|
||||
client: { id: 'orca-desktop', type: 'desktop' }
|
||||
},
|
||||
timeoutMs: 15_000
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ export type RuntimeTerminalProcessInspection = {
|
|||
}
|
||||
|
||||
const REMOTE_PTY_ID_PREFIX = 'remote:'
|
||||
const DESKTOP_RUNTIME_CLIENT = { id: 'orca-desktop', type: 'desktop' } as const
|
||||
|
||||
export function isRemoteRuntimePtyId(ptyId: string): boolean {
|
||||
return ptyId.startsWith(REMOTE_PTY_ID_PREFIX)
|
||||
|
|
@ -87,7 +88,7 @@ export function sendRuntimePtyInput(
|
|||
void callRuntimeRpc(
|
||||
target,
|
||||
'terminal.send',
|
||||
{ terminal, text: data },
|
||||
{ terminal, text: data, client: DESKTOP_RUNTIME_CLIENT },
|
||||
{ timeoutMs: 15_000 }
|
||||
).catch(() => {
|
||||
// Why: web session snapshots can retire a remote handle while xterm still
|
||||
|
|
@ -121,7 +122,7 @@ export async function sendRuntimePtyInputVerified(
|
|||
const result = await callRuntimeRpc<{ send: RuntimeTerminalSend }>(
|
||||
target,
|
||||
'terminal.send',
|
||||
{ terminal, text: data, client: { id: 'orca-desktop', type: 'desktop' } },
|
||||
{ terminal, text: data, client: DESKTOP_RUNTIME_CLIENT },
|
||||
{ timeoutMs: 15_000 }
|
||||
)
|
||||
return result.send.accepted === true
|
||||
|
|
|
|||
Loading…
Reference in New Issue