[brennanb2025]

	Files	Added	Deleted	Net
Test	2	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​176	$\color{#cf222e}{\Huge{\mathbf{−}}}$​2	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​174
Prod	5	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​322	$\color{#cf222e}{\Huge{\mathbf{−}}}$​371	$\color{#cf222e}{\Huge{\mathbf{−}}}$​49

ELI5

Orca can start a terminal two ways: the window asks for it directly, or Orca's host starts it (for the CLI, agent launches and paired devices). Each way kept its own copy of the step that builds the terminal's environment: resuming a Codex session, picking which Codex account home to use, waiting for a managed Codex login, and adding Orca's own variables (agent status hooks, the Orca CLI path, proxy settings and so on). Two copies drift apart, and "the environment differs between the two paths" was one of the reasons an earlier attempt to move the "+" button onto the host's path had to be reverted (#27173). This PR makes that step one shared function. Nothing changes for users yet: the places where the copies still disagree are passed in as explicit inputs, and the temporary ones name the follow-up that removes them.

What Changed

Before (on main): the window lane (pty:spawn, ipc/spawn-env-codex.ts) and the host lane (runtime/spawn-preflight.ts) each had about 120 near-identical lines covering:

• Codex resume preparation and launch;
• Codex home selection and the managed-login wait;
• backfill recovery;
• the decision whether to drop or strip CODEX_HOME;
• the session-id safety check;
• building the daemon spawn's host env, with the same options;
• the two env stamps and the Agent Teams PATH promotion.

After:

• resolvePtySpawnHostEnv (src/main/ipc/pty/spawn-host-env.ts, next to last PR's spawn-startup-target.ts) does all of that once and returns:
  • the resume result and launch command;
  • the env before Orca's additions and the final env;
  • the Codex home and its flags.
• Both lanes call it. The window keeps its two window-only lines; the host keeps its Claude profile/auth, binding check and terminal handle ahead of the call.
• The resume steps' dependency type was declared twice, once per lane; it is now declared once (PtySpawnCodexResumeDeps).

Lane differences, now explicit inputs:

Input	Window	Host	Kind
awaitsWslPiOmpGuestReadiness	no wait	waits for a WSL guest's Pi/OMP relay before a Pi/OMP launch	TEMPORARY. Removed by its own small PR stacked right after this one: "window daemon spawns await WSL Pi/OMP guest readiness, like host spawns". #21721 added the wait to the host path and the local provider but not the window's daemon path.
emptySessionIdSkipsHostEnv	an empty id is refused ("Invalid PTY session id")	an empty id builds no host env and the spawn goes on. The host trims, so a blank id from a paired client becomes empty.	TEMPORARY. Removed by row 7 (one session-id rule for both lanes, trimming included).
callerNamedSession	args.sessionId !== undefined	the trimmed caller id is defined	Intended, same meaning: only a launch whose caller named no session waits on a managed Codex login. A host create operation's derived id is not caller-named, so it waits.
mintedSessionId	the id was minted in this request	requestedSessionId === undefined, which is the same fact (sessionId = requested ?? mint)	Intended, same meaning: a failed env build sweeps per-PTY state only for an id this request minted, never for a caller or create-operation id. Kept byte for byte.
resumeOnSelectedAccount	an account-switch restart (replacesPtyId)	always false	Intended: only the window has a restart-in-place request.

Removed with no behavior change, because it was unreachable:

• The window's "no Codex home when there is a connection id" check. getProvider throws for any non-empty connection id before this step runs, in both lanes. The existing "only this machine spawns here" comment covers it.

The resume step's launchEnv is now the lane's env before the resume strip, in both lanes. The window already passed exactly that. The host passed the raw request env; it now passes the same env plus its Claude profile pointer and terminal handle.

• Since fix(orcad): orca serve owns its Claude and Codex accounts again #29209, orca serve reads CODEX_HOME from this env, to skip resume prep for a client-sent account home. The desktop implementation ignores it.
• CODEX_HOME is the same in both envs: the Claude profile adds only CLAUDE_* keys, the handle adds ORCA_TERMINAL_HANDLE, and Claude auth never runs for a Codex resume.
• A new pin checks that both lanes hand resume prep the request's CODEX_HOME.

Not part of this step, so unchanged here:

• The Codex-home lifecycle report at commit: the window reports the env before Orca's additions, the host the final env. The function returns both, and each commit reads what it read before. Row 6 (the commit step) owns it.
• Inside the host lane, a failed env build sweeps per-PTY state only for a minted id, while the host's own failed-start cleanup also covers create-operation and new-session ids. Aligning them changes behavior, so it is a noted side-fix for row 7.

Orderings that move, with no behavior change:

• The host's resume step now runs after its Claude profile/auth, binding check and handle stamp.
  • The normal Claude-auth and Codex-resume paths remain separate, and successful launches receive the same command and environment. Both callers now await the shared function, adding a promise boundary; identical microtask timing is not claimed. The unchanged launch-race parity suite passes.
  • One failure-path effect: a host spawn that asks to persist its binding without the ids (a caller bug that throws) now throws before resume preparation rather than after. Resume preparation only repairs hooks and migrates legacy rollouts, and the next valid launch does both.
• The host now copies its env before adding Orca's variables, as the window already did. It used to add them to its working env in place. The clone preserves all environment values, and the provider and commit consumers use the returned final environment. The caller's original object is no longer mutated by host injection.

Users see: no change.

Why

This is row 4 (first half) of the launch-convergence plan. Every later step that moves a producer onto one spawn path needs each spawn step to exist once. The host-env step was the largest duplicated block. A one-line host-env option added in one lane and forgotten in the other is exactly how the two paths drift (#28180 currently has to add its new option twice). Passing the differences as named inputs keeps this PR a pure merge. Fixing the Pi/OMP wait or the empty-id split here would change behavior, so each gets its own PR.

Alternatives considered:

• Keep resume preparation in each lane and share only the rest. That leaves about 15 duplicated lines in the step most likely to change next (the account home it resumes into).
• One cleanup input named after the failed-start rule (ownsNewSession). That would quietly widen the host's env-failure sweep to create-operation and new-session ids. It is kept as mintedSessionId and noted for row 7.

Linked Issue

Part of the launch-convergence plan (row 4, first half); no issue.

Visual Proof

Real Electron comparison on Windows high spec, driven through Chrome DevTools Protocol with isolated homes: main d9521f9991b versus PR ccff2d60308. Command Prompt was selected through Settings in both builds. The "+" terminal environment, Codex command/environment up to its sign-in screen, and system-default account-home UI matched. Build-specific labels and resource/shadow rendering differed as expected. No authenticated Codex turn is claimed.

Blocked: Session History resume would inspect WSL homes outside the rig's isolation; Windows PowerShell would load a real Documents profile; an alternate managed account requires a sign-in. Resume and selected-account wiring have automated contract coverage. Real-config hash/stat guards held, and the QA worker stopped every recorded process.

Screenshots (main, then PR):

Testing

Commit 1 pins main's behavior before the refactor (passes on main):

• pty-daemon-spawn-session-identity.test.ts:
  • which failed env builds sweep per-PTY state: window minted (yes), window caller id (no), host minted (yes), host create operation (no), host new-session caller id (no);
  • the empty-id split;
  • the host-only WSL Pi/OMP readiness wait.
• pty-spawn-codex-home-unavailable.test.ts: a host create operation waits on a managed Codex login; a caller-named session does not, in either lane.

After syncing main once (#29209): a pin that both lanes hand Codex resume prep the request's CODEX_HOME. The same test now covers a window account restart with resume metadata: only the restart resumes into the selected home, while ordinary resumes retain their source home. Removing the selected-account forwarding makes that restart row fail.

Commit 2 keeps every suite unchanged and green. After the main sync: 27 files / 414 tests, including #29209's orcad-account-services:

• pty-launch-parity, pty-launch-race-parity, pty-daemon-spawn-session-identity;
• the Codex env/home suites (pty-daemon-spawn-codex-auth, pty-daemon-spawn-agent-home-env, pty-spawn-env-codex-home-routing, pty-spawn-env-codex-resume-provenance, pty-spawn-codex-home-unavailable, pty-codex-account-attribution);
• the startup resume suites (codex-account-restart-resume, codex-session-resume-daemon-guard, codex-launch-per-agent-hook-opt-out);
• pty-spawn-env-agent-overlays, pty-agent-pane-launch, pty-daemon-spawn-wsl-runtime, pty-spawn-cwd-fallback, runtime/spawn-preflight-requested-shell, and the neighbouring spawn suites.

Final main integration: the shared builder preserves main's terminal-image setting once, and both lanes preserve main's dropped-resume decision for the new identity consumer. Final head e874c02c3168b547ad7b086458c21a66efeafda9 passed all 34 explicitly named suites (491 tests), including both new resume-identity contracts, environment, parity/races, SQLite boundary, provider routing and repaired renderer fixtures. The narrow integration reviewer agreed with no findings. Full PR CI passed on this exact head, including static analysis/typecheck, both packaging jobs, all five test shards and the aggregate. The PR is out of draft, CLEAN and has zero unresolved review threads.

The builder checked readiness always on, empty-id skip off, always sweep and always wait against the parity pins. Review also removed selected-account forwarding temporarily: the new restart row failed, then the restored implementation passed.

• Automated tests added/updated
• Remote Electron comparison completed for the reachable rows above; blocked rows and limits recorded. Lint and typecheck: CI.

Platforms: Windows and WSL behavior is covered by the platform-switching suites. SSH: an SSH connection never reaches this step (the provider lookup refuses it first), and the remote host runs its own host lane.

Size

Production net −49 (+322 / −371) against the final merged main. Test additions remain below production additions; review extends the existing home-selection contract test rather than adding another fixture.

Area	Lines
spawn-host-env.ts (new shared step)	+256
window spawn-env-codex.ts	+31 / −161
host runtime/spawn-preflight.ts	+31 / −159
both deps types (one resume-deps type)	+4 / −51

Overlaps with open PRs

• feat(browser): option to route agent browser work to the Orca browser #28180: its new buildPtyHostEnv option lands once in spawn-host-env.ts instead of twice.
• feat(claude-accounts): per-project Claude account (ask on launch, remember per project) #23228 and PR 6a: the runtime holds the tab layout (holder becomes the source in main) #27759: textual conflicts in runtime/spawn-preflight.ts.
• fix: a terminal pane no longer resumes a conversation a native chat owns #28288: no shared hunks.

Agent skill upstream boundary

• Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Checklist

• This PR is small and focused
• I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
• Before/after screenshots or videos attached for UI changes, or N/A with reason
• Self-reviewed for correctness, security, and performance
• Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
• pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

[brennanb2025]

Two terminal-start paths each maintained their own account and environment preparation, allowing the same change to reach one path and miss the other. This PR consolidates that preparation into one function while preserving the existing behavior of each path; users should see the same terminals, commands, accounts and notices.

The review covered the full diff with independent correctness and entry-point traces plus an architecture challenger. It added a missing integrated test for resuming a conversation during an account-switch restart and corrected an unsupported timing claim in the description. Removing selected-account forwarding makes that test fail; restoring it passes. No production regression was found. The final main integrations preserve main's terminal-image setting in the shared builder and carry its dropped-resume decision to the new resume-identity consumer. The continued reviewer approved the image-setting integration, and a fresh narrow reviewer approved the resume-identity integration with no findings.

Verification: 34 named suites / 491 tests passed on this head, including both new resume-identity contracts, the SQLite runtime boundary, existing provider routing contract and main's repaired headless renderer tests. Full CI passed on this exact head, including static analysis/typecheck, both packaging jobs, all five unit-test shards and the aggregate gate. GitHub confirms CLEAN, out of draft and zero unresolved threads. The remote-resume review claim was traced through the actual renderer, preload and both spawn paths and refuted: both reject nonempty connection IDs before shared environment preparation. Production is net −49 lines. The existing differences in WSL readiness, empty-session handling and lifecycle environment reporting remain deliberate follow-ups; the existing host cleanup difference is unchanged. The shared async call adds a promise boundary, so identical microtask timing is not claimed.

Real Electron QA used hidden Windows high spec copies, CDP, isolated homes and Command Prompt: main d9521f9991b versus PR ccff2d60308. The "+" terminal environment, Codex launch command/environment up to the sign-in screen, and system-default account UI matched. Commit labels and resource/shadow paint were expected rig differences. Config hash/stat guards held and recorded processes were stopped. Blocked: Session History resume would inspect WSL homes outside isolation; default PowerShell would load a real Documents profile; an alternate managed account needs a sign-in. Authenticated Codex work and those blocked rows were not verified. Automated tests cover resume and selected-account wiring. Screenshots predate the final main integration; that integration is checked by source review and the post-merge tests.

For Brennan's review

Main/PR screenshots, in pairs:

Screenshots (main, then PR):

Three deliberate boundaries to inspect:

• Preserved WSL readiness and empty-session inputs, removed by their planned follow-ups.
• Pre-injection environment retained for existing lifecycle reporting, until the commit-step consolidation.
• Main's image-setting option preserved once.

[coderabbitai]

📝 Walkthrough

The change adds a shared resolver for PTY spawn environment setup. The IPC and runtime spawn paths now delegate resume, Codex-home, and host-environment resolution to it. Tests cover differences between host and window spawns, including session IDs, readiness waits, managed login, and resume preparation.

Priority: ➖ Normal

Merge Risk: 🟡 Moderate · up to 8028a

Resuming a Codex session on a remote connection can now run local resume setup, which may select or migrate the wrong local Codex home. Pass the connection ID through before merging.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 7 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Title check		The title clearly and concisely describes the main change: consolidating host-environment preparation into one step for both terminal spawn lanes.
Description check		The description is detailed and covers the change, rationale, scope, testing, visual proof, platform considerations, follow-up differences, and checklist. It does not provide a linked issue and omits …

Full details: Description check

Explanation

The description is detailed and covers the change, rationale, scope, testing, visual proof, platform considerations, follow-up differences, and checklist. It does not provide a linked issue and omits explicit AI Disclosure and Review sections, but the remaining information is substantially complete.

• Fix all pre-merge checks with AI

✨ Finishing Touches: 💡 2

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: ad9641b1-4a42-4a5d-8c3a-c340e9a27658

📥 Commits

Reviewing files that changed from the base of the PR and between e687b18 and 8028a7f.

📒 Files selected for processing: 7

• src/main/ipc/pty-daemon-spawn-session-identity.test.ts
• src/main/ipc/pty-spawn-codex-home-unavailable.test.ts
• src/main/ipc/pty/ipc/spawn-env-codex.ts
• src/main/ipc/pty/ipc/spawn-types.ts
• src/main/ipc/pty/runtime/controller-deps.ts
• src/main/ipc/pty/runtime/spawn-preflight.ts
• src/main/ipc/pty/spawn-host-env.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.