[nwparker]

	Files	Added	Deleted	Net
Test	27	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​1942	$\color{#cf222e}{\Huge{\mathbf{−}}}$​137	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​1805
Prod	36	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​1562	$\color{#cf222e}{\Huge{\mathbf{−}}}$​410	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​1152

Reland of #14863

#14863 was reverted in #14975 for two regressions. Both are fixed here, each with a test that fails without the fix. The rest of the change is unmodified.

1. History GC deleted folder-workspace shell history (data loss)

The GC's live set came from getAllWorktreeMeta() alone. A folder workspace's PTY carries folder:<id> as its worktree id and lives in a separate store collection, so every live folder workspace looked orphaned and its history was pruned while the user was still writing to it.

getKnownWorktreeIdsForHistoryGc now unions in getFolderWorkspaces() mapped through folderWorkspaceKey. Both consumers — the history-directory prune and the fish-history file sweep — read that one set, so bash, zsh and fish history are all covered.

Worth noting: the directory prune has had this gap since #1524, so folder-workspace bash/zsh history was already being deleted on main independently of #14863. #14863 widened the blast radius to fish files, which is how it got caught. This fixes both.

2. A copied Codex resume command aborted under set -u

The clear statement has to test $fish_pid. Under set -u that unbound expansion aborts the entire line — and because the clear came first, it took the agent launch down with it. This text is copied, so it runs in a shell Orca never spawned and cannot seed.

The removal now rides on the agent itself:

cd '/path/to/repo' && env -u CODEX_HOME -u ORCA_CODEX_HOME codex resume '<id>'

env -u needs no shell syntax and performs no expansion, so there is nothing for set -u to abort on. Verified byte-identical under set -u in sh, bash, zsh, dash, ksh and fish, including a cwd containing a space.

Two placement details are load-bearing:

• On the agent, not on the chain. env cannot run cd — it is a shell builtin — and a child cd would not move the agent anyway. Prefixing the whole cd … && agent chain is broken on both counts.
• cmd and PowerShell keep the clear ahead of the cd. Neither has a nounset hazard, and leading placement preserves cd … && agent, so a failed cd still cannot launch the agent in the wrong directory. Moving their clear inline would have downgraded that to unconditional & sequencing.

withoutEnvCommand asserts its names are identifiers, matching the existing guard on clearEnvCommand.

Also fixed here: three more live-set gaps found in review

Same class as the revert cause — a live set missing a category of real workspace.

• Profiles. The history root has no profile segment but the Store does, so switching profiles condemned every other profile's history; fish history, which lands in the user's own fish data dir, is shared by every profile on the machine. The live set now unions in the inactive profiles' ids.
• No empty-set guard on the tree scan. The fish sweep already refuses an empty live set because it cannot be told apart from a store that failed to hydrate. The directory scan, which deletes more, had no such guard — a bad load would have taken every worktree's bash and zsh history with it.
• Relay fish history was written into the shared fish data dir under the desktop naming but keyed by the client's worktree ids, so a machine running both Orca and a relay host swept its own remote sessions' history. Relay files are now orca_relay_<hash>, which the sweep's pattern deliberately does not match.

Residual risk

env -u execs its argument directly, so on the copy path it bypasses shell alias and function resolution. Two narrow cases behave differently than before:

• An agent command that exists only as a shell alias or function — the pasted line runs the binary instead, or fails with env: codex: No such file or directory if there is no binary. Note the spawned (non-copied) startup path is unaffected; it removes these through envToDelete at the PTY level, not through the command text.
• A compound agentCmdOverrides entry (one containing &&, ;, or a pipe) — the removal applies to the first segment only.

Both were handled by the previous statement form, and both are accepted here deliberately: that form's $fish_pid test is what aborted the launch under set -u, and there is no spelling of it that detects fish without either an unbound expansion (aborts under nounset) or naming a command one of the two shell families does not have (fish prints a multi-line Unknown command: unset diagnostic on every paste — verified, and 2>/dev/null does not suppress it). command unset is not a workaround: zsh's command does not run the builtin, so it silently fails to unset and falls through.

---

Original description follows.

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]

📝 Walkthrough

Walkthrough

The change adds worktree-scoped Bash, Zsh, and Fish history for local, WSL, relay, and SSH PTY sessions. It adds metadata validation, orphan cleanup, tombstone retry limits, and remote deletion flows. Unix startup commands now use shared portable quoting and environment removal. Zsh wrappers restore history after startup files run. Live-shell and unit tests cover history isolation, cleanup, quoting, and shell portability.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The PR includes substantial changes beyond #11044, including fish history, GC, resume-command portability, relay cleanup, and broad shell-dialect refactoring.	Split unrelated work into separate PRs or link the additional issues that define the fish history, GC, resume-command, and shell-portability requirements.
Docstring Coverage	⚠️ Warning	Docstring coverage is 43.75% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR satisfies #11044 by restoring scoped zsh history after startup files and preserving deliberate HISTFILE values, with live-shell test coverage.
Title check	✅ Passed	The title clearly identifies this as a reland and summarizes the portable Unix startup dialect and fixes for both revert causes.
Description check	✅ Passed	The description is comprehensive and covers the required change summary, rationale, issue, testing, visual proof, review, AI disclosure, and checklist.

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

🧹 Nitpick comments (6)

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

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

Write the wrapper files atomically.

writeFileSync truncates the target before it writes. A relay-spawned shell can source .zshrc or rcfile at the same moment that a second concurrent spawn rewrites it, and then reads a truncated file. The existing !== content guard narrows the window to the first spawns after an upgrade, but two concurrent first spawns still race.

Write to a temporary file in the same directory, then rename it over the target.

♻️ Proposed refactor

-  for (const [path, content] of files) {
-    mkdirSync(dirname(path), { recursive: true })
-    let existing: string | null = null
-    try {
-      existing = readFileSync(path, 'utf8')
-    } catch {
-      existing = null
-    }
-    // Why: relay wrapper files persist under ~/.orca-relay across app
-    // upgrades. Existence alone is not enough; stale wrappers would miss
-    // later fixes such as preserving post-.zshenv ZDOTDIR.
-    if (existing !== content) {
-      writeFileSync(path, content, 'utf8')
-    }
-    chmodSync(path, 0o644)
-  }
+  for (const [path, content] of files) {
+    mkdirSync(dirname(path), { recursive: true })
+    let existing: string | null = null
+    try {
+      existing = readFileSync(path, 'utf8')
+    } catch {
+      existing = null
+    }
+    // Why: relay wrapper files persist under ~/.orca-relay across app
+    // upgrades. Existence alone is not enough; stale wrappers would miss
+    // later fixes such as preserving post-.zshenv ZDOTDIR.
+    if (existing !== content) {
+      // Why: a concurrent spawn must never source a truncated wrapper.
+      const staging = `${path}.${process.pid}.tmp`
+      writeFileSync(staging, content, { encoding: 'utf8', mode: 0o644 })
+      renameSync(staging, path)
+    }
+    chmodSync(path, 0o644)
+  }

Add renameSync to the node:fs import on line 1.

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

372-373: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add coverage for SshPtyProvider.deleteWorktreeHistory.

The provider sends pty.deleteWorktreeHistory with { worktreeId }, but its test file does not cover this method. Add an assertion for this request.

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

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

Pass each path segment to path.join.

Line 67 embeds a / inside a single path.join argument. Split the segments instead.

♻️ Proposed change

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

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

Source: Coding guidelines

---

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

Kill the PTY in a finally block.

If any assertion between lines 116 and 123 fails, term.kill() at line 125 never runs and the fish process survives the test. Wrap the interaction in try/finally so the PTY is always terminated.

♻️ Proposed change

-      expect(await waitUntil(() => rendered.includes(PROMPT_MARK), 15_000)).toBe(true)
-      term.write(`${MARKER}\r`)
-      expect(await waitUntil(() => rendered.includes('orca-worktree-scoped-history'), 5_000)).toBe(
-        true
-      )
-      // fish flushes history on exit, so the read must wait for the process to go.
-      term.write('exit\r')
-      expect(await waitUntil(() => exited, 10_000)).toBe(true)
-      try {
-        term.kill()
-      } catch {
-        // already gone
-      }
+      try {
+        expect(await waitUntil(() => rendered.includes(PROMPT_MARK), 15_000)).toBe(true)
+        term.write(`${MARKER}\r`)
+        expect(
+          await waitUntil(() => rendered.includes('orca-worktree-scoped-history'), 5_000)
+        ).toBe(true)
+        // fish flushes history on exit, so the read must wait for the process to go.
+        term.write('exit\r')
+        expect(await waitUntil(() => exited, 10_000)).toBe(true)
+      } finally {
+        try {
+          term.kill()
+        } catch {
+          // already gone
+        }
+      }

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

126-137: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use the imported cap constant instead of the literal 64.

The file already imports MAX_PENDING_HISTORY_TREE_REMOVALS (Line 33) and uses it at Line 175. Lines 127-129 and Lines 160/165 hardcode 64. If the cap value changes, these assertions fail for a reason unrelated to the behavior under test.

♻️ Proposed change

     await vi.advanceTimersByTimeAsync(0)
-    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 substitution at Lines 160 and 165.

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

239-300: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Condense this comment block and anchor it to clearEnvCommand.

Two problems:

1. The block is about 60 lines of prose for one function. The coding guidelines require concise, brief comments and prefer one line. Keep the decisions that are not recoverable from the code — the fish/sh split, -g scoping to avoid universal-variable data loss, command test for alias safety, the trailing true, and the set -u limit — and move the rejected-alternative narrative (math probe measurements, the ${fish_pid-} exploration) to the PR description or a short ADR.
2. As written, the block sits above const ENV_VAR_NAME, so editors and API docs attach it to the regex instead of clearEnvCommand. Move it below the regex, directly above the function.

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: 1ac40ee0-8b45-4936-a969-d5e2da9a0977

📥 Commits

Reviewing files that changed from the base of the PR and between f660aaa and 7f18b90.

📒 Files selected for processing (60)

• .github/workflows/pr.yml
• config/scripts/pr-workflow-parallelism.test.mjs
• 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/window/history-gc-worktree-ids.test.ts
• src/main/window/history-gc-worktree-ids.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

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

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Guard resolveUnixShellPath so history isolation cannot fail the spawn.

resolveUnixShellPath throws when no candidate shell is executable. withHistoryIsolation runs inside spawn, so that throw rejects the whole spawn before doSpawn runs. History scoping is best effort everywhere else in this method (each helper degrades to shared history), and the daemon resolves its own shell anyway. Catch the failure and return the original options.

🛡️ Proposed fix

-    const shellPath = resolveUnixShellPath(preferredShell)
+    let shellPath: string
+    try {
+      shellPath = resolveUnixShellPath(preferredShell)
+    } catch {
+      // History scoping is best effort; an unresolvable shell must not fail the spawn.
+      return opts
+    }

📝 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 preferredShell = wslContext
	? 'bash'
	: opts.shellOverride || env.SHELL || process.env.SHELL || '/bin/zsh'
	const shellPath = resolveUnixShellPath(preferredShell)
	const historyArgs = [
	env,
	opts.worktreeId,
	shellPath,
	opts.cwd ?? resolveSafePtyDefaultCwd()
	] as const
	const result = wslContext
	? injectHistoryEnv(...historyArgs, { wslDistro: wslContext.distro })
	: injectHistoryEnv(...historyArgs)
	const preferredShell = wslContext
	? 'bash'
	: opts.shellOverride || env.SHELL || process.env.SHELL || '/bin/zsh'
	let shellPath: string
	try {
	shellPath = resolveUnixShellPath(preferredShell)
	} catch {
	// History scoping is best effort; an unresolvable shell must not fail the spawn.
	return opts
	}
	const historyArgs = [
	env,
	opts.worktreeId,
	shellPath,
	opts.cwd ?? resolveSafePtyDefaultCwd()
	] as const
	const result = wslContext
	? injectHistoryEnv(...historyArgs, { wslDistro: wslContext.distro })
	: injectHistoryEnv(...historyArgs)

[coderabbitai]

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Delete remote history before push-target cleanup.

cleanupUnusedWorktreePushTargetRemoteSsh() can reject before these lines run. The normal SSH removal path then leaves scoped command history on the remote host. Move deleteRemoteWorktreeHistory() before the push-target cleanup, as done in the orphaned and manually removed paths.

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Inspect the shell-override resolver and the default-shell fallback.
fd -t f 'pty-shell-utils.ts' src/relay --exec ast-grep outline {} --items all
rg -n -C15 'function resolvePtyShellOverride' src/relay

Repository: stablyai/orca

Length of output: 3422

---

🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- pty-shell-utils.ts ---'
cat -n src/relay/pty-shell-utils.ts | sed -n '80,115p'

printf '%s\n' '--- pty-handler.ts resolver and spawn context ---'
cat -n src/relay/pty-handler.ts | sed -n '280,315p;1485,1545p'

printf '%s\n' '--- resolver call sites and existsSync imports ---'
rg -n -C3 'resolvePtyShellOverride|existsSync|pty\.spawn' src/relay/pty-handler.ts src/relay/pty-shell-utils.ts

Repository: stablyai/orca

Length of output: 12966

---

🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- change introducing requestedEnvShell ---'
git diff --unified=12 -- src/relay/pty-handler.ts | sed -n '/requestedEnvShell/,+35p'

printf '%s\n' '--- shell selection invariant probe ---'
python3 - <<'PY'
def current_selection(resolved_override, requested_env_shell, default_shell):
    return resolved_override or requested_env_shell or default_shell

def fallback_selection(resolved_override, requested_env_shell, default_shell, exists):
    return resolved_override or (
        requested_env_shell if requested_env_shell and exists(requested_env_shell) else ''
    ) or default_shell

cases = [
    ('', '/missing/client-shell', '/bin/bash', lambda path: False),
    ('', '/bin/zsh', '/bin/bash', lambda path: path == '/bin/zsh'),
]
for case in cases:
    print({
        'requested': case[1],
        'current': current_selection(*case[:3]),
        'with_existence_fallback': fallback_selection(*case),
    })
PY

Repository: stablyai/orca

Length of output: 439

---

Validate env.SHELL before spawning it.

On POSIX systems, a missing env.SHELL path bypasses resolveDefaultShell() and reaches pty.spawn, which can fail instead of using the fallback shell. Check existsSync(requestedEnvShell) before selecting it.

Do not route env.SHELL through resolvePtyShellOverride(). That resolver returns an empty string on POSIX systems and validates only Windows shell names.

[coderabbitai]

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

Replace the stale shell-branch comment.

The copied command uses shell-independent env -u cleanup. It does not contain a fish/sh branch. Keep this comment to one accurate line.

Proposed fix

-    // Why identical to the fish case: this text is COPIED, so it may be pasted
-    // into any shell — it carries its own fish/sh branch instead of guessing.
+    // Copied commands use shell-independent `env -u` cleanup.

📝 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

	it('emits the same self-contained teardown under an sh-family login shell', () => {
	// Why identical to the fish case: this text is COPIED, so it may be pasted
	// into any shell — it carries its own fish/sh branch instead of guessing.
	it('emits the same self-contained teardown under an sh-family login shell', () => {
	// Copied commands use shell-independent `env -u` cleanup.

Source: Coding guidelines

[coderabbitai]

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Locate live-shell/live-fish test files and any references to the cited filename.
fd -H -t f -e ts 'live-fish|live-shell' | sort
rg -n 'fish-startup-arg-quoting|startup-shell-portability' -g '!node_modules'

Repository: stablyai/orca

Length of output: 255

---

Update the live-test reference

The referenced fish-startup-arg-quoting.live-fish.test.ts file does not exist. Use src/shared/startup-shell-portability.live-shell.test.ts instead.

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 043e6146-b8c5-4188-a0db-25b5fccfa305

📥 Commits

Reviewing files that changed from the base of the PR and between 7f18b90 and 3f248dd.

📒 Files selected for processing (9)

• src/main/fish-history-session.test.ts
• src/main/fish-history-session.ts
• src/main/terminal-history-gc.ts
• src/main/terminal-history.test.ts
• src/main/window/history-gc-profile-worktree-ids.test.ts
• src/main/window/history-gc-profile-worktree-ids.ts
• src/main/window/history-gc-worktree-ids.test.ts
• src/main/window/history-gc-worktree-ids.ts
• src/relay/terminal-history.ts

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

• src/relay/terminal-history.ts

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

[coderabbitai]

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Drain WSL tombstones before this return.

This return occurs before listWslHistoryRoots() and its schedulePendingHistoryTreeRemovals(distroRoot) calls. An empty live set therefore retries main-root tombstones only. WSL .pending-delete entries can remain until a future GC has a nonempty live set.

Enumerate WSL roots and schedule pending removals before this guard. Keep WSL scanning and orphan deletion after the guard.

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 132011fc-48ef-4af1-b5b9-62b3b1d08e7b

📥 Commits

Reviewing files that changed from the base of the PR and between 3f248dd and 9333bb0.

📒 Files selected for processing (2)

• src/shared/ai-vault-resume-command.test.ts
• 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; 2 remain after this review.

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use a cmd-valid resumeCommand fixture.

cmd.exe does not use single quotes for argument quoting. The expected command passes literal 'resume' and 'sid' arguments. This test does not validate the cmd branch with a command that can execute.

Proposed fix

       buildAiVaultResumeShellCommand({
         ...base,
+        resumeCommand: 'codex resume "sid"',
         cwd: 'C:\\repo',
         platform: 'win32',
         shell: 'cmd'
       })
     ).toBe(
-      'set "CODEX_HOME=" & set "ORCA_CODEX_HOME=" & cd /d "C:\\repo" && codex \'resume\' \'sid\''
+      'set "CODEX_HOME=" & set "ORCA_CODEX_HOME=" & cd /d "C:\\repo" && codex resume "sid"'
     )

📝 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

	it('keeps the cmd clear ahead of the cd so a failed cd cannot launch the agent', () => {
	expect(
	buildAiVaultResumeShellCommand({
	...base,
	cwd: 'C:\\repo',
	platform: 'win32',
	shell: 'cmd'
	})
	).toBe(
	'set "CODEX_HOME=" & set "ORCA_CODEX_HOME=" & cd /d "C:\\repo" && codex \'resume\' \'sid\''
	)
	it('keeps the cmd clear ahead of the cd so a failed cd cannot launch the agent', () => {
	expect(
	buildAiVaultResumeShellCommand({
	...base,
	resumeCommand: 'codex resume "sid"',
	cwd: 'C:\\repo',
	platform: 'win32',
	shell: 'cmd'
	})
	).toBe(
	'set "CODEX_HOME=" & set "ORCA_CODEX_HOME=" & cd /d "C:\\repo" && codex resume "sid"'
	)