From 3979c5a159c91cee48b7d5e77deadf17ee861d21 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Sat, 5 Sep 2026 00:42:37 -0400 Subject: [PATCH] fix(ssh): preserve terminal prompt observation deadlines --- src/main/ssh/ssh-host-cli-deadline.ts | 43 +++++++++++++++ src/main/ssh/ssh-remote-cli-args.ts | 21 +------- .../ssh-remote-cli-host-passthrough.test.ts | 21 ++++++++ .../ssh/ssh-remote-cli-host-passthrough.ts | 53 +++---------------- src/relay/remote-cli-timeout.ts | 43 +++++++-------- 5 files changed, 91 insertions(+), 90 deletions(-) create mode 100644 src/main/ssh/ssh-host-cli-deadline.ts diff --git a/src/main/ssh/ssh-host-cli-deadline.ts b/src/main/ssh/ssh-host-cli-deadline.ts new file mode 100644 index 00000000000..fdf64f6e92c --- /dev/null +++ b/src/main/ssh/ssh-host-cli-deadline.ts @@ -0,0 +1,43 @@ +import { parseRemoteCliArgs } from './ssh-remote-cli-args' +import { clampOrchestrationAskTimeoutMs } from '../../shared/orchestration-ask-timeout' +import { + isSafeTimerDelayMs, + parsePositiveSafeIntegerNumericText, + parsePositiveSafeIntegerText +} from '../../shared/timer-delay' + +const DEFAULT_KILL_TIMEOUT_MS = 10 * 60_000 +const KILL_TIMEOUT_GRACE_MS = 2 * 60_000 + +/** Kill timer for the host CLI subprocess. Long-poll commands carry their wait + * budget in `--timeout-ms`; extend past it so the CLI's own timeout fires + * first and produces a proper error message. */ +export function resolveHostCliKillTimeoutMs(argv: string[]): number { + const parsed = parseRemoteCliArgs(argv) + const rawTimeout = parsed.flags.get('timeout-ms') + if (parsed.commandPath[0] === 'terminal' && parsed.commandPath[1] === 'send') { + const rawWait = parsed.flags.get('wait-submit') + const seconds = + typeof rawWait === 'string' ? parsePositiveSafeIntegerNumericText(rawWait) : null + if (seconds !== null && seconds <= 3600) { + return Math.max(DEFAULT_KILL_TIMEOUT_MS, seconds * 1000 + KILL_TIMEOUT_GRACE_MS) + } + } + if (parsed.commandPath[0] === 'orchestration' && parsed.commandPath[1] === 'ask') { + const explicit = + typeof rawTimeout === 'string' ? parsePositiveSafeIntegerText(rawTimeout) : null + return Math.max( + DEFAULT_KILL_TIMEOUT_MS, + clampOrchestrationAskTimeoutMs(explicit ?? undefined) + KILL_TIMEOUT_GRACE_MS + ) + } + const explicit = + typeof rawTimeout === 'string' ? parsePositiveSafeIntegerNumericText(rawTimeout) : null + // Why: this feeds the kill timer directly, so a post-grace budget outside the + // timer range degrades to the default instead of throwing at spawn time. + const extended = explicit === null ? null : explicit + KILL_TIMEOUT_GRACE_MS + if (extended !== null && isSafeTimerDelayMs(extended)) { + return Math.max(DEFAULT_KILL_TIMEOUT_MS, extended) + } + return DEFAULT_KILL_TIMEOUT_MS +} diff --git a/src/main/ssh/ssh-remote-cli-args.ts b/src/main/ssh/ssh-remote-cli-args.ts index bc73eefadcc..4a2e0779cf3 100644 --- a/src/main/ssh/ssh-remote-cli-args.ts +++ b/src/main/ssh/ssh-remote-cli-args.ts @@ -1,3 +1,4 @@ +import { CLI_BOOLEAN_FLAGS } from '../../shared/cli-argument-boundary' import { RemoteCliArgumentError, type ParsedRemoteCli } from './ssh-remote-cli-argument-error' import { isOrchestrationRetryRequestId, @@ -5,24 +6,6 @@ import { VALUELESS_RETRY_REQUEST_GUIDANCE } from '../../shared/orchestration-retry-request-id' -const REMOTE_BOOLEAN_FLAGS = new Set([ - 'all', - 'attachments', - 'children', - 'comments', - 'current', - 'full', - 'help', - 'inject', - 'include-archived', - 'include-visual-layouts', - 'json', - 'me', - 'relations', - 'parent-current', - 'unread', - 'wait' -]) const REPEATED_FLAG_SEPARATOR = '\u0000' const REPEATABLE_REMOTE_STRING_FLAGS = new Set(['label']) @@ -121,7 +104,7 @@ export function optionalRemoteCliNumber( function isRemoteBooleanFlag(flag: string, commandPath: string[]): boolean { // Why: Android launch already uses --activity ; only Linear issue reads use it as a boolean. return ( - REMOTE_BOOLEAN_FLAGS.has(flag) || + CLI_BOOLEAN_FLAGS.has(flag) || (flag === 'activity' && commandPath[0] === 'linear' && commandPath[1] === 'issue') ) } diff --git a/src/main/ssh/ssh-remote-cli-host-passthrough.test.ts b/src/main/ssh/ssh-remote-cli-host-passthrough.test.ts index e66cbd77507..53334e3cd12 100644 --- a/src/main/ssh/ssh-remote-cli-host-passthrough.test.ts +++ b/src/main/ssh/ssh-remote-cli-host-passthrough.test.ts @@ -160,6 +160,27 @@ describe('buildHostCliEnv', () => { }) describe('resolveHostCliKillTimeoutMs', () => { + it.each([ + ['--wait-submit', '3600'], + ['--wait-submit=3600'], + ['--wait-submit=3600.000000000000001'], + ['--wait-submit', '1', '--wait-submit=3600'] + ])('keeps SSH prompt observation inside both outer deadlines: %j', (...waitFlags) => { + const argv = ['terminal', 'send', '--text', 'review', '--enter', ...waitFlags] + const innerTimeout = 3_600_000 + 10_000 + const hostTimeout = resolveHostCliKillTimeoutMs(argv) + const relayTimeout = remoteCliRequestTimeoutMs({ argv })! + expect(hostTimeout).toBeGreaterThan(innerTimeout) + expect(relayTimeout).toBeGreaterThan(hostTimeout) + }) + + it('keeps pre-command Enter flags inside the prompt observation deadline', () => { + const argv = ['--enter', 'terminal', 'send', '--text', 'review', '--wait-submit', '3600'] + const hostTimeout = resolveHostCliKillTimeoutMs(argv) + expect(hostTimeout).toBeGreaterThan(3_610_000) + expect(remoteCliRequestTimeoutMs({ argv })).toBeGreaterThan(hostTimeout) + }) + it('extends the kill timer past an explicit --timeout-ms budget', () => { expect(resolveHostCliKillTimeoutMs(['terminal', 'wait', '--timeout-ms', '1800000'])).toBe( 1_920_000 diff --git a/src/main/ssh/ssh-remote-cli-host-passthrough.ts b/src/main/ssh/ssh-remote-cli-host-passthrough.ts index 05a785c8482..d62a23d8e30 100644 --- a/src/main/ssh/ssh-remote-cli-host-passthrough.ts +++ b/src/main/ssh/ssh-remote-cli-host-passthrough.ts @@ -1,22 +1,12 @@ -// Why: the SSH relay shim (`~/.orca-relay/bin/orca`) forwards CLI invocations -// to the host app. Instead of re-implementing every command in a hand-rolled -// switch (the cause of "Unsupported SSH Orca CLI command", #7716), the host -// runs the real bundled `orca` CLI entry in Electron node mode — the same -// entry the local shell command uses — so remote invocations get the full -// command surface (orchestration, worktree, terminal, ...) by construction. +// The SSH shim runs the bundled CLI so remote shells get the full command surface. import { app } from 'electron' import { spawn as nodeSpawn } from 'node:child_process' import { existsSync } from 'node:fs' import { join } from 'node:path' import { getCanonicalUserDataPath } from '../persistence' -import { parseRemoteCliArgs } from './ssh-remote-cli-args' -import { clampOrchestrationAskTimeoutMs } from '../../shared/orchestration-ask-timeout' -import { - MAX_TIMER_DELAY_MS, - isSafeTimerDelayMs, - parsePositiveSafeIntegerNumericText, - parsePositiveSafeIntegerText -} from '../../shared/timer-delay' +import { resolveHostCliKillTimeoutMs } from './ssh-host-cli-deadline' +export { resolveHostCliKillTimeoutMs } from './ssh-host-cli-deadline' +import { MAX_TIMER_DELAY_MS, isSafeTimerDelayMs } from '../../shared/timer-delay' import { ORCHESTRATION_COMPATIBILITY_ATTACHMENT_ENV, ORCHESTRATION_COMPATIBILITY_HOST_ID_ENV, @@ -81,10 +71,7 @@ export type HostCliPassthroughOptions = { * working even on broken installs. */ export class HostCliUnavailableError extends Error {} -// Why: only Orca terminal-context vars may cross from the remote shell into -// the host CLI process. Remote PATH / ORCA_USER_DATA_PATH are paths on the -// remote machine (meaningless or instance-hijacking on the host), and -// NODE_OPTIONS-style vars could alter host execution. +// Only terminal identity may cross hosts; remote paths and Node options cannot. const REMOTE_CONTEXT_ENV_VARS = [ 'ORCA_TERMINAL_HANDLE', 'ORCA_WORKTREE_ID', @@ -93,11 +80,8 @@ const REMOTE_CONTEXT_ENV_VARS = [ 'ORCA_WORKSPACE_ID' ] as const -// Why: bound captured output so a runaway command cannot balloon the relay -// JSON-RPC response or main-process memory. +// Bound output retained for the relay response. const MAX_CAPTURED_OUTPUT_BYTES = 8 * 1024 * 1024 -const DEFAULT_KILL_TIMEOUT_MS = 10 * 60_000 -const KILL_TIMEOUT_GRACE_MS = 2 * 60_000 export function resolveHostCliEntryPath(app: { isPackaged: boolean @@ -112,31 +96,6 @@ export function resolveHostCliEntryPath(app: { : join(app.appPath, 'out', 'cli', 'index.js') } -/** Kill timer for the host CLI subprocess. Long-poll commands carry their wait - * budget in `--timeout-ms`; extend past it so the CLI's own timeout fires - * first and produces a proper error message. */ -export function resolveHostCliKillTimeoutMs(argv: string[]): number { - const parsed = parseRemoteCliArgs(argv) - const rawTimeout = parsed.flags.get('timeout-ms') - if (parsed.commandPath[0] === 'orchestration' && parsed.commandPath[1] === 'ask') { - const explicit = - typeof rawTimeout === 'string' ? parsePositiveSafeIntegerText(rawTimeout) : null - return Math.max( - DEFAULT_KILL_TIMEOUT_MS, - clampOrchestrationAskTimeoutMs(explicit ?? undefined) + KILL_TIMEOUT_GRACE_MS - ) - } - const explicit = - typeof rawTimeout === 'string' ? parsePositiveSafeIntegerNumericText(rawTimeout) : null - // Why: this feeds the kill timer directly, so a post-grace budget outside the - // timer range degrades to the default instead of throwing at spawn time. - const extended = explicit === null ? null : explicit + KILL_TIMEOUT_GRACE_MS - if (extended !== null && isSafeTimerDelayMs(extended)) { - return Math.max(DEFAULT_KILL_TIMEOUT_MS, extended) - } - return DEFAULT_KILL_TIMEOUT_MS -} - export function buildHostCliEnv(args: { hostEnv: NodeJS.ProcessEnv remoteEnv: Record diff --git a/src/relay/remote-cli-timeout.ts b/src/relay/remote-cli-timeout.ts index f03f845283b..97cacda3e33 100644 --- a/src/relay/remote-cli-timeout.ts +++ b/src/relay/remote-cli-timeout.ts @@ -1,3 +1,4 @@ +import { CLI_BOOLEAN_FLAGS } from '../shared/cli-argument-boundary' import { clampOrchestrationAskTimeoutMs } from '../shared/orchestration-ask-timeout' import { isSafeTimerDelayMs, @@ -14,23 +15,8 @@ import { const REMOTE_CLI_DEFAULT_TIMEOUT_MS = 5 * 60_000 const REMOTE_CLI_WAIT_TIMEOUT_MS = 10 * 60_000 const REMOTE_CLI_TIMEOUT_GRACE_MS = 60_000 -const ORCHESTRATION_ASK_RELAY_GRACE_MS = 3 * 60_000 -const ORCHESTRATION_ASK_RELAY_BASE_MS = 11 * 60_000 - -const REMOTE_TIMEOUT_BOOLEAN_FLAGS = new Set([ - 'all', - 'attachments', - 'children', - 'comments', - 'current', - 'full', - 'help', - 'inject', - 'json', - 'relations', - 'unread', - 'wait' -]) +const REMOTE_CLI_LONG_WAIT_GRACE_MS = 3 * 60_000 +const REMOTE_CLI_LONG_WAIT_BASE_MS = 11 * 60_000 export function remoteCliRequestTimeoutMs(params: Record): number | undefined { const argv = getStringArgv(params) @@ -38,12 +24,20 @@ export function remoteCliRequestTimeoutMs(params: Record): numb return undefined } const commandPath = parseRemoteCommandPath(argv) - const timeoutFlag = findLastTimeoutMsFlag(argv) + const timeoutFlag = findLastTimeoutFlag(argv, 'timeout-ms') + if (commandPath[0] === 'terminal' && commandPath[1] === 'send') { + const waitFlag = findLastTimeoutFlag(argv, 'wait-submit') + const seconds = + waitFlag?.raw === undefined ? null : parsePositiveSafeIntegerNumericText(waitFlag.raw) + if (seconds !== null && seconds <= 3600) { + return Math.max(REMOTE_CLI_LONG_WAIT_BASE_MS, seconds * 1000 + REMOTE_CLI_LONG_WAIT_GRACE_MS) + } + } if (commandPath[0] === 'orchestration' && commandPath[1] === 'ask') { const parsed = timeoutFlag?.raw === undefined ? null : parsePositiveSafeIntegerText(timeoutFlag.raw) const effective = clampOrchestrationAskTimeoutMs(parsed ?? undefined) - return Math.max(ORCHESTRATION_ASK_RELAY_BASE_MS, effective + ORCHESTRATION_ASK_RELAY_GRACE_MS) + return Math.max(REMOTE_CLI_LONG_WAIT_BASE_MS, effective + REMOTE_CLI_LONG_WAIT_GRACE_MS) } const base = isWaitStyleCliRequest(argv, commandPath) ? REMOTE_CLI_WAIT_TIMEOUT_MS @@ -69,15 +63,16 @@ function isWaitStyleCliRequest(argv: string[], commandPath: string[]): boolean { ) } -function findLastTimeoutMsFlag(argv: string[]): { raw: string | undefined } | null { +function findLastTimeoutFlag(argv: string[], name: string): { raw: string | undefined } | null { + const flag = `--${name}` let result: { raw: string | undefined } | null = null for (let index = 0; index < argv.length; index += 1) { const token = argv[index] - if (token === '--timeout-ms') { + if (token === flag) { const next = argv[index + 1] result = { raw: next?.startsWith('--') ? undefined : next } - } else if (token.startsWith('--timeout-ms=')) { - result = { raw: token.slice('--timeout-ms='.length) } + } else if (token.startsWith(`${flag}=`)) { + result = { raw: token.slice(flag.length + 1) } } } return result @@ -106,7 +101,7 @@ function parseRemoteCommandPath(argv: string[]): string[] { } const next = argv[index + 1] - if (!REMOTE_TIMEOUT_BOOLEAN_FLAGS.has(assignment) && next && !next.startsWith('--')) { + if (!CLI_BOOLEAN_FLAGS.has(assignment) && next && !next.startsWith('--')) { index += 1 } }