[nwparker]

	Files	Added	Deleted	Net
Test	24	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​1566	$\color{#cf222e}{\Huge{\mathbf{−}}}$​120	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​1446
Prod	34	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​1311	$\color{#cf222e}{\Huge{\mathbf{−}}}$​386	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​925

ELI5

If you use fish, three things quietly didn't work: every workspace shared one command history, Orca's draft-prefill cleanup used unset (which fish doesn't have), and anything Orca quoted for fish was quoted with sh rules — so a prompt containing \d+ or \\server\share arrived mangled, and one ending in a backslash didn't launch at all.

The fix turned out smaller than the bug list. Orca was trying to detect which shell would read a queued command line and then emit syntax for that shell. It can't detect reliably — a locally-listed workspace can execute on an SSH or WSL host — and every dialect-dependent function is somewhere to get it wrong. So instead of guessing better, this stops guessing: everything Orca emits for a Unix shell is written to be correct in sh, bash, zsh, dash, ksh and fish at the same time.

That deleted more than it added.

What Changed

One Unix dialect, no shell detection

AgentStartupShell is now 'posix' | 'powershell' | 'cmd'. There is deliberately no 'fish' member — not because fish is unsupported, but because nothing needs to know.

• quoteStartupArg emits backslashes as "\\" and apostrophes as "'" between single-quoted runs. Both families read that identically. The sh '\'' idiom does not survive fish: inside '…' fish treats \\ and \' as escapes, so every backslash is halved and a trailing backslash is a hard syntax error that kills the launch.

• clearEnvCommand emits a self-contained branch:

  command test -n "$fish_pid" && set --erase -g NAME; command test -z "$fish_pid" && unset NAME; true

  It deliberately does not call a helper defined by Orca's shell wrappers. Orca wraps only zsh, bash and fish, so an sh/dash/ksh login shell launches unwrapped — and this same text is copied to the clipboard and pasted into shells Orca never spawned. In both cases a helper would be command not found, which is precisely the failure this exists to avoid.

  Every piece of that line is load-bearing, and each was put there by a demonstrated failure:

  • Two guarded statements, not A && B || C — fish's set -e returns non-zero on an already-unset variable, so || fell through to the sh branch and printed Unknown command: unset.
  • Trailing true — the guard that doesn't fire leaves a non-zero status, and this is the last statement of a launch line, so the user's prompt would render it as a failure.
  • command test, not test — interactive shells expand aliases, and alias test=… silently skipped both branches. Only reproducible in an interactive shell, so the test for it drives -i with the script on stdin.
  • -g, not an unflagged erase — without it, a name that exists only as a fish universal (set -Ux CODEX_HOME …, a normal thing to have) is permanently deleted from every future session. Reachable from the clipboard command, which may run with no injected value at all. Measured: unflagged erase → *** PERMANENTLY DELETED ***; with -g → SURVIVED. A universal shadowed by an injected global is revealed again instead, which is the wanted outcome.
  • set --erase, not set -e — $fish_pid is a heuristic. If it is ever non-empty in an sh-family shell, set -e NAME enables errexit and replaces the positional parameters, silently changing the semantics of the rest of the session. set --erase is the same erase in fish, but -- ends option parsing in sh, so a misfire costs a usage message and nothing else. (Measured: set -e → flags=ehBc; set --erase → flags=hBc.)

  Names are asserted, not escaped. clearEnvCommand requires every name to match /^[A-Za-z_][A-Za-z0-9_]*$/ and throws otherwise. The name is interpolated straight into a shell line, so a non-identifier would be both a command injection and — now that -g erases for real in fish — a way to delete PATH or HOME out of the user's session. Both callers pass literals from a fixed table, so this cannot fire today; the guard exists so it stays that way.

  Known limit, documented in the code: under set -u the sh side aborts on the unset $fish_pid and the variable survives. ${fish_pid-} would be nounset-safe but is a fish parse error, and so is set +u.

  A fish-builtin probe (math 1 >/dev/null 2>&1 && … || …) would satisfy both, since it references no variable. Evaluated and rejected: it executes whatever that name resolves to on the user's PATH, twice. math is a real binary — Wolfram Mathematica ships /usr/local/bin/math. Measured with a slow binary of that name on PATH: 10.4s to clear one variable, and with an exit-0 binary it does not clear at all (unconditional false positive). Trading a rare uncleared prefill for arbitrary program execution on the launch path is the wrong direction.

• One tokenizer for Unix. The input is a string typed into an Orca settings field that the shell never parses — every token is re-quoted before the line is built. Parsing it per-shell only made the same setting mean different things in different workspaces. (Windows is genuinely different: cmd/PowerShell re-parse the built line, so that tokenizer stays.)

Deleted with the guessing: fish-command-tokenizer.ts, local-agent-startup-shell.ts, host-agent-startup-shell.ts, client-agent-startup-shell.ts, resolveLoginShellStartupDialect, and the agentEnv.SHELL probe — which read the user's configured agent env to decide who parses the line, and would have quoted with fish rules on a zsh machine.

Per-worktree zsh history was a no-op on macOS — fixes #11044

/etc/zshrc line 16 assigns HISTFILE=${ZDOTDIR:-$HOME}/.zsh_history with no check-before-set, and it runs before every wrapper file Orca controls. So the injected value was already gone by the first prompt — and because ZDOTDIR still pointed at Orca's wrapper dir, history landed inside Orca's own sandbox:

$ HISTFILE=/tmp/wanted /bin/zsh -ic 'echo $HISTFILE'
/Users/<user>/.zsh_history          ← injected value destroyed

The intended path now rides ORCA_HISTFILE, which /etc/zshrc doesn't touch, and the wrapper restores it once the user's own config has loaded.

The wrapper also repairs the case where Orca injects nothing — history isolation off, or any path that does not inject. There, /etc/zshrc still derives HISTFILE from Orca's wrapper ZDOTDIR, so history accumulated inside the wrapper directory and the user's real history was invisible: the plain #11044 bug with no per-worktree scoping involved. Only the exact clobbered value ($ZDOTDIR/.zsh_history) is rewritten, so a HISTFILE the user set deliberately is never touched — verified. bash is unaffected (no system file assigns HISTFILE); verified.

fish history isolation

fish ignores HISTFILE entirely — history lives at <data dir>/<name>_history, keyed only by the fish_history variable, and there is no custom-directory knob. So the session name is the only lever, and the file necessarily lands in the user's fish data dir rather than Orca's tree. XDG_DATA_HOME is deliberately not redirected: that would move every other tool's data.

Deletion is a recorded directory plus a self-healing GC sweep that reclaims orca_<hex>_history files whose worktree is gone. Fixes that fell out:

• Files are deleted, not truncated. The previous approach called ftruncateSync(fd, 0), leaving a permanent 0-byte file per deleted worktree.
• A failed WSL fish cleanup no longer leaks the history tree. injectWslFishHistoryEnv records a fishSession for every WSL terminal, and cleanup shells out to wsl.exe --exec fish — which fails outright on a distro with no fish, i.e. most of them. That rejection was chained into removeHostTree, so those users' bash/zsh history trees were never reclaimed at all.
• WSL metadata no longer records a host-resolved fish directory. On Windows, resolveFishHistoryDir() resolves from the Windows process environment, naming a host directory unrelated to the distro — which the host GC sweep would then scan. WSL cleanup resolves the path inside the distro instead.
• A symlinked ~/.local/share no longer disables cleanup. The previous guard walked every path component from / and bailed on the first symlink — which rejects an ordinary dotfile-manager setup, silently leaking every file.
• One record instead of two. meta.json holds fishSession + fishHistoryDir. The inode-attestation file, the 16-entry retained-path ledger and its migration flag are gone; the path fields were write-only.

The sweep is guarded twice against deleting live history: it refuses an empty live-worktree set (indistinguishable from a store that failed to hydrate) and skips files younger than GC_MIN_AGE_MS, mirroring the tree GC's existing guard against the live-set snapshot race. readHistoryMeta re-derives the expected session from the directory's own hash, so a tampered meta.json naming someone else's session is ignored, and deletion refuses anything that isn't a regular file.

Why

All of these are silent failures. Shared history is a daily papercut across worktrees; the unset one prints an error into the agent's own terminal on every draft launch and leaves the prefill variable exported, so the next agent start inherits a stale draft; and #11044 means the feature this stack is named after has been doing nothing on the primary platform.

The dialect collapse is the load-bearing idea. isPosixStartupShell('fish') === true is exactly the shape of a predicate that looks like it branches and doesn't, and threading a new dialect through code shaped like that is how a cosmetic bug becomes corrupted prompts. Removing the dialect removes the whole class.

Testing

The shells are the oracle, not hand-written expectations.

src/shared/startup-shell-portability.live-shell.test.ts — 194 assertions across sh, bash, zsh, dash, ksh and fish 4.7.1. Each value is handed to a real shell, echoed back through printf, and compared byte-for-byte. 27 adversarial cases including \\server\share, trailing and lone backslashes, $/backtick/$(…), globs, braces, operators, newlines, tabs and non-BMP unicode; a full argv split on NUL; and the teardown clearing the variable with exit 0 and empty stderr whether the variable was set or already unset.

src/main/zsh-scoped-histfile.live-shell.test.ts — drives a real login zsh through /etc/zshrc with Orca's actual wrapper files, and asserts history never lands in the wrapper dir.

Vacuity-checked — each revert fails in exactly the shell it should:

revert	fails
portable quoting → plain sh quoting	4 fish cases (\\server\share, trailing \, lone \, full argv)
teardown → a command eval form	2 zsh cases — zsh's command resolves external binaries only
teardown → A && B || C	fish, on an already-unset variable
teardown → A && B || C	fish, on an already-unset variable
set --erase → set -e	bash + zsh — errexit silently enabled
set --erase -g → unflagged	fish — a lone universal is destroyed
command test → test	bash + zsh, interactive, with test aliased

| WSL fish-cleanup rethrow restored | the history tree is never removed |
| ZSH_HISTFILE_RESTORE_BLOCK's un-injected branch → removed | zsh — history lands in Orca's wrapper dir |
| ZSH_HISTFILE_RESTORE_BLOCK → no-op | 2 zsh cases, incl. "never leaves history inside Orca's own wrapper directory" |

Cross-platform: the same 27-case quoting corpus plus the teardown was replayed on real Linux (Ubuntu 24.04) against sh, bash and dash, compared base64-exact — 27/27 on each. That matters because /bin/sh is dash there, unlike macOS. Windows/WSL code paths are unchanged: windows-terminal-shell.ts is byte-identical to main, and both Windows tokenizer/quoter branches are untouched.

Observed locally on macOS 15 (fish 4.7.1, zsh 5.9) — all after rebase onto current main:

• npx tsc --noEmit — clean, both projects
• npx oxlint — clean
• full suite — 52,347 passed, 130 skipped. One unrelated flake (native-chat/transcript-watch, a file-watcher timing test) passes 28/28 in isolation; this diff touches zero native-chat files.
• check-max-lines-ratchet.mjs — OK, no new bypasses
• check-reliability-gates.mjs — 85 gates pass

Linked Issue

Fixes #11044

Supersedes #11146 (@innocarpe), which fixes #11044 independently with the same ORCA_HISTFILE mechanism this converged on. Two things from that PR are adopted here with credit:

• Dropping a stale inherited ORCA_HISTFILE when history injection is skipped. An Orca terminal opened from inside another Orca terminal inherits it, and the wrapper would then re-export the parent worktree's history path — the cross-worktree leak this feature exists to prevent. Worse on the early-return path, where it would override a caller-supplied HISTFILE. This branch now clears it at the top of injectHistoryEnv, in the provider when history is off, and on the relay path; three tests pin it, vacuity-checked.
• The [[ -n "${ORCA_HISTFILE:-}" ]] restore placement, which matches what landed here.

This PR additionally repairs the case #11146 does not cover: when Orca injects nothing, /etc/zshrc still derives HISTFILE from Orca's wrapper ZDOTDIR, so history accumulated inside the wrapper directory with no scoping involved.

Visual Proof

N/A — no UI surface changes. Every behavior here is shell-side and is covered by real-shell tests instead.

Review

• Security — the only filesystem write outside Orca's dirs is fish's own history file. Deletion re-derives the session from the directory hash, refuses anything that isn't a regular file, and the sweep matches only Orca's own orca_<hex>_ prefix, so the user's real fish_history is never a candidate.
• Data loss — the sweep's two guards (empty-set refusal, age floor) are the ones that matter; both have tests naming the race they close.
• Cross-platform — verified on macOS and Linux as above; Windows/WSL paths byte-identical to main.
• Remote/SSH — no wire change. The dialect decision no longer exists to get wrong on a remote host, which is the point.
• Backwards compatibility — meta.json gains fishHistoryDir; a file without it still deletes through this process's own fish dir, and the sweep catches anything both miss. The quoting output changes spelling for values containing ' or \ ('a'\''b' → 'a'"'"'b'); both are valid sh, only the second is also valid fish.

Adversarial review

Four adversarial reviewers were run against this branch with instructions to break it, plus my own passes over the release checklist. Ten real defects were found and fixed, eight of them introduced by this change:

found	severity	status
__orca_clear_env helper missing in unwrapped sh/dash/ksh and in clipboard-pasted text	high	fixed — helper approach deleted entirely
fish set -e on an already-unset variable → Unknown command: unset	high	fixed — two guarded statements
non-zero exit status painting the prompt after every launch	medium	fixed — trailing true
set -e NAME silently enabling errexit on a $fish_pid misfire	high	fixed — set --erase
alias test=… silently skipping both branches interactively	medium	fixed — command test
failed WSL fish cleanup blocking bash/zsh history-tree removal	high	fixed — cleanup is best-effort
GC sweep with no age guard / no empty-set refusal	high	fixed — both guards added
unflagged fish erase permanently deleting a user's universal variable	data loss	fixed — scoped to -g
second sh-idiom quoting path in the same resume-command file	medium	fixed — unified
unvalidated variable name interpolated into a shell line	latent injection	fixed — identifier assert

Confirmed clean by review and re-verified independently: the 'unix'→'posix' WSL classifier change causes no consumer behavior change (windows-terminal-shell.ts is byte-identical to main); Windows/WSL path quoting; distro argv with spaces and Unicode; wrapper CRLF generation; and quoteStartupArg against history expansion (!, ^), ~, $'', --, CRLF, RTL/ZWSP/combining marks, 10k-char strings, globs, $(), backticks and operators — none breakable.

One more fell out of chasing the last item myself: ai-vault-resume-command.ts had a second quoting path (quoteShellArg, used when a caller omits the shell) still on the sh '\'' idiom — so the same resume command was quoted two different ways depending on the call site. Unified.

Follow-up, scoped out deliberately: ten other quoting helpers across the codebase still use the sh '\'' idiom and carry the same fish hazard —

pane-helpers.ts, setup-agent-sequencing.ts, setup-runner-command.ts, wsl-login-shell-command.ts, ephemeral-vm-recipe-process.ts, posix-command-path-lookup.ts, cli/shell-command-quote.ts, relay/pty-shell-overlay-wrappers.ts, relay/preflight-handler.ts, main/rate-limits/claude-pty.ts.

Each has a different consumer and a different answer to "who parses this" — several are provably sh -c and safe, others are typed into the user's shell and are not. Converting them blind, with no tests on those paths, is a larger and riskier change than this one; they are listed here so the next person has the inventory rather than having to rediscover it.

Accepted, not fixed: if fish_pid is deleted mid-session the guard misreads (one stderr line, uncleared prefill); zsh's set --erase silently clobbers $1 on a misfire (no errexit, which is the property that matters); alias command=false defeats the guard, as would aliasing anything else; and a NUL byte inside a quoted argument is handled differently by each shell (zsh preserves, bash drops, fish truncates, ksh errors). Unreachable from prompts or settings in practice.

Notes for reviewers

• Supersedes fix(fish): scope shell history per worktree and clear agent draft prefills #13983, which took the opposite approach (thread a fish dialect everywhere). That direction is what produced the quoting regression it then had to fix. Net against that branch: −2,400 lines, 14 files deleted. Against main this is +428 source / +136 test lines.
• Known follow-up, deliberately not fixed here: a startup GC drain can launch up to MAX_PENDING_HISTORY_TREE_REMOVALS concurrent wsl.exe cleanups on a machine with many dead WSL fish worktrees. Bounding that belongs with the tombstone scheduler that owns the admission; two attempts to bound it inside the cleanup module produced worse bugs than the fanout, so it is documented at the call site instead.
• Two items that rode along in fix(fish): scope shell history per worktree and clear agent draft prefills #13983 are not here: the fish startup-env perf cache is reverted (unrelated, and its hardcoded agent-name allowlist silently stops applying when an agent is added), and the relay fish shell-ready wrapper is reverted (a real gap, but untested against a live relay — it deserves its own change).

Checklist

• This PR is small and focused
• I explained what changed and why (including ELI5)
• Before/after screenshots or videos attached for UI changes, or N/A with reason
• Self-reviewed for correctness, security, and performance
• Cross-platform, SSH/remote, and path/shortcut impact considered
• pnpm lint, pnpm typecheck, pnpm test pass

AI Disclosure

Claude Opus.

Made with Orca 🐋

[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: c1b8284f-bb0f-4936-b748-9badd5130c2d

📥 Commits

Reviewing files that changed from the base of the PR and between a9102d09222ab378d7fe0dc730bfd3310aed7a1d and d231126.

📒 Files selected for processing (1)

• src/shared/ai-vault-resume-command.ts

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

• src/shared/ai-vault-resume-command.ts

Included review availability: Your plan includes up to 10 reviews per rolling hour; 4 remain after this review.

---

📝 Walkthrough

Walkthrough

The change adds worktree-scoped Bash, Zsh, and Fish history isolation across local and remote PTY sessions. It adds safe Fish history deletion, WSL cleanup, tombstone retry limits, orphan sweeping, relay persistence, and remote worktree cleanup. It also centralizes shell overlays and updates Unix quoting and environment clearing for Bash, Zsh, Fish, and other POSIX shells. Live-shell and unit tests cover history isolation, cleanup, quoting, and startup portability.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 43.33% 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
Description check	✅ Passed	The description covers the main template sections, links issue #11044, explains the changes, documents testing, and marks visual proof as N/A.
Linked Issues check	✅ Passed	The description links Fixes #11044, and the implementation directly addresses the reported zsh HISTFILE overwrite behavior.
Out of Scope Changes check	✅ Passed	The changes consistently support Unix shell portability, per-worktree history isolation, cleanup, and related remote and WSL behavior.
Title check	✅ Passed	The title clearly identifies the primary refactor from shell detection to one portable Unix startup dialect.

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

🧹 Nitpick comments (6)

src/relay/pty-shell-overlay-wrappers.ts (1)

191-206: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Consider degrading gracefully when wrapper writes fail.

This loop has no error handling. On a remote host with a read-only home, no free space, or restrictive permissions, mkdirSync, writeFileSync, or chmodSync throws. The exception propagates out of getRelayShellLaunchConfig and fails the PTY spawn.

The equivalent local and daemon implementations catch the error, log it, and let the shell start without the wrapper. Consider matching that behavior so a wrapper failure costs the ready marker instead of the session.

src/shared/startup-shell-portability.live-shell.test.ts (1)

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

Condense the module header.

Keep only the non-obvious test purpose. Move regression-specific detail to the affected test cases if needed. The current header is much longer than necessary.

As per coding guidelines, “Comments must be concise, non-obvious, and brief—prefer one line.”

Source: Coding guidelines

src/main/terminal-history-tombstone-retry.test.ts (1)

127-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use MAX_PENDING_HISTORY_TREE_REMOVALS instead of the literal 64.

The test already imports the constant and uses it at Line 173. Lines 127-129 and Line 158 hardcode 64. If the cap changes, these assertions fail without naming the cause.

♻️ Proposed change

-    expect(removeHostTreeMock).toHaveBeenCalledTimes(64)
-    expect(deleteWslFishHistoryFileMock).toHaveBeenCalledTimes(64)
-    expect(releases).toHaveLength(64)
+    expect(removeHostTreeMock).toHaveBeenCalledTimes(MAX_PENDING_HISTORY_TREE_REMOVALS)
+    expect(deleteWslFishHistoryFileMock).toHaveBeenCalledTimes(MAX_PENDING_HISTORY_TREE_REMOVALS)
+    expect(releases).toHaveLength(MAX_PENDING_HISTORY_TREE_REMOVALS)

Apply the same replacement at Line 158 and Line 163.

src/main/remote-worktree-history-cleanup.test.ts (1)

15-29: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Add a case for a rejection that is not an unknown-method error.

This test pins the behavior for an old relay. It does not show whether deleteRemoteWorktreeHistory swallows every rejection or only the unknown-method one. A transient transport failure should be distinguishable from an unsupported relay, because the first one is retryable.

Add a case that rejects with, for example, new Error('connection reset') and assert the intended behavior.

src/main/terminal-history-fish-session.node-pty.test.ts (2)

66-67: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Split the path segments instead of embedding /.

Line 67 passes 'fish/config.fish' as one segment. Use separate segments so no separator is assumed.

As per coding guidelines: "Use path.join or Electron/Node path utilities for file paths; never assume / or \".

♻️ Proposed change

-        path.join(home, 'fish/config.fish'),
+        path.join(home, 'fish', 'config.fish'),

Source: Coding guidelines

---

116-128: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Kill the PTY even when an assertion fails.

If the wait at Line 116 or Line 118 returns false, the expect throws and Lines 122-128 never run. The fish process then survives the test. afterEach removes only the temp home directory.

Register the terminal for teardown, or wrap the interaction in try/finally.

♻️ Proposed change

 describe('fish keeps per-worktree history under the session Orca names', () => {
   let home: string | null = null
+  let live: { kill: () => void } | null = null
 
   afterEach(() => {
+    try {
+      live?.kill()
+    } catch {
+      // already gone
+    }
+    live = null
     if (home) {

Assign live = term right after nodePty.spawn returns, and clear it after the expected exit.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0b49944c-1c19-4354-ae53-aaaafa9d7e7e

📥 Commits

Reviewing files that changed from the base of the PR and between c066a55 and ccb6a5f84976d3a8294a2a93c90b96c5479afdd3.

📒 Files selected for processing (63)

• .github/workflows/pr.yml
• src/main/daemon/daemon-pty-adapter.test.ts
• src/main/daemon/daemon-pty-adapter.ts
• src/main/daemon/shell-ready.test.ts
• src/main/daemon/shell-ready.ts
• src/main/fish-history-session.test.ts
• src/main/fish-history-session.ts
• src/main/ipc/pty.ts
• src/main/ipc/worktrees.test.ts
• src/main/ipc/worktrees.ts
• src/main/providers/local-pty-provider.test.ts
• src/main/providers/local-pty-provider.ts
• src/main/providers/local-pty-shell-ready.test.ts
• src/main/providers/local-pty-shell-ready.ts
• src/main/providers/pty-provider-contract.ts
• src/main/providers/ssh-pty-provider.test.ts
• src/main/providers/ssh-pty-provider.ts
• src/main/providers/ssh-pty-spawn-request.ts
• src/main/remote-worktree-history-cleanup.test.ts
• src/main/remote-worktree-history-cleanup.ts
• src/main/runtime/orca-runtime.test.ts
• src/main/runtime/orca-runtime.ts
• src/main/shell-templates.ts
• src/main/terminal-history-deletion.ts
• src/main/terminal-history-fish-session.node-pty.test.ts
• src/main/terminal-history-gc.ts
• src/main/terminal-history-id.ts
• src/main/terminal-history-paths.ts
• src/main/terminal-history-tombstone-retry.test.ts
• src/main/terminal-history.test.ts
• src/main/terminal-history.ts
• src/main/wsl-fish-history-cleanup.test.ts
• src/main/wsl-fish-history-cleanup.ts
• src/main/zsh-scoped-histfile.live-shell.test.ts
• src/relay/pty-handler.test.ts
• src/relay/pty-handler.ts
• src/relay/pty-shell-launch.ts
• src/relay/pty-shell-overlay-wrappers.ts
• src/relay/terminal-history.test.ts
• src/relay/terminal-history.ts
• src/renderer/src/components/settings/CliSkillRuntimeSetup.tsx
• src/renderer/src/components/sidebar/folder-workspace-composer-submit.test.ts
• src/renderer/src/components/terminal-pane/pty-connection.test.ts
• src/renderer/src/lib/ai-vault-resume-command.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/renderer/src/lib/launch-agent-background-session.test.ts
• src/renderer/src/lib/launch-agent-in-new-tab-windows-quoting.test.ts
• src/renderer/src/lib/launch-work-item-direct.test.ts
• src/renderer/src/lib/resume-sleeping-agent-session.test.ts
• src/renderer/src/lib/tui-agent-startup.test.ts
• src/shared/agent-resume-launch-command.ts
• src/shared/ai-vault-resume-command.ts
• src/shared/hermes-startup-query.ts
• src/shared/shell-wrapper-functions.ts
• src/shared/startup-shell-portability.live-shell.test.ts
• src/shared/tui-agent-startup-session-options.test.ts
• src/shared/tui-agent-startup-shell.test.ts
• src/shared/tui-agent-startup-shell.ts
• src/shared/tui-agent-startup.test.ts
• src/shared/types.ts
• src/shared/windows-terminal-shell.ts

Included review availability: Your plan includes up to 10 reviews per rolling hour; 9 remain after this review.

[coderabbitai]

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Limit the host-global HISTFILE assertion to macOS.

Line 82 requires a system zsh startup file to set HISTFILE. Zsh does not set this variable by default, and Linux distributions can omit or configure the global zshrc differently. This test runs on every non-Windows host, so it can fail without a wrapper defect. Gate this assertion to macOS, or create deterministic startup input.

As per coding guidelines, keep platform-dependent behavior behind runtime checks for macOS, Linux, and Windows.

Source: Coding guidelines

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[coderabbitai]

Actionable comments posted: 3

♻️ Duplicate comments (1)

src/relay/pty-shell-overlay-wrappers.ts (1)

63-76: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Restore HISTFILE from the .zlogin wrapper.

Relay zsh starts with -l. The login guard skips the restore at Line 57. User or system startup files can then overwrite HISTFILE, and the worktree-scoped path is not restored.

Add ZSH_HISTFILE_RESTORE_BLOCK after the user .zlogin source and before the final ZDOTDIR restore.

Proposed fix

 ${getPosixOmpShellWrapper()}
+${ZSH_HISTFILE_RESTORE_BLOCK}
 ${getZshFinalZdotdirRestoreBlock('"${ORCA_USER_ZDOTDIR:-${ORCA_ORIG_ZDOTDIR:-$HOME}}"')}

🧹 Nitpick comments (2)

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

172-190: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Shorten the new explanatory comments. Keep one concise statement for each non-obvious constraint. Move detailed edge-case inventories to tests or documentation.

• src/shared/tui-agent-startup-shell.ts#L172-L190: reduce the quoting rationale to one brief constraint statement.
• src/shared/tui-agent-startup-shell.ts#L3-L9: shorten the startup-dialect explanation.
• src/shared/tui-agent-startup-shell.ts#L155-L159: shorten the tokenizer rationale.
• src/shared/tui-agent-startup-shell.ts#L239-L265: shorten the environment-cleanup rationale.
• src/relay/pty-shell-overlay-wrappers.ts#L12-L14: shorten the module-extraction comment.
• src/shared/startup-shell-portability.live-shell.test.ts#L1-L15: shorten the test-suite introduction.

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/main/fish-history-session.ts (1)

5-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Shorten the implementation comments.

Keep the behavioral contract and safety invariant. Remove the detailed implementation narrative. Use brief comments near the affected code.

As per coding guidelines, comments must be concise, non-obvious, and brief—prefer one line.

Also applies to: 30-32, 40-41, 54-56, 68-85, 121-121

Source: Coding guidelines

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 20073452-76f3-4037-ae06-efc8f0255035

📥 Commits

Reviewing files that changed from the base of the PR and between fd1dba9 and c7b010e.

📒 Files selected for processing (57)

• .github/workflows/pr.yml
• src/main/daemon/daemon-pty-adapter.ts
• src/main/daemon/daemon-zsh-shell-ready-rcfile.ts
• src/main/daemon/shell-ready.ts
• src/main/fish-history-session.test.ts
• src/main/fish-history-session.ts
• src/main/ipc/pty.ts
• src/main/ipc/worktrees.ts
• src/main/providers/local-pty-provider.ts
• src/main/providers/local-pty-shell-ready-wrapper-generation.ts
• src/main/providers/pty-provider-contract.ts
• src/main/providers/ssh-pty-provider.test.ts
• src/main/providers/ssh-pty-provider.ts
• src/main/providers/ssh-pty-session-reattach.ts
• src/main/providers/ssh-pty-spawn-request.ts
• src/main/remote-worktree-history-cleanup.test.ts
• src/main/remote-worktree-history-cleanup.ts
• src/main/runtime/orca-runtime.test.ts
• src/main/runtime/orca-runtime.ts
• src/main/shell-templates.ts
• src/main/terminal-history-deletion.ts
• src/main/terminal-history-fish-session.node-pty.test.ts
• src/main/terminal-history-gc.ts
• src/main/terminal-history-id.ts
• src/main/terminal-history-paths.ts
• src/main/terminal-history-tombstone-retry.test.ts
• src/main/terminal-history.test.ts
• src/main/terminal-history.ts
• src/main/wsl-fish-history-cleanup.test.ts
• src/main/wsl-fish-history-cleanup.ts
• src/main/zsh-scoped-histfile.live-shell.test.ts
• src/relay/pty-handler.ts
• src/relay/pty-shell-launch.ts
• src/relay/pty-shell-overlay-wrappers.ts
• src/relay/terminal-history.test.ts
• src/relay/terminal-history.ts
• src/renderer/src/components/settings/CliSkillRuntimeSetup.tsx
• src/renderer/src/components/sidebar/folder-workspace-composer-submit.test.ts
• src/renderer/src/components/terminal-pane/pty-connection-cold-restore-agent-resume.test.ts
• src/renderer/src/lib/ai-vault-resume-command.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/renderer/src/lib/launch-agent-background-session.test.ts
• src/renderer/src/lib/launch-agent-in-new-tab-windows-quoting.test.ts
• src/renderer/src/lib/launch-work-item-direct.test.ts
• src/renderer/src/lib/resume-sleeping-agent-session.test.ts
• src/renderer/src/lib/tui-agent-startup.test.ts
• src/shared/agent-resume-launch-command.ts
• src/shared/ai-vault-resume-command.ts
• src/shared/global-settings-types.ts
• src/shared/hermes-startup-query.ts
• src/shared/startup-shell-portability.live-shell.test.ts
• src/shared/tui-agent-startup-session-options.test.ts
• src/shared/tui-agent-startup-shell.test.ts
• src/shared/tui-agent-startup-shell.ts
• src/shared/tui-agent-startup.test.ts

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

• src/renderer/src/lib/tui-agent-startup.test.ts
• src/renderer/src/components/sidebar/folder-workspace-composer-submit.test.ts
• src/renderer/src/lib/launch-agent-in-new-tab-windows-quoting.test.ts
• src/main/remote-worktree-history-cleanup.ts
• src/renderer/src/lib/launch-work-item-direct.test.ts
• src/main/providers/ssh-pty-spawn-request.ts
• src/renderer/src/lib/ai-vault-resume-command.test.ts
• src/shared/agent-resume-launch-command.ts
• src/main/runtime/orca-runtime.test.ts
• src/renderer/src/lib/resume-sleeping-agent-session.test.ts
• src/main/ipc/pty.ts
• src/shared/tui-agent-startup.test.ts
• src/shared/tui-agent-startup-shell.test.ts
• src/main/providers/pty-provider-contract.ts
• src/shared/ai-vault-resume-command.ts
• src/renderer/src/components/settings/CliSkillRuntimeSetup.tsx
• src/relay/terminal-history.test.ts
• src/main/remote-worktree-history-cleanup.test.ts
• .github/workflows/pr.yml
• src/renderer/src/lib/launch-agent-background-session.test.ts
• src/main/ipc/worktrees.ts
• src/main/terminal-history-gc.ts
• src/main/terminal-history-tombstone-retry.test.ts
• src/shared/tui-agent-startup-session-options.test.ts
• src/main/terminal-history-id.ts
• src/main/terminal-history-paths.ts
• src/main/runtime/orca-runtime.ts
• src/main/wsl-fish-history-cleanup.test.ts
• src/renderer/src/lib/ai-vault-resume-shell.ts
• src/shared/hermes-startup-query.ts
• src/main/terminal-history-fish-session.node-pty.test.ts
• src/renderer/src/lib/ai-vault-resume-shell.test.ts
• src/relay/terminal-history.ts
• src/main/daemon/daemon-pty-adapter.ts
• src/main/terminal-history.test.ts
• src/main/terminal-history.ts
• src/main/providers/local-pty-provider.ts
• src/relay/pty-handler.ts
• src/main/providers/ssh-pty-provider.test.ts

Included review availability: Your plan includes up to 10 reviews per rolling hour; 7 remain after this review.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

node <<'NODE'
const { spawnSync } = require('node:child_process')

const command =
  'test -n "$fish_pid" && set -e ORCA_PI_PREFILL; ' +
  'test -z "$fish_pid" && unset ORCA_PI_PREFILL; true'

for (const shell of ['sh', 'bash', 'zsh', 'dash', 'ksh']) {
  const env = { ...process.env }
  delete env.fish_pid
  const result = spawnSync(shell, ['-u', '-c', command], { encoding: 'utf8', env })

  if (result.error?.code === 'ENOENT') continue
  console.log(`${shell}: status=${result.status}`)
  process.stdout.write(result.stderr || '')
}
NODE

Repository: stablyai/orca

Length of output: 263

---

🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- source symbols and call sites ---'
rg -n -C 4 'clearEnvCommand|fish_pid|startup-shell-portability' src/shared/tui-agent-startup-shell.ts src/shared/startup-shell-portability.live-shell.test.ts src/shared 2>/dev/null | head -240

printf '%s\n' '--- focused source ---'
sed -n '235,290p' src/shared/tui-agent-startup-shell.ts

printf '%s\n' '--- focused portability test ---'
sed -n '1,260p' src/shared/startup-shell-portability.live-shell.test.ts

printf '%s\n' '--- shell availability and behavior ---'
node <<'NODE'
const { spawnSync } = require('node:child_process')

const command =
  'test -n "$fish_pid" && set -e ORCA_PI_PREFILL; ' +
  'test -z "$fish_pid" && unset ORCA_PI_PREFILL; true'

for (const shell of ['sh', 'bash', 'zsh', 'dash', 'ksh', 'fish']) {
  const result = spawnSync(shell, ['-u', '-c', command], {
    encoding: 'utf8',
    env: { ...process.env, ORCA_PI_PREFILL: 'present' },
  })
  if (result.error?.code === 'ENOENT') {
    console.log(`${shell}: unavailable`)
    continue
  }
  console.log(`${shell}: status=${result.status}, stdout=${JSON.stringify(result.stdout)}, stderr=${JSON.stringify(result.stderr)}`)
}
NODE

Repository: stablyai/orca

Length of output: 27959

---

Avoid expanding fish_pid under nounset.

When user startup enables set -u, the POSIX branch expands unset fish_pid before test runs. sh and bash abort the command, so CODEX_HOME and ORCA_CODEX_HOME can remain set.

Use a branch detector that does not expand an undefined POSIX variable. Add nounset coverage to src/shared/startup-shell-portability.live-shell.test.ts.

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (2)

src/shared/startup-shell-portability.live-shell.test.ts (2)

46-48: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use the Node path utility.

split('/') assumes a Unix separator. Use path.basename() and rename the parameter to avoid shadowing the imported module.

Proposed fix

-function basename(path: string): string {
-  return path.split('/').pop() ?? path
+function basename(filePath: string): string {
+  return path.basename(filePath)
 }

As per coding guidelines, “Use path.join or Electron/Node path utilities for file paths; never assume / or \.”

Source: Coding guidelines

---

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

Shorten the file header comment.

Keep a one-line suite purpose. Move detailed regression rationale next to the related test when it is necessary.

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: 5b7b6171-48e3-486b-891c-e1289140ac11

📥 Commits

Reviewing files that changed from the base of the PR and between c7b010e and d546b26353c62e86d46afb4a8c64f919511342fc.

📒 Files selected for processing (11)

• src/main/terminal-history-deletion.ts
• src/main/terminal-history-tombstone-retry.test.ts
• src/main/terminal-history.ts
• src/renderer/src/lib/ai-vault-resume-command.test.ts
• src/renderer/src/lib/ai-vault-resume-shell.test.ts
• src/renderer/src/lib/launch-work-item-direct.test.ts
• src/renderer/src/lib/tui-agent-startup.test.ts
• src/shared/startup-shell-portability.live-shell.test.ts
• src/shared/tui-agent-startup-shell.test.ts
• src/shared/tui-agent-startup-shell.ts
• src/shared/tui-agent-startup.test.ts

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

• src/renderer/src/lib/tui-agent-startup.test.ts
• src/renderer/src/lib/launch-work-item-direct.test.ts
• src/shared/tui-agent-startup.test.ts
• src/renderer/src/lib/ai-vault-resume-command.test.ts
• src/renderer/src/lib/ai-vault-resume-shell.test.ts
• src/main/terminal-history-deletion.ts
• src/shared/tui-agent-startup-shell.ts
• src/shared/tui-agent-startup-shell.test.ts
• src/main/terminal-history.ts

Included review availability: Your plan includes up to 10 reviews per rolling hour; 7 remain after this review.

[coderabbitai]

🧹 Nitpick comments (2)

src/shared/startup-shell-portability.live-shell.test.ts (2)

46-48: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use the Node path utility.

split('/') assumes a POSIX separator. Rename the parameter and use path.basename(filePath).

Proposed fix

-function basename(path: string): string {
-  return path.split('/').pop() ?? path
+function basename(filePath: string): string {
+  return path.basename(filePath)
 }

As per coding guidelines, use Node path utilities for file paths and do not assume / or \.

Source: Coding guidelines

---

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

Reduce the long test commentary.

Keep the required invariant and remove implementation narration. The test names and assertions already describe most of this behavior.

As per coding guidelines, comments must be concise, non-obvious, and brief.

Also applies to: 197-211

Source: Coding guidelines

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e82db3f5-41a5-4ab1-ae87-26688ce86e15

📥 Commits

Reviewing files that changed from the base of the PR and between d546b26353c62e86d46afb4a8c64f919511342fc and a9102d09222ab378d7fe0dc730bfd3310aed7a1d.

📒 Files selected for processing (2)

• src/shared/startup-shell-portability.live-shell.test.ts
• src/shared/tui-agent-startup-shell.ts

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

• src/shared/tui-agent-startup-shell.ts

Included review availability: Your plan includes up to 10 reviews per rolling hour; 6 remain after this review.

[nwparker]

Review feedback addressed

Rebased onto current main (conflict in relay/pty-shell-launch.ts — main had edited ensureOverlayRestoreWrappers inline while this branch extracted it; the extracted module was rebuilt from main's version so BASH_PROMPT_COMMAND_COMPOSITION_BLOCK is preserved).

Fixed now:

finding	change
relay/terminal-history.ts:80 — set ORCA_HISTFILE with HISTFILE	Done. This was a real gap: #11044 was fixed on the desktop PTY path but not the relay, so a remote macOS host would still lose the injected value to /etc/zshrc.
worktrees.ts:2627 — history deletion gated on push-target cleanup	Done, both remote paths. History deletion now runs first; the worktree is already gone from git and disk by then, so a rejecting push-target cleanup can no longer leave the user's commands on the remote host.
fish-history-session.test.ts:52 — Windows path assertions	Done, expectations built with join.
fish-history-session.test.ts:102, relay/terminal-history.test.ts:48 — symlink tests on Windows	Done, skipIf(win32) on both.
zsh-scoped-histfile.live-shell.test.ts:40 — probe vs execute mismatch	Done. Detection stays spawnSync('zsh', …) and execution uses the resolved absolute path, so both refer to the same binary.
startup-shell-portability.live-shell.test.ts:117 — Unix-only suite on Windows	Done, describe.skipIf(win32).

Already fixed in earlier commits (these were reviewed against older revisions): the fish-sweep young-directory guard (terminal-history-gc.ts:150 — now has both a GC_MIN_AGE_MS floor and a refusal to run on an empty live-worktree set), the WSL fish-cleanup gating (terminal-history-deletion.ts:120), and the teardown spelling (tui-agent-startup-shell.ts:269/277 — now command test + set --erase -g, which also fixes an errexit hazard, an alias bypass, and permanent deletion of a user's fish universal variable).

Local: 53,834 passing, both typechecks, lint, max-lines ratchet, 85 reliability gates. One unrelated flake (worktree-base-directory-poller, a file-watcher timing test) passes 23/23 in isolation and is untouched by this diff.

[AmethystLiang]

@nwparker we reverted #14863 in #14975 because it introduced a launch-blocking regression: folder-workspace Fish history can be deleted for a still-valid folder workspace after Orca restarts, and generated Unix cleanup/resume commands can abort in shells with set -u.

Please reland with a fix. Linear: https://linear.app/stably/issue/STA-4566/reland-14863-portable-unix-startup-dialect-after-revert-14975