From 742955e949b189f16c8ce2804f459703dca23162 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 27 Aug 2026 12:05:41 -0700 Subject: [PATCH] fix(setup-sequencing): stop a failed setup hanging the gated agent terminal for two hours The agent gate's only evidence that setup ran is a marker file written by the gated setup command, and nothing guaranteed that command ever ran. The gated command inlined the runner path three times plus the nonce into one typed line -- 1033 bytes for an ordinary worktree, past the 1024-byte canonical input cap a PTY applies before the shell's line editor takes over -- so the submit byte was dropped, no marker was written, and the waiter sat silent for its full two-hour bound. The background-terminal launcher rebuilt the bare runner and ignored the gated command outright, and a swallowed setup spawn left nothing to record an outcome at all. Move the gated script into ORCA_SEQUENCED_SETUP_SCRIPT (the same indirection the startup gate already used) with the bare runner as an inline fallback; record the status from an EXIT trap / try-finally so an interrupted or unexecutable runner still reports; write a .started sentinel whose absence is how the never-started case is recorded from the execution host itself; and fold the gated command and its env into one WorktreeSetupLaunch record so no launcher can take one without the other. Bound the wait at 30 minutes with 15s progress lines and a 45s start grace that launches the agent unsequenced rather than waiting out a result nobody will record, and name the exit status in the agent terminal instead of leaving the failure on a tab the user has to go find. Split setup-agent-sequencing.ts into env/posix-gate/windows-gate to stay under max-lines. --- LANE-REPORT.md | 246 +++++++++++++ src/main/ipc/worktree-remote.ts | 30 +- .../ipc/worktrees-local-create-flow.test.ts | 7 +- src/main/runtime/orca-runtime.test.ts | 47 ++- src/main/runtime/orca-runtime.ts | 80 ++--- .../launch-worktree-background-terminals.ts | 14 +- .../worktree-activation-setup-script.test.ts | 36 +- .../worktree-activation-web-runtime.test.ts | 5 +- .../src/lib/worktree-default-terminal-tabs.ts | 11 +- .../lib/worktree-initial-terminal-seeding.ts | 34 +- .../lib/worktree-setup-issue-command-queue.ts | 8 +- src/shared/setup-agent-sequencing-env.ts | 6 + .../setup-agent-sequencing-posix-gate.ts | 157 ++++++++ .../setup-agent-sequencing-windows-gate.ts | 126 +++++++ src/shared/setup-agent-sequencing.test.ts | 337 +++++++++++++++++- src/shared/setup-agent-sequencing.ts | 279 +++++---------- 16 files changed, 1078 insertions(+), 345 deletions(-) create mode 100644 LANE-REPORT.md create mode 100644 src/shared/setup-agent-sequencing-env.ts create mode 100644 src/shared/setup-agent-sequencing-posix-gate.ts create mode 100644 src/shared/setup-agent-sequencing-windows-gate.ts 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