From 77c2cd73562d395c4c18d6bd8c6900aae1ba5ea8 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 31 Aug 2026 22:39:01 -0700 Subject: [PATCH] Add startup delivery diagnostics and success announcements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Terminal sessions now report startup command delivery details (whether written, presence, length, and delivery method) without logging the command text—preventing credential leakage and distinguishing missing commands from lost ones in diagnostics. Setup scripts now announce completion on both POSIX and Windows before executing the startup command, so healthy setups don't appear stuck in the UI with "Waiting for setup..." as the last visible line. Diagnostics failures are caught and ignored so they never break session creation. --- .../daemon/terminal-host-session-create.ts | 18 ++++- src/main/daemon/terminal-host-startup.test.ts | 76 +++++++++++++++++++ src/shared/setup-agent-sequencing.test.ts | 47 ++++++++++++ src/shared/setup-agent-sequencing.ts | 7 +- 4 files changed, 146 insertions(+), 2 deletions(-) diff --git a/src/main/daemon/terminal-host-session-create.ts b/src/main/daemon/terminal-host-session-create.ts index 24f6cc2e867..e1c8a22e750 100644 --- a/src/main/daemon/terminal-host-session-create.ts +++ b/src/main/daemon/terminal-host-session-create.ts @@ -150,7 +150,23 @@ async function spawnAndPublishSession( deps.onSessionCreated(opts.sessionId, opts.agentSessionGeneration, session.isAlive) const token = session.attachClient(opts.streamClient) - if (opts.command && !subprocess.startupCommandDeliveredInShellArgs) { + const startupCommandWritten = + Boolean(opts.command) && !subprocess.startupCommandDeliveredInShellArgs + // Why: without this, a missing command and a lost one log identically. + // Length, never the text -- launches can carry credentials. + try { + deps.reportReadinessEvent?.('startup-command-delivery', { + sessionId: opts.sessionId, + written: startupCommandWritten, + hasCommand: Boolean(opts.command), + commandLength: opts.command?.length ?? 0, + viaShellArgs: subprocess.startupCommandDeliveredInShellArgs === true, + queuedByShellReadyBarrier: shellReadySupported + }) + } catch { + // Diagnostics must never turn a live PTY into a failed create. + } + if (startupCommandWritten && opts.command) { const submit = process.platform === 'win32' ? '\r' : '\n' // Why: only Orca-wrapped shells advertise the paste-safe startup barrier. session.write( diff --git a/src/main/daemon/terminal-host-startup.test.ts b/src/main/daemon/terminal-host-startup.test.ts index fafa34eedc0..e7b7c7b4c09 100644 --- a/src/main/daemon/terminal-host-startup.test.ts +++ b/src/main/daemon/terminal-host-startup.test.ts @@ -54,3 +54,79 @@ describe('TerminalHost startup command terminator', () => { expect(sub.write).toHaveBeenCalledWith(sent) }) }) + +// Why: a missing command and a lost one used to log identically. +describe('TerminalHost startup command delivery logging', () => { + let sub: SubprocessHandle + let events: { event: string; details: Record }[] + let host: TerminalHost + + beforeEach(() => { + sub = mockSubprocess() + events = [] + host = new TerminalHost({ + spawnSubprocess: () => sub, + reportReadinessEvent: (event, details) => events.push({ event, details }) + }) + }) + + const delivery = (): Record => + events.find((e) => e.event === 'startup-command-delivery')?.details ?? {} + + it('records a written startup command', async () => { + await host.createOrAttach({ + sessionId: 'delivery-written', + cols: 80, + rows: 24, + command: 'codex', + shellReadySupported: false, + streamClient: { onData: vi.fn(), onExit: vi.fn() } + }) + expect(delivery()).toMatchObject({ written: true, hasCommand: true, commandLength: 5 }) + }) + + it('records a session created with no startup command at all', async () => { + await host.createOrAttach({ + sessionId: 'delivery-none', + cols: 80, + rows: 24, + shellReadySupported: false, + streamClient: { onData: vi.fn(), onExit: vi.fn() } + }) + expect(delivery()).toMatchObject({ written: false, hasCommand: false, commandLength: 0 }) + expect(sub.write).not.toHaveBeenCalled() + }) + + it('never logs the command text, which can carry credentials', async () => { + await host.createOrAttach({ + sessionId: 'delivery-secret', + cols: 80, + rows: 24, + command: 'deploy --token=hunter2', + shellReadySupported: false, + streamClient: { onData: vi.fn(), onExit: vi.fn() } + }) + expect(JSON.stringify(delivery())).not.toContain('hunter2') + }) + + it('still delivers the command when the diagnostic sink throws', async () => { + host = new TerminalHost({ + spawnSubprocess: () => sub, + reportReadinessEvent: () => { + throw new Error('log sink unavailable') + } + }) + + await expect( + host.createOrAttach({ + sessionId: 'delivery-sink-failure', + cols: 80, + rows: 24, + command: 'codex', + shellReadySupported: false, + streamClient: { onData: vi.fn(), onExit: vi.fn() } + }) + ).resolves.toMatchObject({ isNew: true }) + expect(sub.write).toHaveBeenCalledWith(`codex${process.platform === 'win32' ? '\r' : '\n'}`) + }) +}) diff --git a/src/shared/setup-agent-sequencing.test.ts b/src/shared/setup-agent-sequencing.test.ts index 3d51b5b7476..fd567c145b0 100644 --- a/src/shared/setup-agent-sequencing.test.ts +++ b/src/shared/setup-agent-sequencing.test.ts @@ -9,6 +9,7 @@ import { getDefaultRepoHookSettings } from './constants' import { createSequencedSetupAgentCommands, createSetupAgentSequenceNonce, + SETUP_COMPLETE_MESSAGE, getSetupAgentSequenceShellForTests, resolveSetupAgentSequenceLaunchCommand, SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV, @@ -89,6 +90,52 @@ describe('createSequencedSetupAgentCommands', () => { ) }) + it('announces success so the pane stops showing the waiting line', () => { + const commands = createSequencedSetupAgentCommands({ + runnerScriptPath: '/repo/.git/orca/setup-runner.sh', + startupCommand: 'codex', + platform: 'posix', + nonce: 'nonce-1' + }) + const script = commands.startupEnv?.ORCA_SEQUENCED_STARTUP_SCRIPT ?? '' + // Why ordering: `eval`/`exec` never returns, so a later message never renders. + expect(script.indexOf(SETUP_COMPLETE_MESSAGE)).toBeGreaterThan(-1) + expect(script.indexOf(SETUP_COMPLETE_MESSAGE)).toBeLessThan( + script.indexOf('eval "$ORCA_SEQUENCED_STARTUP_COMMAND"') + ) + }) + + it('announces success on the native Windows gate too', () => { + const commands = createSequencedSetupAgentCommands({ + runnerScriptPath: 'C:\\repo\\.git\\orca\\setup-runner.cmd', + platform: 'windows', + startupCommand: 'codex', + nonce: 'nonce-2' + }) + const decoded = Buffer.from( + commands.startupCommand.split('-EncodedCommand ')[1] ?? '', + 'base64' + ).toString('utf16le') + expect(decoded).toContain(SETUP_COMPLETE_MESSAGE) + expect(decoded.indexOf(SETUP_COMPLETE_MESSAGE)).toBeLessThan( + decoded.indexOf('Invoke-Expression $startup') + ) + }) + + it('leaves the failure and timeout messages as the only other outcomes', () => { + const script = + createSequencedSetupAgentCommands({ + runnerScriptPath: '/repo/.git/orca/setup-runner.sh', + startupCommand: 'codex', + platform: 'posix', + nonce: 'nonce-3' + }).startupEnv?.ORCA_SEQUENCED_STARTUP_SCRIPT ?? '' + // Silence on success is what made a healthy worktree look stuck. + expect(script).toContain(SETUP_COMPLETE_MESSAGE) + expect(script).toContain('Setup failed; skipping agent startup.') + expect(script).toContain('Timed out waiting for setup before starting agent.') + }) + it('keeps the POSIX terminal submission below the canonical input floor', () => { const result = createSequencedSetupAgentCommands({ runnerScriptPath: `/repo/${'nested-worktree/'.repeat(100)}setup-runner.sh`, diff --git a/src/shared/setup-agent-sequencing.ts b/src/shared/setup-agent-sequencing.ts index b93fe9c8306..7108360f645 100644 --- a/src/shared/setup-agent-sequencing.ts +++ b/src/shared/setup-agent-sequencing.ts @@ -8,6 +8,8 @@ import { } from './setup-runner-command' const DEFAULT_WAIT_TIMEOUT_SECONDS = 2 * 60 * 60 +// Exported so the gate and its tests share one definition. +export const SETUP_COMPLETE_MESSAGE = 'Setup finished; starting agent.' export const SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV = 'ORCA_SEQUENCED_STARTUP_COMMAND' export const SETUP_AGENT_SEQUENCE_STARTUP_SCRIPT_ENV = 'ORCA_SEQUENCED_STARTUP_SCRIPT' @@ -126,7 +128,9 @@ function buildPosixStartupScript( `IFS=: read -r seen status < ${marker} || true;`, `if [ "$seen" = ${nonceValue} ]; then`, `rm -f ${marker} ${tmp} 2>/dev/null;`, - `if [ "$status" = "0" ]; then if [ -n "\${${SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV}:-}" ]; then eval "\$${SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV}"; exit "$?"; else ${startupSuccessCommand}; fi; fi;`, + // Why: failure and timeout announce themselves; a silent success left + // "Waiting for setup..." as the pane's last line forever. + `if [ "$status" = "0" ]; then echo ${quotePosixArg(SETUP_COMPLETE_MESSAGE)} >&2; if [ -n "\${${SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV}:-}" ]; then eval "\$${SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV}"; exit "$?"; else ${startupSuccessCommand}; fi; fi;`, 'echo "Setup failed; skipping agent startup." >&2;', 'exit "${status:-1}";', 'fi;', @@ -247,6 +251,7 @@ function buildWindowsStartupCommand( ' [Console]::Error.WriteLine("Missing sequenced startup command.")', ' exit 1', ' }', + ` [Console]::Error.WriteLine(${quotePowerShellString(SETUP_COMPLETE_MESSAGE)})`, ' Invoke-Expression $startup', ' if ($global:LASTEXITCODE -ne $null) { exit $global:LASTEXITCODE }', ' if (-not $?) { exit 1 }',