diff --git a/LANE-REPORT.md b/LANE-REPORT.md new file mode 100644 index 00000000000..9e8284fa551 --- /dev/null +++ b/LANE-REPORT.md @@ -0,0 +1,246 @@ +# Lane 2 — Setup failed, but the sequenced agent startup waited forever + +Branch: `brennanb2025/setup-agent-seq-hang` (off `origin/main` @ fecdf0bde8) +Worktree: `/Users/brennanbenson/orca/workspaces/orca/setup-agent-seq-hang` + +## Root cause + +The agent gate's only evidence that setup happened is a marker file written by the gated setup +command, and nothing guaranteed that command ever ran: in +`src/shared/setup-agent-sequencing.ts`, `buildPosixSetupCommand` inlined the runner path three +times plus the nonce into one typed line — **1033 bytes for the worktree in the recording, past the +1024-byte canonical input cap a PTY applies before the shell's line editor takes over** — so the +marker was never written and `buildPosixStartupScript` sat silent for the full +`DEFAULT_WAIT_TIMEOUT_SECONDS = 2 * 60 * 60`. + +That is one instance of a structural defect with three faces, all fixed here: + +| Face | Where | Effect | +|---|---|---| +| Gated command too long to submit | `buildPosixSetupCommand` | marker never written | +| Bare runner launched instead of the gated command | `launch-worktree-background-terminals.ts` `buildSetupCommand` ignored `setup.command` outright | marker never written | +| Setup never launched at all | spawn failures swallowed in `provisionManagedWorktreeTerminals`; the non-awaited branch sets `didSpawnSetup = true` optimistically | marker never written, and the client is told not to retry | + +In every case the waiter had no second source of truth and no way to say so. + +### On the coordinator's lead + +The lead named `activationSetup` in `src/main/ipc/worktree-remote.ts` only carrying +`command: wrappedSetupCommandStr` when **both** `startupTerminalHandle` and +`wrappedSetupCommandStr` are set. **Refuted as the trigger**, but it was a real smell: at that +point `startupTerminalHandle` is always non-null (the only path that leaves it null returns early +with no `activationSetup`), so the conjunct was dead weight hiding the real seam — that the gated +command and the state it depends on were two separable values threaded by hand through four +launchers. It is gone now. + +### Evidence for the length finding + +- The runner for the recorded worktree still exists on disk: + `/Users/brennanbenson/orca/orca/.git/worktrees/fix-agent-hooks-post-posix-payloads-as-json/orca/setup-runner.sh`, + written `Aug 27 07:04` — matching the 7:04 phone clock in `t009.png`. **No `.done` marker + sibling was ever written**, and the pre-fix gate deletes the marker only when it consumes it, + which it demonstrably never did. +- Feeding that exact path through the unfixed `createSequencedSetupAgentCommands` yields a + **1033-byte** `setupCommand`. `TTYHOG` on Darwin/BSD is 1024. +- Frame `e045.png` (t≈45s, re-extracted with output seeking) shows the origin: the mobile + **Create worktree** sheet, Project `orca`, Run on **Local Mac**, from issue #11292, Agent + **Claude** — a host-side create, exactly as the brief assumed. + +Honest limit: I proved the marker was never written and that the command exceeds the cap for this +worktree. I could not replay which of the three faces fired in the recording — the host does not +retain the PTY command line, and the terminal-history entries for those two PTYs had already +rotated. The fix closes all three regardless, which is what the brief asked for. + +## The fix + +### (a) An outcome is recorded on every path — structurally + +1. **The gated setup command is now short and unsubmittable-proof.** The long script moved into + `ORCA_SEQUENCED_SETUP_SCRIPT`, the same env-var indirection the startup gate already used (its + own comment cites this hazard). The command is ~235 bytes and, critically, carries an inline + fallback: if the env var is missing it runs the bare runner rather than nothing. +2. **The status is written from a shell `trap ... EXIT`** (POSIX) / `try…finally` (Windows), so a + runner that exits non-zero, aborts under `set -e`, cannot be executed at all (127), or is torn + down with its pane still records an outcome. Previously only a clean fall-through did. +3. **A `.started` sentinel is written before the script body runs.** Its *absence* is how the + never-started case gets recorded — from the waiter, which runs on the execution host, rather + than from a main process that may not share a filesystem with it. This is what makes the + guarantee structural rather than one more patched branch. +4. **The gated command and its env can no longer be separated.** `applySequencedSetupLaunch()` + folds both into the `WorktreeSetupLaunch` record every launcher already consumes, and the + separate `wrappedSetupCommandStr` parameter threaded through four call sites is deleted. A + launcher cannot now pick up `command` while dropping the half that records the outcome. +5. `launch-worktree-background-terminals.ts` honours `setup.command` instead of always rebuilding + the bare runner. + +### (b) The bound, and why this number + +`DEFAULT_WAIT_TIMEOUT_SECONDS` **2 h → 30 min**, plus `SETUP_START_GRACE_SECONDS = 45` and a +progress line every `WAIT_PROGRESS_INTERVAL_SECONDS = 15`. + +- **30 minutes.** The slowest legitimate setup we ship against is a cold monorepo install plus a + native rebuild — single-digit minutes even on a slow link (this repo's own `pnpm install` + + `rebuild-native-deps` failed at 10.2 s in the recording). 30 min is several times that headroom, + so it will not cut off a genuinely slow install, while still surfacing inside one sitting rather + than half a workday. The bound is now a backstop, not the primary signal. +- **15 s progress ticks** are the actual fix for "indistinguishable from a hang" — the terminal is + never silent again, which is why the bound can stay generous. +- **45 s start grace.** The setup terminal is spawned in the same host operation as the agent + terminal and writes its sentinel before running a line of script, so 45 s is far beyond a slow + shell profile plus an SSH round trip. Expiring does **not** fail the launch: it starts the agent + unsequenced and says why, so a false positive costs a warning line, never a dead terminal. + +### (c) The user is told in the agent terminal + +Three messages, all on the terminal the user is already looking at: + +- setup failed → `Setup failed; skipping agent startup. Setup exited with status 9; open the Setup + tab for its output, then start the agent yourself once it is fixed.` +- setup never reported starting → `Setup never reported starting within 45s, so this terminal + cannot tell whether it ran. Starting the agent without waiting for setup.` +- bound reached → `Timed out waiting for setup before starting agent. Waited 1800s without a + result; the agent was not started. Open the Setup tab for its output.` + +Wording is deliberate on the second: not being able to see setup is not evidence that it died, so +it says the terminal cannot tell, and proceeds. That trades the #6298 ordering guarantee for a +live terminal — the pre-#6298 behaviour — only in the case where the guarantee was already void. + +## Regression tests — failing against the unfixed code + +`src/shared/setup-agent-sequencing.prefix-proof.test.ts` (scratch, not committed) expressed the +required scenarios in HEAD's API and ran against the **unfixed** `setup-agent-sequencing.ts`. +**7 of 7 failed.** + +``` + ❯ src/shared/setup-agent-sequencing.prefix-proof.test.ts (7 tests | 7 failed) + × setup submission stays under the canonical input floor 3ms + × setup exits non-zero: the gate names the status in the agent terminal 1398ms + × setup killed mid-run still records an outcome 5005ms + × setup never started: the agent still launches instead of waiting out the bound 4116ms + × wrapped command absent: the bare runner launch still records an outcome 5002ms + × the wait reports progress instead of sitting silent 5002ms + × the default bound is not a two-hour silent wait 1ms + +AssertionError: expected 1033 to be less than 1024 + ❯ src/shared/setup-agent-sequencing.prefix-proof.test.ts:28:40 + +AssertionError: expected 'Waiting for setup to finish before st…' to contain 'Setup exited with status 7' +- Setup exited with status 7 ++ Waiting for setup to finish before starting agent... ++ Setup failed; skipping agent startup. + +AssertionError: expected 124 to be +0 // Object.is equality (setup never started) +- 0 ++ 124 + +AssertionError: expected 'deadline=$((SECONDS + 7200)); echo "W…' to contain 'deadline=$((SECONDS + 1800))' + +Error: Test timed out in 5000ms. (killed mid-run / wrapped command absent / progress) +``` + +Note `expected 1033 to be less than 1024` — the incident measurement, from the recorded worktree's +own runner path. The three "test timed out" entries are the unfixed code literally hanging. + +The committed equivalents live in `src/shared/setup-agent-sequencing.test.ts` under +`describe('setup outcome recording')` and spawn real `bash`: + +- `records a non-zero setup status so the agent gate reports the failure` — **setup exits non-zero** +- `records an outcome when the setup runner cannot be executed at all` — **setup exits non-zero (127)** +- `records an outcome when the setup pane is torn down mid-run` — signals the pane's process group; + asserts the marker reads `killed-setup:143` +- `starts the agent unsequenced when setup is never started at all` — **setup never started** +- `still records an outcome when the setup script env never reaches the setup terminal` — + **wrapped command absent**: the setup PTY is spawned without `setupEnv`, the fallback still runs + setup, and the gate still resolves +- `reports progress instead of waiting silently` +- `keeps the POSIX setup submission below the canonical input floor` +- `pairs the gated setup command with the env that carries its script` +- `bounds the default wait well under the two-hour silent timeout it replaced` + +## Electron QA — rendered, on my own instance + +Own dev instance: CDP `9336`, renderer `5180`, `ORCA_DEV_USER_DATA_PATH=/tmp/orca-lane2-profile.*`, +`ORCA_DEV_INSTANCE_KEY=lane2-setup-seq`. `HOME` untouched. Identity verified **before** any +capture: `devRepoRoot: /Users/brennanbenson/orca/workspaces/orca/setup-agent-seq-hang`, +`devBranch: brennanb2025/setup-agent-seq-hang`. Playwright CDP only — no computer-use, no OS +automation. Torn down by pgid `74450` after confirming its argv pointed at this worktree; the +user's `:5173` dev server was still listening afterward. + +Scenario: fixture repo with an `orca.yaml` setup hook that exits 9, repo policy set to +`wait-for-setup` through the real store action, workspace created through `createWorktree` with an +agent startup. + +Screenshots in `~/orca-qa/mobile-video-triage-2026-08-27/lane2-setup-hang/`: + +- `fix-agent-terminal-reports-failure.png` — the agent terminal, same shape as the recording + (`bash -lc 'eval "$ORCA_SEQUENCED_STARTUP_SCRIPT"'` → `Waiting for setup to finish before + starting agent...`), now followed within seconds by `Setup started; waiting for it to finish.` + and `Setup failed; skipping agent startup. Setup exited with status 9; open the Setup tab…` +- `fix-setup-tab-runs-gated-command.png` — the Setup tab running the new short gated command with + its `ORCA_SEQUENCED_SETUP_SCRIPT` branch, then the fixture's failure output +- `fix-success-path-agent-starts.png` — setup flipped to exit 0: `Setup started; waiting for it to + finish.` → `AGENT_ACTUALLY_STARTED`. The #6298 ordering guarantee is intact. + +**Not verified in the rendered app:** the never-started and torn-down-mid-run arms (covered by the +shell-level tests only — forcing them through the UI is racy), and every Windows path. The Windows +gate changes mirror the POSIX ones and are covered by unit assertions on the generated PowerShell, +but no Windows machine ran them. + +## Scope constraints + +- **SSH / remote.** The outcome is recorded by, and read by, shell running on the execution host — + no main-process filesystem write is introduced, so nothing assumes the client shares a disk with + the worktree. The never-started message says the terminal cannot tell whether setup ran; it never + claims setup died. No `live`/`unverifiable`/`exited` verdict is emitted or changed. +- **Remote wire.** No new field and no new stream opcode. `command` and `envVars` are existing + `WorktreeSetupLaunch` fields that already cross the boundary; the change is additive keys inside + the existing `envVars` map. New host + old client: the client forwards `envVars` verbatim, so the + gate works. Old host + new client: the client receives a long unwrapped command as before, and + the new start-grace and 30-minute bound now protect it instead of a two-hour silence. +- **Windows.** The runner file's `.cmd`-vs-`#!` selection is untouched; no `cmd.exe /c` is + introduced; the Windows gate stays on `-EncodedCommand` (a single base64 token, not subject to + the POSIX cap) and gained only the sentinel and the `try…finally` status write. No direct + `child_process` use added. +- **Folder workspaces.** No new assumption that a workspace is a git worktree; the runner path + still comes from the existing `createSetupRunnerScript` resolution. +- `src/shared/setup-agent-sequencing.ts` crossed the 300-line `max-lines` limit, so it was **split** + into `-env`, `-posix-gate`, and `-windows-gate` modules. No `max-lines` disable was added. + +## Gates + +| Gate | Result | +|---|---| +| `pnpm tc` | clean | +| Affected vitest (12 files) | **1371 passed, 10 skipped, 0 failed** | +| oxlint — code quality | clean | +| oxlint — type-aware code quality | clean | +| oxlint — React Doctor | clean | + +`pnpm run check:code-quality:changed` **crashes on Node 26** before linting anything — the pnpm +engine warning lands in the stream it `JSON.parse`s, unrelated to this change. The three +`OXLINT_SCANS` from that script were run directly against the changed files instead (results +above); oxlint confirmed live at 167 rules, and it did catch the `max-lines` violation, which was +fixed by splitting rather than suppressing. + +One process note: `pnpm format ` reflowed 51 unrelated files across the repo despite the path +argument. Those were reverted; the change set is 9 files. + +## Files changed + +``` +src/shared/setup-agent-sequencing.ts (split; gate now records + reports) +src/shared/setup-agent-sequencing-env.ts (new) +src/shared/setup-agent-sequencing-posix-gate.ts (new) +src/shared/setup-agent-sequencing-windows-gate.ts (new) +src/shared/setup-agent-sequencing.test.ts (regression tests) +src/main/ipc/worktree-remote.ts +src/main/runtime/orca-runtime.ts +src/main/ipc/worktrees-local-create-flow.test.ts +src/main/runtime/orca-runtime.test.ts +src/renderer/src/lib/launch-worktree-background-terminals.ts +src/renderer/src/lib/worktree-initial-terminal-seeding.ts +src/renderer/src/lib/worktree-default-terminal-tabs.ts +src/renderer/src/lib/worktree-setup-issue-command-queue.ts +src/renderer/src/lib/worktree-activation-setup-script.test.ts +src/renderer/src/lib/worktree-activation-web-runtime.test.ts +``` diff --git a/src/main/ipc/worktree-remote.ts b/src/main/ipc/worktree-remote.ts index 5e1f004972b..3d2117c2ec3 100644 --- a/src/main/ipc/worktree-remote.ts +++ b/src/main/ipc/worktree-remote.ts @@ -127,7 +127,10 @@ import { buildSetupRunnerCommand, getSetupRunnerCommandPlatformForPath } from '../../shared/setup-runner-command' -import { createSequencedSetupAgentCommands } from '../../shared/setup-agent-sequencing' +import { + applySequencedSetupLaunch, + createSequencedSetupAgentCommands +} from '../../shared/setup-agent-sequencing' import { shouldWaitForSetupBeforeAgentStartup } from '../../shared/setup-agent-startup-policy' import { createWorktreeCreateTimingRecorder } from '../worktree-create-timing' import { @@ -364,7 +367,10 @@ async function spawnLocalStartupAndSetupTerminals(args: { let startupTerminal: CreateWorktreeResult['startupTerminal'] let sequencedStartup = startup - let wrappedSetupCommandStr: string | undefined + // Why: the gated setup command and the env carrying its script travel together as one launch + // record, so no downstream branch can run the setup runner without the half that records the + // outcome the agent terminal is waiting on. + let sequencedSetup: CreateWorktreeResult['setup'] if (startup && setup?.waitForAgentStartup === true) { const platform = getSetupRunnerCommandPlatformForLaunch( setup, @@ -381,7 +387,7 @@ async function spawnLocalStartupAndSetupTerminals(args: { command: sequenced.startupCommand, ...(sequenced.startupEnv ? { env: { ...startup.env, ...sequenced.startupEnv } } : {}) } - wrappedSetupCommandStr = sequenced.setupCommand + sequencedSetup = applySequencedSetupLaunch(setup, sequenced) } try { @@ -426,8 +432,9 @@ async function spawnLocalStartupAndSetupTerminals(args: { let didSpawnSetup = false if (setup) { try { + const setupLaunch = sequencedSetup ?? setup const setupCommand = - wrappedSetupCommandStr ?? + setupLaunch.command ?? buildSetupRunnerCommand( setup.runnerScriptPath, getSetupRunnerCommandPlatformForLaunch( @@ -446,14 +453,14 @@ async function spawnLocalStartupAndSetupTerminals(args: { await runtime.splitTerminal(startupTerminalHandle, { direction: setupLaunchMode === 'split-horizontal' ? 'horizontal' : 'vertical', command: setupCommand, - env: setup.envVars, + env: setupLaunch.envVars, activate: false }) } else { await runtime.createTerminal(`id:${worktree.id}`, { title: 'Setup', command: setupCommand, - env: setup.envVars, + env: setupLaunch.envVars, activate: false }) } @@ -467,16 +474,7 @@ async function spawnLocalStartupAndSetupTerminals(args: { } return { - ...(setup && !didSpawnSetup - ? { - activationSetup: { - ...setup, - ...(startupTerminalHandle && wrappedSetupCommandStr - ? { command: wrappedSetupCommandStr } - : {}) - } - } - : {}), + ...(setup && !didSpawnSetup ? { activationSetup: sequencedSetup ?? setup } : {}), ...(startupTerminal ? { startupTerminal } : {}), didSpawnSetup, ...(warning ? { warning } : {}) diff --git a/src/main/ipc/worktrees-local-create-flow.test.ts b/src/main/ipc/worktrees-local-create-flow.test.ts index d01efc489da..2d8ce0a1180 100644 --- a/src/main/ipc/worktrees-local-create-flow.test.ts +++ b/src/main/ipc/worktrees-local-create-flow.test.ts @@ -1,6 +1,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { resolve } from 'node:path' import type { CreateWorktreeResult } from '../../shared/worktree/create-types' +import { SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV } from '../../shared/setup-agent-sequencing' import { resolveRegisteredWorktreePath } from './registered-worktree-roots-cache' import { listWorktreesMock, @@ -738,7 +739,9 @@ describe('registerWorktreeHandlers', () => { request_kind: 'new' } } - })) as { setup?: { command?: string; runnerScriptPath: string } } + })) as { + setup?: { command?: string; runnerScriptPath: string; envVars?: Record } + } expect(result.setup).toEqual( expect.objectContaining({ @@ -746,7 +749,7 @@ describe('registerWorktreeHandlers', () => { command: expect.stringContaining('bash /mnt/c/workspace/repo/.git/orca/setup-runner.sh') }) ) - expect(result.setup?.command).toContain('printf') + expect(result.setup?.envVars?.[SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV]).toContain('printf') }) it('rejects ask-policy creates before mutating git state when setup decision is missing', async () => { diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index e4c39a32583..e95d3e4e0a0 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -126,6 +126,7 @@ import { import { advertisedUrlWatcher } from '../ports/advertised-url-watcher' import { makePaneKey } from '../../shared/stable-pane-id' import { + SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV, SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV, SETUP_AGENT_SEQUENCE_STARTUP_SCRIPT_ENV } from '../../shared/setup-agent-sequencing' @@ -7066,15 +7067,21 @@ describe('OrcaRuntimeService', () => { } const startupCommand = startup.command const startupScript = startup.env[SETUP_AGENT_SEQUENCE_STARTUP_SCRIPT_ENV]! - const setupCommand = (spawn.mock.calls[1]![0] as { command: string }).command + const setupSpawn = spawn.mock.calls[1]![0] as { + command: string + env: Record + } + const setupCommand = setupSpawn.command + const setupScript = setupSpawn.env[SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV]! const nonceMatch = startupScript.match(/if \[ "\$seen" = ([0-9a-f-]+) \]/) expect(nonceMatch?.[1]).toBeTruthy() const markerPath = `/remote/repo/.git/worktrees/mobile-setup/orca/setup-runner.sh.${nonceMatch![1]}.done` expect(startupCommand.length).toBeLessThan(256) - expect(setupCommand).toContain('printf') - expect(setupCommand).toContain(`${nonceMatch![1]} "$status"`) + expect(setupCommand.length).toBeLessThan(1024) + expect(setupScript).toContain('printf') + expect(setupScript).toContain(`${nonceMatch![1]} "$1"`) expect(startupScript).toContain(markerPath) - expect(setupCommand).toContain(markerPath) + expect(setupScript).toContain(markerPath) expect(revealTerminalSession).toHaveBeenLastCalledWith( result.worktree.id, expect.objectContaining({ @@ -45973,15 +45980,20 @@ describe('OrcaRuntimeService', () => { } const startupCommand = startup.command const startupScript = startup.env[SETUP_AGENT_SEQUENCE_STARTUP_SCRIPT_ENV]! - const setupCommand = (spawn.mock.calls[1]![0] as { command: string }).command + const setupSpawn = spawn.mock.calls[1]![0] as { + command: string + env: Record + } + const setupCommand = setupSpawn.command + const setupScript = setupSpawn.env[SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV]! const nonceMatch = startupScript.match(/if \[ "\$seen" = ([0-9a-f-]+) \]/) expect(nonceMatch?.[1]).toBeTruthy() expect(startupCommand.length).toBeLessThan(256) expect(startupScript).toContain('exec claude') expect(startupScript).toContain('/mnt/c/tmp/repo/.git/orca/setup-runner.sh') expect(setupCommand).toContain('bash /mnt/c/tmp/repo/.git/orca/setup-runner.sh') - expect(setupCommand).toContain('printf') - expect(setupCommand).toContain(`${nonceMatch![1]} "$status"`) + expect(setupScript).toContain('printf') + expect(setupScript).toContain(`${nonceMatch![1]} "$1"`) expect(result.setup).toBeUndefined() }) @@ -47358,16 +47370,21 @@ describe('OrcaRuntimeService', () => { } const startupCommand = startup.command const startupScript = startup.env[SETUP_AGENT_SEQUENCE_STARTUP_SCRIPT_ENV]! - const setupCommand = (spawn.mock.calls[1]![0] as { command: string }).command + const setupSpawn = spawn.mock.calls[1]![0] as { + command: string + env: Record + } + const setupScript = setupSpawn.env[SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV]! const nonceMatch = startupScript.match(/if \[ "\$seen" = ([0-9a-f-]+) \]/) expect(nonceMatch?.[1]).toBeTruthy() const markerPath = `/tmp/repo/.git/orca/setup-runner.sh.${nonceMatch![1]}.done` expect(startupCommand.length).toBeLessThan(256) + expect(setupSpawn.command.length).toBeLessThan(1024) expect(startupScript).toContain('--dangerously-bypass-approvals-and-sandbox') - expect(setupCommand).toContain('printf') - expect(setupCommand).toContain(`${nonceMatch![1]} "$status"`) + expect(setupScript).toContain('printf') + expect(setupScript).toContain(`${nonceMatch![1]} "$1"`) expect(startupScript).toContain(markerPath) - expect(setupCommand).toContain(markerPath) + expect(setupScript).toContain(markerPath) const mainEnv = (spawn.mock.calls[0]![0] as { env?: Record }).env ?? {} const setupEnv = (spawn.mock.calls[1]![0] as { env?: Record }).env ?? {} expect(result.setup).toBeUndefined() @@ -47466,8 +47483,12 @@ describe('OrcaRuntimeService', () => { undefined, undefined ) - const activationSetup = activateWorktree.mock.calls[0]?.[2] as { command?: string } | undefined - expect(activationSetup?.command).toContain('printf') + const activationSetup = activateWorktree.mock.calls[0]?.[2] as + | { command?: string; envVars?: Record } + | undefined + // Why: the retry the renderer performs must carry the gate script alongside the command, + // or the Setup tab it opens records no outcome for the waiting agent terminal. + expect(activationSetup?.envVars?.[SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV]).toContain('printf') }) it('lets explicit startup draft agents override the desktop default', async () => { diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index c8a9c7eb376..78c1cc3c0f1 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -617,6 +617,7 @@ import { getSetupRunnerCommandPlatformForPath } from '../../shared/setup-runner-command' import { + applySequencedSetupLaunch, createSequencedSetupAgentCommands, SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV } from '../../shared/setup-agent-sequencing' @@ -26807,10 +26808,10 @@ export class OrcaRuntimeService { setupCommandPlatform: 'windows' | 'posix' observeSetupCompletion?: boolean // Why: when the agent startup is sequenced to wait for setup - // (waitForAgentStartup), the startup PTY runs a wrapper that already embeds - // the setup command. Pass that wrapped command through so the Setup tab runs - // the same script the agent is waiting on instead of a bare runner. - wrappedSetupCommand?: string + // (waitForAgentStartup), the startup PTY polls for an outcome only this gated launch + // records. Pass the whole sequenced launch record — command plus the env carrying its + // script — so the Setup tab runs the script the agent waits on, not a bare runner. + sequencedSetup?: CreateWorktreeResult['setup'] // Why: a workspace provisioned in the background must not pull the sidebar // to itself; the user never asked to look at these tabs. surfaceOwner?: false @@ -26840,8 +26841,9 @@ export class OrcaRuntimeService { primaryTerminalHandle = terminal.handle } if (args.setup) { + const setupLaunch = args.sequencedSetup ?? args.setup const completionToken = - args.observeSetupCompletion && !args.wrappedSetupCommand ? randomUUID() : null + args.observeSetupCompletion && !setupLaunch.command ? randomUUID() : null const observedCommand = completionToken ? buildObservedSetupCommand( args.setup.runnerScriptPath, @@ -26851,14 +26853,14 @@ export class OrcaRuntimeService { ) : null const setupCommand = - args.wrappedSetupCommand ?? + setupLaunch.command ?? observedCommand?.command ?? buildSetupRunnerCommand( args.setup.runnerScriptPath, args.setupCommandPlatform, args.setup.shell ) - const setupEnv = { ...args.setup.envVars, ...observedCommand?.env } + const setupEnv = { ...setupLaunch.envVars, ...observedCommand?.env } const shouldSplitSetup = primaryTerminalHandle && (setupLaunchMode === 'split-vertical' || setupLaunchMode === 'split-horizontal') @@ -27941,7 +27943,7 @@ export class OrcaRuntimeService { let startupTerminalPtyId: string | null = null let sequencedStartup = effectiveStartup - let wrappedSetupCommandStr: string | undefined + let sequencedSetup: CreateWorktreeResult['setup'] if (effectiveStartup && setup?.waitForAgentStartup === true) { const platform = getSetupRunnerCommandPlatformForLaunch( setup, @@ -27960,7 +27962,7 @@ export class OrcaRuntimeService { ? { env: { ...effectiveStartup.env, ...sequenced.startupEnv } } : {}) } - wrappedSetupCommandStr = sequenced.setupCommand + sequencedSetup = applySequencedSetupLaunch(setup, sequenced) } if (sequencedStartup && this.ptyController?.spawn) { @@ -28025,9 +28027,9 @@ export class OrcaRuntimeService { hasStartupTerminal: didSpawnStartup, setupCommandPlatform: getSetupRunnerCommandPlatformForLaunch(setup, 'posix'), observeSetupCompletion: args.observeSetupCompletion, - // Why: carry the wait-for-agent wrapped setup command (#6298) so the + // Why: carry the wait-for-agent gated setup launch (#6298) so the // Setup tab runs the same script the sequenced agent waits on. - ...(wrappedSetupCommandStr ? { wrappedSetupCommand: wrappedSetupCommandStr } : {}) + ...(sequencedSetup ? { sequencedSetup } : {}) }) didSpawnSetup = provisioned.setupSpawned setupTerminalHandle = provisioned.setupTerminalHandle @@ -28037,14 +28039,9 @@ export class OrcaRuntimeService { // activation retries it. const activationSetup = didSpawnSetup ? undefined - : setup - ? { - ...setup, - ...(didSpawnStartup && wrappedSetupCommandStr - ? { command: wrappedSetupCommandStr } - : {}) - } - : undefined + : didSpawnStartup && sequencedSetup + ? sequencedSetup + : setup const activationDefaultTabs = runtimeWillProvisionTerminals ? undefined : defaultTabs if (effectiveStartup && !didSpawnStartup) { this.notifyActivateWorktree(repo.id, worktree.id, { @@ -28073,7 +28070,7 @@ export class OrcaRuntimeService { hasStartupTerminal: didSpawnStartup, setupCommandPlatform: getSetupRunnerCommandPlatformForLaunch(setup, 'posix'), observeSetupCompletion: args.observeSetupCompletion, - ...(wrappedSetupCommandStr ? { wrappedSetupCommand: wrappedSetupCommandStr } : {}), + ...(sequencedSetup ? { sequencedSetup } : {}), surfaceOwner: false }) // Why: runtime owns setup spawning here, so the RPC result must omit setup @@ -28101,14 +28098,9 @@ export class OrcaRuntimeService { } const returnedSetup = didSpawnSetup ? undefined - : setup - ? { - ...setup, - ...(didSpawnStartup && wrappedSetupCommandStr - ? { command: wrappedSetupCommandStr } - : {}) - } - : undefined + : didSpawnStartup && sequencedSetup + ? sequencedSetup + : setup this.emitWorktreeLifecycle({ kind: 'created', worktreeId: worktree.id, @@ -28292,7 +28284,7 @@ export class OrcaRuntimeService { let startupTerminalPtyId: string | null = null let sequencedStartup = args.startup - let wrappedSetupCommandStr: string | undefined + let sequencedSetup: CreateWorktreeResult['setup'] if (args.startup && result.setup?.waitForAgentStartup === true) { const platform = getSetupRunnerCommandPlatformForLaunch(result.setup, 'posix') const sequenced = createSequencedSetupAgentCommands({ @@ -28306,7 +28298,7 @@ export class OrcaRuntimeService { command: sequenced.startupCommand, ...(sequenced.startupEnv ? { env: { ...args.startup.env, ...sequenced.startupEnv } } : {}) } - wrappedSetupCommandStr = sequenced.setupCommand + sequencedSetup = applySequencedSetupLaunch(result.setup, sequenced) } if (sequencedStartup && this.ptyController?.spawn) { @@ -28368,9 +28360,9 @@ export class OrcaRuntimeService { hasStartupTerminal: didSpawnStartup, setupCommandPlatform: getSetupRunnerCommandPlatformForLaunch(result.setup, 'posix'), observeSetupCompletion: args.observeSetupCompletion, - // Why: carry the wait-for-agent wrapped setup command (#6298) so the + // Why: carry the wait-for-agent gated setup launch (#6298) so the // remote Setup tab runs the same script the sequenced agent waits on. - ...(wrappedSetupCommandStr ? { wrappedSetupCommand: wrappedSetupCommandStr } : {}) + ...(sequencedSetup ? { sequencedSetup } : {}) }) didSpawnSetup = provisioned.setupSpawned setupTerminalHandle = provisioned.setupTerminalHandle @@ -28379,14 +28371,9 @@ export class OrcaRuntimeService { // failure fall through with the wrapped command so renderer retries. const activationSetup = didSpawnSetup ? undefined - : result.setup - ? { - ...result.setup, - ...(didSpawnStartup && wrappedSetupCommandStr - ? { command: wrappedSetupCommandStr } - : {}) - } - : undefined + : didSpawnStartup && sequencedSetup + ? sequencedSetup + : result.setup const activationDefaultTabs = runtimeWillProvisionTerminals ? undefined : result.defaultTabs if (args.startup && !didSpawnStartup) { this.notifyActivateWorktree(repo.id, result.worktree.id, { @@ -28421,7 +28408,7 @@ export class OrcaRuntimeService { hasStartupTerminal: didSpawnStartup, setupCommandPlatform: getSetupRunnerCommandPlatformForLaunch(result.setup, 'posix'), observeSetupCompletion: args.observeSetupCompletion, - ...(wrappedSetupCommandStr ? { wrappedSetupCommand: wrappedSetupCommandStr } : {}), + ...(sequencedSetup ? { sequencedSetup } : {}), surfaceOwner: false }) // Why: runtime owns setup spawning here, so omit setup from the RPC result @@ -28449,14 +28436,9 @@ export class OrcaRuntimeService { const returnedSetup = didSpawnSetup ? undefined - : result.setup - ? { - ...result.setup, - ...(didSpawnStartup && wrappedSetupCommandStr - ? { command: wrappedSetupCommandStr } - : {}) - } - : undefined + : didSpawnStartup && sequencedSetup + ? sequencedSetup + : result.setup const resultForRenderer = returnedSetup ? { ...result, setup: returnedSetup } : (() => { diff --git a/src/renderer/src/lib/launch-worktree-background-terminals.ts b/src/renderer/src/lib/launch-worktree-background-terminals.ts index 791504d6948..7f2a8b0ea91 100644 --- a/src/renderer/src/lib/launch-worktree-background-terminals.ts +++ b/src/renderer/src/lib/launch-worktree-background-terminals.ts @@ -125,11 +125,17 @@ function registerBackgroundPaneBuffer(tabId: string, leafId: string, pane: Spawn } function buildSetupCommand(setup: WorktreeSetupLaunch): string { + // Why: a sequenced launch carries the gated command that records setup's outcome for a waiting + // agent terminal; rebuilding the bare runner here ran setup but recorded nothing, so the agent + // waited out its whole bound. // Why: background setup tabs can launch later, so they must reuse the same shell chosen when the runner was written. - return buildSetupRunnerCommand( - setup.runnerScriptPath, - getSetupRunnerCommandPlatformForPath(setup.runnerScriptPath, 'posix'), - setup.shell + return ( + setup.command ?? + buildSetupRunnerCommand( + setup.runnerScriptPath, + getSetupRunnerCommandPlatformForPath(setup.runnerScriptPath, 'posix'), + setup.shell + ) ) } diff --git a/src/renderer/src/lib/worktree-activation-setup-script.test.ts b/src/renderer/src/lib/worktree-activation-setup-script.test.ts index ff9bc0874b8..75f8196197d 100644 --- a/src/renderer/src/lib/worktree-activation-setup-script.test.ts +++ b/src/renderer/src/lib/worktree-activation-setup-script.test.ts @@ -1,5 +1,8 @@ import { describe, expect, it, vi } from 'vitest' -import { SETUP_AGENT_SEQUENCE_STARTUP_SCRIPT_ENV } from '../../../shared/setup-agent-sequencing' +import { + SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV, + SETUP_AGENT_SEQUENCE_STARTUP_SCRIPT_ENV +} from '../../../shared/setup-agent-sequencing' import { ensureWorktreeHasInitialTerminal } from './worktree-initial-terminal-seeding' import { createMockStore, @@ -121,7 +124,11 @@ describe('ensureWorktreeHasInitialTerminal', () => { expect(store.queueTabStartupCommand).toHaveBeenCalledWith( 'tab-2', expect.objectContaining({ - command: expect.stringContaining('printf') + env: expect.objectContaining({ + [SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV]: expect.stringContaining( + '/tmp/repo/.git/orca/setup-runner.sh.' + ) + }) }) ) expect(store.queueTabSetupSplit).not.toHaveBeenCalled() @@ -188,7 +195,11 @@ describe('ensureWorktreeHasInitialTerminal', () => { expect(store.queueTabStartupCommand).toHaveBeenCalledWith( 'tab-2', expect.objectContaining({ - command: expect.stringContaining('printf') + env: expect.objectContaining({ + [SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV]: expect.stringContaining( + '/tmp/repo/.git/orca/setup-runner.sh.' + ) + }) }) ) expect(store.queueTabStartupCommand).toHaveBeenCalledWith( @@ -250,12 +261,12 @@ describe('ensureWorktreeHasInitialTerminal', () => { ) expect(store.queueTabSetupSplit).toHaveBeenCalledWith('tab-1', { command: expect.stringContaining('bash /tmp/repo/.git/orca/setup-runner.sh'), - env: { ORCA_ROOT_PATH: '/tmp/repo' }, - direction: 'vertical' - }) - expect(store.queueTabSetupSplit).toHaveBeenCalledWith('tab-1', { - command: expect.stringContaining('printf'), - env: { ORCA_ROOT_PATH: '/tmp/repo' }, + env: expect.objectContaining({ + ORCA_ROOT_PATH: '/tmp/repo', + [SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV]: expect.stringContaining( + '/tmp/repo/.git/orca/setup-runner.sh.' + ) + }), direction: 'vertical' }) }) @@ -288,7 +299,12 @@ describe('ensureWorktreeHasInitialTerminal', () => { ) expect(store.queueTabSetupSplit).toHaveBeenCalledWith('tab-1', { command: expect.stringContaining('bash /mnt/c/repo/.git/orca/setup-runner.sh'), - env: { ORCA_ROOT_PATH: 'C:\\repo' }, + env: expect.objectContaining({ + ORCA_ROOT_PATH: 'C:\\repo', + [SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV]: expect.stringContaining( + '/mnt/c/repo/.git/orca/setup-runner.sh.' + ) + }), direction: 'vertical' }) }) diff --git a/src/renderer/src/lib/worktree-activation-web-runtime.test.ts b/src/renderer/src/lib/worktree-activation-web-runtime.test.ts index 98a7679ae6d..702d3b7aaf6 100644 --- a/src/renderer/src/lib/worktree-activation-web-runtime.test.ts +++ b/src/renderer/src/lib/worktree-activation-web-runtime.test.ts @@ -1,3 +1,4 @@ +import { SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV } from '../../../shared/setup-agent-sequencing' import { describe, expect, it, vi } from 'vitest' import { ensureWorktreeHasInitialTerminal } from './worktree-initial-terminal-seeding' import type { AppStoreState } from './worktree-activation-test-harness' @@ -88,7 +89,9 @@ describe('ensureWorktreeHasInitialTerminal', () => { expect(store.queueTabStartupCommand).toHaveBeenCalledWith( 'tab-2', expect.objectContaining({ - command: expect.stringContaining('printf') + env: expect.objectContaining({ + [SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV]: expect.stringContaining('printf') + }) }) ) }) diff --git a/src/renderer/src/lib/worktree-default-terminal-tabs.ts b/src/renderer/src/lib/worktree-default-terminal-tabs.ts index 5a9d48723f0..aec48555230 100644 --- a/src/renderer/src/lib/worktree-default-terminal-tabs.ts +++ b/src/renderer/src/lib/worktree-default-terminal-tabs.ts @@ -28,7 +28,6 @@ export function applyDefaultTerminalTabs( setup: WorktreeSetupLaunch | undefined, issueCommand: IssueCommandLaunch | undefined, defaultTabs: WorktreeDefaultTabsLaunch | undefined, - wrappedSetupCommandStr: string | undefined, opts: InitialTerminalOptions | undefined ): string | null { if (!defaultTabs || store.defaultTerminalTabsAppliedByWorktreeId[worktreeId]) { @@ -99,14 +98,6 @@ export function applyDefaultTerminalTabs( } store.queueTabStartupCommand(firstTabId, startup) } - queueSetupAndIssueCommands( - store, - worktreeId, - firstTabId, - setup, - issueCommand, - wrappedSetupCommandStr, - opts - ) + queueSetupAndIssueCommands(store, worktreeId, firstTabId, setup, issueCommand, opts) return firstTabId } diff --git a/src/renderer/src/lib/worktree-initial-terminal-seeding.ts b/src/renderer/src/lib/worktree-initial-terminal-seeding.ts index c3d58b190ba..451a78e159e 100644 --- a/src/renderer/src/lib/worktree-initial-terminal-seeding.ts +++ b/src/renderer/src/lib/worktree-initial-terminal-seeding.ts @@ -3,7 +3,10 @@ import type { WorktreeSetupLaunch } from '../../../shared/worktree/launch-types' import { shouldAutoCreateInitialTerminal } from '@/components/terminal/initial-terminal' -import { createSequencedSetupAgentCommands } from '../../../shared/setup-agent-sequencing' +import { + applySequencedSetupLaunch, + createSequencedSetupAgentCommands +} from '../../../shared/setup-agent-sequencing' import { getSetupRunnerCommandPlatformForPath } from '../../../shared/setup-runner-command' import { agentKindToTuiAgent } from '../../../shared/agent-kind' import { useAppStore } from '@/store' @@ -51,7 +54,10 @@ export function ensureWorktreeHasInitialTerminal( ? store : useAppStore.getState() let sequencedStartup = startup - let wrappedSetupCommandStr: string | undefined + // Why: sequencing rewrites both halves at once — the gate the agent pane waits on and the + // gated setup launch that records the outcome — so the setup record itself carries the pairing + // from here on instead of a second argument every caller has to remember to thread. + let sequencedSetup = setup if (startup && setup?.waitForAgentStartup === true) { const platform = getSetupRunnerCommandPlatformForLaunch(setup) @@ -66,7 +72,7 @@ export function ensureWorktreeHasInitialTerminal( command: sequenced.startupCommand, ...(sequenced.startupEnv ? { env: { ...startup.env, ...sequenced.startupEnv } } : {}) } - wrappedSetupCommandStr = sequenced.setupCommand + sequencedSetup = applySequencedSetupLaunch(setup, sequenced) } const backendStartupTerminalSpawned = opts?.backendStartupTerminalSpawned === true @@ -79,9 +85,8 @@ export function ensureWorktreeHasInitialTerminal( store, worktreeId, existingTerminalTabId, - setup, + sequencedSetup, issueCommand, - wrappedSetupCommandStr, opts ) return existingTerminalTabId @@ -98,9 +103,8 @@ export function ensureWorktreeHasInitialTerminal( state, worktreeId, firstTerminalTabId, - setup, + sequencedSetup, issueCommand, - wrappedSetupCommandStr, opts ) }) @@ -136,9 +140,8 @@ export function ensureWorktreeHasInitialTerminal( store, worktreeId, existingTerminalTabId, - setup, + sequencedSetup, issueCommand, - wrappedSetupCommandStr, opts ) return existingTerminalTabId @@ -150,10 +153,9 @@ export function ensureWorktreeHasInitialTerminal( store, worktreeId, sequencedStartup, - setup, + sequencedSetup, issueCommand, defaultTabs, - wrappedSetupCommandStr, opts ) if (templatedTabId) { @@ -200,15 +202,7 @@ export function ensureWorktreeHasInitialTerminal( } store.queueTabStartupCommand(terminalTab.id, sequencedStartup) } - queueSetupAndIssueCommands( - store, - worktreeId, - terminalTab.id, - setup, - issueCommand, - wrappedSetupCommandStr, - opts - ) + queueSetupAndIssueCommands(store, worktreeId, terminalTab.id, sequencedSetup, issueCommand, opts) return terminalTab.id } diff --git a/src/renderer/src/lib/worktree-setup-issue-command-queue.ts b/src/renderer/src/lib/worktree-setup-issue-command-queue.ts index 3c474d98c6a..13f1e1da6e0 100644 --- a/src/renderer/src/lib/worktree-setup-issue-command-queue.ts +++ b/src/renderer/src/lib/worktree-setup-issue-command-queue.ts @@ -17,17 +17,15 @@ export function queueSetupAndIssueCommands( terminalTabId: string, setup: WorktreeSetupLaunch | undefined, issueCommand: IssueCommandLaunch | undefined, - wrappedSetupCommandStr: string | undefined, opts: InitialTerminalOptions | undefined ): void { // Why: setup launch location is user-configurable — 'new-tab' keeps setup output off the primary pane; splits keep it adjacent. if (setup) { const mode = useAppStore.getState().settings?.setupScriptLaunchMode ?? 'new-tab' + // Why: `command` and `envVars` arrive as one sequenced launch record, so the gated command + // and the env carrying its script cannot be separated by a caller that forgets one of them. const setupCommand = { - command: - wrappedSetupCommandStr ?? - setup.command ?? - buildSetupRunnerCommand(setup.runnerScriptPath, setup.shell), + command: setup.command ?? buildSetupRunnerCommand(setup.runnerScriptPath, setup.shell), env: setup.envVars } if (mode === 'new-tab') { diff --git a/src/shared/setup-agent-sequencing-env.ts b/src/shared/setup-agent-sequencing-env.ts new file mode 100644 index 00000000000..90506fd9221 --- /dev/null +++ b/src/shared/setup-agent-sequencing-env.ts @@ -0,0 +1,6 @@ +/** Env keys that carry the setup-to-agent gate between the host and the terminals it launches. + * Their own module so the POSIX and Windows gate builders can share them without importing + * each other. */ +export const SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV = 'ORCA_SEQUENCED_STARTUP_COMMAND' +export const SETUP_AGENT_SEQUENCE_STARTUP_SCRIPT_ENV = 'ORCA_SEQUENCED_STARTUP_SCRIPT' +export const SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV = 'ORCA_SEQUENCED_SETUP_SCRIPT' diff --git a/src/shared/setup-agent-sequencing-posix-gate.ts b/src/shared/setup-agent-sequencing-posix-gate.ts new file mode 100644 index 00000000000..362cbdbbdc4 --- /dev/null +++ b/src/shared/setup-agent-sequencing-posix-gate.ts @@ -0,0 +1,157 @@ +import { SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV, SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV } from './setup-agent-sequencing-env' + +/** Shell fragments for the POSIX half of the setup-to-agent gate: the setup launch that records + * an outcome, and the agent launch that waits for one. */ +export function setupStartedPath(markerPath: string): string { + return `${markerPath}.started` +} + +export function buildPosixSetupCommand(bareRunnerCommand: string): string { + const script = [ + `if [ -n "\${${SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV}:-}" ]; then`, + `eval "\$${SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV}";`, + 'else', + `${bareRunnerCommand};`, + 'fi' + ].join(' ') + return `bash -lc ${quotePosixArg(script)}` +} + +export function buildPosixSetupScript(setupCommand: string, markerPath: string, nonce: string): string { + const marker = quotePosixArg(markerPath) + const tmp = quotePosixArg(`${markerPath}.tmp`) + const started = quotePosixArg(setupStartedPath(markerPath)) + const nonceValue = quotePosixArg(nonce) + + // Why: the gate's only evidence is this file, so the status is recorded from an EXIT trap — + // a runner that is interrupted, killed, or exits non-zero still records an outcome instead of + // leaving the agent terminal waiting on a marker nobody will ever write. + return [ + `rm -f ${marker} ${tmp} ${started} 2>/dev/null`, + `printf '%s\\n' ${nonceValue} > ${started} 2>/dev/null`, + `orca_record_setup_status() { printf '%s:%s\\n' ${nonceValue} "$1" > ${tmp} 2>/dev/null && mv -f ${tmp} ${marker} 2>/dev/null; }`, + 'trap \'orca_record_setup_status "$?"\' EXIT', + "trap 'exit 129' HUP", + "trap 'exit 130' INT", + "trap 'exit 143' TERM", + `( ${setupCommand} )`, + 'exit "$?"' + ].join('; ') +} + +export function buildPosixStartupScript( + startupCommand: string, + markerPath: string, + nonce: string, + waitTimeoutSeconds: number, + startGraceSeconds: number, + progressIntervalSeconds: number +): string { + const marker = quotePosixArg(markerPath) + const tmp = quotePosixArg(`${markerPath}.tmp`) + const started = quotePosixArg(setupStartedPath(markerPath)) + const nonceValue = quotePosixArg(nonce) + const timeout = Math.max(1, Math.floor(waitTimeoutSeconds)) + const grace = Math.max(1, Math.floor(startGraceSeconds)) + const progressInterval = Math.max(1, Math.floor(progressIntervalSeconds)) + const launchAgent = buildPosixLaunchAgentClause(startupCommand) + // Why: the PTY launch path feeds this command through an interactive shell, + // so keeping the wrapper on one line avoids visible `quote>` continuation + // prompts while still preserving valid `while`/`if` shell syntax. + const script = [ + `deadline=$((SECONDS + ${timeout}));`, + `start_deadline=$((SECONDS + ${grace}));`, + `next_report=$((SECONDS + ${progressInterval}));`, + 'setup_started=0;', + 'echo "Waiting for setup to finish before starting agent..." >&2;', + 'while :; do', + `if [ -f ${marker} ]; then`, + `IFS=: read -r seen status < ${marker} || true;`, + `if [ "$seen" = ${nonceValue} ]; then`, + `rm -f ${marker} ${tmp} ${started} 2>/dev/null;`, + `if [ "$status" = "0" ]; then ${launchAgent} fi;`, + 'echo "Setup failed; skipping agent startup. Setup exited with status ${status:-1}; open the Setup tab for its output, then start the agent yourself once it is fixed." >&2;', + 'exit "${status:-1}";', + 'fi;', + 'fi;', + `if [ "$setup_started" = "0" ] && [ -f ${started} ]; then`, + 'setup_started=1;', + 'echo "Setup started; waiting for it to finish." >&2;', + 'fi;', + 'if [ "$setup_started" = "0" ] && [ "$SECONDS" -ge "$start_deadline" ]; then', + // Why: nothing reported starting, so we cannot claim setup failed — only that this terminal + // has no way to observe it. Starting the agent unsequenced beats a terminal that waits out + // the whole bound for an outcome no one is going to record. + `echo "Setup never reported starting within ${grace}s, so this terminal cannot tell whether it ran. Starting the agent without waiting for setup." >&2;`, + `${launchAgent}`, + 'fi;', + 'if [ "$SECONDS" -ge "$deadline" ]; then', + `echo "Timed out waiting for setup before starting agent. Waited ${timeout}s without a result; the agent was not started. Open the Setup tab for its output." >&2;`, + 'exit 124;', + 'fi;', + 'if [ "$SECONDS" -ge "$next_report" ]; then', + `next_report=$((SECONDS + ${progressInterval}));`, + 'echo "Still waiting for setup to finish before starting agent... (${SECONDS}s elapsed)" >&2;', + 'fi;', + 'sleep 1;', + 'done' + ].join(' ') + + return script +} + +function buildPosixLaunchAgentClause(startupCommand: string): string { + const startupSuccessCommand = buildPosixStartupSuccessCommand(startupCommand) + return `if [ -n "\${${SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV}:-}" ]; then eval "\$${SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV}"; exit "$?"; else ${startupSuccessCommand}; fi;` +} + +function buildPosixStartupSuccessCommand(startupCommand: string): string { + if ( + hasUnquotedPosixCommandSeparator(startupCommand) || + hasLeadingPosixEnvAssignment(startupCommand) + ) { + return `eval ${quotePosixArg(startupCommand)}; exit "$?"` + } + return `exec ${startupCommand}` +} + +function hasLeadingPosixEnvAssignment(command: string): boolean { + return /^[A-Za-z_][A-Za-z0-9_]*=/.test(command.trimStart()) +} + +function hasUnquotedPosixCommandSeparator(command: string): boolean { + let quote: "'" | '"' | null = null + let escaped = false + for (const char of command) { + if (escaped) { + escaped = false + continue + } + if (char === '\\') { + escaped = true + continue + } + if (quote) { + if (char === quote) { + quote = null + } + continue + } + if (char === "'" || char === '"') { + quote = char + continue + } + if (char === ';' || char === '&' || char === '|' || char === '\n' || char === '\r') { + return true + } + } + return false +} + +function quotePosixArg(value: string): string { + if (/^[A-Za-z0-9_./:-]+$/.test(value)) { + return value + } + return `'${value.replace(/'/g, `'\\''`)}'` +} + diff --git a/src/shared/setup-agent-sequencing-windows-gate.ts b/src/shared/setup-agent-sequencing-windows-gate.ts new file mode 100644 index 00000000000..cc9d08d6d29 --- /dev/null +++ b/src/shared/setup-agent-sequencing-windows-gate.ts @@ -0,0 +1,126 @@ +import { encodePowerShellCommand } from './powershell-command-encoding' +import { SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV } from './setup-agent-sequencing-env' +import { setupStartedPath } from './setup-agent-sequencing-posix-gate' + +/** Shell fragments for the native-Windows half of the setup-to-agent gate. Both sides run under + * PowerShell so the marker write and the bounded poll stay parseable without a batch label loop. */ +export function buildWindowsSetupCommand( + runnerScriptPath: string, + markerPath: string, + nonce: string +): string { + // Why: delayed expansion keeps path metacharacters as data when cmd invokes the batch runner. + // Why: the status write lives in `finally` for the same reason the POSIX side uses an EXIT + // trap — a terminated runner must still record an outcome for the waiting agent terminal. + const script = [ + `$runner = ${quotePowerShellString(runnerScriptPath)}`, + `$marker = ${quotePowerShellString(markerPath)}`, + '$tmp = $marker + ".tmp"', + `$started = ${quotePowerShellString(setupStartedPath(markerPath))}`, + `$nonce = ${quotePowerShellString(nonce)}`, + '$utf8 = [System.Text.UTF8Encoding]::new($false)', + 'Remove-Item -LiteralPath $marker, $tmp, $started -Force -ErrorAction SilentlyContinue', + '[System.IO.File]::WriteAllText($started, ($nonce + [Environment]::NewLine), $utf8)', + '$setupStatus = 1', + 'try {', + ' $processInfo = [System.Diagnostics.ProcessStartInfo]::new()', + ' $processInfo.FileName = $env:ComSpec', + ' $processInfo.Arguments = \'/d /s /v:on /c ""!ORCA_SETUP_RUNNER!""\'', + ' $processInfo.UseShellExecute = $false', + ' $processInfo.EnvironmentVariables["ORCA_SETUP_RUNNER"] = $runner', + ' $process = [System.Diagnostics.Process]::Start($processInfo)', + ' $process.WaitForExit()', + ' $setupStatus = $process.ExitCode', + '} finally {', + ' [System.IO.File]::WriteAllText($tmp, ($nonce + ":" + $setupStatus + [Environment]::NewLine), $utf8)', + ' Move-Item -LiteralPath $tmp -Destination $marker -Force', + '}', + 'exit $setupStatus' + ].join('; ') + + return encodePowerShellInvocation(script) +} + +export function buildWindowsStartupCommand( + markerPath: string, + nonce: string, + waitTimeoutSeconds: number, + startGraceSeconds: number, + progressIntervalSeconds: number +): string { + const timeout = Math.max(1, Math.floor(waitTimeoutSeconds)) + const grace = Math.max(1, Math.floor(startGraceSeconds)) + const progressInterval = Math.max(1, Math.floor(progressIntervalSeconds)) + // Why: native Windows setup runners launch through cmd.exe, but PowerShell + // gives us safe bounded file polling/parsing without a fragile batch label loop. + const script = [ + `$marker = ${quotePowerShellString(markerPath)}`, + 'if ([string]::IsNullOrWhiteSpace($marker)) {', + ' [Console]::Error.WriteLine("Missing setup marker path.")', + ' exit 1', + '}', + '$tmp = $marker + ".tmp"', + `$started = ${quotePowerShellString(setupStartedPath(markerPath))}`, + `$nonce = ${quotePowerShellString(nonce)}`, + '$begunAt = Get-Date', + `$deadline = $begunAt.AddSeconds(${timeout})`, + `$startDeadline = $begunAt.AddSeconds(${grace})`, + `$nextReport = $begunAt.AddSeconds(${progressInterval})`, + '$setupStarted = $false', + '[Console]::Error.WriteLine("Waiting for setup to finish before starting agent...")', + '$launchAgent = {', + ` $startup = $env:${SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV}`, + ' if ([string]::IsNullOrWhiteSpace($startup)) {', + ' [Console]::Error.WriteLine("Missing sequenced startup command.")', + ' exit 1', + ' }', + ' Invoke-Expression $startup', + ' if ($global:LASTEXITCODE -ne $null) { exit $global:LASTEXITCODE }', + ' if (-not $?) { exit 1 }', + ' exit 0', + '}', + 'while ($true) {', + ' if (Test-Path -LiteralPath $marker) {', + ' $content = Get-Content -LiteralPath $marker -TotalCount 1', + ' if ($content -match "^([0-9A-Za-z_-]+):([0-9]+)$" -and $Matches[1] -eq $nonce) {', + ' $setupStatus = [int]$Matches[2]', + ' Remove-Item -LiteralPath $marker, $tmp, $started -Force -ErrorAction SilentlyContinue', + ' if ($setupStatus -ne 0) {', + ' [Console]::Error.WriteLine("Setup failed; skipping agent startup. Setup exited with status $setupStatus; open the Setup tab for its output, then start the agent yourself once it is fixed.")', + ' exit $setupStatus', + ' }', + ' & $launchAgent', + ' }', + ' }', + ' if (-not $setupStarted -and (Test-Path -LiteralPath $started)) {', + ' $setupStarted = $true', + ' [Console]::Error.WriteLine("Setup started; waiting for it to finish.")', + ' }', + ' if (-not $setupStarted -and (Get-Date) -ge $startDeadline) {', + ` [Console]::Error.WriteLine("Setup never reported starting within ${grace}s, so this terminal cannot tell whether it ran. Starting the agent without waiting for setup.")`, + ' & $launchAgent', + ' }', + ' if ((Get-Date) -ge $deadline) {', + ` [Console]::Error.WriteLine("Timed out waiting for setup before starting agent. Waited ${timeout}s without a result; the agent was not started. Open the Setup tab for its output.")`, + ' exit 124', + ' }', + ' if ((Get-Date) -ge $nextReport) {', + ` $nextReport = (Get-Date).AddSeconds(${progressInterval})`, + ' $elapsed = [int]((Get-Date) - $begunAt).TotalSeconds', + ' [Console]::Error.WriteLine("Still waiting for setup to finish before starting agent... (${elapsed}s elapsed)")', + ' }', + ' Start-Sleep -Seconds 1', + '}' + ].join('; ') + + return encodePowerShellInvocation(script) +} + +function encodePowerShellInvocation(script: string): string { + return `powershell.exe -NoProfile -NonInteractive -ExecutionPolicy Bypass -EncodedCommand ${encodePowerShellCommand(script)}` +} + +function quotePowerShellString(value: string): string { + return `'${value.replace(/'/g, "''")}'` +} + diff --git a/src/shared/setup-agent-sequencing.test.ts b/src/shared/setup-agent-sequencing.test.ts index 3d51b5b7476..eced2d6b0cd 100644 --- a/src/shared/setup-agent-sequencing.test.ts +++ b/src/shared/setup-agent-sequencing.test.ts @@ -7,10 +7,12 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { getDefaultRepoHookSettings } from './constants' import { + applySequencedSetupLaunch, createSequencedSetupAgentCommands, createSetupAgentSequenceNonce, getSetupAgentSequenceShellForTests, resolveSetupAgentSequenceLaunchCommand, + SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV, SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV, SETUP_AGENT_SEQUENCE_STARTUP_SCRIPT_ENV } from './setup-agent-sequencing' @@ -20,6 +22,9 @@ import { } from './setup-agent-startup-policy' const TEMP_DIRS: string[] = [] +// Why: TTYHOG, the canonical-mode input queue a PTY applies before the shell's line editor +// takes over. macOS and the BSDs use 1024; anything longer loses its submit byte. +const POSIX_CANONICAL_INPUT_FLOOR_BYTES = 1024 const WINDOWS_PROCESS_TEST_TIMEOUT_MS = 30_000 afterEach(() => { @@ -61,13 +66,16 @@ describe('createSequencedSetupAgentCommands', () => { waitTimeoutSeconds: 9 }) + const setupScript = result.setupEnv?.[SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV] expect(result.setupCommand).toMatch(/^bash -lc /) + // Why: the command stays short enough to survive a PTY's canonical input cap; the gate that + // records setup's outcome rides in the env, with the bare runner as the inline fallback. expect(result.setupCommand).toContain('bash /repo/.git/orca/setup-runner.sh') - expect(result.setupCommand).toContain('printf') - expect(result.setupCommand).toContain('nonce-123 "$status"') - expect(result.setupCommand).toContain( - 'mv -f /repo/.git/orca/setup-runner.sh.nonce-123.done.tmp' - ) + expect(setupScript).toContain('printf') + expect(setupScript).toContain('orca_record_setup_status "$?"\' EXIT') + expect(setupScript).toContain('nonce-123 "$1"') + expect(setupScript).toContain('mv -f /repo/.git/orca/setup-runner.sh.nonce-123.done.tmp') + expect(setupScript).toContain('/repo/.git/orca/setup-runner.sh.nonce-123.done.started') const startupScript = result.startupEnv?.[SETUP_AGENT_SEQUENCE_STARTUP_SCRIPT_ENV] expect(result.startupCommand).toBe( `bash -lc 'eval "$${SETUP_AGENT_SEQUENCE_STARTUP_SCRIPT_ENV}"'` @@ -118,16 +126,18 @@ describe('createSequencedSetupAgentCommands', () => { nonce: 'second-launch' }) - expect(first.setupCommand).toContain('/repo/.git/orca/setup-runner.sh.first-launch.done') + const firstSetupScript = first.setupEnv?.[SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV] + const secondSetupScript = second.setupEnv?.[SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV] + expect(firstSetupScript).toContain('/repo/.git/orca/setup-runner.sh.first-launch.done') expect(first.startupEnv?.[SETUP_AGENT_SEQUENCE_STARTUP_SCRIPT_ENV]).toContain( '/repo/.git/orca/setup-runner.sh.first-launch.done' ) - expect(second.setupCommand).toContain('/repo/.git/orca/setup-runner.sh.second-launch.done') + expect(secondSetupScript).toContain('/repo/.git/orca/setup-runner.sh.second-launch.done') expect(second.startupEnv?.[SETUP_AGENT_SEQUENCE_STARTUP_SCRIPT_ENV]).toContain( '/repo/.git/orca/setup-runner.sh.second-launch.done' ) - expect(first.setupCommand).not.toContain('/repo/.git/orca/setup-runner.sh.second-launch.done') - expect(second.setupCommand).not.toContain('/repo/.git/orca/setup-runner.sh.first-launch.done') + expect(firstSetupScript).not.toContain('/repo/.git/orca/setup-runner.sh.second-launch.done') + expect(secondSetupScript).not.toContain('/repo/.git/orca/setup-runner.sh.first-launch.done') }) it('keeps simple POSIX startup commands eligible for exec when quoted text has separators', () => { @@ -172,7 +182,7 @@ describe('createSequencedSetupAgentCommands', () => { expect(result.setupCommand).toContain( 'bash /home/jin/repo/.git/worktrees/feature/orca/setup-runner.sh' ) - expect(result.setupCommand).toContain( + expect(result.setupEnv?.[SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV]).toContain( '/home/jin/repo/.git/worktrees/feature/orca/setup-runner.sh.nonce-wsl.done' ) expect(result.setupCommand).not.toContain('wsl.localhost') @@ -204,7 +214,7 @@ describe('createSequencedSetupAgentCommands', () => { }) expect(result.setupCommand).toContain('bash /mnt/c/repo/.git/orca/setup-runner.sh') - expect(result.setupCommand).toContain( + expect(result.setupEnv?.[SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV]).toContain( '/mnt/c/repo/.git/orca/setup-runner.sh.nonce-wsl-shell.done' ) expect(result.startupEnv?.[SETUP_AGENT_SEQUENCE_STARTUP_SCRIPT_ENV]).toContain( @@ -237,7 +247,7 @@ describe('createSequencedSetupAgentCommands', () => { expect(startupPowerShell).toContain('Timed out waiting for setup before starting agent.') expect(startupPowerShell).toContain('Setup failed; skipping agent startup.') expect(startupPowerShell).toContain( - 'Remove-Item -LiteralPath $marker, $tmp -Force -ErrorAction SilentlyContinue' + 'Remove-Item -LiteralPath $marker, $tmp, $started -Force -ErrorAction SilentlyContinue' ) expect(result.startupCommand).not.toContain('%ERRORLEVEL%') expect(startupPowerShell).toContain('Invoke-Expression') @@ -275,7 +285,7 @@ describe('createSequencedSetupAgentCommands', () => { 'eval "$ORCA_SEQUENCED_STARTUP_COMMAND"' ) // Why: bash writes and reads the marker here, so it needs the /c/... form of the path. - expect(result.setupCommand).toContain( + expect(result.setupEnv?.[SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV]).toContain( '/c/repo/.git/orca/setup-runner.cmd.nonce-gitbash-cmd.done' ) expect(result.startupEnv?.[SETUP_AGENT_SEQUENCE_STARTUP_SCRIPT_ENV]).toContain( @@ -378,7 +388,10 @@ describe('createSequencedSetupAgentCommands', () => { expect(readFileSync(markerPath, 'utf8')).toBe('stale:0\n') const setupExit = await waitForExit( - spawn('bash', ['-lc', commands.setupCommand], { stdio: 'pipe' }) + spawn('bash', ['-lc', commands.setupCommand], { + stdio: 'pipe', + env: { ...process.env, ...commands.setupEnv } + }) ) expect(setupExit.code).toBe(0) @@ -412,7 +425,10 @@ describe('createSequencedSetupAgentCommands', () => { }) const setupExitPromise = waitForExit( - spawn('bash', ['-lc', commands.setupCommand], { stdio: 'pipe' }) + spawn('bash', ['-lc', commands.setupCommand], { + stdio: 'pipe', + env: { ...process.env, ...commands.setupEnv } + }) ) const startupExit = await waitForExit( spawn('bash', ['-lc', commands.startupCommand], { @@ -462,7 +478,10 @@ describe('createSequencedSetupAgentCommands', () => { }) const setupExitPromise = waitForExit( - spawn('bash', ['-lc', commands.setupCommand], { stdio: 'pipe' }) + spawn('bash', ['-lc', commands.setupCommand], { + stdio: 'pipe', + env: { ...process.env, ...commands.setupEnv } + }) ) const startupExit = await waitForExit( spawn('bash', ['-lc', commands.startupCommand], { @@ -593,3 +612,289 @@ function waitForExit( }) }) } + +describe('setup outcome recording', () => { + it('keeps the POSIX setup submission below the canonical input floor', () => { + // Regression: the gated setup command inlined the runner path three times plus the nonce. + // For an ordinary worktree that is 1033 bytes, past the 1024-byte canonical input cap a PTY + // applies before the shell's line editor takes over, so the submit byte was dropped and the + // marker the agent gate polls was never written — a two-hour silent wait. + const result = createSequencedSetupAgentCommands({ + runnerScriptPath: + '/Users/exampleuser12/orca/orca/.git/worktrees/fix-agent-hooks-post-posix-payloads-as-json/orca/setup-runner.sh', + startupCommand: 'claude', + platform: 'posix', + nonce: 'b3f1c0de-1234-4abc-9def-0123456789ab' + }) + + expect(result.setupCommand.length).toBeLessThan(POSIX_CANONICAL_INPUT_FLOOR_BYTES) + expect(result.setupEnv?.[SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV]).toContain( + 'fix-agent-hooks-post-posix-payloads-as-json' + ) + }) + + it('pairs the gated setup command with the env that carries its script', () => { + const sequenced = createSequencedSetupAgentCommands({ + runnerScriptPath: '/repo/.git/orca/setup-runner.sh', + startupCommand: 'claude', + platform: 'posix', + nonce: 'paired-nonce' + }) + const launch = applySequencedSetupLaunch( + { + runnerScriptPath: '/repo/.git/orca/setup-runner.sh', + envVars: { ORCA_WORKSPACE_PATH: '/repo' }, + waitForAgentStartup: true + }, + sequenced + ) + + expect(launch.command).toBe(sequenced.setupCommand) + expect(launch.envVars).toEqual({ + ORCA_WORKSPACE_PATH: '/repo', + [SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV]: + sequenced.setupEnv?.[SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV] + }) + }) + + it.skipIf(process.platform === 'win32')( + 'records a non-zero setup status so the agent gate reports the failure', + async () => { + const tempDir = makeTempDir() + const runnerScriptPath = join(tempDir, 'setup-runner.sh') + const logPath = join(tempDir, 'sequence.log') + + writeExecutable(runnerScriptPath, '#!/bin/sh\nexit 7\n') + + const commands = createSequencedSetupAgentCommands({ + runnerScriptPath, + startupCommand: `printf 'agent-start\\n' >> ${quoteSh(logPath)}`, + platform: 'posix', + nonce: 'failing-setup', + waitTimeoutSeconds: 30, + startGraceSeconds: 25 + }) + + const startupExitPromise = waitForExit( + spawn('bash', ['-lc', commands.startupCommand], { + stdio: 'pipe', + env: { ...process.env, ...commands.startupEnv } + }) + ) + const setupExit = await waitForExit( + spawn('bash', ['-lc', commands.setupCommand], { + stdio: 'pipe', + env: { ...process.env, ...commands.setupEnv } + }) + ) + const startupExit = await startupExitPromise + + expect(setupExit.code).toBe(7) + expect(startupExit.code).toBe(7) + expect(startupExit.stderr).toContain('Setup failed; skipping agent startup.') + expect(startupExit.stderr).toContain('Setup exited with status 7') + expect(readIfExists(logPath)).toBe('') + } + ) + + it.skipIf(process.platform === 'win32')( + 'records an outcome when the setup pane is torn down mid-run', + async () => { + const tempDir = makeTempDir() + const runnerScriptPath = join(tempDir, 'setup-runner.sh') + const markerPath = `${runnerScriptPath}.killed-setup.done` + + writeExecutable(runnerScriptPath, '#!/bin/sh\nsleep 30\n') + + const commands = createSequencedSetupAgentCommands({ + runnerScriptPath, + startupCommand: 'printf ready', + platform: 'posix', + nonce: 'killed-setup' + }) + + // Why: closing a Setup tab signals the pane's whole process group, not just the gate, so + // the teardown is reproduced that way — signalling the gate alone leaves it blocked in its + // foreground child and bash defers the trap until that child returns. + const setup = spawn('bash', ['-lc', commands.setupCommand], { + stdio: 'pipe', + detached: true, + env: { ...process.env, ...commands.setupEnv } + }) + const setupExitPromise = waitForExit(setup) + for (let attempt = 0; attempt < 60 && !readIfExists(`${markerPath}.started`); attempt += 1) { + await sleep(50) + } + expect(readIfExists(`${markerPath}.started`)).toContain('killed-setup') + process.kill(-(setup.pid as number), 'SIGTERM') + await setupExitPromise + + expect(readIfExists(markerPath)).toBe('killed-setup:143\n') + }, + 15_000 + ) + + it.skipIf(process.platform === 'win32')( + 'records an outcome when the setup runner cannot be executed at all', + async () => { + const tempDir = makeTempDir() + const runnerScriptPath = join(tempDir, 'setup-runner.sh') + const markerPath = `${runnerScriptPath}.missing-runner.done` + const logPath = join(tempDir, 'sequence.log') + + // The runner file is never written, so the gate's own launch of it fails. + const commands = createSequencedSetupAgentCommands({ + runnerScriptPath, + startupCommand: `printf 'agent-start\\n' >> ${quoteSh(logPath)}`, + platform: 'posix', + nonce: 'missing-runner', + waitTimeoutSeconds: 30, + startGraceSeconds: 25 + }) + + const startupExitPromise = waitForExit( + spawn('bash', ['-lc', commands.startupCommand], { + stdio: 'pipe', + env: { ...process.env, ...commands.startupEnv } + }) + ) + const setupExit = await waitForExit( + spawn('bash', ['-lc', commands.setupCommand], { + stdio: 'pipe', + env: { ...process.env, ...commands.setupEnv } + }) + ) + const startupExit = await startupExitPromise + + expect(setupExit.code).toBe(127) + expect(readIfExists(markerPath)).toBe('') + expect(startupExit.code).toBe(127) + expect(startupExit.stderr).toContain('Setup exited with status 127') + expect(readIfExists(logPath)).toBe('') + }, + 15_000 + ) + + it.skipIf(process.platform === 'win32')( + 'starts the agent unsequenced when setup is never started at all', + async () => { + const tempDir = makeTempDir() + const runnerScriptPath = join(tempDir, 'setup-runner.sh') + const logPath = join(tempDir, 'sequence.log') + + writeExecutable(runnerScriptPath, '#!/bin/sh\nexit 0\n') + + const commands = createSequencedSetupAgentCommands({ + runnerScriptPath, + startupCommand: `printf 'agent-start\\n' >> ${quoteSh(logPath)}`, + platform: 'posix', + nonce: 'never-started', + waitTimeoutSeconds: 120, + startGraceSeconds: 1 + }) + + // No setup process is ever launched, so nothing will write the marker. + const startupExit = await waitForExit( + spawn('bash', ['-lc', commands.startupCommand], { + stdio: 'pipe', + env: { ...process.env, ...commands.startupEnv } + }) + ) + + expect(startupExit.code).toBe(0) + expect(startupExit.stderr).toContain('Setup never reported starting within 1s') + expect(readFileSync(logPath, 'utf8')).toBe('agent-start\n') + } + ) + + it.skipIf(process.platform === 'win32')( + 'still records an outcome when the setup script env never reaches the setup terminal', + async () => { + const tempDir = makeTempDir() + const runnerScriptPath = join(tempDir, 'setup-runner.sh') + const setupLogPath = join(tempDir, 'setup.log') + const logPath = join(tempDir, 'sequence.log') + + writeExecutable( + runnerScriptPath, + ['#!/bin/sh', `printf 'setup-ran\\n' >> ${quoteSh(setupLogPath)}`].join('\n') + ) + + const commands = createSequencedSetupAgentCommands({ + runnerScriptPath, + startupCommand: `printf 'agent-start\\n' >> ${quoteSh(logPath)}`, + platform: 'posix', + nonce: 'missing-setup-env', + waitTimeoutSeconds: 120, + startGraceSeconds: 2 + }) + + const startupExitPromise = waitForExit( + spawn('bash', ['-lc', commands.startupCommand], { + stdio: 'pipe', + env: { ...process.env, ...commands.startupEnv } + }) + ) + // The wrapped launch record was dropped on the way to the setup terminal, so the gated + // script is absent from its env; the fallback must still run setup. + const setupExit = await waitForExit( + spawn('bash', ['-lc', commands.setupCommand], { stdio: 'pipe', env: { ...process.env } }) + ) + const startupExit = await startupExitPromise + + expect(setupExit.code).toBe(0) + expect(readFileSync(setupLogPath, 'utf8')).toBe('setup-ran\n') + expect(startupExit.code).toBe(0) + expect(startupExit.stderr).toContain('Setup never reported starting within 2s') + expect(readFileSync(logPath, 'utf8')).toBe('agent-start\n') + } + ) + + it.skipIf(process.platform === 'win32')( + 'reports progress instead of waiting silently', + async () => { + const tempDir = makeTempDir() + const runnerScriptPath = join(tempDir, 'setup-runner.sh') + + writeExecutable(runnerScriptPath, '#!/bin/sh\nexit 0\n') + + const commands = createSequencedSetupAgentCommands({ + runnerScriptPath, + startupCommand: 'printf ready', + platform: 'posix', + nonce: 'progress-sequence', + waitTimeoutSeconds: 4, + startGraceSeconds: 120, + progressIntervalSeconds: 1 + }) + + const startupExit = await waitForExit( + spawn('bash', ['-lc', commands.startupCommand], { + stdio: 'pipe', + env: { ...process.env, ...commands.startupEnv } + }) + ) + + expect(startupExit.code).toBe(124) + expect(startupExit.stderr).toContain( + 'Still waiting for setup to finish before starting agent' + ) + expect(startupExit.stderr).toContain('Waited 4s without a result') + }, + 15_000 + ) + + it('bounds the default wait well under the two-hour silent timeout it replaced', () => { + const result = createSequencedSetupAgentCommands({ + runnerScriptPath: '/repo/.git/orca/setup-runner.sh', + startupCommand: 'claude', + platform: 'posix', + nonce: 'default-bound' + }) + const startupScript = result.startupEnv?.[SETUP_AGENT_SEQUENCE_STARTUP_SCRIPT_ENV] ?? '' + + expect(startupScript).toContain('deadline=$((SECONDS + 1800))') + expect(startupScript).toContain('start_deadline=$((SECONDS + 45))') + expect(startupScript).toContain('next_report=$((SECONDS + 15))') + }) +}) diff --git a/src/shared/setup-agent-sequencing.ts b/src/shared/setup-agent-sequencing.ts index b93fe9c8306..064464ae7a9 100644 --- a/src/shared/setup-agent-sequencing.ts +++ b/src/shared/setup-agent-sequencing.ts @@ -1,4 +1,18 @@ -import { encodePowerShellCommand } from './powershell-command-encoding' +import type { WorktreeSetupLaunch } from './worktree/launch-types' +import { + SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV, + SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV, + SETUP_AGENT_SEQUENCE_STARTUP_SCRIPT_ENV +} from './setup-agent-sequencing-env' +import { + buildPosixSetupCommand, + buildPosixSetupScript, + buildPosixStartupScript +} from './setup-agent-sequencing-posix-gate' +import { + buildWindowsSetupCommand, + buildWindowsStartupCommand +} from './setup-agent-sequencing-windows-gate' import { nativeWindowsPathToPosixShellPath, resolveSetupRunnerCommand, @@ -7,16 +21,51 @@ import { type SetupRunnerShell } from './setup-runner-command' -const DEFAULT_WAIT_TIMEOUT_SECONDS = 2 * 60 * 60 -export const SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV = 'ORCA_SEQUENCED_STARTUP_COMMAND' -export const SETUP_AGENT_SEQUENCE_STARTUP_SCRIPT_ENV = 'ORCA_SEQUENCED_STARTUP_SCRIPT' +// Why: a cold monorepo install plus a native rebuild is the slowest legitimate setup we ship +// against, and that lands in single-digit minutes even on a slow link. 30 minutes leaves several +// times that headroom while still telling the user something is wrong inside one sitting; the +// two-hour bound this replaced was indistinguishable from a hang. Progress ticks below are the +// primary signal — this is only the backstop. +const DEFAULT_WAIT_TIMEOUT_SECONDS = 30 * 60 +// Why: the setup terminal is spawned in the same host operation as the agent terminal and writes +// its start sentinel before running a single line of the script, so anything past a slow shell +// profile plus an SSH round trip means nobody is going to run setup at all. Expiring does not +// fail the launch — it starts the agent unsequenced and says so — so a false positive costs a +// warning line, never a dead terminal. +const SETUP_START_GRACE_SECONDS = 45 +// Why: a silent terminal reads as a hang, so the wait reports itself on a human interval. +const WAIT_PROGRESS_INTERVAL_SECONDS = 15 + +export { + SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV, + SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV, + SETUP_AGENT_SEQUENCE_STARTUP_SCRIPT_ENV +} from './setup-agent-sequencing-env' export type SequencedSetupAgentCommands = { setupCommand: string + /** Must be merged into the setup terminal's env; without it `setupCommand` degrades to the + * bare runner and the gate below reports setup as never-started instead of hanging. */ + setupEnv?: Record startupCommand: string startupEnv?: Record } +/** Folds a sequenced pair's setup half into the launch record every setup launcher already + * consumes. Keeping the gated command and the env that feeds it in one object is what makes the + * pairing structural: a launcher cannot pick up `command` while dropping `setupEnv`, and both + * fields already cross the client/host wire, so no new field is introduced. */ +export function applySequencedSetupLaunch( + setup: WorktreeSetupLaunch, + sequenced: SequencedSetupAgentCommands +): WorktreeSetupLaunch { + return { + ...setup, + command: sequenced.setupCommand, + envVars: { ...setup.envVars, ...sequenced.setupEnv } + } +} + export function resolveSetupAgentSequenceLaunchCommand( env: Record, fallbackCommand: string | undefined @@ -40,6 +89,8 @@ export function createSequencedSetupAgentCommands(args: { shell?: SetupRunnerShell nonce?: string waitTimeoutSeconds?: number + startGraceSeconds?: number + progressIntervalSeconds?: number }): SequencedSetupAgentCommands { const nonce = args.nonce ?? createSetupAgentSequenceNonce() const resolution = resolveSetupRunnerCommand(args.runnerScriptPath, args.platform, args.shell) @@ -55,6 +106,8 @@ export function createSequencedSetupAgentCommands(args: { // a shared completion marker. const markerPath = `${markerBasePath}.${nonce}.done` const waitTimeoutSeconds = args.waitTimeoutSeconds ?? DEFAULT_WAIT_TIMEOUT_SECONDS + const startGraceSeconds = args.startGraceSeconds ?? SETUP_START_GRACE_SECONDS + const progressIntervalSeconds = args.progressIntervalSeconds ?? WAIT_PROGRESS_INTERVAL_SECONDS if (resolution.shell === 'windows' && !posixGateForWindowsRunner) { return { @@ -63,7 +116,13 @@ export function createSequencedSetupAgentCommands(args: { markerPath, nonce ), - startupCommand: buildWindowsStartupCommand(markerPath, nonce, waitTimeoutSeconds), + startupCommand: buildWindowsStartupCommand( + markerPath, + nonce, + waitTimeoutSeconds, + startGraceSeconds, + progressIntervalSeconds + ), startupEnv: { [SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV]: args.startupCommand } @@ -74,10 +133,24 @@ export function createSequencedSetupAgentCommands(args: { args.startupCommand, markerPath, nonce, - waitTimeoutSeconds + waitTimeoutSeconds, + startGraceSeconds, + progressIntervalSeconds ) return { - setupCommand: buildPosixSetupCommand(resolution.command, markerPath, nonce), + // Why: the wrapper embeds the runner path three times plus the nonce, which for an ordinary + // worktree exceeds the 1024-byte canonical input cap a PTY applies before the shell's line + // editor takes over — the submit byte is dropped and setup never records an outcome. Same + // env-var indirection the startup gate already uses; the inline branch keeps a launcher that + // forgets `setupEnv` running the bare runner instead of nothing. + setupCommand: buildPosixSetupCommand(resolution.command), + setupEnv: { + [SETUP_AGENT_SEQUENCE_SETUP_SCRIPT_ENV]: buildPosixSetupScript( + resolution.command, + markerPath, + nonce + ) + }, // Why: long worktree paths can push the gate past a PTY's canonical input cap and drop its submit byte. startupCommand: `bash -lc 'eval "$${SETUP_AGENT_SEQUENCE_STARTUP_SCRIPT_ENV}"'`, startupEnv: { @@ -87,198 +160,6 @@ export function createSequencedSetupAgentCommands(args: { } } -function buildPosixSetupCommand(setupCommand: string, markerPath: string, nonce: string): string { - const marker = quotePosixArg(markerPath) - const tmp = quotePosixArg(`${markerPath}.tmp`) - const nonceValue = quotePosixArg(nonce) - - const script = [ - `rm -f ${marker} ${tmp} 2>/dev/null`, - `( ${setupCommand} )`, - 'status=$?', - `printf '%s:%s\\n' ${nonceValue} "$status" > ${tmp}`, - `mv -f ${tmp} ${marker}`, - 'exit "$status"' - ].join('; ') - - return `bash -lc ${quotePosixArg(script)}` -} - -function buildPosixStartupScript( - startupCommand: string, - markerPath: string, - nonce: string, - waitTimeoutSeconds: number -): string { - const marker = quotePosixArg(markerPath) - const tmp = quotePosixArg(`${markerPath}.tmp`) - const nonceValue = quotePosixArg(nonce) - const timeout = Math.max(1, Math.floor(waitTimeoutSeconds)) - const startupSuccessCommand = buildPosixStartupSuccessCommand(startupCommand) - // Why: the PTY launch path feeds this command through an interactive shell, - // so keeping the wrapper on one line avoids visible `quote>` continuation - // prompts while still preserving valid `while`/`if` shell syntax. - const script = [ - `deadline=$((SECONDS + ${timeout}));`, - 'echo "Waiting for setup to finish before starting agent..." >&2;', - 'while :; do', - `if [ -f ${marker} ]; then`, - `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;`, - 'echo "Setup failed; skipping agent startup." >&2;', - 'exit "${status:-1}";', - 'fi;', - 'fi;', - 'if [ "$SECONDS" -ge "$deadline" ]; then', - 'echo "Timed out waiting for setup before starting agent." >&2;', - 'exit 124;', - 'fi;', - 'sleep 1;', - 'done' - ].join(' ') - - return script -} - -function buildPosixStartupSuccessCommand(startupCommand: string): string { - if ( - hasUnquotedPosixCommandSeparator(startupCommand) || - hasLeadingPosixEnvAssignment(startupCommand) - ) { - return `eval ${quotePosixArg(startupCommand)}; exit "$?"` - } - return `exec ${startupCommand}` -} - -function hasLeadingPosixEnvAssignment(command: string): boolean { - return /^[A-Za-z_][A-Za-z0-9_]*=/.test(command.trimStart()) -} - -function hasUnquotedPosixCommandSeparator(command: string): boolean { - let quote: "'" | '"' | null = null - let escaped = false - for (const char of command) { - if (escaped) { - escaped = false - continue - } - if (char === '\\') { - escaped = true - continue - } - if (quote) { - if (char === quote) { - quote = null - } - continue - } - if (char === "'" || char === '"') { - quote = char - continue - } - if (char === ';' || char === '&' || char === '|' || char === '\n' || char === '\r') { - return true - } - } - return false -} - -function buildWindowsSetupCommand( - runnerScriptPath: string, - markerPath: string, - nonce: string -): string { - // Why: delayed expansion keeps path metacharacters as data when cmd invokes the batch runner. - const script = [ - `$runner = ${quotePowerShellString(runnerScriptPath)}`, - `$marker = ${quotePowerShellString(markerPath)}`, - '$tmp = $marker + ".tmp"', - `$nonce = ${quotePowerShellString(nonce)}`, - 'Remove-Item -LiteralPath $marker, $tmp -Force -ErrorAction SilentlyContinue', - '$processInfo = [System.Diagnostics.ProcessStartInfo]::new()', - '$processInfo.FileName = $env:ComSpec', - '$processInfo.Arguments = \'/d /s /v:on /c ""!ORCA_SETUP_RUNNER!""\'', - '$processInfo.UseShellExecute = $false', - '$processInfo.EnvironmentVariables["ORCA_SETUP_RUNNER"] = $runner', - '$process = [System.Diagnostics.Process]::Start($processInfo)', - '$process.WaitForExit()', - '$setupStatus = $process.ExitCode', - '$utf8 = [System.Text.UTF8Encoding]::new($false)', - '[System.IO.File]::WriteAllText($tmp, ($nonce + ":" + $setupStatus + [Environment]::NewLine), $utf8)', - 'Move-Item -LiteralPath $tmp -Destination $marker -Force', - 'exit $setupStatus' - ].join('; ') - - return encodePowerShellInvocation(script) -} - -function buildWindowsStartupCommand( - markerPath: string, - nonce: string, - waitTimeoutSeconds: number -): string { - const timeout = Math.max(1, Math.floor(waitTimeoutSeconds)) - // Why: native Windows setup runners launch through cmd.exe, but PowerShell - // gives us safe bounded file polling/parsing without a fragile batch label loop. - const script = [ - `$marker = ${quotePowerShellString(markerPath)}`, - 'if ([string]::IsNullOrWhiteSpace($marker)) {', - ' [Console]::Error.WriteLine("Missing setup marker path.")', - ' exit 1', - '}', - '$tmp = $marker + ".tmp"', - `$nonce = ${quotePowerShellString(nonce)}`, - `$deadline = (Get-Date).AddSeconds(${timeout})`, - '[Console]::Error.WriteLine("Waiting for setup to finish before starting agent...")', - 'while ($true) {', - ' if (Test-Path -LiteralPath $marker) {', - ' $content = Get-Content -LiteralPath $marker -TotalCount 1', - ' if ($content -match "^([0-9A-Za-z_-]+):([0-9]+)$" -and $Matches[1] -eq $nonce) {', - ' $setupStatus = [int]$Matches[2]', - ' Remove-Item -LiteralPath $marker, $tmp -Force -ErrorAction SilentlyContinue', - ' if ($setupStatus -ne 0) {', - ' [Console]::Error.WriteLine("Setup failed; skipping agent startup.")', - ' exit $setupStatus', - ' }', - ` $startup = $env:${SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV}`, - ' if ([string]::IsNullOrWhiteSpace($startup)) {', - ' [Console]::Error.WriteLine("Missing sequenced startup command.")', - ' exit 1', - ' }', - ' Invoke-Expression $startup', - ' if ($global:LASTEXITCODE -ne $null) { exit $global:LASTEXITCODE }', - ' if (-not $?) { exit 1 }', - ' exit 0', - ' }', - ' }', - ' if ((Get-Date) -ge $deadline) {', - ' [Console]::Error.WriteLine("Timed out waiting for setup before starting agent.")', - ' exit 124', - ' }', - ' Start-Sleep -Seconds 1', - '}' - ].join('; ') - - return encodePowerShellInvocation(script) -} - -function encodePowerShellInvocation(script: string): string { - return `powershell.exe -NoProfile -NonInteractive -ExecutionPolicy Bypass -EncodedCommand ${encodePowerShellCommand(script)}` -} - -function quotePosixArg(value: string): string { - if (/^[A-Za-z0-9_./:-]+$/.test(value)) { - return value - } - return `'${value.replace(/'/g, `'\\''`)}'` -} - -function quotePowerShellString(value: string): string { - return `'${value.replace(/'/g, "''")}'` -} - export function getSetupAgentSequenceShellForTests( runnerScriptPath: string, platform: SetupRunnerCommandPlatform