[nwparker]

Summary

^[[?997;1n no longer leaks into fish. Interactive prompts that read a keystroke right after a fish prompt (brew, npx, git add -p, any [y/N]) stop swallowing escape bytes.

Closes #9993. Supersedes #12580 and #10054 — see Why not the proposed fixes.

Root cause

fish arms CSI ?2031h immediately before painting each prompt and withdraws it with CSI ?2031l when it hands the tty to a child. Measured on real fish 4.7.1, that window is ~1 ms. Orca answered the subscribe with CSI ?997;Nn across a renderer IPC hop of 1–3 ms, so the reply arrived after the withdrawal and was read as stdin by whatever fish had just handed the tty to.

Two measurements decide the fix:

1. The reply is not stale by Orca's own view when it is written (staleReplies: 0). Any "drop the stale reply" design can only drop what it already knows is stale, so it under-drops in exactly the failure case.
2. Nothing asked for the reply. The Contour spec is explicit: "A terminal emulator should only send out the DSR when the palette has been updated." Subscribing is not a palette update; the protocol's query is CSI ?996n.

You cannot win a race you enter 1–3 ms late. So don't enter it.

Verified against real implementations, not just the spec

Terminal	On CSI ?2031h
Ghostty	nothing. force=true is reachable only from the ?996n DSR (stream_handler.zig:869); every other emit is gated on the mode (Termio.zig:729)
iTerm2	nothing — case 2031: sendUnsolicitedDarkModeDSR = mode (VT100Terminal.m:995)
xterm.js (what Orca embeds)	nothing — sets colorSchemeUpdates (InputHandler.ts:2035)
Warp	does not implement mode 2031

17 real programs measured under a pty. Only four subscribe: fish, tmux, claude, opencode. None blocks on a reply. A paired 5×2 fish benchmark gave deterministic byte counts per arm (1911 vs 1935 — the delta is exactly two redundant OSC 11 + DA1 re-queries) with fully overlapping startup latency; the no-answer arm was faster in 2 of 5 runs. Rendering diffs across all four subscribers show one redundant palette re-query and zero rendering change. tmux is the only program that sends ?996n, which Orca still answers. The retained flip push still reaches all four.

fish genuinely decodes the report — it reacts to a real ?997;2n but ignores garbage and CPR — and uses it purely as a "re-query OSC 11" trigger, which it already does unprompted at every prompt paint.

What changed

• Subscribes are record-only at all four emitters: the live chunk scan and hidden-gate fact path in pty-connection.ts, parked-terminal-byte-watcher.ts, and parked-terminal-mode2031-responder.ts (deleted — it existed only to reply).
• Unchanged on purpose: the CSI ?996n DSR answer, the theme-flip push, and the paneMode2031 / paneLastThemeMode registries. paneLastThemeMode is still seeded at subscribe time, or the next unrelated appearance re-apply would read "no last mode" as a flip and push a spurious 997.
• Replay grammar carries ?2031l alongside ?2031h (terminal-reply-query-scan.ts). Snapshots drop control bytes, so the arm must still be replayed for a late-attaching remote client to build its registry — but an arm with no withdraw left that client subscribed for a TUI that had already retired the subscription, reproducing [Bug]: ?997;1n color-scheme replies leak into fish and corrupt stdin of interactive prompts (repro on v1.4.152) #9993 on the remote path. Renamed isReplyElicitingCsi → isReplayableCsi.

Fish-integration gaps closed alongside

Gap	Failure
clearEnvCommand emitted unset VAR, which fish lacks	AI-vault resume left CODEX_HOME at Orca's routed value, so codex resume <id> reported the session missing — a silent wrong result
shell-startup-env.ts handled only zsh/bash	Orca could not detect a fish user's own CODEX_HOME / PI_CODING_AGENT_DIR / OMP_CODING_AGENT_DIR
bracketedPasteSafe differed across local / daemon / relay for one fish user	Multiline startup commands submitted line-by-line locally, executing piecemeal

The dialect is chosen only where the client's login shell actually parses the command. A locally-scanned Codex session can target a remote worktree, and emitting fish set -e into remote bash would not clear the variable — it would enable errexit.

Why not the proposed fixes

#12580 (drop stale replies at the write boundary) — refuted by measurement 1: the reply is usually not stale by main's view at write time, so the gate under-drops precisely when it matters. Its predicate also exact-matches a 2-element set, so a coalesced doubled ?997;1n — the literal payload in the issue title — passes through. And #13309 moved the real write boundary below it into PtyStartupReplyDelivery. The byte-level trace in that PR is accurate and was genuinely useful; only the conclusion drawn from it doesn't hold.

#10054 reached the right answer but justified it with "fish probes with ?2031h + ?996n", which is false — fish never sends ?996n (0 occurrences across a full session).

Deliberately out of scope

• Emitter de-duplication. An earlier revision of this PR made maybePushMode2031Flip the sole owner of the flip push and suppressed xterm's own report. Adversarial review found it regressed real cases twice: a flip-triggered push structurally cannot cover what a write-triggered push covered (a palette change within one darkness class notified nothing; a reattached pane was left with no emitter at all). It was never needed for [Bug]: ?997;1n color-scheme replies leak into fish and corrupt stdin of interactive prompts (repro on v1.4.152) #9993 — it only addressed a cosmetic doubling on theme flip. Reverted. Filing separately.
• Fish agent-env restore / markerless wrapping — STA-3937, filed by @brennanb2025 against fix(pty): give fish login shells the Prime Agent status wrapper #13875, which restructures the same fish arm. I had it implemented and dropped it rather than collide.
• CPR/OSC-11 stdin leak — [Bug]: OSC/DSR query replies are reordered behind CPR/DA1 by the POSIX echo-safe write path, stranding them into the next child's stdin #13892.

Screenshots

No visual change.

Testing

• pnpm lint — oxlint 0 errors; code-quality native and type-aware clean; max-lines ratchet and reliability gates pass, no new suppressions
• pnpm typecheck — node / web / cli clean
• pnpm test — 2673 files / 32405 tests passing across src/
• pnpm build — not run. No build-surface change: renderer + main TypeScript and generated shell text only, all covered by typecheck.
• Added or updated high-quality tests that would catch regressions

The merge-blocking proof is fish-color-scheme-child-stdin.node-pty.test.ts: real fish 4.7.1 under node-pty, driving the real pty-connection module, asserting on what the child process read. A screen-level assertion would pass while npx/brew stayed broken.

Verified as a real guard rather than assumed: re-injecting the reply makes it fail with the child reading the CPR reply, the OSC 11 report, then ?997;1n immediately ahead of hello — the exact shape from the issue. Restoring makes it pass.

CI (shell_contracts, required) installs fish 4 via ppa:fish-shell/release-4 with retry and gates on the version — 2031 arming lives in fish 4's Rust tty_handoff, so on Noble's fish 3.7 this test would fail rather than skip. ORCA_REQUIRE_FISH=1 turns a missing-fish skip into a failure in CI while still skipping cleanly on a dev machine without fish.

tests/tools/fish-mode2031-repro/ holds the standalone trace harness.

AI Review Report

Multiple adversarial review rounds with a fix pass between each. Findings fixed and re-verified:

• Emitter de-dup regressions (twice) — palette-luminance flips and fit-override panes missed; then within-darkness-class palette changes missed and reattach left no emitter. Resolution: the cleanup was removed, not patched further.
• [Bug]: ?997;1n color-scheme replies leak into fish and corrupt stdin of interactive prompts (repro on v1.4.152) #9993 still reproduced on the remote path via the arm-without-withdraw replay grammar. Fixed.
• ai-vault remote dialect — fish set -e could reach a remote POSIX shell where it enables errexit. Fixed; the pre-existing test pinned isLocalSession: false, which short-circuits before the fish branch, so it was false coverage. Replaced with the reachable case plus an end-to-end assertion.
• Two vacuous tests that passed unchanged against pre-fix code. Rewritten.
• Two CI blockers (25 oxlint errors in the harness; a duplicate vitest import). Fixed.

Final round returned zero blocker/major findings. Three minors were addressed: stale comments describing the removed reply, a retry around the fish PPA install so a Launchpad blip cannot red an unrelated PR, and a note that buildAgentDraftLaunchPlan's fish branch is contract-only for now (selectedRepoStartupShell is Windows-only, so fish users still get unset on that path — follow-up).

Cross-platform checked: byte-level and platform-neutral. Windows/ConPTY query replies untouched. SSH, relay, folder workspaces, and remote clients cross the same emitters.

Remote wire (docs/reference/remote-wire-compatibility.md): no RPC param, stream opcode, or fact-kind change — only what a client publishes into the PTY. Old host + new client: the client stops writing a byte the host merely forwarded. New host + old client: 2031 query authority was always renderer-side, so an old client keeps replying, and the host's echo-safe coalesced-997 handling is retained for exactly that case. No capability negotiation needed.

Security Audit

No new command execution, filesystem access, network surface, or IPC endpoint. The change removes an outbound byte sequence and classifies sequences already flowing through existing paths; it cannot inject or transform input. The fish set -e change replaces a command that was a no-op error under fish with the correct spelling, narrowing rather than widening what executes. config.fish parsing is read-only, bounded, and does not execute the file.

Notes

• pr.yml will conflict mechanically with fix(pty): give fish login shells the Prime Agent status wrapper #13875, which edits the same hunks. The fish-4 PPA is load-bearing for this PR's test; fix(pty): give fish login shells the Prime Agent status wrapper #13875's own fish tests do not need 2031 and run fine on 3.7.
• One pre-existing mobile test failure (mobile-create-pr-action.test.ts) is unrelated and reproduces at the branch base — verified by stashing.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4d9a117d-9b5a-40c4-975a-13b789bf5f5a

📥 Commits

Reviewing files that changed from the base of the PR and between 8fd462e1c440b4bf1d892f2395efede4848eafac and 8ce34e3.

📒 Files selected for processing (1)

• src/main/pty/shell-startup-env.test.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/main/pty/shell-startup-env.test.ts

---

📝 Walkthrough

Walkthrough

The change adds Fish 4 detection, CI enforcement, startup-file parsing, session-aware environment lookup, Fish shell command handling, and Fish resume-shell support. It preserves XDG_CONFIG_HOME in Codex shell-startup contexts. Terminal mode-2031 handling now records subscriptions without hidden-terminal replies. New tests cover Fish startup behavior, shell resolution, PTY handoffs, and child-process input integrity.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 32.56% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the primary change: making DECSET 2031 subscriptions silent to prevent fish stdin leakage.
Description check	✅ Passed	The description thoroughly explains the problem, solution, issue linkage, testing, compatibility impact, security review, and intentional scope exclusions.
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.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

---

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: 7

🧹 Nitpick comments (5)

src/renderer/src/lib/client-login-shell.ts (1)

1-8: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Condense this JSDoc.

Retain one brief rationale about Fish requiring a distinct startup dialect. Remove the return-value and fallback details that the function already makes clear.

As per coding guidelines, “Comments must be concise, non-obvious, and brief—prefer one line; do not explain obvious behavior or walk through code.”

Source: Coding guidelines

src/shared/fish-binary-requirement.ts (1)

3-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Condense the new block comments.

Keep one short comment for the CI skip-to-failure contract. Remove the repeated implementation explanation.

As per coding guidelines: “Comments must be concise, non-obvious, and brief—prefer one line; do not explain obvious behavior or walk through code.”

Also applies to: 51-55

Source: Coding guidelines

src/renderer/src/components/terminal-pane/fish-color-scheme-child-stdin.node-pty.test.ts (1)

407-409: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

The harness answers an OSC 10 query with an OSC 11 report.

The condition matches \x1b]10;? (foreground query) or \x1b]11;? (background query), but the reply is always \x1b]11;.... fish receives a background report for a foreground query. Today fish does not block on OSC 10, so the test still reaches the prompt. Answer each query with its matching OSC number so the harness stays faithful if fish's probe handling changes.

