fix(fish): scope shell history per worktree and clear agent draft prefills by nwparker · Pull Request #13983 · stablyai/orca

nwparker · 2026-08-12T06:40:39Z

ELI5

If you use fish, three things quietly didn't work. Every workspace shared one command history (arrow-up showed another worktree's commands); when Orca pre-filled an agent draft it tried to clean up with unset, which fish doesn't have, so fish printed Unknown command: unset and the variable stayed set; and — this is the one the first two revisions of this PR created — anything Orca quoted for fish was quoted with sh rules, which fish does not use. A prompt containing \d+ or \\server\share arrived at the agent with half its backslashes gone, and a prompt ending in a backslash killed the launch outright with a syntax error.

What Changed

0. fish quoting — the part that made the earlier revision a regression

Read this first: as originally written, this PR made fish users worse off. It was the first change to thread the fish dialect through to real callers, and quoteStartupArg had no fish branch, so every prompt and CLI argument was quoted with sh rules and handed to fish.

That is not a cosmetic mismatch. POSIX sh single quotes are fully literal; fish's are not — inside '…', \\ and \' are escapes, and a trailing backslash before the closing quote is a hard syntax error. Measured on fish 4.7.1:

sh   -c "printf '%s\n' 'a\\b'"    ->  a\\b     (literal)
fish -c "printf '%s\n' 'a\\b'"    ->  a\b      (collapses)
fish -c "printf '%s\n' 'ends\'"   ->  fish: Unexpected end of string, quotes are not balanced

So a composer prompt use \d+ and path \\server\share was silently mangled, and a prompt ending in \ failed to launch. Fixed:

quoteStartupArg gains a fish branch: ' + s.replace(/\\/g,'\\\\').replace(/'/g, "\\'") + '. Verified to round-trip through real fish for regex backslashes, UNC paths, drive paths, apostrophes, trailing/lone backslashes, mixed quotes, newlines, $-expansions, glob characters, backticks, parens, operators, comments, tabs and non-BMP unicode.
New src/shared/fish-command-tokenizer.ts. tokenizeStartupCommand(value, 'fish') was routing to the sh tokenizer, which is wrong in the same way and in one more: fish decodes the full C escape set unquoted (\n is a newline, not n; \x41/\101/A/\cA all decode), and inside double quotes only \\, \$, \" and a line continuation are escapes. Since callers tokenize a user's CLI-args string and then re-quote it, a wrong tokenizer corrupts the value even with correct quoting. The new tokenizer also flags fish-specific live syntax the span splice cannot model — (…) command substitution, {a,b} brace expansion, and globs (an unmatched wildcard is a hard error in fish, not a literal).
agent-resume-argv-drop: RESUME_ARGV_SHELLS did not include fish, so a fish-quoted resume suffix would not match and main would report unrecognized and refuse the launch. Added.
agent-resume-launch-command: the NAME=value cmd prefix skip was gated on shell === 'posix'; fish supports that form (3.1+), so it now uses isPosixStartupShell.
ai-vault-resume-command: the non-Windows cd '<cwd>' && CODEX_HOME='<x>' … prefix quoted with sh rules even when the live shell was known to be fish. It now quotes for the live shell. (NAME=value cmd itself parses in fish — verified — so only the quoting was wrong.)
hermes-startup-query: the outer sh -c <arg> was quoted with a hardcoded 'posix' even though that quoting is parsed by the shell that types the line. Now quoted for the real dialect. Honest scoping: today's payload is a run of \0NNN octal escapes and never contains \\ or a trailing \, so it happened to survive sh quoting in fish; this removes the dependence on that accident rather than fixing an observed break.
isPosixStartupShell's doc comment claimed fish "shares POSIX quoting/word rules". That claim is what produced this bug, so it is replaced with what the predicate actually means (sh-family grammar: NAME=value prefixes, sh -c, ; chaining) and an explicit warning never to hardcode 'posix' behind it.

Audited and left alone: commandSeparator (; is correct in fish), buildShellCommandFromArgv (delegates to quoteStartupArg), resolveStartupShell (never defaults to fish), normalizeHermesArgv's env prefix (correct for fish), and the inner buildShellCommandFromArgv(argv, 'posix') in the hermes script body (genuinely parsed by sh).

1. Worktree-scoped shell history now works for fish

historyFilename returned null for fish, so injectHistoryEnv did nothing and every fish worktree shared the global history. The bash/zsh mechanism (a per-worktree directory + HISTFILE) cannot transfer, because fish ignores HISTFILE entirely: history lives at $XDG_DATA_HOME/fish/${fish_history}_history, and the only isolation knob is the session name.

New src/main/fish-history-session.ts derives a stable, filesystem-safe session name (orca_<first 16 hex of the same worktree hash bash/zsh already use>) and resolves the file path fish will write.
injectHistoryEnv exports fish_history=orca_<hash> into the spawn env. fish imports environment variables as global variables at startup, so this needs no --init-command — verified against fish 4.7.1, which also means attribution-only fish panes (no -C) are covered.
Check-before-set is preserved: a caller-supplied fish_history wins, exactly as a caller-supplied HISTFILE does.

XDG_DATA_HOME is deliberately not touched — redirecting it would move every other tool's data for that session. The tradeoff is that the file lands in the user's fish data dir rather than Orca's history root, so it can't ride the existing tree tombstone. Instead the session is recorded in that worktree's meta.json, and scheduleWorktreeHistoryTreeDeletion removes the out-of-tree file first — one hook covering both the explicit worktree delete and the startup GC sweep.

Deletion now resolves that path from the right environment. The first revision resolved it from the main process env, but the file is written by the PTY's fish, and the two disagree whenever Orca was launched with a different XDG_DATA_HOME/HOME than the shells it spawns — a permanent one-file-per-deleted-worktree leak. injectHistoryEnv now records fishHistoryPath resolved from the spawn env in meta.json, and deletion tries that first with the main-process guess kept as a fallback for meta files written before this change. The recorded path is only trusted if it is absolute and still basenames to <session>_history, so a tampered meta.json cannot steer rmSync.

One case remains genuinely unresolvable from Node: a set -gx XDG_DATA_HOME … that lives inside the user's config.fish is applied after we have handed off, so neither env can see it. Rather than dropping that silently, deletion now warns explicitly, naming the session and the paths it tried.

updateHistFileForFallback → updateHistoryEnvForFallback: on a fish→zsh/bash fallback spawn it drops the injected fish_history and points HISTFILE at the worktree directory. It only ever undoes what this spawn injected, so a user-supplied value survives.

2. The fish draft-prefill teardown is now reachable

clearEnvCommand already had a correct fish branch (set -e VAR), but no POSIX caller ever threaded a fish shell into buildAgentDraftLaunchPlan — selectedRepoStartupShell came from resolveLocalWindowsAgentStartupShell, which returns undefined off win32. So a fish user got pi; unset ORCA_PI_PREFILL.

New src/shared/local-agent-startup-shell.ts extends the Windows-only resolver to POSIX, reusing the pattern and the hard-won guard from #13904: the local login shell may pick the dialect only when it is provably the shell that parses the line. It refuses when the target is remote, when the execution host is ssh/runtime, or when the target platform differs from this machine's (which is how WSL is caught). Everything else stays posix.

Two thin wrappers supply the login shell, because the source differs by process:

src/renderer/src/lib/client-agent-startup-shell.ts — uses getClientLoginShell() + CLIENT_PLATFORM, and requires an explicit executionHostKind so no call site can forget that a locally-listed workspace may execute elsewhere.
src/main/host-agent-startup-shell.ts — the renderer helper doesn't exist in main, and on a remote Orca host it's that host's $SHELL that runs the command, not the connected client's. Uses process.platform / process.env.SHELL. Main's launch scopes carry only repo/folder + connectionId (no runtime dimension), so non-remote + same platform is sufficient there.

Every resolveLocalWindowsAgentStartupShell call site moved to one of the two wrappers, so all draft-prefill entry points (composer, folder-workspace composer, source-control action, work-item direct launch, new-tab/background launch, and both main-process paths) are covered rather than just the one in the report. launch-work-item-direct never threaded a shell at all and now does.

