Repository navigation
Conversation
📝 Walkthrough
Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: 🔵 Low · up to
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 9b3b50bd-f1d6-4006-bcfa-426a13200638
📒 Files selected for processing (11)
src/main/daemon/pty-subprocess/shell-launch-plan.tssrc/main/daemon/terminal-host-session-create.tssrc/main/daemon/terminal-host-startup.test.tssrc/main/providers/local-pty-finalize-environment.tssrc/main/providers/local-pty-provider-windows-shell-launch.test.tssrc/main/providers/local-pty-shell-readiness-session.tssrc/main/providers/local-pty-shell-ready.tssrc/main/pty/wsl-startup-shell-ready.test.tssrc/main/pty/wsl-startup-shell-ready.tssrc/shared/wsl-login-shell-command.test.tssrc/shared/wsl-login-shell-command.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
|
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- WSL readiness predicate — new
src/main/pty/wsl-startup-shell-ready.tsgates the wait on win32 +wsl.exe+ a typed (not argv-delivered) startup command, and requests the existing OSC 777 marker viaORCA_SHELL_FEATURES=ready(WSLENV). - Daemon path —
shell-launch-plan.tsrequests the marker;terminal-host-session-create.tsderiveswslShellReadySupportedfrom the spawned subprocess (main's win32 shell-ready hint is always false), ORs it intoshellReadySupported, and keeps WSL writes raw. - In-process path —
local-pty-finalize-environment.tsoverridesplan.shellReadyLaunchwithsupportsReadyMarker: trueand a 15 sreadyTimeoutMs;local-pty-shell-ready.tsand the readiness session thread that timeout through. - Guest login script —
WSL_UNWRAPPED_SHELL_READY_BLOCKfires the marker before the finalexecfor shells Orca does not wrap, gated by_orca_wsl_shell_wrappedso wrapped bash/zsh self-report and consume the channel. - Tests — new daemon, in-process, predicate, request, and real-
shlogin-script coverage; I ran the changed suites plussession,shell-ready,windows-shell-args, andwsl-orca-envlocally (all green).
I traced both transports end to end: the in-process override survives local-pty-spawn.ts because getFallbackShellReadyConfig is only set on the non-win32 branch, WSL never embeds its command in shell args (buildWslShellArgs ignores startupCommand), and addOrcaWslInteropEnv already publishes ORCA_SHELL_READY_ROOT, so the marker arrives from either the wrapper or the login script. The bracketed-paste change is not a regression for WSL (the flag was already false there) and is left unchanged for non-WSL shells.
ℹ️ Unwrapped guest shells report ready before their rc runs
For fish/sh/dash (or bash/zsh with a missing wrapper), WSL_UNWRAPPED_SHELL_READY_BLOCK emits the marker immediately before exec "$_orca_wsl_shell" -l, so the held write lands while the guest's startup files are still running. That still cures the reported WSL-boot race, but a config that drains tty input at startup can drop the command in this fallback path. The wrapped bash/zsh path (marker from the line editor at the prompt) is not affected, and wrappers normally exist for those shells — noting it only so the limitation is on the record, no change requested.
Technical details
# Unwrapped-shell marker precedes login rc
## Affected sites
- `src/shared/wsl-login-shell-command.ts:158` — `WSL_UNWRAPPED_SHELL_READY_BLOCK` runs before `exec "$_orca_wsl_shell" -l`, so zsh/bash rc files execute after the marker.
- `src/main/pty/wsl-startup-shell-ready.ts` — the host writes ~30–200 ms after the marker; an rc that reads/drains the tty in that window still loses the command.
## Required outcome
- None requested. Documented limitation: the wrapped path reports from the prompt (correct); unwrapped is best-effort.
## Open questions for the human
- Is the unwrapped fallback's early marker acceptable given wrappers are generated for bash/zsh, or should fish/sh be treated differently (e.g. don't hold)?DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 99cced72-2bd6-4f0c-9aff-e94546aa83c7
📒 Files selected for processing (4)
src/main/pty/wsl-startup-shell-ready.test.tssrc/main/pty/wsl-startup-shell-ready.tssrc/shared/wsl-login-shell-command.test.tssrc/shared/wsl-login-shell-command.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/pty/wsl-startup-shell-ready.ts
- src/main/pty/wsl-startup-shell-ready.test.ts
- src/shared/wsl-login-shell-command.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Incremental re-review of commit 6751b98 on top of the previously reviewed 3bda7f0; it reworks _orca_wsl_shell's readiness reporting so a marker can only come from a prompt hook, addressing the prior review's noted unwrapped-shell limitation. Four files changed.
- Prompt-hook-only readiness — removed the pre-
execWSL_UNWRAPPED_SHELL_READY_BLOCK; bash/zsh wrappers still self-report, and fish now starts viafish -l -C <WSL_FISH_SHELL_READY_INIT>, whose--on-event fish_prompthandler fires afterconfig.fish. - Channel consumption — added
_orca_wsl_shell_wrapped, set only by the zsh fall-through path, and a final[ -n "$_orca_wsl_shell_wrapped" ] || unset ORCA_SHELL_FEATURES; wrapped bash execs into its rcfile and consumes the channel there, soORCA_SHELL_FEATUREScannot leak into the user's shell. - Unwrapped shells —
sh/dash/missing-wrapper bash-zsh no longer emit an early marker and instead fall back to the host's 15 s cap. - Tests — replaced the unwrapped-block suite with dispatch tests over the real login script (fish args, hookless timeout, wrapped zsh) and asserted
WSL_FISH_SHELL_READY_INITequals the host'sgetFishShellReadyInitCommand.
Prior pullfrog feedback (the unwrapped-shell early-marker note) is addressed by this commit: readiness is now only reported from a prompt position, which is exactly what the note asked to put on record.
ℹ️ PR description still documents the removed fallback
The PR body's "What Changed", "Testing", and visual-proof table describe an unwrapped-shell marker emitted from the login script right before exec, and claim an unwrapped dash guest runs "at ~3.4 s, no 15 s stall". Commit 6751b98 deleted that path: only bash/zsh wrappers and fish's prompt hook report ready, while sh/dash/missing-wrapper shells now wait out the full 15 s cap. The code is fine — the description just no longer matches what it ships, so please update it (and confirm the timeout-fallback tradeoff is intended).
Technical details
# Stale PR description after the prompt-hook rework
## Affected sites
- `src/shared/wsl-login-shell-command.ts:140-153` — the fis case reports at the prompt; everything else falls through to the final consume + `exec`.
- `src/main/providers/local-pty-finalize-environment.ts:54-59` — unwrapped WSL shells inherit `readyTimeoutMs: WSL_SHELL_READY_TIMEOUT_MS` (15 s), replacing the old fast path.
- `src/shared/wsl-login-shell-command.ts:163-169` — `WSL_UNWRAPPED_SHELL_READY_BLOCK` no longer exists.
## Required outcome
- PR description's mechanism, testing bullet, and visual-proof table reflect prompt-hook-only readiness and the 15 s fallback for hookless shells.
## Open questions for the human
- Is the 15 s hold acceptable for `sh`/`dash`/missing-wrapper panes, or should those be treated as non-waiting?DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
6751b98 to
c096ce3
Compare
|
Hi @AmethystLiang! This one is ready for a look. I rebased it onto current main and resolved the conflict with the startup-command staging from #23962. Main also gave guest fish its own Tested locally: typecheck, oxlint and oxfmt are clean, and the changed and related suites pass. The only Windows failures I see ( The CI workflows are waiting for maintainer approval since I'm an outside contributor. Could you approve them when you have a moment? Thanks! |
…ready On Windows, Orca typed the startup command into wsl.exe about 30 ms after the first output, while the distro was still booting and the login shell was still reading its config. A config that reads or flushes pending input (zsh plugins, update prompts) dropped it: the command showed echoed above a fresh prompt and nothing ran (stablyai#24188). The guest login script already starts bash and zsh through Orca's wrappers, which emit the OSC 777 ready marker when ORCA_SHELL_FEATURES asks for it, but the host never asked across wsl.exe. Now both the in-process provider and the terminal daemon request the marker for a WSL pane with a typed startup command and hold the write until it arrives (15 s cap, then the command is written anyway). A guest shell Orca does not wrap gets the marker from the login script right before exec, so it waits out the WSL boot without sitting through the cap. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review feedback: the fallback marker for shells Orca does not wrap came before the shell's own startup files, so a config that reads pending input could still eat the startup command. Guest fish now starts with the same fish_prompt hook host fish uses (after config.fish); any other unwrapped shell sends no marker and the host falls back to its 15 s timeout. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
c096ce3 to
b9961d0
Compare
|
Hi @nwparker! Could you approve CI for this one? It fixes #24188: an agent command gets lost when an automation starts it in a WSL terminal. I rebased it onto current main ( |

ELI5
When an automation (or any agent launch) starts an agent in a WSL terminal, Orca typed the command before WSL had finished starting. Some shell setups throw away anything typed that early, so the agent never started. Now Orca waits until the shell inside WSL says it is ready, then types the command.
What Changed
Before: the terminal shows the command echoed at the top, then a fresh prompt, and nothing runs (the screenshot in #24188:
pi 'test'above➜ rcm).After: the command is typed at the prompt and runs.
Mechanism. On Windows the host wrote the startup command into
wsl.exeabout 30 ms after the first output. At that point the distro may still be booting and zsh/bash is still reading its config. Inside the guest, Orca already starts bash and zsh through its shell-ready wrappers, and those emit the OSC 777 ready marker whenORCA_SHELL_FEATUREScontainsready. The host just never asked for it acrosswsl.exe.ORCA_SHELL_FEATURES=ready, passed throughWSLENV, and the host holds the write until the marker arrives. If the marker never comes, the command is written after 15 s anyway, the same cap the daemon already uses.local-pty-finalize-environment.ts) and the terminal daemon (shell-launch-plan.tsasks for the marker,terminal-host-session-create.tsqueues the write). Both use one predicate,wslStartupCommandWaitsForShellReady. The daemon has to decide after spawn, because main can't tell a pane will bewsl.exe.fish_prompthook host fish uses. Other unwrapped shells (sh/dash, bash/zsh with a missing wrapper) report nothing, and the command goes out at the 15 s cap.Plain WSL terminals without a startup command, and every non-WSL shell, are unchanged.
Why
This reuses the readiness handshake Orca already has for local bash/zsh/fish. No new wire fields, no renderer changes. Alternatives I rejected:
startupfeature): an unwrapped guest shell would silently drop it.#23962 and #24257 (both merged) change how long launch lines and prompts arrive, but the launch line is still typed into the shell as soon as the pane starts, so they don't cover this. #23748 lists "WSL gets no shell check" as a known gap.
Linked Issue
Fixes #24188
Visual Proof
End-to-end run on Windows 11 with a real ConPTY and
wsl.exeinto Ubuntu 26.04: zsh + oh-my-zsh, WSL shut down before each run, startup commandecho ORCA_$((20+22))_DONE. To reproduce the reporter's lost input,.zshrcgot one line that drains pending tty input at startup, which stands in for plugins or update prompts that read the tty.ORCA_42_DONEat ~5 sconfig.fishwithsleep 2, afterdash), afterWith stock oh-my-zsh (no draining line) the command is also typed before the prompt, but zsh keeps it, which is why the bug depends on the setup.
Testing
I manually tested these changes locally
Automated tests added/updated, or explained why not below
Daemon:
terminal-host-startup.test.tschecks that the command is held until the marker, and delivered on timeout. Fails without the fix.In-process:
local-pty-provider-windows-shell-launch.test.tschecks the env and WSLENV request and that nothing is written before the marker. Fails without the fix.Login script:
wsl-login-shell-command.test.tsruns the shell dispatch under a realsh: the script never prints the marker itself, fish gets the prompt hook only when ready was requested, a hookless shell never seesORCA_SHELL_FEATURES, and wrapped shells are left alone.wsl-startup-shell-ready.test.tscovers the predicate and checks that the marker matchesSHELL_READY_MARKER_ESCAPED.Ran on Windows and on Linux (Ubuntu in WSL). Across
src/main/providers,src/main/daemon,src/main/ptythe set of failing tests is the same before and after, apart from two real-shell timing tests that failed once under full parallel load and passed 3/3 in isolation.uses the preferred WSL distro for Windows cwd WSL terminalsfails on currentmaintoo (theHISTFILEassertion), on both platforms. A fullpnpm testin WSL showed 7 more failing files outside these folders; run one by one, each gives the same result onmain.tsc -p config/tsconfig.node.json, oxlint, oxfmt andcheck:code-quality:changedare clean.macOS not tested; the new code only runs when
process.platform === 'win32'and the shell iswsl.exe.AI Disclosure
Written with Claude Code (Claude Opus 5.5): root-cause analysis, the patch, the tests and the WSL end-to-end runs. I reviewed the change and the test results.
Review
AI self-review (Claude Code), re-run after rebasing onto
e04c2f9e:process.platform === 'win32'and the shell iswsl.exe. macOS and Linux take the existing paths.claudefunction (Re-land Claude per-account folders (#24434, #26405) with the upgrade and proxy fixes #26801) next to the ready hook.quotePosixShell.Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)pnpm lintpasses. Typecheck passes, each project run alone with--singleThreaded; the parallel run runs out of memory in this 7.7 GB VM. Everypnpm buildstep passes exceptbuild:mobile-web, which needs themobile/dependencies that aren't installed here; this PR touches nothing inmobile/.pnpm test: the affected folders pass. The full run couldn't finish in WSL, and every file that failed there fails the same way onmain. CI covers the rest.🤖 Generated with Claude Code