🎨 Proposed probe-reply split

-        if (chunk.includes('\x1b]10;?') || chunk.includes('\x1b]11;?')) {
-          term.write('\x1b]11;rgb:1e1e/1e1e/1e1e\x1b\\')
-        }
+        if (chunk.includes('\x1b]10;?')) {
+          term.write('\x1b]10;rgb:eeee/eeee/eeee\x1b\\')
+        }
+        if (chunk.includes('\x1b]11;?')) {
+          term.write('\x1b]11;rgb:1e1e/1e1e/1e1e\x1b\\')
+        }

tests/tools/fish-mode2031-repro/capture-fish-mode2031-trace.mjs (1)

40-42: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider a POSIX-independent default shell and a file URL for the dynamic import.

Line 54 defaults SHELL to /opt/homebrew/bin/fish, which exists only on Apple Silicon Homebrew. A Linux user must always pass --shell=. Line 42 imports a .ts file through a bare absolute path; ESM on Windows requires a file:// URL, so await import fails there. This tool is a manual repro harness, so neither blocks the PR. Resolving fish from PATH and using pathToFileURL would remove the friction.

🔧 Proposed portability adjustments

+import { pathToFileURL } from 'node:url'
+
 const pty = require(path.join(REPO_ROOT, 'node_modules/node-pty'))
 const { INITIAL_MODE_2031_REPLY_SCAN_STATE, mode2031SequenceFor, scanMode2031ReplyDecision } =
