Skip to content

fix(pty): Pi and OMP agents started from the window in WSL report status from their first launch - #29780

Merged
brennanb2025 merged 2 commits into
mainfrom
brennanb2025/conv-4b-window-daemon-wsl-readiness
Oct 16, 2026
Merged

brennanb2025 merged 2 commits into
mainfrom
brennanb2025/conv-4b-window-daemon-wsl-readiness

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​3 $\color{#cf222e}{\Huge{\mathbf{−}}}$​4 $\color{#cf222e}{\Huge{\mathbf{−}}}$​1
Prod 3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​7 $\color{#cf222e}{\Huge{\mathbf{−}}}$​13 $\color{#cf222e}{\Huge{\mathbf{−}}}$​6

ELI5

A Pi or OMP agent started from the Orca window inside WSL could run without working/idle status because its terminal started before the WSL status relay was ready. The window now uses the same preparation already used by the other terminal launch paths.

What Changed

The shared daemon environment builder now waits for Pi/OMP guest preparation before reading the endpoint and extension paths. The caller can no longer skip that preparation merely because a window requested the terminal. Local-provider and adopted terminals return before this step, so they gain no duplicate wait.

When preparation finishes within the existing ten-second limit, the first launch receives the guest status endpoint and extensions. If preparation fails or times out, the agent still starts and status may remain degraded. A cold launch can therefore take up to ten seconds longer; tab order, selection, setup and notices are unchanged.

Why

The earlier consolidation in #29297 preserved the window/host difference as a temporary boolean. Removing that input fixes the omitted launch path at the existing shared boundary. Turning the synchronous environment builder into another asynchronous layer would add changes without another launch path to cover.

This follows the common pattern of preparing terminal integration before process creation. The intended difference is that optional status preparation remains bounded and nonfatal.

Linked Issue

Follow-up to #29297 in the terminal launch convergence work. The separate issue where a daemon startup fallback lacks the entire host environment remains tracked in Linear STA-10858.

Visual Proof

Windows Electron comparison completed on main 79b916395150eeee2954f03c30cc1744d95f6bcf and final PR 0a278751079c4c316e1b8c4fdadb59be9e365c87. Native launch/environment parity passed; normalized recorded fields have no differences.

Flow Main PR
Settings: Command Prompt selected Main Settings PR Settings
Normal + shell environment Main shell PR shell
Absolute-path stand-in environment Main stand-in PR stand-in

The comparison covers Command Prompt selected through Settings, a normal + terminal, and an absolute-path stand-in launched from the window. Stand-ins verify commands/environment only. WSL endpoint/status and resume rows are blocked: the available distro uses a real Linux home that the rig cannot isolate, and suitable Pi/OMP are unavailable without installation/sign-in. No live first-launch WSL status result is claimed.

Both heads also reproduce three pre-existing failures in the hidden rig: the first default tab needs a tab switch to start, a launch-failure notice precedes the successful stand-in, and CODEX_HOME is absent from panes. No fix is included for those independent behaviors, and no real-agent success is claimed. An automatic session-search promotion appeared during PR Settings navigation; this timing difference was not established as a regression, and search was never opened.

Both rig process trees stopped and ports cleared; protected hashes were stable within each arm. A config hash changed between arms while neither rig was active; the report leaves its writer unattributed.

An earlier QA attempt was stopped after a rig isolation failure and is excluded from acceptance evidence. Final launch fixtures use absolute-path stand-ins; no real-agent success is claimed.

Testing

  • Existing two-caller regression test updated: both daemon callers prepare WSL Pi/OMP once. The changed pin fails against the original production code.
  • Six named suites, 157 tests passed again after the final main merge: daemon session identity, launch parity, launch race parity, daemon WSL runtime, WSL relay manager, and unavailable Codex home.
  • Final Windows Electron native comparison, with shared failures and blocked WSL/real-agent rows disclosed.
  • Exact-head full PR workflow passed: PR Checks, including the aggregate gate.

Review

The independent correctness review and architecture challenger found no proven defects; no production fixes were needed. The retained reviewer also checked the final main merge on 0a27875 and found no new defects.

The wait is a no-op outside WSL and for ordinary other-agent commands. Global hooks-off returns promptly. Individual Pi/OMP opt-out or inconsistent agent/command metadata still follows the existing helper's classification and may incur the bounded wait. No saved data, dependency, wire format or minimum server version changes; paired hosts retain their existing path. No mobile-facing surface touched.

Agent skill upstream boundary

  • Not applicable; no upstream skill-installer material copied or translated.

Notes

Production +7/-13; tests +3/-4. No new tests, saved fields or protocol operations.

Checklist

  • Small and focused; problem, before/after, mechanism and alternatives explained
  • Main/PR screenshots attached
  • Correctness, security, performance, cross-platform and SSH/paired behavior reviewed
  • Full PR workflow passes on the exact head (CI provides lint/typecheck/build gates)

…ike host spawns

A Pi or OMP agent started from the window inside WSL, with the terminal
daemon on, could start before the distro's hook relay was up. It then
reported no working/idle status for that session. Host starts and
local-provider starts already waited (#21721); the window's daemon path
was the one left out.

The shared host-env step now always runs the wait on the daemon spawns
it builds, so the lane input that skipped it for window starts is gone.
The wait is a no-op for anything but an explicit Pi/OMP launch in WSL.
@brennanb2025
brennanb2025 marked this pull request as ready for review October 16, 2026 20:00
@coderabbitai

coderabbitai Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 99818359-987f-4e29-aa5d-79a4f0d0a645
📥 Commits

Reviewing files that changed from the base of the PR and between 79b9163 and 0a27875.

📒 Files selected for processing: 4
  • src/main/ipc/pty-daemon-spawn-session-identity.test.ts
  • src/main/ipc/pty/ipc/spawn-env-codex.ts
  • src/main/ipc/pty/runtime/spawn-preflight.ts
  • src/main/ipc/pty/spawn-host-env.ts
💤 Files with no reviewable changes: 2
  • src/main/ipc/pty/runtime/spawn-preflight.ts
  • src/main/ipc/pty/ipc/spawn-env-codex.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.

📝 Walkthrough

Host environment setup now always waits for explicit Pi/OMP guest readiness. The readiness option was removed from the resolver and its callers. The test now expects both daemon spawn lanes to invoke the wait once.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 0a278

Explicit Pi/OMP WSL daemon launches may wait up to ten seconds for guest readiness; timeout allows launch to proceed. The supplied evidence establishes no actionable merge-blocking risk.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning 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 1 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The description is complete and directly matches the implementation. It explains the user impact, mechanism, rationale, limitations, testing, review findings, and checklist status. It references the f…
Title check Passed The title clearly identifies the fix and its affected behavior: Pi and OMP agents launched from the WSL window now report status on their first launch.
Full details: Description check

Explanation

The description is complete and directly matches the implementation. It explains the user impact, mechanism, rationale, limitations, testing, review findings, and checklist status. It references the follow-up issue #29297, although it does not use the template's exact “Fixes #” form.

  • 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.

❤️ Share

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

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Pi and OMP terminals started from the window inside WSL could beat the guest status relay because that daemon launch path skipped preparation that the other paths already awaited. The shared environment builder now applies the same bounded preparation to both callers, so a relay that becomes ready within ten seconds is wired before the first agent process starts. A failed or slow relay still lets the agent launch, with potentially degraded status.

The architecture challenger and independent correctness review found no proven defects, and no production fixes were needed during the loop. The kept differences are intended: preparation is best-effort, global hooks-off returns promptly, and individual-agent opt-out or inconsistent agent/command metadata still follow the existing helper's classification.

Verification: six named suites passed again after the once-only main merge (157 tests); the retained reviewer found no defects in that integration delta. The final source readiness scan found no additional candidates. Full PR Checks passed on 0a278751079c4c316e1b8c4fdadb59be9e365c87, including static analysis, typecheck, five test shards, relay integration, packaging and the aggregate gate. The PR is out of draft, has zero unresolved threads, and its merge state is CLEAN.
Windows Electron main-versus-PR comparison completed on the final head: Settings-selected Command Prompt, the normal + shell, and an absolute-path stand-in have matching recorded launch/environment fields. Both hidden rigs reproduce three pre-existing failures: the first default tab starts after switching tabs, a launch-failure notice precedes stand-in success, and CODEX_HOME is absent from panes. No real-agent success is claimed. An automatic session-search promotion appeared during PR Settings navigation, but the timing difference was not established as a regression. Both rig process trees stopped; per-arm protected hashes stayed stable. The report records an unattributed config change between inactive arms.
Blocked: WSL endpoint/status and resume checks cannot run safely on this shared Windows box, where the available distros use real guest homes; OMP is absent and Pi is only a Windows npm shim. No first-launch WSL status result is claimed.

For Brennan's review

Flow Main PR
Settings: Command Prompt selected Main Settings PR Settings
Normal + shell environment Main shell PR shell
Absolute-path stand-in environment Main stand-in PR stand-in

Human judgment: the shared preparation step retains the existing ten-second, nonfatal status-setup tradeoff. The separate degraded daemon fallback missing the full host environment remains tracked in STA-10858.

@brennanb2025
brennanb2025 merged commit 69d877e into main Oct 16, 2026
55 checks passed
@brennanb2025
brennanb2025 deleted the brennanb2025/conv-4b-window-daemon-wsl-readiness branch October 16, 2026 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant