Repository navigation
Native chat: every agent launches through one composed launch path - #27204
Merged
Merged
Conversation
…ity oracle Captures the exact child each native-chat agent spawns (Claude, Codex, Grok, OpenCode, OMP, Pi), worktree and floating, fresh and resumed, plus a registered worker with visuals off, at the real spawn boundary (the process host's spawn) and through production's own host dependency wiring. The inherited environment holds every pane, hook, caller, terminal-handle and visuals key a parent Orca pane could hand down, so the snapshot records today's per-agent differences as they are. Each case also records the launch's permission posture, which for Codex (thread policy) and the ACP agents (full access) acts only after spawn. Each agent also runs fresh and resumed with every launch setting moved off its default: a Command setting naming another binary, the permission posture flipped, an environment overlay over inherited and shell keys, and only named login-shell variables inherited (the fake login shell carries its own PATH and a variable no case names). A second test fails when a registered agent has no snapshot, since the oracle lists its agents by hand. Test only: the launch-path refactor that follows must leave both tests and the snapshot file unchanged.
Every native-chat agent (Claude, Codex, Grok, OpenCode, OMP, Pi) now builds its child through one composer instead of six hand-written resolvers. Each agent contributes only its own part (command, args, agent env, keys it must strip); the shared composer admits the session record, reads the launch directory, base environment, launch overrides and visuals once each (memoized, in the agent's original order), sets cwd, and seals the child. The seal (sealStructuredSessionChild) is the one place that names a child: session id, structured marker, worker terminal handle, the Orca CLI env, the spawn token under one shared constant, and the per-launch list of inherited keys to delete, minus any key the seal itself sets. Claude's delete list now reaches its SDK spawn after buildClaudeChildProcessEnv's merge. Behaviour is unchanged: the parity snapshot from the previous commit passes byte for byte, including each agent's existing differences (Claude and Codex keep inherited pane and hook env, Pi has no visuals and always full access, the ACP history read stays unsealed). Admission refusals keep their reasons; only log-only wording moved to the shared check. Drops launch dependencies no production caller wires (resolveLaunchEnv, resolveCodexOverrides) and folds the per-agent env overlay hooks into the existing resolveAgentLaunchEnv.
Follow-ups from review of the composed launch: - The spawn token is required on every resolveLaunch. The one read that starts no child, ACP's restart-recovery history read, now has its own path (composeStructuredLaunchRead, deps.readLaunch) that returns the agent's own env unsealed, as before. Agent parts receive the session identity only, never the token. - structuredSessionChildIdentityEnv is gone; the seal is the only way to build a child's identity env, and tests go through it. - A launch's environment is the runtime's resolveAgentEnvironment, so the base-plus-overlay rule exists once. - Claude's connection test now covers a non-empty envToDelete applied after the inherited env is merged. No change to any spawned child: the parity snapshot is unchanged.
brennanb2025
force-pushed
the
brennanb2025/native-chat-one-launch-path
branch
from
October 10, 2026 09:55
efa20c3 to
d5fd690
Compare
The seal moved from structured-session-child-identity-env to structured-session-child-env, but the PR job classifier still listed the old path, so a change to only the seal would skip the live login-shell contract job. The prefix now covers the seal, its test and its test support (which the login-shell harness imports).
Contributor
Author
Review status: ready for your review (draft, not merged)The problem. Every native-chat agent (Claude, Codex, OpenCode, Grok, OMP, Pi) has to take the same steps to start:
Each agent family had its own copy of those steps, and two user-facing bugs came from a newer agent's copy missing one:
What this PR changes for users. Nothing. This is a strict-parity refactor: every agent now launches through one shared composer and one "seal". Agents can't set their own folder or final environment, so a new agent can't skip a step. Fixed during review:
Deferred. Per-agent differences the snapshot keeps exactly as on main, each a separate follow-up:
Also deferred:
Verified:
Not verified:
Seen in QA, not from this PR:
|
brennanb2025
marked this pull request as ready for review
October 12, 2026 01:12
Contributor
📝 Walkthrough
Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to This refactor is intended to preserve launch behavior, and no concrete regression was identified. The surviving comment is a small readability suggestion and does not block merge. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 42 functions across 51 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
-
Check name Status Explanation Title check The title clearly and concisely summarizes the main change: routing every native-chat agent through one composed launch path. Description check The description is comprehensive and follows the required template. It explains the user impact, mechanism, rationale, parity evidence, testing, cross-platform considerations, and checklist status. Th… 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.
- Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
-
📝 Generate docstrings 💡
-
- 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.
Comment @coderabbitai help to get the list of available commands.
Contributor
Author
Live Electron QA: every native-chat agent still launches and behaves as beforeA hidden macOS build of this PR at 422a743, with the real Claude, Codex, OpenCode, Grok, OMP and Pi CLIs, each on its own existing sign-in. A separate agent drove every click. All rows pass.
Environment seal. The app was started with a fake
That is the same split as main. Permission cards. Only OMP showed one ( Agent config files. The agents' real config files were hashed before, during and after the run, with no changes. Not covered: a side-by-side run of an origin/main build. Equality with main rests on the 43-case spawn snapshot in this PR. Pi in a worktree: OMP's permission card, with the new-tab menu that opened each chat: Floating panel: an OpenCode chat runs in the floating folder: After quit and relaunch, Claude remembers its earlier reply (its environment list still has the pane and tab keys): After quit and relaunch, OpenCode remembers its earlier reply (pane and tab keys removed): An OpenCode orchestration worker reports |
…hat-one-launch-path # Conflicts: # src/main/codex/codex-structured-launch-resolution.ts
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/codex/codex-structured-launch-resolution.ts (1)
72-76: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePass the already-resolved environment instead of calling
basis.environmenttwice.
basis.environmentis wrapped inonce, so both calls return the same promise. This means the behavior is correct. However, the code awaits the environment twice through two paths. Resolveenvironmentfirst and passasync () => environmenttoresolveCodexStructuredInvocation. That makes the single-read intent explicit.♻️ Proposed refactor
- const { command } = await resolveCodexStructuredInvocation({ - ...(deps.resolveCommand ? { resolveCommand: deps.resolveCommand } : {}), - resolveEnvironment: basis.environment - }) - const environment = await basis.environment() + const environment = await basis.environment() + const { command } = await resolveCodexStructuredInvocation({ + ...(deps.resolveCommand ? { resolveCommand: deps.resolveCommand } : {}), + resolveEnvironment: async () => environment + })
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
5c351d84-0c14-447a-909b-740121b0fb0a
⛔ Files ignored due to path filters (1)
src/main/runtime/__snapshots__/structured-agent-launch-parity.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
config/scripts/pr-code-change-scope.mjssrc/main/codex/codex-structured-launch-resolution.tssrc/main/runtime/structured-agent-launch-composition.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.
brennanb2025
added a commit
that referenced
this pull request
Oct 12, 2026
Main's #27204 moved every native-chat agent onto one composed launch path. This PR's launch-side changes are carried onto it: - The composer's sources take the runtime's refreshAfterMissingProgram, and the launch basis exposes it; after a refresh its environment reads return the refreshed environment. Each agent's part (Claude, Codex, ACP, Pi) resolves its program through resolveAgainAfterRefreshOnMissingProgram with it, so a launch that finds no program waits for the one shared refresh. - Claude resolves its command from the launch's own environment. - The parity snapshot gains the three remote selectors the seal sets empty; nothing else in it changed.
2 of 3 tasks
brennanb2025
added a commit
that referenced
this pull request
Oct 15, 2026
…27204) * test(chat): snapshot every native-chat agent's spawned child as a parity oracle Captures the exact child each native-chat agent spawns (Claude, Codex, Grok, OpenCode, OMP, Pi), worktree and floating, fresh and resumed, plus a registered worker with visuals off, at the real spawn boundary (the process host's spawn) and through production's own host dependency wiring. The inherited environment holds every pane, hook, caller, terminal-handle and visuals key a parent Orca pane could hand down, so the snapshot records today's per-agent differences as they are. Each case also records the launch's permission posture, which for Codex (thread policy) and the ACP agents (full access) acts only after spawn. Each agent also runs fresh and resumed with every launch setting moved off its default: a Command setting naming another binary, the permission posture flipped, an environment overlay over inherited and shell keys, and only named login-shell variables inherited (the fake login shell carries its own PATH and a variable no case names). A second test fails when a registered agent has no snapshot, since the oracle lists its agents by hand. Test only: the launch-path refactor that follows must leave both tests and the snapshot file unchanged. * refactor(chat): one composed launch for every native-chat agent Every native-chat agent (Claude, Codex, Grok, OpenCode, OMP, Pi) now builds its child through one composer instead of six hand-written resolvers. Each agent contributes only its own part (command, args, agent env, keys it must strip); the shared composer admits the session record, reads the launch directory, base environment, launch overrides and visuals once each (memoized, in the agent's original order), sets cwd, and seals the child. The seal (sealStructuredSessionChild) is the one place that names a child: session id, structured marker, worker terminal handle, the Orca CLI env, the spawn token under one shared constant, and the per-launch list of inherited keys to delete, minus any key the seal itself sets. Claude's delete list now reaches its SDK spawn after buildClaudeChildProcessEnv's merge. Behaviour is unchanged: the parity snapshot from the previous commit passes byte for byte, including each agent's existing differences (Claude and Codex keep inherited pane and hook env, Pi has no visuals and always full access, the ACP history read stays unsealed). Admission refusals keep their reasons; only log-only wording moved to the shared check. Drops launch dependencies no production caller wires (resolveLaunchEnv, resolveCodexOverrides) and folds the per-agent env overlay hooks into the existing resolveAgentLaunchEnv. * refactor(chat): only the seal names a native-chat child Follow-ups from review of the composed launch: - The spawn token is required on every resolveLaunch. The one read that starts no child, ACP's restart-recovery history read, now has its own path (composeStructuredLaunchRead, deps.readLaunch) that returns the agent's own env unsealed, as before. Agent parts receive the session identity only, never the token. - structuredSessionChildIdentityEnv is gone; the seal is the only way to build a child's identity env, and tests go through it. - A launch's environment is the runtime's resolveAgentEnvironment, so the base-plus-overlay rule exists once. - Claude's connection test now covers a non-empty envToDelete applied after the inherited env is merged. No change to any spawned child: the parity snapshot is unchanged. * ci: run the login-shell contract job for the renamed child-env seal The seal moved from structured-session-child-identity-env to structured-session-child-env, but the PR job classifier still listed the old path, so a change to only the seal would skip the live login-shell contract job. The prefix now covers the seal, its test and its test support (which the login-shell harness imports). (cherry picked from commit d3182d4) Backport to 1.4.224: tests import spawnProcess/runProcess/ProcessSpec from src/shared/child-process instead of @orca/process-host (the parity test mocks run-process at the same spawn boundary).
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ELI5
Orca's chat view runs six agents: Claude, Codex, OpenCode, Grok, OMP and Pi. Starting any of them takes the same handful of steps: pick the folder the chat runs in, build its environment, tell it which Orca chat it is, and strip settings it must not inherit.
Today each agent family has its own copy of those steps. When a newer agent's copy missed one, users saw bugs:
This PR gives every agent one shared launch path, so a new agent can't skip a step. Nothing changes for users.
What Changed
User-facing: nothing. This is a strict-parity refactor. Every agent starts the same program, with the same arguments, folder and environment, in the same order as before.
Mechanism:
composeStructuredLaunch).sealStructuredSessionChildis the only code that builds a chat process's identity environment. That environment is the chat's Orca session id, Orca's own CLI on PATH, and the spawn token.Why
The bugs all came from one agent's launch skipping a step that the others repeat. Making each agent repeat the step correctly would leave the next agent to miss it again. Here the shared steps live in one composer and one seal that every registration has to go through.
Alternatives considered:
Parity evidence
ORCA_PANE_KEY, hook environment, and tab, worktree and launch-token keys. OpenCode, Grok, OMP and Pi strip them.ORCA_TERMINAL_HANDLE; Pi strips it unless the chat is a worker.Linked Issue
N/A: maintainer refactor. It follows up the orchestration-workers fix (#26776) and the floating-folder fix (#26981).
Visual Proof
N/A: no user-visible change. Parity is proven by the spawn snapshot above.
Testing
@orca/process-hostspawn boundary: command, arguments, folder, environment and deletions, plus each agent's permission settings.ORCA_REAL_CLAUDE_CLI_TEST=1) pass 13/13.pnpm tc:nodeand all 24orca-ci-checkspass.Review
Agent skill upstream boundary
Notes
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)