-  await import(path.join(REPO_ROOT, 'src/shared/terminal-color-scheme-protocol.ts'))
+  await import(pathToFileURL(path.join(REPO_ROOT, 'src/shared/terminal-color-scheme-protocol.ts')).href)

-const SHELL = opt('shell', '/opt/homebrew/bin/fish')
+// Resolve from PATH so Linux and Intel-Homebrew hosts do not need --shell=.
+const SHELL = opt('shell', 'fish')

Also applies to: 54-54

mobile/src/terminal/terminal-webview-color-scheme-push-injected.ts (1)

32-35: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low value

The mobile matcher is stricter than the desktop one.

Desktop uses isMode2031ColorSchemeReport(data) in src/renderer/src/components/terminal-pane/xterm-theme-write-color-scheme-push.ts. This twin uses exact string equality against the two report constants. If xterm ever emits the report joined with other bytes in one onData call, the desktop side still drops it and this side forwards it.

xterm emits _reportColorScheme as its own onData call today, so the behavior matches in practice. Add a short note here recording that assumption, so a future xterm upgrade does not silently split the twins.

💡 Proposed comment

   function isSuppressedXtermColorSchemePush(data) {
+    // Exact match is enough: xterm emits _reportColorScheme as its own onData call, never
+    // concatenated. Desktop's isMode2031ColorSchemeReport is the tolerant twin of this check.
     return xtermColorSchemePushSuppressionDepth > 0 &&
       (data === MODE_2031_DARK_REPORT || data === MODE_2031_LIGHT_REPORT);
   }

🔇 Additional comments (58)

src/main/ipc/pty-hidden-delivery-gate.ts (1)

21-21: LGTM!

src/renderer/src/components/terminal-pane/pty-input-write-queue.test.ts (1)

472-473: LGTM!

Also applies to: 525-564

src/renderer/src/lib/client-login-shell.ts (1)

9-15: LGTM!

src/shared/terminal-mode-2031-final-state.test.ts (1)

7-19: LGTM!

Also applies to: 140-209

src/main/daemon/shell-ready.test.ts (1)

8-8: LGTM!

Also applies to: 20-21, 170-174

.github/workflows/pr.yml (1)

179-184: LGTM!

Also applies to: 195-226, 240-240, 275-275

src/relay/plugin-overlay-env.ts (1)

1-1: 🩺 Stability & Availability

Verify relay bundle compatibility for this new main-process import.

src/relay is deployed as a separate bundle. Confirm that src/main/pty/shell-startup-env.ts is relay-safe and run the established relay bundle check before merge. The PR reports that build was not run.

Based on learnings: “src/relay is bundled separately for remote deploy,” and imports from src/main require bundle-safety confirmation.

Source: Learnings

src/relay/plugin-overlay-env.test.ts (1)

107-133: LGTM!

src/main/codex/codex-pane-account-registry.ts (1)

114-114: LGTM!

Also applies to: 211-211

src/main/codex/codex-real-home-path.test.ts (1)

1-1: LGTM!

Also applies to: 14-22, 65-67, 87-132

src/main/codex/codex-real-home-path.ts (1)

8-9: LGTM!

Also applies to: 59-72, 94-95, 111-118

src/renderer/src/lib/ai-vault-resume-shell.test.ts (1)

1-127: LGTM!

src/renderer/src/lib/ai-vault-resume-shell.ts (1)

7-11: LGTM!

Also applies to: 32-35

src/shared/tui-agent-startup-shell.ts (1)

3-24: LGTM!

Also applies to: 86-88, 126-130

src/shared/tui-agent-startup-shell.test.ts (1)

13-55: LGTM!

src/shared/startup-command-submission.test.ts (1)

2-5: LGTM!

Also applies to: 50-68

src/shared/startup-command-submission.ts (1)

26-51: LGTM!

src/main/providers/local-pty-provider.ts (1)

16-16: LGTM!

Also applies to: 1050-1057

src/main/providers/local-pty-shell-ready.ts (1)

436-436: LGTM!

src/shared/hermes-startup-query.ts (1)

4-4: LGTM!

Also applies to: 118-118, 143-143

src/main/pty/shell-startup-env.ts (1)

1-1: LGTM!

Also applies to: 14-79, 91-139, 210-289

src/main/pty/shell-startup-env.test.ts (1)

3-33: LGTM!

Also applies to: 337-541

