Repository navigation
test(launch): pin what each agent launcher produces on main today - #27189
Conversation
…lemetry per launcher
…red new-tab launches
… tabs and the WSL cwd shell rule
…ed tables; fold duplicates
…ty, pane size, env values, hidden and paired-host spawns Round-2 review fixes: local Windows ConPTY panes withhold the kitty flag, the pane's measured grid reaches the provider, agent/caller env values and the hidden mark are provider facts, rows 5r/7 run through the host's RPC handlers, SSH folder rows quote an apostrophe, Windows clients use their real '' default shell, titled automations only, the quick composer drives its real selection hook, and inputs no producer builds are gone.
…e paired RPC request
… 10, real desktop caller, older-host seeding, local Windows folder composer, gone automation root
…their OS, wsl$ and SSH folders quote for Linux, older paired hosts get the host's quoting
Review status (coordinator)The problem. Orca starts terminal agents from about ten different places (typed-prompt tabs, AI buttons, OpenCode panes, desktop automations, Antigravity "continue", paired-server tabs, the workspace composers, phones, workers and other host callers). They go through two separate planners and two separate spawn code paths, which have drifted apart: different shells, quoting, environment variables, start sizes, telemetry and folder fallbacks. A planned series of PRs moves every launcher onto one host launch path, and each step must keep each launcher exactly as it behaves today. Nothing recorded what "today" is. The user-facing change. None. Tests only; no production code changes. The tests record, per launcher, what main produces at the point where the terminal process is started (both code paths, observed at the same layer) and what the desktop window sends to the main process, in one table per launcher, so each later PR can run the same table against its new path. The "+" launchers, the AI buttons' window-side route and the "+"-specific open questions come in a small follow-up PR once #27173 (the revert of #26489) merges, because until then main's "+" behaviour is the route being reverted. Review. Five rounds with fresh reviewers each round, tracked in a ledger:
Proof that the tests pin main. Production code in this branch is identical to main. Across rounds, fresh reviewers independently removed one production line per suite and per launcher table (over 70 in total, mostly different lines from the builder's), and every test went red for the right reason and green again when restored; the few that stayed green exposed rows that could not fail, which were fixed or removed. Two images in the PR body show the suites passing and one such removal. Live QA. Not applicable: no production change, so there is nothing to exercise in the app. The Windows and WSL cases are pinned at the code level; their behaviour on a real Windows machine was not checked. CI. Green at 3cbc1cb (after the one merge of main): all 14 required checks pass (static analysis + typecheck, repository guards, the 5 test shards, relay integration, both package jobs, line-count and selection checks), 17 skipped by path filters; no failures, so nothing to classify against main's known failures. Not verified.
Update after the value audit (head 18ad96f)Why it changed. Brennan asked that every line of these tests earn its place, and an independent audit listed the cuts. They were applied exactly:
3,844 added lines in 21 files became 1,967 in 11. Re-checked. The branch merged main once (no production change), and nothing left needed re-pinning. A fresh reviewer confirmed the audit was applied as written, the moved files match byte for byte, and the suites pass on the newest main. It broke one production line per remaining suite: each went red. One break stayed green because production sets that value in two places, so a different line was broken and that one went red. Every remaining suite names the later PR that relies on it. CI. Green at 18ad96f, no failures. Not pinned here. The "+" launchers come in C0a-2. The phone's path from its launch request up to the terminal options is not pinned (it needs the native-chat database). SSH launches wait for the SSH-runtime work. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The parity tests introduce no established merge-blocking risk. Windows behavior remains simulated rather than tested on a real Windows machine. Pre-merge checks |
|
…s against Drop SSH rows (the SSH PTY provider is gone from main), rows whose behaviour is already decided to change, rows existing tests already pin, and the hostAdmittedMembership assertion main removed. Workspace-creation and paired new-tab producer tests move to C4. Each remaining suite names the later PR that relies on it.
…ture The focus-read ratchet counts getDefaultSettings in a new test fixture; the suites build the settings instead. Type the row 8 telemetry literal.
ELI5
Orca starts a terminal agent (Claude, Codex, OpenCode and so on) from many places: the "+" menu, a typed prompt, the fix-checks and other AI buttons, automations, the phone, workers. Those launchers reach the agent through two separate code paths. One is planned in the desktop window and spawned when the tab's pane mounts (the window lane). The other is planned and spawned by the host process (the host lane). The two were written at different times and have drifted: they disagree on which shell runs the command, how the command is quoted on Windows and WSL, which environment variables the agent sees, and which telemetry fires.
A coming migration moves every launcher onto one path. It has to keep each launcher behaving as it does today, except where a change is decided on purpose, and nothing currently pins that. This PR writes down what each launcher produces on main right now, as tables the later PRs run against their new path.
It keeps only the cases a named later PR of the migration checks against (each suite says which one). Cases that only one later PR (the workspace-creation and paired-tab move) uses move to that PR, which will pin main as it is when that work starts.
Coming in a small follow-up PR, not here. These wait until #27173 (the revert of #26489) merges, because until then main's "+" behaviour is the route being reverted:
\\wsl$folder is quoted.What Changed
Tests only. No production file changes, so nothing is different for users. (The size table above counts the seven
*.test-fixture.ts/*.test-cases.tsfiles as production because the counter only recognises*.test.ts; they are test-only tables and fixtures that nothing in production imports.)How the tests are built. Each launcher ("producer") has one table of cases: a workspace (repo or folder; local
C:\,\\wsl$or POSIX; macOS, Linux or Windows), its saved settings, and what main produces for it. The tests observe each lane at the layer the migration keeps:pty:spawnrequest it sends to main. The table holds that exact request.pty:spawnhandler to a fake terminal provider. The table holds what reaches the provider: command, folder, the pane's size, shell, WSL distro, the pane'sORCA_*keys, the agent's saved env values, whether main marks the terminal hidden,agent_startedtelemetry and the saved pane record.createTerminal, or the host's own RPC handler for paired and OpenCode requests) goes through the real runtime controller to the same fake provider. The table also holds the phone's view of the tab, the saved pane record and telemetry. The AI-button rows and the phone quick-command rows enter at thecreateTerminaloptions the host's launch handler (agent.launch) builds, not atagent.launchitself: the phone path from its launch request to those options needs the native-chat database, and the AI buttons' route is unified away by C2.Each suite starts with a one-line comment saying it pins main's current launch behaviour as the convergence parity baseline, and names the later PR that checks against it.
Tables (one per producer):
src/shared/launch-parity-window-cases.test-fixture.tslaunch-parity-window-tab.test.ts(renderer),pty-launch-parity.test.ts(main)src/shared/launch-parity-automation-cases.test-fixture.tslaunch-agent-background-session-launch-parity.test.ts(renderer),pty-launch-parity.test.ts(main)pty-launch-parity-host.test-cases.tspty-launch-parity.test.tspty-launch-parity-caller.test-cases.tspty-launch-parity.test.tsOutside the tables (
pty-launch-parity.test.ts): a failed window-lane spawn reportsagent_error(2 cases, C5), an orchestration worker asks the window to reveal its titled tab without activating it (C5), and main rewrites stale pane identity keys from the request's tab and leaf (C5).terminal-startup-queue-restart.test.tspins that a queued startup that never reached a pane is not saved across a restart (C2).Findings on main (pinned as-is). The tests keep these exactly as they are, so the migration changes each one only on purpose:
ORCA_WORKSPACE_IDis always sent by a window pane, never by an automation, and by the host side only for folders.agent_startedsaysunknown.What this round removed, and why
This round halves the PR: it goes from 21 files and 3,844 added lines to 11 files and 1,967. Every remaining case is one a named later PR checks against.
hostAdmittedMembershipassertion. Main removed that field (Save runtime-started tabs in the tab bar before their terminal starts #27083).agent_error(decided to report it), the Codex no-prompt shell wait (decided: always wait), and rules a moving PR rewrites anyway (the host's\\wsl$repo quoting, the deleted-folder fallback, an automation in aC:\WSL project, a work item's PowerShell quoting).\\wsl$rows (pty-subprocess-wsl-launch.test.tsis back to main).web-runtime-session-terminal-host-authority.test.ts. They pin main as of before this round's merge, so C4 re-pins them when it starts.Why
The migration is meant to change nothing a user can see until each difference is changed on purpose. Without a written baseline, a reviewer can't tell a deliberate change from an accidental one. The tables are data, so the PR that moves a launcher can feed the same cases to the new path and must produce the same facts.
Alternatives considered:
Linked Issue
N/A (maintainer change; no issue).
Visual Proof
Tests only, so there is no UI change to show. These are the terminal outputs.
All 4 changed test files pass (57 tests) at
18ad96f59aa, merged with main, with no production file changed:One ablation: the line that makes a local Windows
C:\path quote for the client's OS is replaced, the WindowsC:\folder row goes red (Linux quoting comes back), the line is restored, the tree is clean, and the suite is green again:Testing
The 4 changed test files pass on this branch, merged with current main (57 tests).
Ablation: for each remaining table and suite, I changed one production line it pins and confirmed the suite fails, then restored the line by writing back the original text. I made 14 changes. 13 turned their suite red. One stayed green because the transcript path is rewritten in two places; I replaced it with a change to the shared function both use, which turned red. The tree was clean after every change.
pnpm tc:node,pnpm tc:weband the CI static checks pass locally. The one local red istypecheck:ci, which needs a newer Bun than this machine has (bun checkis missing in 1.3.14);tc:nodeandtc:webcover the two projects this PR touches.Platforms: the tests simulate Windows, macOS and Linux clients and hosts. I ran them on macOS.
I manually tested these changes locally
Automated tests added/updated, or explained why not below
Open questions the tests answer
What shell does a
\\wsl$folder tab get, and how is its command quoted?VERIFIED by running the real producer, store and pane.
createTabgives every local tab on Windows a shell, and for a\\wsl$path, folders included, that shell iswsl.exe. The window treats a\\wsl$folder's host as Linux, so the command is quoted for Linux, and main receiveswsl.exewith distroUbuntu. The two agree. Not run on a real Windows machine.Do automations quote by the same rule as window tabs?
VERIFIED, and pinned. Yes: both quote for the OS of the host that runs the agent. An automation's terminal shell is
wsl.exeonly for a local\\wsl$workspace; everything else gets the settings shell.Which rules does an OpenCode pane with a model pick use?
VERIFIED, and pinned. The host's: the command is quoted by the host's rule and the shell comes from the host's spawn path. With cmd.exe as the Windows shell, a Windows
C:\repo gets cmd quoting in cmd.exe.Is a queued tab startup saved across an app restart?
VERIFIED with a test: no. A window-side new tab is saved with its agent identity but not its queued command, so after a restart it opens as a plain shell.
AI Disclosure
Review
Agent skill upstream boundary
Notes
Ensure no issues in: Security, Cross-platoform support (Linux, Windows, Mac), Remote SSH, Mobile, general backwards compatibility, performance
Tests only. No wire, host, storage or UI change.
Author: @BrennanKB5
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)