Out of scope on purpose: no fish Prime Agent command wrapper (#13875 was closed, #13906 removed that interception).

Why

All three are silent failures. Shared history is a daily papercut for anyone running several 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 in that shell inherits a stale draft.

The quoting one is the reason this PR is bigger than the report. Threading a new dialect into a code path is only safe if every dialect-dependent function actually branches on it, and isPosixStartupShell('fish') === true is exactly the shape of a function that looks like it does and doesn't. Without the fix, this PR would have taken fish users from "one wrong unset line" to "your regex and Windows-path prompts arrive corrupted, and some prompts don't launch at all" — a strictly worse trade.

The dialect guard is the other subtle part and is why this isn't just "read $SHELL". A locally-scanned session can target a remote worktree, and set -e ORCA_PI_PREFILL in bash does not clear a variable — it enables errexit, which silently changes the semantics of the rest of that shell. So the resolver fails closed to posix unless the local shell is provably the one parsing the line.

Linked Issue

Fixes #

Visual Proof

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

Testing

Three real-fish suites (itWithFish guard + src/shared/fish-binary-requirement.ts), all registered in the shell contracts run list and the shard --exclude list in .github/workflows/pr.yml:

src/shared/fish-startup-arg-quoting.live-fish.test.ts (new) — the shell is the oracle, not a hand-written expectation. Each case is written to a script file, run by a config-free fish in a pinned temp HOME/XDG_*, and echoed back through string join0 (a NUL separator no test input can forge). Covers 15 adversarial arguments individually and all of them in one argv; the fish tokenizer against fish's own splitting for five command lines including \x41/\101/\cA; a full buildAgentStartupPlan composer prompt delivered to a real child process's argv; and the hermes sh -c wrapper end to end. Two cases assert the old behavior was broken: sh quoting of \\server\share comes back as \server\share, and sh quoting of a trailing backslash throws.
Vacuity-checked: deleting the fish branch from quoteStartupArg fails 8 of these, including "delivers a composer prompt to the agent argv intact".
src/main/terminal-history-fish-session.node-pty.test.ts — spawns real interactive fish in a fully pinned temp HOME/XDG_* (no ambient env reaches the shell), types a command, exits, and asserts the record landed in $XDG_DATA_HOME/fish/orca_<hash>_history in fish's YAML-ish - cmd: / when: format, and that the default shared fish_history file was never created. Vacuity-checked: renaming the injected env var makes it fail with ENOENT on the scoped path.
src/shared/fish-draft-prefill-teardown.node-pty.test.ts — proves the difference is shell-observable rather than just a string: in real fish, the posix spelling prints Unknown command and set -q still reports the variable present, while the fish spelling clears it.

Unit coverage: src/shared/fish-command-tokenizer.test.ts (single/double-quote escapes, the unquoted escape set, unclosed-quote errors, divergence flags, and the sh-vs-fish disagreement); the fish startup shell dialect block in tui-agent-startup-shell.test.ts, whose old "shares POSIX quoting" assertion is replaced with the real rule plus a quote→tokenize round trip; injectHistoryEnv fish session + per-worktree distinctness + check-before-set + the new spawn-env fishHistoryPath record; deletion preferring the recorded path, refusing a recorded path that doesn't name its own session file, and warning when nothing is found; updateHistoryEnvForFallback in both directions; resolveFishHistoryFilePath including its refusal of a name it did not mint; and every guard branch of resolveLocalAgentStartupShell / resolveHostAgentStartupShell. Every test that reads ambient env (XDG_DATA_HOME, SHELL) pins and restores it.

Observed locally on macOS 15 (fish 4.7.1, /opt/homebrew/bin/fish):

npx tsc --noEmit -p config/tsconfig.node.json — clean
npx tsc --noEmit -p config/tsconfig.tc.web.json — clean
npx oxlint — exit 0, no errors (only pre-existing warnings in unrelated files)
npx vitest run --config config/vitest.config.ts src/shared/ — 4526 passed, 18 skipped
npx vitest run --config config/vitest.config.ts src/main/ — 20802 passed, 87 skipped
npx vitest run --config config/vitest.config.ts src/renderer/ — 22322 passed, 4 skipped
node config/scripts/check-max-lines-ratchet.mjs — OK, no new bypasses
node config/scripts/check-reliability-gates.mjs — 74 gates pass

Not exercised on a real machine: Windows and WSL (unchanged code paths — WSL's inner shell is resolved as bash before history injection, and resolveLocalAgentStartupShell delegates win32 to the existing Windows resolver untouched), and SSH hosts (the new guard's whole job is to not apply a local dialect there).

I manually tested these changes locally
Automated tests added/updated, or explained why not below

AI Disclosure

Claude Opus.

Review

Security — the only new filesystem write outside Orca's own dirs is fish's own history file. Deletion is guarded twice: /^orca_[0-9a-f]{1,64}$/ on the session name, and a recorded path must be absolute and basename to <session>_history, so a tampered meta.json cannot steer rmSync at an arbitrary path.
Cross-platform — fish injection is env-only and inert on Windows; WSL terminals resolve their inner shell to bash before history injection, so they are untouched; the Windows startup-shell families and both Windows tokenizer/quoter branches route through the existing code unchanged.
Remote SSH — no wire change (both the spawn env and the dialect decision are resolved on the machine that runs the shell). The dialect guard explicitly refuses to apply a local shell's syntax to a remote host.
Mobile — mobile clients drive the main-process launch paths, which use the host's own $SHELL, so a phone on iOS cannot push an iOS dialect onto a macOS/Linux workspace.
Backwards compatibility — posix remains the default for every shell that isn't fish; the sh quoting and tokenizing paths are byte-for-byte unchanged, and existing bash/zsh HISTFILE behavior and the settings kill switch are untouched. fishHistoryPath is an additive meta.json field; a meta file without it still deletes through the main-env fallback.
Performance — one extra meta.json read on worktree-history deletion.

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 (or N/A)
pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

Made with Orca 🐋

coderabbitai · 2026-08-12T06:47:04Z

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

📝 Walkthrough

Walkthrough

The PR adds worktree-scoped fish history sessions with validated paths, metadata persistence, fallback cleanup, and file deletion. It adds interactive fish integration coverage and updates shell contract test routing. It also adds host- and client-aware startup-shell resolution based on platform, login shell, remote state, terminal settings, and execution-host kind. Agent launch paths now use the resolved shell for runtime, workspace, background, resume, direct, and source-control launches.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 31.96% 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 identifies the primary fish history and draft-prefill fixes, although it omits the related quoting and tokenization changes.
Description check	✅ Passed	The description covers the required sections with detailed changes, rationale, testing, review considerations, and an explicit N/A for visual proof.
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

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

coderabbitai

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

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

833-867: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Handle fish as the WSL inner shell.

WSL selects the user’s login shell from getent passwd, but effectiveShellPath always passes bash to injectHistoryEnv. If that shell is fish, the code sets HISTFILE, which fish ignores, and does not set fish_history; history remains shared. Pass the actual WSL shell to history injection or disable scoped history when it cannot be determined.

🧹 Nitpick comments (2)

src/main/fish-history-session.ts (1)

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

Reduce the long narrative comments.

Keep only the behavior that the next maintainer cannot infer from the code or test name.

src/main/fish-history-session.ts#L5-L20: replace the module-length fish behavior summary with a short contract comment.

src/main/terminal-history-fish-session.node-pty.test.ts#L1-L15: keep one short comment for the interactive PTY and probe requirement.

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

Source: Coding guidelines
src/main/terminal-history.ts (1)
217-217: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use path.posix.join for the fallback history path.

Line 217 builds a Linux-visible WSL path with string concatenation. Use path.posix.join(injected.historyDir, newFilename).
Proposed fix
-import { join, basename } from 'node:path'
+import { join, basename, posix } from 'node:path'
...
-  spawnEnv.HISTFILE = `${injected.historyDir}/${newFilename}`
+  spawnEnv.HISTFILE = posix.join(injected.historyDir, newFilename)
As per coding guidelines, “Use path.join or Electron/Node path utilities for file paths; never assume / or \.”

Source: Coding guidelines

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 690e98a0-b600-4d9f-8b69-ae750de34091

📥 Commits

Reviewing files that changed from the base of the PR and between fd2afc1 and 8558987.

📒 Files selected for processing (30)

.github/workflows/pr.yml
src/main/fish-history-session.ts
src/main/host-agent-startup-shell.test.ts
src/main/host-agent-startup-shell.ts
src/main/providers/local-pty-provider.ts
src/main/runtime/orca-runtime.ts
src/main/terminal-history-deletion.ts
src/main/terminal-history-fish-session.node-pty.test.ts
src/main/terminal-history.test.ts
src/main/terminal-history.ts
src/renderer/src/components/right-sidebar/buildSourceControlAgentDeliveryPlan.ts
src/renderer/src/components/right-sidebar/useSourceControlAgentActionStart.ts
src/renderer/src/components/sidebar/folder-workspace-composer-submit.ts
src/renderer/src/hooks/useComposerState.ts
src/renderer/src/lib/agent-background-session-launch-host.ts
src/renderer/src/lib/agent-resume-launch-target.test.ts
src/renderer/src/lib/agent-resume-launch-target.ts
src/renderer/src/lib/client-agent-startup-shell.ts
src/renderer/src/lib/launch-agent-background-session.ts
src/renderer/src/lib/launch-agent-in-new-tab-cwd.test.ts
src/renderer/src/lib/launch-agent-in-new-tab.ts
src/renderer/src/lib/launch-work-item-direct-agent.ts
src/renderer/src/lib/launch-work-item-direct.test.ts
src/renderer/src/lib/launch-work-item-direct.ts
src/renderer/src/lib/source-control-agent-action-plan.ts
src/shared/fish-draft-prefill-teardown.node-pty.test.ts
src/shared/local-agent-startup-shell.test.ts
src/shared/local-agent-startup-shell.ts
src/shared/tui-agent-startup-shell.test.ts
src/shared/types.ts

💤 Files with no reviewable changes (1)

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

coderabbitai · 2026-08-12T06:47:07Z

+  if (args.platform === 'win32' || args.isRemote) {
+    return resolveLocalWindowsAgentStartupShell(args)
+  }
+  if (args.executionHostKind === 'ssh' || args.executionHostKind === 'runtime') {
+    return undefined
+  }


🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Move the ssh/runtime guard before the Windows branch.

If platform === 'win32', isRemote === false, and executionHostKind is 'ssh' or 'runtime', the function returns at Line 30 and never evaluates the fail-closed guard at Line 32. resolveLocalWindowsAgentStartupShell then returns a dialect derived from terminalWindowsShell, which describes the local host shell rather than the execution host. That contradicts the documented rule that the dialect applies only when the local shell provably parses the line.

The test suite does not cover this combination, so the gap is invisible today. Add a case with platform: 'win32' and executionHostKind: 'ssh' in src/shared/local-agent-startup-shell.test.ts.

🐛 Proposed fix for the guard order

+ if (args.executionHostKind === 'ssh' || args.executionHostKind === 'runtime') { + return undefined + } if (args.platform === 'win32' || args.isRemote) { return resolveLocalWindowsAgentStartupShell(args) } - if (args.executionHostKind === 'ssh' || args.executionHostKind === 'runtime') { - return undefined - }

📝 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

if (args.platform === 'win32' || args.isRemote) {

return resolveLocalWindowsAgentStartupShell(args)

}

if (args.executionHostKind === 'ssh' || args.executionHostKind === 'runtime') {

return undefined

}

if (args.executionHostKind === 'ssh' || args.executionHostKind === 'runtime') {

return undefined

}

if (args.platform === 'win32' || args.isRemote) {

return resolveLocalWindowsAgentStartupShell(args)

}

coderabbitai

Actionable comments posted: 2

🧹 Nitpick comments (3)

src/main/fish-history-session.ts (1)

61-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reduce the implementation comment.

Keep the non-obvious fallback behavior. Reduce this comment to one short description of the spawn-time path preference and the config.fish limitation.

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

Source: Coding guidelines

src/shared/fish-startup-arg-quoting.live-fish.test.ts (2)

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

Shorten the file header.

Keep one short comment that states why this suite runs real fish. The detailed shell behavior belongs in the focused test names and assertions.

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

Source: Coding guidelines

184-229: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert that agentArgs reaches Hermes.

The test passes agentArgs: '--yolo', but Line 229 only verifies the query argument. A regression that drops agent arguments will still pass this test. Assert that the captured argv contains --yolo.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7a4e9ba9-6553-4b01-93ae-325022c0c9b1

📥 Commits

Reviewing files that changed from the base of the PR and between 8558987 and a4e421e.

📒 Files selected for processing (14)

.github/workflows/pr.yml
src/main/fish-history-session.ts
src/main/terminal-history-deletion.ts
src/main/terminal-history.test.ts
src/main/terminal-history.ts
src/shared/agent-resume-argv-drop.ts
src/shared/agent-resume-launch-command.ts
src/shared/ai-vault-resume-command.ts
src/shared/fish-command-tokenizer.test.ts
src/shared/fish-command-tokenizer.ts
src/shared/fish-startup-arg-quoting.live-fish.test.ts
src/shared/hermes-startup-query.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 (4)

.github/workflows/pr.yml
src/main/terminal-history.test.ts
src/main/terminal-history-deletion.ts
src/main/terminal-history.ts

coderabbitai · 2026-08-12T07:28:35Z

+  writeFileSync(scriptPath, `string join0 -- ${commandLine} > ${outPath}\nexit 0\n`)
+  execFileSync(FISH.path as string, [scriptPath], {


🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Pass outPath as a script argument.

Line 37 injects outPath into fish source without quoting. If the temporary directory contains whitespace or fish metacharacters, the redirection can parse incorrectly and fail the test.

Proposed fix

- writeFileSync(scriptPath, `string join0 -- ${commandLine} > ${outPath}\nexit 0\n`) - execFileSync(FISH.path as string, [scriptPath], { + writeFileSync(scriptPath, `string join0 -- ${commandLine} > $argv[1]\nexit 0\n`) + execFileSync(FISH.path as string, [scriptPath, outPath], {

📝 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

writeFileSync(scriptPath, `string join0 -- ${commandLine} > ${outPath}\nexit 0\n`)

execFileSync(FISH.path as string, [scriptPath], {

writeFileSync(scriptPath, `string join0 -- ${commandLine} > $argv[1]\nexit 0\n`)

execFileSync(FISH.path as string, [scriptPath, outPath], {

coderabbitai

🧹 Nitpick comments (1)

src/shared/fish-draft-prefill-teardown.node-pty.test.ts (1)

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

Shorten the test comments.

Keep only the non-obvious test invariant. Move detailed fish, Ubuntu, and implementation rationale to test names or documentation if it must remain.

src/shared/fish-draft-prefill-teardown.node-pty.test.ts#L9-L15: reduce the diagnostic-variance explanation to one concise invariant.

src/shared/fish-draft-prefill-teardown.node-pty.test.ts#L115-L120: reduce the regex and latency explanation to concise comments.

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

Source: Coding guidelines

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 29181d6e-e0cd-4e2a-96ba-befc11bb43a0

📥 Commits

Reviewing files that changed from the base of the PR and between a4e421e and 94cd699.

📒 Files selected for processing (3)

src/main/terminal-history-fish-session.node-pty.test.ts
src/shared/fish-draft-prefill-teardown.node-pty.test.ts
src/shared/fish-startup-arg-quoting.live-fish.test.ts

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

src/main/terminal-history-fish-session.node-pty.test.ts
src/shared/fish-startup-arg-quoting.live-fish.test.ts

coderabbitai · 2026-08-13T01:50:22Z

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

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 43410a43-6816-4f25-98db-e44b8e0d8ae9

📥 Commits

Reviewing files that changed from the base of the PR and between f1ee308 and f5aca4a.

📒 Files selected for processing (41)

.github/workflows/pr.yml
src/main/fish-history-session.ts
src/main/host-agent-startup-shell.test.ts
src/main/host-agent-startup-shell.ts
src/main/providers/local-pty-provider.ts
src/main/runtime/orca-runtime.ts
src/main/terminal-history-deletion.ts
src/main/terminal-history-fish-session.node-pty.test.ts
src/main/terminal-history.test.ts
src/main/terminal-history.ts
src/renderer/src/components/right-sidebar/buildSourceControlAgentDeliveryPlan.ts
src/renderer/src/components/right-sidebar/useSourceControlAgentActionStart.ts
src/renderer/src/components/sidebar/folder-workspace-composer-submit.ts
src/renderer/src/hooks/useComposerState.ts
src/renderer/src/lib/agent-background-session-launch-host.ts
src/renderer/src/lib/agent-resume-launch-target.test.ts
src/renderer/src/lib/agent-resume-launch-target.ts
src/renderer/src/lib/ai-vault-resume-command.ts
src/renderer/src/lib/ai-vault-resume-shell.test.ts
src/renderer/src/lib/client-agent-startup-shell.ts
src/renderer/src/lib/fish-ai-vault-resume-env.live-fish.test.ts
src/renderer/src/lib/launch-agent-background-session.ts
src/renderer/src/lib/launch-agent-in-new-tab-cwd.test.ts
src/renderer/src/lib/launch-agent-in-new-tab.ts
src/renderer/src/lib/launch-work-item-direct-agent.ts
src/renderer/src/lib/launch-work-item-direct.test.ts
src/renderer/src/lib/launch-work-item-direct.ts
src/renderer/src/lib/source-control-agent-action-plan.ts
src/shared/agent-resume-argv-drop.ts
src/shared/agent-resume-launch-command.ts
src/shared/ai-vault-resume-command.ts
src/shared/fish-command-tokenizer.test.ts
src/shared/fish-command-tokenizer.ts
src/shared/fish-draft-prefill-teardown.node-pty.test.ts
src/shared/fish-startup-arg-quoting.live-fish.test.ts
src/shared/hermes-startup-query.ts
src/shared/local-agent-startup-shell.test.ts
src/shared/local-agent-startup-shell.ts
src/shared/tui-agent-startup-shell.test.ts
src/shared/tui-agent-startup-shell.ts
src/shared/types.ts

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

src/renderer/src/lib/launch-agent-in-new-tab-cwd.test.ts
src/shared/types.ts
src/renderer/src/lib/agent-background-session-launch-host.ts
src/shared/agent-resume-argv-drop.ts
src/renderer/src/components/sidebar/folder-workspace-composer-submit.ts
src/renderer/src/components/right-sidebar/useSourceControlAgentActionStart.ts
src/main/host-agent-startup-shell.test.ts
src/renderer/src/lib/launch-agent-in-new-tab.ts
src/shared/tui-agent-startup-shell.ts
src/main/host-agent-startup-shell.ts
src/renderer/src/hooks/useComposerState.ts
.github/workflows/pr.yml
src/renderer/src/lib/launch-work-item-direct.ts
src/shared/agent-resume-launch-command.ts
src/renderer/src/lib/source-control-agent-action-plan.ts
src/main/runtime/orca-runtime.ts
src/renderer/src/lib/agent-resume-launch-target.ts
src/shared/hermes-startup-query.ts
src/renderer/src/lib/agent-resume-launch-target.test.ts
src/main/terminal-history.test.ts
src/main/providers/local-pty-provider.ts
src/shared/local-agent-startup-shell.ts
src/renderer/src/lib/client-agent-startup-shell.ts
src/shared/ai-vault-resume-command.ts
src/main/fish-history-session.ts
src/shared/local-agent-startup-shell.test.ts
src/main/terminal-history.ts
src/shared/fish-command-tokenizer.test.ts
src/main/terminal-history-fish-session.node-pty.test.ts
src/renderer/src/components/right-sidebar/buildSourceControlAgentDeliveryPlan.ts
src/renderer/src/lib/launch-work-item-direct.test.ts
src/renderer/src/lib/launch-agent-background-session.ts
src/renderer/src/lib/launch-work-item-direct-agent.ts
src/main/terminal-history-deletion.ts
src/shared/tui-agent-startup-shell.test.ts

coderabbitai · 2026-08-13T01:55:41Z

+      let rendered = ''
+      term.onData((chunk) => {
+        rendered += chunk
+        if (chunk.includes('\x1b[0c') || chunk.includes('\x1b[c')) {
+          term.write('\x1b[?62;4;6;22c')
+        }
+        if (chunk.includes('\x1b[6n')) {
+          term.write('\x1b[1;1R')
+        }
+        if (chunk.includes('\x1b]10;?') || chunk.includes('\x1b]11;?')) {
+          term.write('\x1b]11;rgb:1e1e/1e1e/1e1e\x1b\\')
+        }
+      })


🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Buffer terminal capability queries before replying.

node-pty can split \x1b[c, \x1b[6n, and OSC queries across onData callbacks. This handler checks only one chunk. A split query receives no reply, so fish can wait for its terminal probe and the test can time out.

Keep a bounded carry window. Reply only when a complete query ends in newly received data. Do not reply again when a complete query exists only in the carry window.

Based on learnings: retained terminal-query bytes must produce one reply for tail-aligned, split, and distinct queries. [retrieved_learnings]

Source: Learnings

Two fish ergonomics gaps. Worktree-scoped history did nothing for fish: `historyFilename` returned null, so every fish worktree shared one global history. fish ignores HISTFILE entirely — history lives at `$XDG_DATA_HOME/fish/${fish_history}_history`, so the only isolation knob is the session NAME. fish imports env vars as global variables at startup, so exporting `fish_history=orca_<hash>` in the spawn env picks the session with no `--init-command`. Consequence: the file sits in the user's fish data dir rather than Orca's history root, so the session name is recorded in that worktree's meta.json and the tombstone path (explicit delete and GC alike) removes the out-of-tree file first. The fish draft-prefill teardown was unreachable: `clearEnvCommand`'s fish branch existed, but no POSIX caller ever threaded a fish shell in, so fish users still got `pi; unset ORCA_PI_PREFILL` and "Unknown command: unset". The client's login-shell dialect now flows through on POSIX, behind the guard #13904 established — the local shell may pick the dialect only when it is provably the one parsing the line, since `set -e` in remote bash enables errexit instead of clearing anything. Main-process launch paths use this process's own `$SHELL`, because the renderer helper does not exist there and on a remote host it is that host's shell that runs the command.

Threading the `fish` dialect to real callers made quoting a correctness problem: `quoteStartupArg` had no fish branch, so prompts and CLI args were quoted with sh rules and handed to fish. fish single quotes are not literal — `\\` and `\'` are escapes inside them — so every backslash silently halved and a trailing backslash was a hard syntax error that killed the launch. - `quoteStartupArg` gains a fish branch escaping `\` and `'`. - New `fish-command-tokenizer.ts`: fish's single/double-quote and unquoted escape rules, plus divergence flags for command substitution, brace expansion and globs (an unmatched wildcard is a hard error in fish). - `agent-resume-argv-drop` adds fish to the quoting candidates, so a fish-quoted resume suffix is recognised instead of refusing the launch. - `agent-resume-launch-command` skips `NAME=value` prefixes for fish too. - `ai-vault-resume-command` quotes its cd/CODEX_HOME prefix for the live shell. - `hermes-startup-query` quotes the `sh -c` wrapper for the real dialect. - `isPosixStartupShell`'s doc no longer claims fish shares sh quoting. fish history deletion now uses the path resolved from the PTY's spawn env (recorded in meta.json) rather than the main process env, and warns explicitly when no file is found instead of leaking one per deleted worktree in silence.

… text Co-authored-by: Orca <help@stably.ai>

coderabbitai · 2026-08-13T07:29:04Z

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

🧹 Nitpick comments (16)

src/renderer/src/lib/source-control-agent-action-plan.ts (1)

42-50: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add a remote Windows regression test for shell resolution. Production callers propagate executionHostKind; only the source-control plan tests lack remote Windows coverage.
src/shared/fish-startup-arg-quoting.live-fish.test.ts (1)
165-170: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Replace the Node binary copies with shebang stubs.

copyFileSync(process.execPath, …) copies the full Node binary twice per suite run. A #!/usr/bin/env node stub named claude or hermes satisfies the same requirement at a fraction of the I/O cost, and the sibling suite src/renderer/src/lib/fish-ai-vault-resume-env.live-fish.test.ts already uses that pattern.
♻️ Proposed refactor for the `claude` stub
-    copyFileSync(process.execPath, claudePath)
-    chmodSync(claudePath, 0o755)
     writeFileSync(
-      path.join(root, 'capture.js'),
-      `require('node:fs').writeFileSync(${JSON.stringify(capturePath)}, JSON.stringify(process.argv.slice(2)))`
+      claudePath,
+      `#!/usr/bin/env node\nrequire('node:fs').writeFileSync(${JSON.stringify(capturePath)}, JSON.stringify(process.argv.slice(2)))\n`
     )
+    chmodSync(claudePath, 0o755)
Drop the capture.js argument from cmdOverrides when you apply this.
Also applies to: 205-210
src/main/fish-history-session.ts (1)

39-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Share one canonical-path predicate between the two fish-history modules.

canonicalPath here duplicates canonicalPath and safeSession in src/main/fish-history-location-attestation.ts (Lines 35-44). MAX_PATH_CANDIDATES is also duplicated. These predicates gate destructive file operations. If one copy changes, the attestation writer and the deletion reader can accept different path sets.

Export the predicate from one module and import it in the other.

src/main/fish-history-location-attestation.ts (1)

248-256: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Both fish-history records are written non-atomically. writeFileSync replaces each file in place, so an interrupted write leaves truncated JSON. Both readers fail closed and return no data, which means the fish history file in the user's data directory can no longer be located or removed.

src/main/fish-history-location-attestation.ts#L248-L256: write fish-history-locations.json to a temporary file in the same directory, then rename it.

src/main/terminal-history.ts#L126-L137: write meta.json the same way, so readHistoryMeta cannot lose fishSession after an interrupted write.

src/main/fish-history-session.test.ts (1)

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

Add a test that proves deleteFishHistoryFile clears an attested file.

Every test in this suite asserts a rejection. None asserts the success path: attest a location, write history content, call deleteFishHistoryFile, and expect the file to be empty.

Without that test, a platform-specific failure inside clearFile cannot be detected. clearFile selects a different open path per platform (/proc/self/fd on Linux, /dev/fd elsewhere), so the success path needs coverage on each supported POSIX platform.

Do you want me to generate this test?

src/main/wsl-fish-history-cleanup.ts (1)

77-79: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

A full queue rejects and blocks the history tree removal.

When queuedCleanups reaches MAX_QUEUED_CLEANUPS, this returns a rejected promise. In src/main/terminal-history-deletion.ts Line 112, that rejection prevents removeHostTree(dir) and consumes one of the two retry attempts. MAX_PENDING_HISTORY_TREE_REMOVALS is also 64, so a startup drain can fill this queue and defer local tree removal for reasons unrelated to the tree.

Consider resolving instead of rejecting when the queue is full, and letting the next launch retry the fish cleanup. The tombstone and its meta.json still record the session.

src/main/terminal-history-deletion.ts (1)

23-35: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Export and reuse the WSL history root name. Export HISTORY_DIR_NAME_WSL from src/main/terminal-history-paths.ts and use it in wslDistroForHistoryRoot. Keep PENDING_DELETE_DIR_NAME as one path segment because historyRootForTombstone depends on the current tombstone layout.

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

878-881: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

Capture the narrowed history result before the callback.

spawnShellWithFallback uses /bin/zsh, /bin/bash, and /bin/sh as Unix fallbacks, so fish is not selected by this path. Replace the closure’s HistoryInjectionResult assertion with a narrowed local constant.
src/relay/terminal-history.ts (1)
87-98: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

One failed unlink stops the remaining history file deletion.

The inner throw propagates to the outer catch, so the loop ends. If bash_history removal fails with a non-ENOENT error, zsh_history stays on the remote host. Scope the failure handling per file so both entries are always attempted.
♻️ Proposed per-file failure handling
     for (const filename of ['bash_history', 'zsh_history']) {
       const path = join(HISTORY_ROOT, `${hashWorktreeId(worktreeId)}-${filename}`)
       try {
         if (!lstatSync(path).isSymbolicLink()) {
           unlinkSync(path)
         }
       } catch (error) {
         if ((error as NodeJS.ErrnoException).code !== 'ENOENT') {
-          throw error
+          console.warn(
+            `[pty:history] Failed to delete relay shell history ${filename}: ${error instanceof Error ? error.message : String(error)}`
+          )
         }
       }
     }
src/relay/pty-handler.ts (2)
870-876: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

pty.deleteWorktreeHistory reports success for a missing or non-string worktreeId.

The handler returns { ok: true } even when it performs no deletion. The host then treats the sweep as complete. Reject the invalid payload so a protocol mismatch is observable.
♻️ Proposed validation
     this.dispatcher.onRequest('pty.deleteWorktreeHistory', async (p) => {
-      if (typeof p.worktreeId === 'string') {
-        deleteRelayFishHistory(p.worktreeId)
-        deleteRelayHistory(p.worktreeId)
-      }
+      if (typeof p.worktreeId !== 'string' || p.worktreeId.length === 0) {
+        throw new Error('worktreeId required')
+      }
+      deleteRelayFishHistory(p.worktreeId)
+      deleteRelayHistory(p.worktreeId)
       return { ok: true }
     })
1545-1559: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Fish history injection is duplicated between spawn and revive.

Both blocks apply the same four conditions, the same fish_history session name, the same metadata record, and the same injectRelayHistoryEnv call. Extract one helper that takes spawnEnv, worktreeId, shell, and the isolation flag. This keeps the spawn and revive contracts from drifting.

Also applies to: 2133-2150
src/main/terminal-history-tombstone-retry.test.ts (1)
126-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert the batch size with MAX_PENDING_HISTORY_TREE_REMOVALS, not the literal 64.

The suite already imports the constant and uses it at Line 173. If the cap changes, these literals turn into a confusing failure instead of tracking the contract.
♻️ Proposed constant use
-    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)
-    expect(vi.getTimerCount()).toBe(64)
+    expect(vi.getTimerCount()).toBe(MAX_PENDING_HISTORY_TREE_REMOVALS)
Also applies to: 156-159
src/main/terminal-history.test.ts (1)

574-593: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Strengthen the bounded-cleanup assertion.

The attested design refuses raw metadata paths, so rmSyncMock is likely never called for a fish history path here. toBeLessThanOrEqual(MAX_RETAINED_FISH_HISTORY_PATHS + 1) then passes for any correct or incorrect bounded value. Assert the exact expected number of fish-path removals so a regression that starts trusting retained paths fails this test.
src/relay/fish-history-metadata.test.ts (1)
77-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add the positive deletion case.

This suite proves that deleteRelayFishHistory refuses an unattested inode, and that it removes the metadata file. No test proves that it removes the fish history file when the inode still matches the attestation. A regression that always refuses deletion would keep every remote fish history file and still pass this suite.
💚 Proposed additional test
it('removes an attested history file', () => {
  const metadataRoot = join(root, 'metadata')
  const historyFile = join(dataRoot, 'fish', `${session}_history`)
  mkdirSync(join(dataRoot, 'fish'), { recursive: true })
  writeFileSync(historyFile, 'attested')
  recordRelayFishHistoryPath(worktreeId, { XDG_DATA_HOME: dataRoot }, metadataRoot)

  deleteRelayFishHistory(worktreeId, metadataRoot)

  expect(existsSync(historyFile)).toBe(false)
  expect(existsSync(join(metadataRoot, `${hash}.json`))).toBe(false)
})
src/relay/pty-handler.test.ts (1)
741-747: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Mock ./terminal-history and assert the bash/zsh deletion too.

The test mocks only ./fish-history-metadata. deleteRelayHistory therefore runs for real and touches the developer's home directory under ~/.orca-remote/terminal-history. The test also does not prove that pty.deleteWorktreeHistory deletes bash and zsh history.
♻️ Proposed mock and assertion
+vi.mock('./terminal-history', () => ({
+  deleteRelayHistory: deleteRelayHistoryMock,
+  injectRelayHistoryEnv: injectRelayHistoryEnvMock
+}))
     expect(deleteRelayFishHistoryMock).toHaveBeenCalledWith('repo-1::/remote/wt')
+    expect(deleteRelayHistoryMock).toHaveBeenCalledWith('repo-1::/remote/wt')
src/relay/terminal-history.test.ts (1)

8-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

The suite writes into the real user home directory.

HISTORY_ROOT in src/relay/terminal-history.ts is a module-level constant derived from homedir(), so these tests create files under ~/.orca-remote/terminal-history on the machine that runs them. Consider an optional root parameter on injectRelayHistoryEnv and deleteRelayHistory, matching the root = metadataRoot() pattern already used in src/relay/fish-history-metadata.ts. The tests can then use a temporary directory.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7ee33c9b-d2c8-47f7-bc53-dbd172175358

📥 Commits

Reviewing files that changed from the base of the PR and between c61f296 and 3af2787.

📒 Files selected for processing (82)

.github/workflows/pr.yml
src/main/daemon/daemon-pty-adapter.test.ts
src/main/daemon/daemon-pty-adapter.ts
src/main/fish-history-location-attestation.ts
src/main/fish-history-session.test.ts
src/main/fish-history-session.ts
src/main/host-agent-startup-shell.test.ts
src/main/host-agent-startup-shell.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/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/terminal-history-attested-deletion.test.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/relay/fish-history-metadata.test.ts
src/relay/fish-history-metadata.ts
src/relay/pty-handler.test.ts
src/relay/pty-handler.ts
src/relay/terminal-history.test.ts
src/relay/terminal-history.ts
src/renderer/src/components/floating-terminal/FloatingTerminalWindowControls.tsx
src/renderer/src/components/right-sidebar/buildSourceControlAgentDeliveryPlan.ts
src/renderer/src/components/right-sidebar/useSourceControlAgentActionStart.ts
src/renderer/src/components/settings/CliSkillRuntimeSetup.tsx
src/renderer/src/components/sidebar/folder-workspace-composer-submit.ts
src/renderer/src/hooks/useComposerState.ts
src/renderer/src/lib/agent-background-session-launch-host.ts
src/renderer/src/lib/agent-resume-launch-target.test.ts
src/renderer/src/lib/agent-resume-launch-target.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/client-agent-startup-shell.ts
src/renderer/src/lib/fish-ai-vault-resume-env.live-fish.test.ts
src/renderer/src/lib/launch-agent-background-session.ts
src/renderer/src/lib/launch-agent-in-new-tab-cwd.test.ts
src/renderer/src/lib/launch-agent-in-new-tab.ts
src/renderer/src/lib/launch-work-item-direct-agent.ts
src/renderer/src/lib/launch-work-item-direct.test.ts
src/renderer/src/lib/launch-work-item-direct.ts
src/renderer/src/lib/onboarding-folder-agent-startup.ts
src/renderer/src/lib/project-skill-runtime.ts
src/renderer/src/lib/source-control-agent-action-plan.test.ts
src/renderer/src/lib/source-control-agent-action-plan.ts
src/renderer/src/lib/tui-agent-startup.test.ts
src/shared/agent-resume-argv-drop.ts
src/shared/agent-resume-launch-command.test.ts
src/shared/agent-resume-launch-command.ts
src/shared/ai-vault-resume-command.ts
src/shared/fish-command-tokenizer.test.ts
src/shared/fish-command-tokenizer.ts
src/shared/fish-draft-prefill-teardown.node-pty.test.ts
src/shared/fish-startup-arg-quoting.live-fish.test.ts
src/shared/hermes-startup-query.ts
src/shared/local-agent-startup-shell.test.ts
src/shared/local-agent-startup-shell.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/tui-agent-startup.ts
src/shared/types.ts
src/shared/windows-terminal-shell.test.ts
src/shared/windows-terminal-shell.ts

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

src/renderer/src/lib/agent-resume-launch-target.ts
src/shared/agent-resume-argv-drop.ts
src/renderer/src/lib/launch-work-item-direct.ts
src/shared/ai-vault-resume-command.ts
src/renderer/src/lib/launch-agent-in-new-tab-cwd.test.ts
src/shared/agent-resume-launch-command.ts
src/renderer/src/hooks/useComposerState.ts
src/renderer/src/lib/launch-work-item-direct-agent.ts
src/main/host-agent-startup-shell.test.ts
src/renderer/src/lib/agent-background-session-launch-host.ts
src/renderer/src/lib/launch-agent-in-new-tab.ts
src/shared/fish-command-tokenizer.test.ts
src/renderer/src/lib/launch-work-item-direct.test.ts
src/renderer/src/lib/ai-vault-resume-shell.test.ts
src/renderer/src/components/sidebar/folder-workspace-composer-submit.ts
src/shared/local-agent-startup-shell.test.ts
src/shared/types.ts
src/shared/hermes-startup-query.ts
src/renderer/src/components/right-sidebar/useSourceControlAgentActionStart.ts
src/shared/tui-agent-startup-shell.test.ts
src/renderer/src/lib/launch-agent-background-session.ts
src/renderer/src/lib/client-agent-startup-shell.ts
.github/workflows/pr.yml
src/main/host-agent-startup-shell.ts
src/main/terminal-history-fish-session.node-pty.test.ts
src/renderer/src/components/right-sidebar/buildSourceControlAgentDeliveryPlan.ts
src/shared/local-agent-startup-shell.ts

coderabbitai · 2026-08-13T07:48:12Z

+    const filePath =
+      process.platform === 'win32'
+        ? path
+        : join(
+            process.platform === 'linux' ? '/proc/self/fd' : '/dev/fd',
+            String(directoryFd),
+            basename(path)
+          )
+    fileFd = openSync(
+      filePath,
+      fsConstants.O_RDWR | (process.platform === 'win32' ? 0 : fsConstants.O_NOFOLLOW)
+    )


🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash # Description: Check whether any test covers clearFile on darwin, and whether other modules already solve dir-relative opens. set -eu printf '=== platform branches in fish history modules ===\n' rg -n --type=ts "process\.platform|/dev/fd|/proc/self/fd" src/main/fish-history-location-attestation.ts src/main/fish-history-session.ts printf '\n=== existing dir-relative open helpers in repo ===\n' rg -n --type=ts "O_NOFOLLOW|O_DIRECTORY|/proc/self/fd|/dev/fd" src printf '\n=== tests that skip or gate on darwin ===\n' rg -n --type=ts "platform === 'darwin'" src | head -50

Repository: stablyai/orca

Length of output: 8801

🏁 Script executed:

#!/bin/bash set -eu printf '%s\n' '=== attestation implementation ===' sed -n '1,170p' src/main/fish-history-location-attestation.ts printf '%s\n' '=== callers and tests ===' rg -n -C 4 --type=ts 'clearFile|deleteFishHistoryFile|fish-history-location-attestation|attest' src/main printf '%s\n' '=== package/runtime context ===' rg -n '"(node|electron)"|engines|vitest|jest' package.json

Repository: stablyai/orca

Length of output: 50370

🌐 Web query:

macOS /dev/fd file descriptor directory lookup open /dev/fd/3/name ENOTDIR fdesc

💡 Result:

On macOS, the error "ENOTDIR" when attempting to access a path such as /dev/fd/3/name occurs because the entries within /dev/fd do not behave as standard traversable directories [1][2]. The /dev/fd directory is implemented by the fdesc (file descriptor) filesystem [3][4]. While this filesystem allows processes to access their own open file descriptors via the /dev/fd/N path [5][3], the entries (e.g., /dev/fd/3) are not directories in the structural sense [1]. Consequently, attempting to perform a directory lookup or traversal—such as appending /name to a file descriptor path—will fail with ENOTDIR because the kernel does not treat /dev/fd/N as a directory node that can contain further filesystem objects [1][6]. Key points regarding this behavior: - Nature of /dev/fd: The /dev/fd interface is designed to provide access to open file descriptors by referencing them as files [5][3]. It is not a general-purpose directory that supports the hierarchical traversal of the underlying file (if the descriptor happens to be a directory) or its metadata [1]. - Source of Error: The ENOTDIR error is returned by the kernel when a system call expects a directory component in a path but encounters a non-directory, such as a file descriptor handle [6]. - Compatibility and Workarounds: Applications, including language runtimes like Python, often encounter this limitation when attempting to use recursive file system operations (like globbing or path traversal) on /dev/fd [7][8]. To interact with file descriptors, applications should open the descriptor path directly (e.g., open("/dev/fd/3")) rather than attempting to traverse it [5][3]. For tasks requiring the inspection of open file descriptors, developers are advised to use platform-specific APIs such as the proc_pidinfo function from the undocumented libproc.h header, which provides a more robust method for counting and inspecting descriptors on macOS [2].

Citations:

1: Fix macOS receipt file references iseppo/e-arveldaja-mcp#56

2: https://zameermanji.com/blog/2021/8/1/counting-open-file-descriptors-on-macos/

3: https://leopard-adc.pepas.com/documentation/Darwin/Reference/ManPages/man8/mount_fdesc.8.html

4: https://www.oreilly.com/library/view/mac-os-x/0596003706/re293.html

5: https://manp.gs/mac/4/fd

6: https://github.com/apple/darwin-xnu/blob/main/bsd/miscfs/devfs/devfs_tree.c

7: python/cpython@2cf3316

8: Python fails to read a script whose path is /dev/fd/X python/cpython#87235

🏁 Script executed:

#!/bin/bash set -eu printf '%s\n' '=== fish-history deletion tests ===' sed -n '70,190p' src/main/fish-history-session.test.ts sed -n '190,330p' src/main/fish-history-session.test.ts printf '%s\n' '=== test platform controls ===' rg -n -C 2 --type=ts "darwin|process\.platform|platform" src/main/fish-history-session.test.ts src/main/terminal-history-attested-deletion.test.ts printf '%s\n' '=== runtime versions and filesystem API references ===' rg -n -C 2 '"(node|electron)"|engines|openSync|openat' package.json src/main/fish-history-location-attestation.ts

Repository: stablyai/orca

Length of output: 5754

🌐 Web query:

Node.js fs openat API directory file descriptor relative open O_NOFOLLOW

💡 Result:

Node.js does not provide a direct, public-facing fs.openat API. While Node.js relies on libuv for cross-platform file system operations, the high-level fs module functions (such as fs.open and fsPromises.open) do not expose the openat system call or the ability to pass a directory file descriptor to perform path-relative lookups [1][2]. The openat system call (which supports flags like O_NOFOLLOW to prevent following symbolic links at the final path component) is a feature of POSIX-compliant operating systems rather than the Node.js runtime itself [3][4][5]. Key technical considerations regarding your query include: 1. Absence of API: There is no fs.openat method in the Node.js documentation [1][6][7]. Developers needing to operate relative to a directory must generally resolve paths manually or use library-specific workarounds that do not leverage the kernel-level directory file descriptor directly [8]. 2. O_NOFOLLOW: While O_NOFOLLOW is a valid flag in the underlying C/C++ system-level implementation (and can sometimes be passed through low-level bindings or modules that interact with system headers), it is not a documented or supported flag in the high-level fs.open flags parameter [1][9]. Attempting to pass numeric flags directly into Node.js fs methods can sometimes bypass internal validation, but this is undocumented and unreliable [9]. 3. Security Risks: Node.js has faced security concerns related to permission logic and symlink handling precisely because it lacks native, secure openat-style primitives for path resolution [9]. Without native openat support, preventing TOCTOU (Time-of-Check to Time-of-Use) vulnerabilities and symlink escapes requires careful, manual path canonicalization and verification [8]. If your application requires secure path resolution relative to a base directory or strict symlink prevention, you must perform these checks manually (e.g., using fs.realpathSync or similar path-validation strategies) rather than relying on an openat wrapper in Node.js [8].

Citations:

1: https://nodejs.org/api/fs.html

2: https://nodejs.org/api/fs.md

3: https://www.man7.org/linux/man-pages/man2/openat.2.html

4: https://man7.org/linux/man-pages/man2/openat.2.html

5: https://man.archlinux.org/man/openat.2.en.txt

6: https://nodejs.org/docs/latest-v26.x/api/fs.html

7: https://beta.docs.nodejs.org/fs.html

8: Discussion: openat support libuv/libuv#4167

9: Permission logic in fs.open(), fs.openSync(), and fsPromises.open() can easily be bypassed nodejs/node#47090

Use the attested path directly on Darwin

On macOS, /dev/fd/<fd>/<name> fails with ENOTDIR, so clearFile cannot truncate fish history. Add a Darwin-specific direct-path branch with O_NOFOLLOW, retain both identity checks, and add a Darwin-gated truncation test.

coderabbitai · 2026-08-13T07:48:12Z

+  it('rejects a final history symlink replacement', () => {
+    const dataRoot = join(root, 'data')
+    const owned = historyPath(dataRoot)
+    attestFishHistoryLocation(attestationPath, session, owned)
+    const target = join(root, 'target-history')
+    writeFileSync(target, 'unrelated')
+    rmSync(owned)
+    symlinkSync(target, owned)
+
+    deleteFishHistoryFile(session, attestationPath)
+
+    expect(readFileSync(target, 'utf8')).toBe('unrelated')
+  })


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

Guard symlink-based filesystem tests on Windows.

These tests call symlinkSync, which commonly fails on Windows CI without elevation or Developer Mode. Add it.skipIf(process.platform === 'win32') to both symlink tests so the suites do not fail because of runner permissions.

This also applies to the corresponding test in src/relay/terminal-history.test.ts.

📍 Affects 2 files

src/main/fish-history-session.test.ts#L114-L126 (this comment)

src/relay/terminal-history.test.ts#L43-L54

Source: Learnings

coderabbitai · 2026-08-13T07:48:12Z

+  if (!injected.historyDir) {
    return
  }

-  const newShell = resolveShellKind(fallbackShellPath)
-  const newFilename = historyFilename(newShell)
+  const newFilename = historyFilename(resolveShellKind(fallbackShellPath))
  if (!newFilename) {
-    // Fallback to an unknown shell — remove HISTFILE override entirely
-    // so the shell uses its own default.
+    // Fallback to an unknown shell — drop the override so it uses its own default.
    delete spawnEnv.HISTFILE
    return
  }
-
-  // Replace the filename portion of the HISTFILE path.
-  const dir = spawnEnv.HISTFILE.replace(/[/\\][^/\\]+$/, '')
-  spawnEnv.HISTFILE = `${dir}/${newFilename}`
+  spawnEnv.HISTFILE = `${injected.historyDir}/${newFilename}`
 }


🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Do not hardcode / when rebuilding HISTFILE.

Line 330 joins injected.historyDir and newFilename with a literal /. Line 284 already avoids path.dirname for the same reason: under WSL the value must stay a POSIX path. Derive the separator from injected.historyDir instead of assuming one.

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

♻️ Proposed change

- spawnEnv.HISTFILE = `${injected.historyDir}/${newFilename}` + // WSL history dirs are POSIX paths, so reuse the separator the dir already uses. + const separator = injected.historyDir.includes('\\') ? '\\' : '/' + spawnEnv.HISTFILE = `${injected.historyDir}${separator}${newFilename}`

📝 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

if (!injected.historyDir) {

return

}

const newShell = resolveShellKind(fallbackShellPath)

const newFilename = historyFilename(newShell)

const newFilename = historyFilename(resolveShellKind(fallbackShellPath))

if (!newFilename) {

// Fallback to an unknown shell — remove HISTFILE override entirely

// so the shell uses its own default.

// Fallback to an unknown shell — drop the override so it uses its own default.

delete spawnEnv.HISTFILE

return

}

// Replace the filename portion of the HISTFILE path.

const dir = spawnEnv.HISTFILE.replace(/[/\\][^/\\]+$/, '')

spawnEnv.HISTFILE = `${dir}/${newFilename}`

spawnEnv.HISTFILE = `${injected.historyDir}/${newFilename}`

}

if (!injected.historyDir) {

return

}

const newFilename = historyFilename(resolveShellKind(fallbackShellPath))

if (!newFilename) {

// Fallback to an unknown shell — drop the override so it uses its own default.

delete spawnEnv.HISTFILE

return

}

// WSL history dirs are POSIX paths, so reuse the separator the dir already uses.

const separator = injected.historyDir.includes('\\') ? '\\' : '/'

spawnEnv.HISTFILE = `${injected.historyDir}${separator}${newFilename}`

}

Source: Coding guidelines

coderabbitai · 2026-08-13T07:48:12Z

+function fishCleanupScript(session: string): string {
+  return [
+    'set -l data_home $XDG_DATA_HOME',
+    'string match -qr "^/" -- $data_home; or set data_home "$HOME/.local/share"',
+    `command rm -f -- "$data_home/fish/${session}_history"`
+  ].join('; ')
+}


🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Quote $data_home, or the command blocks until the 5 s timeout.

Line 59 passes $data_home unquoted to string match. In fish, an unquoted expansion of an unset or empty variable produces zero arguments. string match with no string arguments reads from standard input. execFile opens a stdin pipe and never closes it, so string match blocks until the 5 s timeout kills the process.

XDG_DATA_HOME is unset by default on most Linux distributions, so this is the common path, not an edge case.

The rejection then propagates: src/main/terminal-history-deletion.ts Line 112 skips removeHostTree(dir), Line 122 schedules a retry, and after the retries the tombstone waits for the next app launch. The WSL fish history file is never removed.

Use a double-quoted expansion, which always yields exactly one argument.

🐛 Proposed fix

function fishCleanupScript(session: string): string { return [ 'set -l data_home $XDG_DATA_HOME', - 'string match -qr "^/" -- $data_home; or set data_home "$HOME/.local/share"', + 'string match -qr "^/" -- "$data_home"; or set data_home "$HOME/.local/share"', `command rm -f -- "$data_home/fish/${session}_history"` ].join('; ') }

The command-injection hint from static analysis does not apply here: execFile receives an argument array, and isSafeFishHistorySession restricts session to orca_ plus hex characters before it reaches the script.

📝 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

function fishCleanupScript(session: string): string {

return [

'set -l data_home $XDG_DATA_HOME',

'string match -qr "^/" -- $data_home; or set data_home "$HOME/.local/share"',

`command rm -f -- "$data_home/fish/${session}_history"`

].join('; ')

}

function fishCleanupScript(session: string): string {

return [

'set -l data_home $XDG_DATA_HOME',

'string match -qr "^/" -- "$data_home"; or set data_home "$HOME/.local/share"',

`command rm -f -- "$data_home/fish/${session}_history"`

].join('; ')

}

🧰 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 { execFile } from 'node:child_process'
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(detect-child-process-typescript)

Source: Linters/SAST tools

coderabbitai · 2026-08-13T07:48:13Z

+  if (char === 'x' || char === 'X') {
+    const digits = readRadix(value, start + 1, /^[0-9a-fA-F]{1,2}/)
+    return digits ? fromCode(digits, 16, 1) : { text: char, next: start + 1, diverges: false }
+  }
+  if (char === 'u' || char === 'U') {
+    const digits = readRadix(
+      value,
+      start + 1,
+      char === 'u' ? /^[0-9a-fA-F]{1,4}/ : /^[0-9a-fA-F]{1,8}/
+    )
+    return digits ? fromCode(digits, 16, 1) : { text: char, next: start + 1, diverges: false }
+  }
+  if (char >= '0' && char <= '7') {
+    const digits = readRadix(value, start, /^[0-7]{1,3}/)
+    // fish caps an octal escape at one byte and errors above it.
+    return Number.parseInt(digits, 8) > 0xff
+      ? {
+          text: value.slice(start, start + digits.length),
+          next: start + digits.length,
+          diverges: true
+        }
+      : fromCode(digits, 8, 0)
+  }


🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

fish shell 4 documentation escape sequences \xHH versus \XHH and octal \ooo byte semantics

💡 Result:

In fish shell 4, the escape sequences \xHH and \XHH are treated as equivalent, both representing a byte of data with the specified hexadecimal value HH [1][2][3]. This represents a change from older versions of fish (prior to version 3.6), where \xHH was restricted to ASCII values (typically up to 0x7f) and \XHH was used to represent raw byte values, which could be used to insert invalid strings in multibyte encodings [4]. As of fish 3.6 and continuing into version 4, these sequences have been unified to simplify usage; both now represent the specified byte value, and the restriction on \xHH has been removed [5]. Regarding octal sequences, \ooo (where ooo is an octal number) represents the ASCII character with the specified value [1][2]. The documentation specifies that the highest allowed value for this sequence is \177 [1][6][3]. It is important to note that these escape sequence behaviors apply to the fish language syntax itself [1][3]. For the built-in echo command, which may accept its own set of escape sequences when the -e flag is used, the documentation similarly identifies \xHH as a byte with hexadecimal value and \0NNN (1 to 3 digits) as a byte with octal value [7][8].

Citations:

1: https://fishshell.com/docs/current/language.html

2: https://fishshell.com/docs/4.4/language.html

3: https://fishshell.com/docs/4.8/language.html

4: https://fishshell.com/docs/3.5/language.html

5: Make \x the same as \X fish-shell/fish-shell#9247

6: https://fishshell.com/docs/current/language.html?highlight=brace+expansion

7: https://fishshell.com/docs/current/cmds/echo.html

8: https://fishshell.com/docs/4.4/cmds/echo.html

🏁 Script executed:

#!/bin/bash set -e printf '%s\n' '--- candidate file map ---' ast-grep outline src/shared/fish-command-tokenizer.ts printf '%s\n' '--- relevant source ---' sed -n '1,180p' src/shared/fish-command-tokenizer.ts printf '%s\n' '--- fish references and tests ---' rg -n -i --glob '!node_modules' 'fish-command-tokenizer|fromCode|readRadix|diverg|\\\\X|\\\\x|octal|fish version' . | head -240 printf '%s\n' '--- package/version context ---' rg -n -i --glob 'package.json' --glob '*.md' 'fish|version' . | head -160

Repository: stablyai/orca

Length of output: 50369

🌐 Web query:

fish shell current language escape sequences \xHH \XHH octal \ooo maximum value official documentation

💡 Result:

In the fish shell, the escape sequences \xHH/\XHH and \ooo are defined as follows: - \xHH or \XHH: Where HH is a hexadecimal number, this represents a byte of data with the specified value [1][2]. There is no explicit maximum value defined for HH in the official documentation, as it represents a byte value [1][2]. - \ooo: Where ooo is an octal number, this represents an ASCII character with the specified value [1][2]. The official documentation explicitly states that the highest allowed value for this sequence is \177 [1][2]. These escape sequences are provided for characters that cannot be written directly on the command line [1][2]. For handling broader Unicode character requirements, fish also supports \uXXXX (16-bit) and \UXXXXXXXX (32-bit) escape sequences [2].

Citations:

1: https://fishshell.com/docs/current/language.html

2: https://man.archlinux.org/man/fish-language.1.en.txt

🏁 Script executed:

#!/bin/bash set -e python3 - <<'PY' from pathlib import Path p = Path("src/shared/fish-command-tokenizer.ts") text = p.read_text() print("--- escape decoder implementation ---") for i, line in enumerate(text.splitlines(), 1): if 1 <= i <= 120: print(f"{i:4}: {line}") print("--- relevant test files ---") for path in Path(".").rglob("*"): if path.is_file() and ("fish" in path.name.lower() or "token" in path.name.lower()): print(path) PY

Repository: stablyai/orca

Length of output: 19021

🌐 Web query:

site:fishshell.com/docs/current/language.html escape sequences \xHH \XHH octal

💡 Result:

Escape sequences are used in programming and command-line environments to represent characters that cannot be written directly. Their syntax and validity depend on the specific language or environment being used [1]. Hexadecimal Escape Sequences (\xHH and \XHH) The sequence \xHH represents a byte of data using a hexadecimal number where HH are the hex digits [1]. Some environments, such as the fish shell, treat \xHH and \XHH interchangeably, allowing both lowercase and uppercase variants to represent the same hexadecimal value [1]. However, this behavior is not universal; in many programming languages, only the lowercase \x prefix is standard, and \X may be treated as a literal character or an error depending on the language specification. Octal Escape Sequences (\ooo) The sequence \ooo represents a character using an octal number, where ooo consists of octal digits [1]. For example, in the fish shell, \011 represents the tab character [1]. Octal sequences typically have a defined range of allowed values, such as up to \177 for ASCII characters [1]. Summary of notation: \xHH: Hexadecimal sequence (widely supported in lowercase) [1]. \XHH: Hexadecimal sequence (supported in specific environments like fish shell) [1]. \ooo: Octal sequence [1].

Citations:

1: https://fishshell.com/docs/current/language.html

🏁 Script executed:

#!/bin/bash set -e printf '%s\n' '--- tokenizer tests ---' sed -n '1,280p' src/shared/fish-command-tokenizer.test.ts printf '%s\n' '--- live fish quoting tests ---' sed -n '1,250p' src/shared/fish-startup-arg-quoting.live-fish.test.ts printf '%s\n' '--- tokenizer integration ---' sed -n '1,190p' src/shared/tui-agent-startup-shell.ts sed -n '90,150p' src/shared/agent-resume-launch-command.ts printf '%s\n' '--- fish availability ---' if command -v fish >/dev/null 2>&1; then fish --version else echo 'fish: unavailable' fi

Repository: stablyai/orca

Length of output: 23220

🏁 Script executed:

#!/bin/bash set -e python3 - <<'PY' from pathlib import Path p = Path("src/shared/fish-command-tokenizer.test.ts") for i, line in enumerate(p.read_text().splitlines(), 1): print(f"{i:4}: {line}") PY

Repository: stablyai/orca

Length of output: 4230

🏁 Script executed:

#!/bin/bash set -e if ! command -v fish >/dev/null 2>&1; then echo 'fish unavailable; no runtime comparison' exit 0 fi python3 - <<'PY' import subprocess cases = [r'\x7f', r'\x80', r'\xE9', r'\X7f', r'\XE9', r'\101', r'\177', r'\200', r'\377', r'\400'] for expr in cases: # fish runs a command whose Python child reports argv[1] as raw bytes. script = ( "python3 -c 'import sys; print(sys.argv[1].encode(\"utf-8\", \"surrogateescape\").hex())' " + "'" + expr.replace("'", "'\\''") + "'" ) result = subprocess.run(["fish", "-c", script], text=True, capture_output=True) print(expr, "status=", result.returncode, "stdout=", result.stdout.strip(), "stderr=", result.stderr.strip()) PY

Repository: stablyai/orca

Length of output: 191

🌐 Web query:

fish-shell source parse escape xHH byte escape UTF-8 \xE9 \XHH

💡 Result:

In the fish shell, the escape sequences \xHH and \XHH are used to represent a single byte of data with the hexadecimal value HH [1][2][3]. Key details regarding these escape sequences include: 1. Functionality: Both \xHH and \XHH are functionally identical in modern versions of fish [4]. They allow you to insert arbitrary byte values into a string [1][2]. 2. Encoding and UTF-8: Because these sequences represent individual bytes rather than Unicode characters, they can be used to construct invalid UTF-8 sequences if the byte values provided do not form valid character encodings [1][2]. Fish typically operates with UTF-8 encoding, so while these sequences are useful for raw byte manipulation, caution is advised when using them to construct strings intended for display or text processing [1][2]. 3. Evolution: In older versions of fish, there was a distinction where \xHH was intended for ASCII-range characters (up to 0x7F) and would error on higher values, while \XHH was used for raw byte data [3][5]. This distinction has been simplified, and \xHH now represents the byte value regardless of whether it is within the standard ASCII range [4]. 4. Comparison with other escapes: While \xHH and \XHH handle raw bytes, fish also provides dedicated sequences for Unicode characters, such as \uXXXX (for 16-bit values) and \UXXXXXXXX (for 32-bit values) [6][5]. These should be used instead of raw byte escapes when the goal is to represent specific Unicode characters [6]. If you need to generate or parse these escapes programmatically, you can use the built-in string escape and string unescape commands, which can handle various styles, including hex-encoded representations [7][8][9].

Citations:

1: https://fishshell.com/docs/current/language.html

2: https://fishshell.com/docs/4.8/language.html

3: https://github.com/fish-shell/fish-shell/blob/a6959aba/doc_src/language.rst

4: Make \x the same as \X fish-shell/fish-shell#9247

5: https://fishshell.com/docs/3.5/language.html

6: https://man.archlinux.org/man/fish-language.1.en.txt

7: https://fishshell.com/docs/current/cmds/string-unescape.html

8: https://fishshell.com/docs/current/cmds/string.html

9: https://fishshell.com/docs/current/cmds/string-escape.html

🌐 Web query:

site:github.com/fish-shell/fish-shell \"XHH\" \"synonymous\" escape

💡 Result:

In the context of the fish shell, the escape sequences \xHH and \XHH are synonymous [1][2]. Both represent a byte of data where HH corresponds to a hexadecimal number [1][2]. Historically, \xHH and \XHH were functionally similar, though \xHH previously included an error check that restricted values to 0x7f or below; this restriction was removed, making the two sequences effectively identical in current versions of the shell [3][4]. This syntax is also commonly used in other programming and command-line contexts, such as the printf command, to represent hexadecimal byte values [5].

Citations:

1: https://github.com/fish-shell/fish-shell/blob/acae88e6/doc_src/language.rst

2: https://github.com/fish-shell/fish-shell/blob/a6959aba/doc_src/language.rst

3: Make \x the same as \X fish-shell/fish-shell#9247

4: Make \x the same as \X fish-shell/fish-shell#9247

5: Add string builtin fish-shell/fish-shell#2296

🏁 Script executed:

#!/bin/bash set -e printf '%s\n' '--- tokenizer decode call and span propagation ---' sed -n '180,265p' src/shared/fish-command-tokenizer.ts printf '%s\n' '--- fish-related package and CI configuration ---' rg -n -i --glob '!node_modules/**' 'fishRequirementViolation|resolveFishBinary|fish 4|fish-shell|fish version|FISH' src config tests package.json .github 2>/dev/null | head -220

Repository: stablyai/orca

Length of output: 28903

Preserve fish byte-escape semantics

In fish 4, \xHH and \XHH are equivalent raw-byte escapes, while octal escapes cannot exceed \177. The current String.fromCodePoint path misrepresents \xE9, and the octal guard incorrectly allows \200–\377. Preserve raw bytes or mark these escapes as divergent, and cap octal values at 0o177.

coderabbitai · 2026-08-13T07:48:13Z

+    if (/\s/.test(char)) {
+      if (started) {
+        tokens.push(token)
+        spans.push({
+          start: tokenStart,
+          end: index,
+          divergesFromShell: diverges
+        })
+        token = ''
+        started = false
+        diverges = false
+      }
+      index += 1
+      continue
+    }


🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Restrict the word separator to the characters fish splits on.

/\s/ also matches U+00A0, U+2028, U+2029, U+FEFF and other Unicode spaces. fish splits words on space, tab and newline only. A pasted CLI-args string that contains a no-break space is therefore split into two tokens, and the emitted spans no longer match the real word boundaries.

🐛 Proposed fix

- if (/\s/.test(char)) { + if (char === ' ' || char === '\t' || char === '\n' || char === '\r') {

📝 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

if (/\s/.test(char)) {

if (started) {

tokens.push(token)

spans.push({

start: tokenStart,

end: index,

divergesFromShell: diverges

})

token = ''

started = false

diverges = false

}

index += 1

continue

}

if (char === ' ' || char === '\t' || char === '\n' || char === '\r') {

if (started) {

tokens.push(token)

spans.push({

start: tokenStart,

end: index,

divergesFromShell: diverges

})

token = ''

started = false

diverges = false

}

index += 1

continue

}

nwparker · 2026-08-16T07:41:32Z

Superseded by #14863.

That branch keeps the three fixes here and adds the real-fish quoting work, but takes the opposite architectural approach: instead of threading a fish dialect through every call site, it removes the dialect. Everything emitted for a Unix shell is written to be correct in sh/bash/zsh/dash/fish simultaneously, so nothing has to detect which shell parses a queued line — which was never reliable for a remote or WSL host anyway.

Net against this branch: −1,854 lines, 14 files deleted. It also fixes #11044 (per-worktree zsh history was a no-op on macOS — /etc/zshrc clobbers the injected HISTFILE), which this branch's own feature depends on.

nwparker · 2026-08-16T22:29:29Z

Closing — superseded by #14863, which is now merged (b6ea3f17a97).

That PR keeps the three fixes from this branch and adds the real-fish quoting work, but takes the opposite architectural approach: rather than threading a fish dialect through every call site, it removes the dialect. Everything emitted for a Unix shell is written to be correct in sh/bash/zsh/dash/ksh/fish simultaneously, so nothing has to detect which shell parses a queued line — which was never reliable for a remote or WSL host anyway.

Net against this branch: roughly −2,400 lines, 14 files deleted. It also fixes #11044 (per-worktree zsh history was a no-op on macOS — /etc/zshrc clobbers the injected HISTFILE), which this branch's own feature depended on.

coderabbitai Bot reviewed Aug 12, 2026

View reviewed changes

nwparker force-pushed the nwparker/fish-history-and-draft-prefill branch from 94cd699 to f5aca4a Compare August 13, 2026 01:50

coderabbitai Bot reviewed Aug 13, 2026

View reviewed changes

nwparker and others added 9 commits August 12, 2026 23:37

test(fish): grade the teardown control on status, not on fish's error…

27cf309

… text Co-authored-by: Orca <help@stably.ai>

fix(fish): scope Codex resume env cleanup

191954b

fix(fish): align shell and history isolation

609ec7e

fix fish startup and scoped history ownership

0ab1dde

fix(history): harden cleanup ownership

0b2f698

Fix relay history isolation and safe fish cleanup

ce7c6e0

Harden Fish and relay history cleanup

43d0f17

nwparker force-pushed the nwparker/fish-history-and-draft-prefill branch from f5aca4a to 43d0f17 Compare August 13, 2026 07:03

nwparker added 2 commits August 13, 2026 00:15

test: replace attested fish history inode

32f2d08

fix: complete fish history and draft prefill CI fixes

3af2787

coderabbitai Bot reviewed Aug 13, 2026

View reviewed changes

perf(fish): cache startup environment snapshot

fa00cf1

nwparker mentioned this pull request Aug 16, 2026

refactor(shell): one portable Unix startup dialect instead of shell detection #14863

Merged

6 tasks done

nwparker closed this Aug 16, 2026

nwparker mentioned this pull request Aug 17, 2026

reland(shell): one portable Unix startup dialect, with both revert causes fixed #15018

Merged

6 tasks done

		writeFileSync(scriptPath, `string join0 -- ${commandLine} > ${outPath}\nexit 0\n`)
		execFileSync(FISH.path as string, [scriptPath], {

Conversation

nwparker commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ELI5

What Changed

0. fish quoting — the part that made the earlier revision a regression

1. Worktree-scoped shell history now works for fish

2. The fish draft-prefill teardown is now reachable

Why

Linked Issue

Visual Proof

Testing

AI Disclosure

Review

Checklist

Uh oh!

coderabbitai Bot commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented Aug 13, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented Aug 13, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nwparker commented Aug 16, 2026

Uh oh!

nwparker commented Aug 16, 2026

nwparker commented Aug 12, 2026 •

edited

Loading

coderabbitai Bot commented Aug 12, 2026 •

edited

Loading