src/main/ipc/pty.ts (1)

130-130: LGTM!

Also applies to: 1501-1501, 1516-1516, 1670-1670

src/shared/ai-vault-resume-command.ts (1)

102-107: 🎯 Functional Correctness

Keep the existing Fish environment syntax.

Fish 3.1 and later support CODEX_HOME=value command for a single command. The existing construction is valid, so the proposed env branch is unnecessary.

			> Likely an incorrect or invalid review comment.

src/shared/terminal-color-scheme-protocol.ts (1)

14-18: LGTM!

src/shared/terminal-reply-query-scan.ts (1)

27-38: LGTM!

Also applies to: 55-58, 98-98

src/shared/terminal-reply-query-scan.test.ts (1)

6-23: LGTM!

Also applies to: 60-112

src/shared/terminal-output-side-effects.ts (1)

68-72: LGTM!

src/shared/terminal-query-reply.ts (1)

95-97: LGTM!

src/shared/terminal-side-effect-facts.ts (1)

32-35: LGTM!

src/renderer/src/components/terminal-pane/pty-connection-types.ts (1)

102-105: LGTM!

src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts (1)

882-886: LGTM!

src/renderer/src/components/terminal-pane/terminal-side-effect-facts-handler.ts (1)

88-91: LGTM!

src/renderer/src/components/terminal-pane/terminal-hidden-delivery-gate.ts (1)

7-7: LGTM!

tests/e2e/artificial-opencode-revisit-pressure-scenario.ts (1)

30-30: Already covered by the debug-field rename verification on tests/e2e/artificial-opencode-hidden-pressure-scenario.ts.

tests/e2e/artificial-opencode-terminal-load.spec.ts (2)

59-60: Already covered by the debug-field rename verification on tests/e2e/artificial-opencode-hidden-pressure-scenario.ts.

---

373-373: LGTM!

Also applies to: 391-391

src/renderer/src/components/terminal-pane/terminal-mode-2031-replies.ts (1)

16-19: LGTM!

src/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.ts (1)

65-65: LGTM!

Also applies to: 208-210, 249-249, 288-290

src/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.test.ts (1)

111-123: LGTM!

Also applies to: 363-373, 866-874, 899-916

src/renderer/src/components/terminal-pane/terminal-parked-pty-watcher.ts (1)

2-2: LGTM!

Also applies to: 85-85

mobile/src/terminal/terminal-webview-theme-injected.ts (1)

105-111: 🩺 Stability & Availability

⚠️ Unverified finding
Sandbox verification was unavailable.

Verify that the color-scheme push script is injected before the theme script.

applyTerminalTheme now calls withSuppressedXtermColorSchemePush, which TERMINAL_COLOR_SCHEME_PUSH_JS defines. If terminal-webview-html.ts injects TERMINAL_WEBVIEW_THEME_JS first, the first theme apply throws a ReferenceError and the WebView keeps the default theme. The unit test builds its own load order, so it does not cover the shipped HTML.

src/renderer/src/components/terminal-pane/fish-color-scheme-child-stdin.node-pty.test.ts (1)

223-225: 🩺 Stability & Availability

⚠️ Unverified finding
Sandbox verification was unavailable.

Confirm that this always-on test does not fail where fish is absent.

itWithFish skips the regression when fish 4 is missing, but this test always runs and asserts fishRequirementViolation(FISH) is null. If fishRequirementViolation reports a violation whenever fish 4 is unavailable, the suite fails on every macOS, Linux, and Windows machine without fish 4, not only on the CI lane that requires it. src/shared/fish-binary-requirement.ts is not in this review context.

tests/e2e/artificial-opencode-hidden-pressure-scenario.ts (1)

57-58: 📐 Maintainability & Code Quality

⚠️ Unverified finding
Sandbox verification was unavailable.

Confirm the renderer debug snapshot emits hiddenRendererSkipCount.

This type now declares hiddenRendererSkipCount. The producer of __terminalPtyOutputDebug.snapshot() is not in this review context. If the producer still emits hiddenRendererMode2031ReplyCount, the annotation prints undefined and the diagnostic silently loses value. The same rename appears in tests/e2e/artificial-opencode-revisit-pressure-scenario.ts, tests/e2e/artificial-opencode-terminal-load.spec.ts, and tests/e2e/terminal-hidden-tui-visual-restore.spec.ts.

src/renderer/src/components/terminal-pane/terminal-appearance.test.ts (3)

226-242: 📐 Maintainability & Code Quality

The static analysis hint on this line is a false positive.

OpenGrep reports child_process.exec command injection at line 233. The call is RegExp.prototype.exec on a color string, not child_process.exec. No action is required.

Source: Linters/SAST tools

---

336-353: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

⚠️ Unverified finding
Sandbox verification was unavailable.

Consider clearing the fit override instead of overwriting it.

The finally block writes a desktop-fit entry at 0x0 for pty-held. That leaves module-level state behind for later tests in this file. If mobile-fit-overrides exposes a clear/delete API, prefer it so the module returns to its pre-test state.

---

202-224: LGTM!

Also applies to: 251-334

src/renderer/src/components/terminal-pane/xterm-theme-write-color-scheme-push.ts (1)

1-34: LGTM!

src/renderer/src/components/terminal-pane/terminal-appearance.ts (1)

113-147: LGTM!

Also applies to: 191-191, 201-205, 251-256

src/renderer/src/components/terminal-pane/pty-connection.ts (2)

3874-3884: 🗄️ Data Integrity & Integration

⚠️ Unverified finding
Sandbox verification was unavailable.

Verify the recordPaneMode2031Subscription dependency and the new module edge.

Two points need confirmation:

1. deps.recordPaneMode2031Subscription?.() is optional. If no caller supplies it, a gate-managed pane records no subscription and never receives a later theme flip. Confirm every production caller wires it.
2. pty-connection.ts now imports resolveTerminalPaneColorSchemeMode from ./terminal-appearance. Confirm terminal-appearance.ts does not import back from pty-connection.ts, so no module cycle is created.

---

4103-4108: LGTM!

Also applies to: 6344-6368

src/renderer/src/components/terminal-pane/pty-connection.test.ts (1)

12250-12256: LGTM!

Also applies to: 12292-12320, 13499-13536, 13596-13687, 13721-13924, 13967-13969, 15216-15217

mobile/src/terminal/terminal-webview-color-scheme-push-injected.ts (1)

18-30: LGTM!

mobile/src/terminal/terminal-webview-color-scheme-push-injected.test.ts (1)

1-127: LGTM!

mobile/src/terminal/terminal-webview-html.ts (1)

13-13: LGTM!

Also applies to: 223-223

mobile/src/terminal/terminal-webview-query-reply-injected.ts (1)

14-20: LGTM!

mobile/src/terminal/terminal-webview-query-reply.test.ts (1)

4-4: LGTM!

Also applies to: 23-23

mobile/src/terminal/terminal-webview-theme-injected.test.ts (1)

5-21: LGTM!

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 93f5c478-bfdd-4098-9b61-c94a1d849da3

📥 Commits

Reviewing files that changed from the base of the PR and between 96c17b4 and dc15aa3.

📒 Files selected for processing (63)

• .github/workflows/pr.yml
• mobile/src/terminal/terminal-webview-color-scheme-push-injected.test.ts
• mobile/src/terminal/terminal-webview-color-scheme-push-injected.ts
• mobile/src/terminal/terminal-webview-html.ts
• mobile/src/terminal/terminal-webview-query-reply-injected.ts
• mobile/src/terminal/terminal-webview-query-reply.test.ts
• mobile/src/terminal/terminal-webview-theme-injected.test.ts
• mobile/src/terminal/terminal-webview-theme-injected.ts
• src/main/codex/codex-pane-account-registry.ts
• src/main/codex/codex-real-home-path.test.ts
• src/main/codex/codex-real-home-path.ts
• src/main/daemon/shell-ready.test.ts
• src/main/ipc/pty-hidden-delivery-gate.ts
• src/main/ipc/pty.ts
• src/main/providers/local-pty-provider.ts
• src/main/providers/local-pty-shell-ready.ts
• src/main/pty/shell-startup-env.test.ts
• src/main/pty/shell-startup-env.ts
• src/relay/plugin-overlay-env.test.ts
• src/relay/plugin-overlay-env.ts
• src/renderer/src/components/terminal-pane/fish-color-scheme-child-stdin.node-pty.test.ts
• src/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.test.ts
• src/renderer/src/components/terminal-pane/parked-terminal-byte-watcher.ts
• src/renderer/src/components/terminal-pane/parked-terminal-mode2031-responder.test.ts
• src/renderer/src/components/terminal-pane/parked-terminal-mode2031-responder.ts
• src/renderer/src/components/terminal-pane/pty-connection-types.ts
• src/renderer/src/components/terminal-pane/pty-connection.test.ts
• src/renderer/src/components/terminal-pane/pty-connection.ts
• src/renderer/src/components/terminal-pane/pty-input-write-queue.test.ts
• src/renderer/src/components/terminal-pane/terminal-appearance.test.ts
• src/renderer/src/components/terminal-pane/terminal-appearance.ts
• src/renderer/src/components/terminal-pane/terminal-hidden-delivery-gate.ts
• src/renderer/src/components/terminal-pane/terminal-mode-2031-replies.ts
• src/renderer/src/components/terminal-pane/terminal-parked-pty-watcher.ts
• src/renderer/src/components/terminal-pane/terminal-parked-tab-watchers.test.ts
• src/renderer/src/components/terminal-pane/terminal-side-effect-facts-handler.ts
• src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts
• src/renderer/src/components/terminal-pane/xterm-theme-write-color-scheme-push.ts
• src/renderer/src/lib/ai-vault-resume-shell.test.ts
• src/renderer/src/lib/ai-vault-resume-shell.ts
• src/renderer/src/lib/client-login-shell.ts
• src/shared/ai-vault-resume-command.ts
• src/shared/fish-binary-requirement.test.ts
• src/shared/fish-binary-requirement.ts
• src/shared/hermes-startup-query.ts
• src/shared/startup-command-submission.test.ts
• src/shared/startup-command-submission.ts
• src/shared/terminal-color-scheme-protocol.ts
• src/shared/terminal-mode-2031-final-state.test.ts
• src/shared/terminal-output-side-effects.ts
• src/shared/terminal-query-reply.ts
• src/shared/terminal-reply-query-scan.test.ts
• src/shared/terminal-reply-query-scan.ts
• src/shared/terminal-side-effect-facts.ts
• src/shared/terminal-view-attributes.ts
• src/shared/tui-agent-startup-shell.test.ts
• src/shared/tui-agent-startup-shell.ts
• tests/e2e/artificial-opencode-hidden-pressure-scenario.ts
• tests/e2e/artificial-opencode-revisit-pressure-scenario.ts
• tests/e2e/artificial-opencode-terminal-load.spec.ts
• tests/e2e/terminal-hidden-tui-visual-restore.spec.ts
• tests/e2e/terminal-tab-switch-visual-restore.spec.ts
• tests/tools/fish-mode2031-repro/capture-fish-mode2031-trace.mjs

💤 Files with no reviewable changes (4)

• tests/e2e/terminal-tab-switch-visual-restore.spec.ts
• src/renderer/src/components/terminal-pane/parked-terminal-mode2031-responder.test.ts
• src/renderer/src/components/terminal-pane/terminal-parked-tab-watchers.test.ts
• src/renderer/src/components/terminal-pane/parked-terminal-mode2031-responder.ts

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

The internal wait deadlines can exceed the test timeout.

The sequential waitUntil budgets are 15s (prompt) + 5s (arm) + 5s (withdrawal #1) + 5s (withdrawal #2) + 10s (child read), which totals 40s. The test timeout is 30s. On a slow runner the suite reports a generic Vitest timeout instead of the specific waitUntil assertion that failed, which makes triage harder. Raise the test timeout above the sum of the internal deadlines, or lower the internal deadlines.

⏱️ Proposed timeout budget fix

-    30_000
+    // Must exceed the sum of the internal waitUntil deadlines (15+5+5+5+10s),
+    // or a slow runner reports a generic timeout instead of the failing wait.
+    45_000

Also applies to: 474-474

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Guard the teardown write so it cannot mask an assertion failure.

term.write('exit\r') runs in finally. If fish already exited, the write can throw on the closed master fd. That throw replaces the original assertion error and hides the real failure. term.kill() below already handles this case.

🛡️ Proposed teardown guard

       } finally {
-        term.write('exit\r')
-        await waitUntil(() => exited, 3_000)
+        try {
+          term.write('exit\r')
+          await waitUntil(() => exited, 3_000)
+        } catch {
+          // already gone
+        }
         try {
           term.kill()
         } catch {
           // already gone
         }
       }

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Continue after an incompatible Fish candidate.

Line 37 returns when fish on PATH is too old. This prevents probing /opt/homebrew/bin/fish or /usr/local/bin/fish when either contains a compatible Fish version. Record the incompatible result, continue probing, and report it only when no candidate meets the minimum. Add a regression test with an old PATH candidate followed by a compatible fixed-path candidate.

Proposed fix

+  let incompatibleFish:
+    | { path: string; majorVersion: number; version: string }
+    | undefined
+
   for (const path of FISH_CANDIDATES) {
     const probe = spawnSync(path, ['--version'], { encoding: 'utf8' })
     if (probe.status !== 0) {
       continue
     }
     const majorVersion = Number(/version (\d+)/.exec(probe.stdout ?? '')?.[1] ?? '0')
     if (majorVersion < minMajorVersion) {
-      return {
-        available: false,
-        path: null,
-        majorVersion,
-        reason: `fish ${minMajorVersion}+ required, found ${probe.stdout?.trim() || 'an unknown version'} at ${path}`
-      }
+      incompatibleFish = {
+        path,
+        majorVersion,
+        version: probe.stdout?.trim() || 'an unknown version'
+      }
+      continue
     }
     return { available: true, path, majorVersion }
   }
 
+  if (incompatibleFish) {
+    return {
+      available: false,
+      path: null,
+      majorVersion: incompatibleFish.majorVersion,
+      reason: `fish ${minMajorVersion}+ required, found ${incompatibleFish.version} at ${incompatibleFish.path}`
+    }
+  }
   return { available: false, path: null, majorVersion: 0, reason: 'no fish binary on PATH' }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	for (const path of FISH_CANDIDATES) {
	const probe = spawnSync(path, ['--version'], { encoding: 'utf8' })
	if (probe.status !== 0) {
	continue
	}
	const majorVersion = Number(/version (\d+)/.exec(probe.stdout ?? '')?.[1] ?? '0')
	if (majorVersion < minMajorVersion) {
	return {
	available: false,
	path: null,
	majorVersion,
	reason: `fish ${minMajorVersion}+ required, found ${probe.stdout?.trim() || 'an unknown version'} at ${path}`
	}
	let incompatibleFish:
	| { path: string; majorVersion: number; version: string }
	| undefined
	for (const path of FISH_CANDIDATES) {
	const probe = spawnSync(path, ['--version'], { encoding: 'utf8' })
	if (probe.status !== 0) {
	continue
	}
	const majorVersion = Number(/version (\d+)/.exec(probe.stdout ?? '')?.[1] ?? '0')
	if (majorVersion < minMajorVersion) {
	incompatibleFish = {
	path,
	majorVersion,
	version: probe.stdout?.trim() || 'an unknown version'
	}
	continue
	}
	return { available: true, path, majorVersion }
	}
	if (incompatibleFish) {
	return {
	available: false,
	path: null,
	majorVersion: incompatibleFish.majorVersion,
	reason: `fish ${minMajorVersion}+ required, found ${incompatibleFish.version} at ${incompatibleFish.path}`
	}
	}
	return { available: false, path: null, majorVersion: 0, reason: 'no fish binary on PATH' }

🧰 Tools

🪛 ast-grep (0.45.1)

[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from 'node:child_process'
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(detect-child-process-typescript)

🪛 OpenGrep (1.26.0)

[ERROR] 36-36: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.

(coderabbit.command-injection.exec-js)

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Rename the mode in this contract.

colorSchemeMode is terminal-background-derived. It is not an app color-scheme mode. Line 28 conflicts with Line 31 and can mislead consumers of TerminalViewAttributes.

Proposed fix

-  /** Resolved APP color-scheme mode. NOT a color-scheme answer for programs:
+  /** Resolved terminal color-scheme mode. NOT a color-scheme answer for programs:

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	/** Resolved APP color-scheme mode. NOT a color-scheme answer for programs:
	* both the DSR ?996n reply and the 2031/997 flip push follow the composed
	* background's relative luminance like a visible xterm (_reportColorScheme),
	* and the two can disagree (e.g. dark terminal theme in light app mode). */
	/** Resolved terminal color-scheme mode. NOT a color-scheme answer for programs:
	* both the DSR ?996n reply and the 2031/997 flip push follow the composed
	* background's relative luminance like a visible xterm (_reportColorScheme),
	* and the two can disagree (e.g. dark terminal theme in light app mode). */

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

The comment above this field now contradicts the field name.

The preceding comment states that only the mode-2031 fact-reply counter survives and that the hidden-skip counters were deleted. This change renames the field to hiddenRendererSkipCount, and this PR removes the mode-2031 reply counter. The comment now describes the opposite of the code. Update the comment text.

📝 Proposed comment fix

-  // Why: only the mode-2031 fact-reply counter survives Phase 6 — the
-  // hidden-skip counters were deleted with the renderer skip grammar.
+  // Why: the mode-2031 reply counter is gone (`#9993` made subscribes silent);
+  // the hidden-renderer skip count is the remaining withheld-output signal.

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Document the --typeahead, --typeahead-at, and --settle options.

Lines 62 to 64 implement these three options, but the usage block does not list them. The comment at Lines 294 to 298 describes --typeahead as the most realistic leak window, so it is the primary mode of this harness. A user reading the usage block cannot discover it.

📝 Proposed usage-block additions

  *   --accept-delay=<ms> gap between typing the command and pressing Enter (300)
  *   --paste             send command + Enter in a single write (paste-like)
+ *   --typeahead         queue the command while an external command still owns
+ *                       the tty; the most realistic leak window
+ *   --typeahead-at=<ms> delay before the typed-ahead line is written (150)
+ *   --settle=<ms>       quiet period after the first prompt (400)
  *   --no-reply          never answer 2031 (control run)
  *   --json              append a machine-readable summary

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	* Usage:
	* node tests/tools/fish-mode2031-repro/capture-fish-mode2031-trace.mjs [options]
	*
	* --delay=<ms> reply delay; 0 = synchronous in-process (default 0)
	* --shell=<path> fish binary (default /opt/homebrew/bin/fish)
	* --user-config use the caller's real fish config (Tide etc.) instead of
	* an isolated minimal config
	* --prompt-cmd add an external command substitution to fish_prompt, so
	* the prompt itself forces extra tty handoffs
	* --no-probes do NOT answer DA1/DA2/CPR/XTVERSION/OSC 10/11 (fish then
	* stalls 10s on its DA1 wait)
	* --accept-delay=<ms> gap between typing the command and pressing Enter (300)
	* --paste send command + Enter in a single write (paste-like)
	* --no-reply never answer 2031 (control run)
	* --json append a machine-readable summary
	*/
	* Usage:
	* node tests/tools/fish-mode2031-repro/capture-fish-mode2031-trace.mjs [options]
	*
	* --delay=<ms> reply delay; 0 = synchronous in-process (default 0)
	* --shell=<path> fish binary (default /opt/homebrew/bin/fish)
	* --user-config use the caller's real fish config (Tide etc.) instead of
	* an isolated minimal config
	* --prompt-cmd add an external command substitution to fish_prompt, so
	* the prompt itself forces extra tty handoffs
	* --no-probes do NOT answer DA1/DA2/CPR/XTVERSION/OSC 10/11 (fish then
	* stalls 10s on its DA1 wait)
	* --accept-delay=<ms> gap between typing the command and pressing Enter (300)
	* --paste send command + Enter in a single write (paste-like)
	* --typeahead queue the command while an external command still owns
	* the tty; the most realistic leak window
	* --typeahead-at=<ms> delay before the typed-ahead line is written (150)
	* --settle=<ms> quiet period after the first prompt (400)
	* --no-reply never answer 2031 (control run)
	* --json append a machine-readable summary
	*/

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Validate the numeric options so a typo cannot silently change the experiment.

Number(opt('delay', '0')) returns NaN for a non-numeric value such as --delay=abc. NaN <= 0 is false, so the code takes the delayed branch at Line 255 and setTimeout(..., NaN) runs at 1ms. The harness then reports a delay the run did not use. The same problem applies to --accept-delay, --typeahead-at, and --settle. Reject non-numeric values instead.

🛡️ Proposed numeric-option validation

+const num = (name, fallback) => {
+  const raw = opt(name, fallback)
+  const value = Number(raw)
+  if (!Number.isFinite(value)) {
+    console.error(`--${name} must be a number, got "${raw}"`)
+    process.exit(2)
+  }
+  return value
+}
+
-const REPLY_DELAY_MS = Number(opt('delay', '0'))
+const REPLY_DELAY_MS = num('delay', '0')
 const SHELL = opt('shell', '/opt/homebrew/bin/fish')
 const USE_USER_CONFIG = flag('user-config')
 const ANSWER_PROBES = !flag('no-probes')
 const REPLY_ENABLED = !flag('no-reply')
 const EMIT_JSON = flag('json')
-const ACCEPT_DELAY_MS = Number(opt('accept-delay', '300'))
+const ACCEPT_DELAY_MS = num('accept-delay', '300')
 const PASTE = flag('paste')
 const PROMPT_CMD = flag('prompt-cmd')
 const TYPEAHEAD = flag('typeahead')
-const TYPEAHEAD_AT_MS = Number(opt('typeahead-at', '150'))
-const SETTLE_MS = Number(opt('settle', '400'))
+const TYPEAHEAD_AT_MS = num('typeahead-at', '150')
+const SETTLE_MS = num('settle', '400')

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	const REPLY_DELAY_MS = Number(opt('delay', '0'))
	const SHELL = opt('shell', '/opt/homebrew/bin/fish')
	const USE_USER_CONFIG = flag('user-config')
	const ANSWER_PROBES = !flag('no-probes')
	const REPLY_ENABLED = !flag('no-reply')
	const EMIT_JSON = flag('json')
	const ACCEPT_DELAY_MS = Number(opt('accept-delay', '300'))
	const PASTE = flag('paste')
	const PROMPT_CMD = flag('prompt-cmd')
	const TYPEAHEAD = flag('typeahead')
	const TYPEAHEAD_AT_MS = Number(opt('typeahead-at', '150'))
	const SETTLE_MS = Number(opt('settle', '400'))
	const num = (name, fallback) => {
	const raw = opt(name, fallback)
	const value = Number(raw)
	if (!Number.isFinite(value)) {
	console.error(`--${name} must be a number, got "${raw}"`)
	process.exit(2)
	}
	return value
	}
	const REPLY_DELAY_MS = num('delay', '0')
	const SHELL = opt('shell', '/opt/homebrew/bin/fish')
	const USE_USER_CONFIG = flag('user-config')
	const ANSWER_PROBES = !flag('no-probes')
	const REPLY_ENABLED = !flag('no-reply')
	const EMIT_JSON = flag('json')
	const ACCEPT_DELAY_MS = num('accept-delay', '300')
	const PASTE = flag('paste')
	const PROMPT_CMD = flag('prompt-cmd')
	const TYPEAHEAD = flag('typeahead')
	const TYPEAHEAD_AT_MS = num('typeahead-at', '150')
	const SETTLE_MS = num('settle', '400')

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (1)

src/renderer/src/components/terminal-pane/fish-color-scheme-child-stdin.node-pty.test.ts (1)

1-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Condense the file header comment.

Lines 1-19 repeat test mechanics that the code already shows. Keep a brief comment that states the regression contract and references #9993.

As per coding guidelines: “Comments must be concise, non-obvious, and brief—prefer one line; do not explain obvious behavior or walk through code.”

Source: Coding guidelines

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9764a60a-a94f-40df-8121-9506e6fccb58

📥 Commits

Reviewing files that changed from the base of the PR and between dc15aa3 and e8a8b58.

📒 Files selected for processing (11)

• src/main/ipc/pty.ts
• src/main/providers/local-pty-provider.ts
• src/renderer/src/components/terminal-pane/fish-color-scheme-child-stdin.node-pty.test.ts
• src/renderer/src/components/terminal-pane/pty-connection.test.ts
• src/renderer/src/components/terminal-pane/pty-connection.ts
• src/renderer/src/components/terminal-pane/pty-input-write-queue.test.ts
• src/renderer/src/lib/ai-vault-resume-command.ts
• src/renderer/src/lib/ai-vault-resume-shell.test.ts
• src/renderer/src/lib/ai-vault-resume-shell.ts
• src/shared/tui-agent-startup-shell.test.ts
• src/shared/tui-agent-startup-shell.ts

🚧 Files skipped from review as they are similar to previous changes (6)

• src/main/ipc/pty.ts
• src/renderer/src/lib/ai-vault-resume-shell.test.ts
• src/main/providers/local-pty-provider.ts
• src/shared/tui-agent-startup-shell.test.ts
• src/renderer/src/lib/ai-vault-resume-shell.ts
• src/shared/tui-agent-startup-shell.ts

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

rg -n -C 8 'getExecutionHostIdForWorktree|return .*executionHostId|return .*hostId|runtime:unresolved-owner' src/renderer/src/lib/worktree-runtime-owner.ts
rg -n -C 8 'resolveAgentResumeLaunchTarget|isRemote|connectionId|parseExecutionHostId' src/renderer/src/lib/agent-resume-launch-target.ts

Repository: stablyai/orca

Length of output: 5041

---

🏁 Script executed:

#!/bin/bash
sed -n '1,230p' src/renderer/src/lib/worktree-runtime-owner.ts
rg -n -C 6 'function parseExecutionHostId|export function parseExecutionHostId|type ExecutionHostId|ExecutionHostId =' src/shared src/renderer/src
rg -n -C 5 'getExecutionHostIdForWorktree\(' src/renderer/src src/main

Repository: stablyai/orca

Length of output: 50370

---

🏁 Script executed:

#!/bin/bash
sed -n '1,180p' src/shared/execution-host.ts
rg -n -C 8 'getExecutionHostIdForWorktree|runsOnClientLoginShell|runtime:unresolved-owner|malformed|unknown host|parseExecutionHostId' src/renderer/src/lib src/shared --glob '*.test.ts' --glob '*.test.tsx'

Repository: stablyai/orca

Length of output: 50370

---

🏁 Script executed:

#!/bin/bash
python3 - <<'PY'
from urllib.parse import unquote

def parse_execution_host_id(value):
    normalized = value.strip() if isinstance(value, str) else None
    if not normalized:
        return None
    if normalized == "local":
        return {"kind": "local"}
    for prefix, kind in (("ssh:", "ssh"), ("runtime:", "runtime")):
        if normalized.startswith(prefix):
            encoded = normalized[len(prefix):]
            if not encoded:
                return None
            try:
                decoded = unquote(encoded)
            except Exception:
                return None
            return {"kind": kind} if decoded else None
    return None

def current_decision(host_id, platform):
    execution_host = parse_execution_host_id(host_id)
    return execution_host is None or platform == "client"

def strict_decision(host_id, platform):
    return parse_execution_host_id(host_id) == {"kind": "local"}

for host_id in ("local", "ssh:host", "runtime:env", "runtime:", "unknown-host", "ssh:%E0%A4%A"):
    print(host_id, parse_execution_host_id(host_id), current_decision(host_id, "client"), strict_decision(host_id, "client"))
PY

Repository: stablyai/orca

Length of output: 358

---

🏁 Script executed:

#!/bin/bash
python3 - <<'PY'
from urllib.parse import unquote

def parse_execution_host_id(value):
    normalized = value.strip() if isinstance(value, str) else None
    if not normalized:
        return None
    if normalized == "local":
        return {"kind": "local"}
    for prefix, kind in (("ssh:", "ssh"), ("runtime:", "runtime")):
        if normalized.startswith(prefix):
            encoded = normalized[len(prefix):]
            if not encoded:
                return None
            try:
                decoded = unquote(encoded, errors="strict")
            except Exception:
                return None
            return {"kind": kind} if decoded else None
    return None

def current_decision(host_id, platform):
    execution_host = parse_execution_host_id(host_id)
    if execution_host and execution_host["kind"] in ("ssh", "runtime"):
        return False
    return platform == "client"

def strict_decision(host_id, platform):
    return parse_execution_host_id(host_id) == {"kind": "local"}

for host_id in ("local", "ssh:host", "runtime:env", "runtime:", "unknown-host", "ssh:%E0%A4%A"):
    print(host_id, parse_execution_host_id(host_id), current_decision(host_id, "client"), strict_decision(host_id, "client"))
PY

Repository: stablyai/orca

Length of output: 349

---

Treat only an explicit local host as client-local.

Unknown or malformed host IDs parse as null, then fall through to the client-platform check. Return true only when executionHost?.kind === 'local', and add tests for unknown and malformed IDs.