Skip to content

fix(terminal): restore agent sessions after interactive shell startup - #30978

Open
brennanb2025 wants to merge 4 commits into
mainfrom
brennanb2025/zsh-interactive-init-restore
Open

brennanb2025 wants to merge 4 commits into
mainfrom
brennanb2025/zsh-interactive-init-restore

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

ELI5

When a shell startup file asks for an SSH-key passphrase, restoring a saved agent session can freeze the terminal. Orca holds both the resume command and the user’s answer until shell startup finishes; after a timeout it can submit the resume command as the passphrase and the passphrase as a shell command.

Blocked — do not merge. Two delivery designs have concrete failures: shell-owned execution changes command status, suspension, carriage-return handling and shell options; typed delivery can send a command into an unanswered startup question or append it to unfinished user input. The recovery behavior needs a product decision, and full review and real restoration QA remain incomplete.

This change lets the user answer startup questions immediately. On bash, zsh, and fish, the initialized shell executes the restored command once at its command prompt instead of Orca guessing when to type it.

What Changed

  • Daemon terminals, including SSH hosts, pass startup commands through the existing shell startup channel and skip the second typed copy. The local terminal provider uses the same mechanism for every agent.
  • Long and multiline commands retain the existing temporary-script staging and cleanup so they do not exceed process environment limits.
  • Zsh distinguishes its command editor from an editor opened inside startup files and supports shells with line editing disabled. Bash reports readiness before running a long-lived startup command.
  • Deadlines report delayed startup without transferring command ownership. Late readiness output is still recognized. Failure to prepare startup files fails before spawning instead of falling back to typing into an unknown reader.

Why

A timeout, process name, or terminal mode cannot establish whether a password question or a command prompt is reading input. Shell-owned execution follows the common pattern and removes that guess for integrated POSIX launches. Windows and older execution hosts keep their existing launch paths; this changes no saved fields or client/server messages.

Linked Issue

Tracked in STA-11198.

Visual Proof

Pending real Electron and SSH QA with before/after screenshots. This PR is not ready to merge.

Testing

  • Real Electron and SSH main-path QA completed
  • Automated regression tests added/updated

On macOS, eight named test files passed: 118 tests passed and one optional benchmark was skipped. The real-shell regression covers bash/zsh secret questions, bash Readline questions, zsh nested editors, zsh without line editing, and fish secret questions before and after the deadline, checking the user answer, readiness, and exactly-once execution. Existing local-provider, daemon-delivery, snapshot, and startup-timing contracts also passed. The pre-commit checks passed. Earlier Linux bash/read-e checks passed (4 tests); real Electron and SSH restoration QA remain pending; independent review completed one round and is blocked on the findings below. The full workflow skipped this PR while draft; it is now opened for review to run full CI, without a ready-to-merge claim.

The staging follow-up passed 96 tests across six named files, including a 200 KB command and cleanup after fallback and app quit. Existing replacement-shell compatibility tests have 4 failures, 10 passes, and 2 unavailable-binary skips; the failures are the shipping blocker below, not waived acceptance.

Review

Independent review is blocked. Executing restored commands inside prompt callbacks changes ordinary shell behavior: Bash and zsh without line editing can lose command status events, fish cannot suspend the command with Ctrl-Z, and zsh without line editing can use the wrapper’s options instead of the user’s. Short commands ending with a carriage return also need the existing normalization restored. These remain unresolved and prevent shipping. The tested execution alternatives have not yet preserved the full initialized-shell behavior.

The follow-up commit 0e4fd6730718 corrects seven compile errors and releases the spawn environment after launch rather than retaining it through staging cleanup callbacks. Five named contract files passed: 85 tests passed and four skipped. On this head, CI static analysis and typecheck passed. The shell-contract job has the same four unresolved replacement-shell recovery failures described below (450 tests passed and 30 skipped); the rest of the workflow is still running. This follow-up does not resolve the execution findings or establish a QA pass.

A bounded independent check also tested keeping typed startup-command delivery while allowing user input immediately. Waiting only after user activity still sends the command into an unanswered startup question before any input arrives. Waiting for a valid command-prompt marker can concatenate the startup command with unfinished user input. These results do not establish a preserving replacement for the current execution design.

Full review path: this changes startup ordering, process launch, and recovery behavior across local and SSH terminals. It is not eligible for the light path.

Agent skill upstream boundary

  • Not applicable; this copies no upstream skill-installer material.

Notes

Shipping blocker: replacement-shell recovery decision pending. A startup file that replaces the integrated shell with an unintegrated shell, tmux, SSH, or a terminal relay discards its automatic continuation. Existing replacement-shell recovery tests represent behavior users already have; this implementation does not yet preserve that automatic recovery. Brennan must decide whether recovery should become deliberate in those cases or whether cooperating inner-shell integration must be implemented before shipping. A technical panel rejected process/argument/terminal-mode inference as a safe fallback. No loss of support is approved, and this PR must not land until that decision and its acceptance tests are settled.

The intended difference from the common pattern is execution after the user’s startup files, so restored commands use the user’s initialized configuration.

Checklist

  • One focused startup/restoration change
  • User-facing before/after, mechanism, and alternatives explained
  • Before/after behavior evidence attached
  • Self-reviewed; independent review has unresolved findings
  • Cross-platform, SSH, folder-workspace, and mixed-version behavior considered
  • Full PR workflow, review, and real QA passed on the current head

@brennanb2025
brennanb2025 marked this pull request as ready for review October 20, 2026 02:58
@coderabbitai

coderabbitai Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

📝 Walkthrough

The change adds wrapper-based startup-command delivery for supported POSIX shells. Launch plans prepare and track staged commands, and PTY lifecycle paths clean up staged data. Sessions distinguish startup commands from ordinary input. Readiness handling accounts for wrapper-delivered commands, and tests cover shell prompts, timeouts, fallback launch, and command execution.

Priority: ⬆️ High

Merge Risk: 🟠 High · up to 0e4fd

This change moves restored agent commands into shell startup hooks for bash, zsh and fish. The way those commands now run causes several regressions. Restored agents in bash and in zsh without line editing do not report their start and finish, so stale "working" status can return. Fish runs the command inside a prompt event, which can change Ctrl-Z behavior and skip command events. Zsh without line editing runs the command under Orca's private function scope and default options instead of the user's settings. Short commands ending in a carriage return keep that character and can fail to run. Because these issues affect every restored session on the supported POSIX shells, they should be fixed before merging.

Pre-merge checks: Passed 3 · Failed 2

Failed Failed checks: 2 warnings

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 20.83% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 29 files. (3 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check Warning The description covers the required sections and clearly explains the change, rationale, testing status, known blockers, and review state. However, the Linked Issue section contains only STA-11198, … Add a direct link to the tracked issue in the Linked Issue section. Attach before-and-after behavior evidence, or use N/A only if the change has no visual or interaction impact. Complete or explicitly justify any remaining required checkl…
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: restoring agent sessions after interactive shell startup.
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.
Full details: Docstring Coverage

Explanation

Docstring coverage is 20.83% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 29 files. (3 skipped: 3 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Full details: Description check

Explanation

The description covers the required sections and clearly explains the change, rationale, testing status, known blockers, and review state. However, the Linked Issue section contains only STA-11198, not an actual issue link, and the required behavior evidence is still pending.

Resolution

Add a direct link to the tracked issue in the Linked Issue section. Attach before-and-after behavior evidence, or use N/A only if the change has no visual or interaction impact. Complete or explicitly justify any remaining required checklist items before merge.

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 2
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@brennanb2025

Copy link
Copy Markdown
Contributor Author

A startup authentication question can hold the user’s input while Orca waits to restore a terminal command. After the old deadline, the restored command can become the answer to that question. This PR lets user input through immediately and gives the shell responsibility for executing the restored command after initialization.

Independent review: BLOCKED at 8ba17b07f2dcfa15e81121207e5cfb5c34fc99f2. One full review round completed. No production changes were made during this review.

Four confirmed regressions remain open:

  • P1: restored commands in the daemon’s Bash path, and in zsh without its line editor, omit normal command-start/completion events. Terminal status and command-lifetime consumers depend on these events.
  • P1: fish executes the command inside a prompt event, putting the child in the shell’s foreground process group. Ctrl-Z does not suspend it and return to the prompt as ordinary command entry does.
  • P2: zsh without its line editor runs the command under the initialization function’s options and scope, changing initialized user settings and declaration persistence.
  • P2: a short command ending in carriage return no longer receives the existing terminator normalization; /usr/bin/true with that terminator fails instead of succeeding.

Three execution findings share a cause: running ordinary commands inside prompt instrumentation. Bounded reader experiments support a fish continuation, but no complete preserving continuation is established for the required Bash and zsh modes. The tested Bash input-file alternative also lets user prompt code consume the saved command, so it was rejected. No support reduction, changed editor/default, replacement shell, or inferred terminal typing was accepted.

Separate existing support gate: startup configurations that replace the shell or enter relays still lose automatic restoration. The existing replacement-shell suite remains 4 failed / 10 passed / 2 skipped; those tests and the support requirement were retained. That product decision is separate from the four findings above.

The final source readiness audit found no additional proven PR defect. A preexisting cleanup concern remains deferred after a bounded source trace: another host sharing the temporary directory can sweep a staged command file older than one hour while its shell still waits for startup input. The complete two-host failure was not reproduced. Staging failure with an oversized raw environment fallback also remains an unverified concern.

Verification: nine named test files passed 170 tests during the review; isolated real-shell probes reproduced the findings and compared ordinary command entry. Full current-head CI fails on seven TypeScript errors: four direct test handle constructors omit the new required ownership flag, and three environment objects infer optional undefined values. The packaged Windows test stops on the same errors before its runtime flow. Genuine saved-agent restoration through the Electron renderer, a real reply after restoration, main-versus-branch screenshots, and real SSH QA were not completed. The QA queue ticket was released without machine admission or an app launch. Automated tests and shell probes do not count as that QA.

No ready-for-merge claim or completed-review marker is issued.

For Brennan’s review: no main-versus-branch screenshots are available; rendered restoration evidence remains required. The three execution boundaries that need a preserving design are Bash startup execution, fish prompt-event execution, and zsh initialization execution.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 09c4b172-005e-4c27-b081-df7d6a37bb6c
📥 Commits

Reviewing files that changed from the base of the PR and between 912c130 and 0e4fd67.

📒 Files selected for processing: 32
  • src/main/__fixtures__/shell-wrapper-snapshots/daemon-bash-rcfile.txt
  • src/main/__fixtures__/shell-wrapper-snapshots/daemon-zsh-zshenv.txt
  • src/main/__fixtures__/shell-wrapper-snapshots/local-zsh-zshenv.txt
  • src/main/daemon/agent-startup-prompt-latency.node-pty.test.ts
  • src/main/daemon/daemon-bash-shell-ready-rcfile.ts
  • src/main/daemon/daemon-zsh-shell-ready-wrapper-spec.ts
  • src/main/daemon/pty-subprocess-env-retention.test.ts
  • src/main/daemon/pty-subprocess-io-failure-cleanup.test.ts
  • src/main/daemon/pty-subprocess-io-failure-native.test.ts
  • src/main/daemon/pty-subprocess-spawn-file-foreground.test.ts
  • src/main/daemon/pty-subprocess.ts
  • src/main/daemon/pty-subprocess/shell-launch-plan.ts
  • src/main/daemon/pty-subprocess/subprocess-handle.ts
  • src/main/daemon/repro-13767-shell-ready-marker-lost-to-exec.test.ts
  • src/main/daemon/session-shell-ready-barrier.ts
  • src/main/daemon/session-shell-ready-interactive-rc.test.ts
  • src/main/daemon/session-subprocess-handle.ts
  • src/main/daemon/session.test.ts
  • src/main/daemon/session.ts
  • src/main/daemon/shell-ready.ts
  • src/main/daemon/terminal-host-session-create.ts
  • src/main/providers/local-pty-finalize-environment.ts
  • src/main/providers/local-pty-launch-plan.ts
  • src/main/providers/local-pty-provider-shell-readiness.test.ts
  • src/main/providers/local-pty-session-activation.ts
  • src/main/providers/local-pty-shell-readiness-session.ts
  • src/main/providers/local-pty-spawn.ts
  • src/main/pty/posix-shell-startup-command.ts
  • src/main/runtime/terminal/output/headless-terminal-state.ts
  • src/main/shell-templates.ts
  • src/main/zsh-deferred-startup-line-init.ts
  • src/main/zsh-startup-wrapper-builder.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

${BASH_PROMPT_COMMAND_COMPOSITION_BLOCK}
__orca_prepend_prompt_command "__orca_osc133_precmd"
__orca_append_prompt_command '__orca_in_debug_capture=1; __orca_prompt_had_functrace=""; if [[ -o functrace ]]; then __orca_prompt_had_functrace=1; set +T; fi; __orca_outer_debug_trap_spec="$(trap -p DEBUG)"; [[ -z "$__orca_prompt_had_functrace" ]] || set -T; unset __orca_prompt_had_functrace __orca_in_debug_capture'
${getBashStartupCommandPromptBlock(SHELL_READY_MARKER)}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

The restored bash command runs inside PROMPT_COMMAND, so OSC 133 C/D is never emitted for it.

This line adds __orca_run_startup_command to the daemon rcfile. It is appended after __orca_osc133_precmd and before __orca_osc133_epilogue. Here is what happens:

  1. __orca_osc133_precmd sets __orca_in_prompt_command=1.
  2. __orca_run_startup_command runs eval "$__orca_command" while that flag is still set.
  3. __orca_osc133_preexec returns early at [[ -z "${__orca_in_prompt_command:-}" ]] || return 0, so it never prints 133;C and never sets __orca_in_command.
  4. At the next prompt, __orca_osc133_precmd finds no __orca_in_command, so it never prints 133;D.

The same thing happens on bash < 5.1, where the hook runs from __orca_prompt_command_suffix inside __orca_run_prompt_command_array.

Consequence: terminal-command-lifecycle never sees the restored agent start or exit. The stale "working" status that the C/D markers exist to clear (see the comment above __orca_osc133_precmd) comes back for every restored session.

Fix: have the startup hook mark the command boundary itself before it runs eval.

Proposed change in `getBashStartupCommandPromptBlock` (src/main/pty/posix-shell-startup-command.ts)
     builtin history -s "$__orca_command" 2>/dev/null || true
     builtin printf '%s\n' "$__orca_command"
+    # Prompt hooks suppress preexec; mark the restored command as a foreground command.
+    unset __orca_in_prompt_command
+    builtin printf "\\033]133;C\\007"
+    __orca_in_command=1
     eval "$__orca_command"

Update the snapshot fixture after this change.

Comment on lines +80 to 91
// Shell-owned delivery still needs file staging to stay below exec's environment limits.
const staging =
launch.startupCommandDeliveredByWrapper && opts.command
? stageStartupCommand({
command: opts.command,
shellPath: launch.shellPath,
orcaBuiltLine: opts.launchAgent !== undefined
})
: undefined
if (staging) {
env[POSIX_SHELL_STARTUP_COMMAND_ENV] = staging.command
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

#!/bin/bash
# Confirm how the typed path normalizes terminators and whether stripSubmitTerminator is exported.
rg -nP -C5 '\bstripSubmitTerminator\b|\bbuildStartupCommandSubmission\b' src/shared src/main | head -80

Repository: stablyai/orca

Length of output: 6899


🏁 Script executed:

set -eu
printf '%s\n' '--- startup-command-staging.ts ---'
nl -ba src/shared/startup-command-staging.ts | sed -n '45,145p'
printf '%s\n' '--- direct consumers of startup-command environment ---'
rg -n -F --glob '*.{ts,tsx}' -e 'POSIX_SHELL_STARTUP_COMMAND_ENV' -e 'startupCommandDeliveredByWrapper' -e 'stageStartupCommand' src
printf '%s\n' '--- local finalize and daemon subprocess context ---'
nl -ba src/main/daemon/pty-subprocess.ts | sed -n '1,140p'
nl -ba src/main/pty/local-pty-finalize-environment.ts | sed -n '70,140p'
printf '%s\n' '--- wrapper-related source blocks ---'
nl -ba src/main/pty/posix-shell-startup-command.ts | sed -n '1,180p'
nl -ba src/main/shell-templates.ts | sed -n '170,240p'
nl -ba src/main/zsh-startup-wrapper-builder.ts | sed -n '140,205p'
printf '%s\n' '--- changed files versus requested base ---'
git diff --stat 98ee2688250027353232d6d9504c828ef636525b 0e4fd67307186799fd9857c0f1e2ed89121537d1 -- src/shared/startup-command-staging.ts src/main/daemon/pty-subprocess.ts src/main/pty/local-pty-finalize-environment.ts src/main/pty/posix-shell-startup-command.ts src/main/shell-templates.ts src/main/zsh-startup-wrapper-builder.ts

Repository: stablyai/orca

Length of output: 32981


🏁 Script executed:

set -eu
nl -ba src/shared/startup-command-staging.ts | sed -n '45,145p'
printf '%s\n' '--- consumers and launch paths ---'
rg -n -F --glob '*.{ts,tsx}' -e 'POSIX_SHELL_STARTUP_COMMAND_ENV' -e 'startupCommandDeliveredByWrapper' -e 'stageStartupCommand' src
nl -ba src/main/daemon/pty-subprocess.ts | sed -n '1,140p'
nl -ba src/main/pty/local-pty-finalize-environment.ts | sed -n '70,140p'
printf '%s\n' '--- wrapper consumers ---'
nl -ba src/main/pty/posix-shell-startup-command.ts | sed -n '1,180p'
nl -ba src/main/shell-templates.ts | sed -n '170,240p'
nl -ba src/main/zsh-startup-wrapper-builder.ts | sed -n '140,205p'

Repository: stablyai/orca

Length of output: 32513


🏁 Script executed:

set -eu
printf '%s\n' '--- local wrapper staging ---'
nl -ba src/main/providers/local-pty-session-activation.ts | sed -n '165,220p'
nl -ba src/main/providers/local-pty-finalize-environment.ts | sed -n '85,135p'
printf '%s\n' '--- daemon host staging ---'
nl -ba src/main/daemon/terminal-host-session-create.ts | sed -n '215,270p'
printf '%s\n' '--- staging call sites and tests ---'
rg -n -F --glob '*.{ts,tsx}' -e 'staging.command' -e 'staging.delivery' -e 'startupCommandStaging' -e 'stageStartupCommand({' src/main src/shared
nl -ba src/shared/startup-command-staging.test.ts | sed -n '25,225p'

Repository: stablyai/orca

Length of output: 19730


🏁 Script executed:

set -eu
nl -ba src/main/providers/local-pty-session-activation.ts | sed -n '170,215p'
nl -ba src/main/providers/local-pty-finalize-environment.ts | sed -n '90,130p'
nl -ba src/main/daemon/terminal-host-session-create.ts | sed -n '225,265p'
printf '%s\n' '--- all staging result consumers ---'
rg -n -F --glob '*.{ts,tsx}' -e 'staging.command' -e 'staging.delivery' -e 'startupCommandStaging' src/main src/shared

Repository: stablyai/orca

Length of output: 8023


Strip submit terminators from every non-file staging result.

When stageStartupCommand does not create a file, it returns the original command unchanged. Its staging-failure path does the same. Wrapper delivery then evaluates the trailing \r as part of the command string. Normalize both fallback results.

🐛 Suggested fix
   if (!shouldStageStartupCommand({ ...args, platform })) {
-    return { command: args.command, delivery: 'typed' }
+    return { command: stripSubmitTerminator(args.command), delivery: 'typed' }
   }
...
   } catch (error) {
     return {
-      command: args.command,
+      command: stripSubmitTerminator(args.command),
       delivery: 'typed-after-stage-failed',
       failure: error instanceof Error ? error.message : String(error)

Add coverage for a short command ending in \r and assert that the executed command does not receive the terminator.

Comment on lines +217 to +219
`${getFishShellReadyInitCommand(SHELL_READY_MARKER, features.includes('ready'), options?.startupCommand !== undefined)}\n${getFishCodexShellLaunchPreflight() + getFishClaudeShellFunction()}`
],
env: {},
env: startupEnv,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

🔎 Supported by static analysis

🏁 Script executed:

#!/bin/bash
# Inspect the fish template and every caller that enables startup-command delivery.
rg -nP -C4 'getFishShellReadyInitCommand\s*\(' src
sed -n 195,230p src/main/shell-templates.ts

Repository: stablyai/orca

Length of output: 3556


🏁 Script executed:

set -o pipefail
printf '%s\n' '--- daemon caller ---'
sed -n '160,230p' src/main/daemon/shell-ready.ts
printf '%s\n' '--- local caller ---'
sed -n '95,170p' src/main/providers/local-pty-shell-ready.ts
printf '%s\n' '--- startup env references ---'
rg -n -F --glob '*.ts' -e 'POSIX_SHELL_STARTUP_COMMAND_ENV' -e 'startupCommand' src/main | sed -n '1,240p'
printf '%s\n' '--- fish tests and related assertions ---'
rg -n -i -F --glob '*.{ts,tsx,test,spec}' -e 'fish_prompt' -e 'getFishShellReadyInitCommand' -e 'startup command' -e 'startupCommand' src test tests 2>/dev/null | sed -n '1,260p' || true
printf '%s\n' '--- fish availability ---'
command -v fish || true
fish --version 2>/dev/null || true

Repository: stablyai/orca

Length of output: 41900


🌐 Web query:

official fish shell documentation fish_prompt event commandline -f execute fish_preexec fish_postexec event handler job control

💡 Result:

Fish’s current official docs (fish 4.9.3) distinguish these related mechanisms:

- **`fish_prompt`** is a function fish runs when it is about to display the prompt; its output becomes the prompt. Separately, fish emits a named **`fish_prompt` event**, which you can subscribe to with `function --on-event fish_prompt`. ([fishshell.com](https://fishshell.com/docs/current/cmds/fish_prompt.html?utm_source=openai))
- **`fish_preexec` / `fish_postexec`** are named events for interactive commands: preexec fires right before execution and postexec right after. Register handlers with `function --on-event fish_preexec` or `--on-event fish_postexec`; the command line is passed as the first argument. ([fishshell.com](https://fishshell.com/docs/current/cmds/function.html))
- **`commandline -f execute`** queues the `execute` input function; `execute` runs the current command line. `commandline --is-valid` reports whether that line is complete and would execute when `execute` is called. ([fishshell.com](https://fishshell.com/docs/current/cmds/commandline.html?utm_source=openai))
- **Job control is separate from those events.** A foreground job takes control of the terminal; `&` starts a job in the background, `Ctrl-Z` suspends it, and `fg`/`bg` manage it. Fish also supports job-exit event handlers via `function --on-job-exit PID`; that is not the same as `fish_postexec`, which marks completion of an interactive command. ([fishshell.com](https://fishshell.com/docs/current/language.html))

In short: use **`fish_preexec`/`fish_postexec`** to observe interactive command execution, **`fish_prompt`** to customize prompt output (or subscribe to its event), and **job-exit handlers** when you need to react to a particular job ending.

Citations:

- 1: https://fishshell.com/docs/current/cmds/fish_prompt.html?utm_source=openai
- 2: https://fishshell.com/docs/current/cmds/function.html
- 3: https://fishshell.com/docs/current/cmds/commandline.html?utm_source=openai
- 4: https://fishshell.com/docs/current/language.html

🌐 Web query:

site:fishshell.com/docs fish event handler command execution job control fish_prompt eval fish_preexec fish_postexec commandline execute queued

💡 Result:

The fish documentation distinguishes **event hooks** from **queued command-line input**:

- `fish_preexec` fires just before an interactive command executes; `fish_postexec` fires just after. Both receive the command line as an argument, and neither fires for an empty command. These are event handlers, not documented mechanisms for inserting or scheduling another command. ([fishshell.com](https://fishshell.com/docs/4.6/language.html?utm_source=openai))
- `fish_prompt` is emitted when a prompt is about to appear. The `fish_prompt` function itself runs to generate the prompt; with transient prompts enabled, it may also run immediately before a command line executes. ([fishshell.com](https://fishshell.com/docs/4.6/language.html?utm_source=openai))
- `commandline -f …` queues **input functions** to be read before further keypresses. The docs do not say this queues arbitrary shell commands. `commandline --is-valid` only reports whether the current buffer could be executed by the `execute` input function. ([fishshell.com](https://fishshell.com/docs/current/cmds/commandline.html?utm_source=openai))
- `eval` evaluates its arguments as fish commands when called; its documentation does not describe it as a command-queueing or job-control facility. ([fishshell.com](https://fishshell.com/docs/current/cmds/eval.html?utm_source=openai))

**Bottom line:** these docs establish that event hooks run around interactive commands and that `commandline -f` queues input functions. They do not establish that a `fish_preexec`/`fish_postexec` handler can safely or directly execute a queued shell command. The cited event-handler detail is from the fish 4.6 language docs, while the command pages are for fish 4.9.3; behavior for another version may differ. ([fishshell.com](https://fishshell.com/docs/4.6/language.html?utm_source=openai))

Citations:

- 1: https://fishshell.com/docs/4.6/language.html?utm_source=openai
- 2: https://fishshell.com/docs/4.6/language.html?utm_source=openai
- 3: https://fishshell.com/docs/current/cmds/commandline.html?utm_source=openai
- 4: https://fishshell.com/docs/current/cmds/eval.html?utm_source=openai
- 5: https://fishshell.com/docs/4.6/language.html?utm_source=openai

Run the restored command through fish’s command-line execution path.

The daemon and local fish paths execute the restored command with eval inside a fish_prompt event handler. This bypasses the normal interactive command-line path and can skip fish_preexec/fish_postexec instrumentation. It may also change job-control behavior for long-running agents.

Queue the command for normal fish input execution after the prompt event returns. Do not execute it directly with eval from the event handler.

Comment on lines +212 to +225
${
spec.startupCommandDelivery
? ` if [[ ! -o zle ]] && (( \${+${POSIX_SHELL_STARTUP_COMMAND_ENV}} )); then
local __orca_command="\$${POSIX_SHELL_STARTUP_COMMAND_ENV}"
builtin unset ${POSIX_SHELL_STARTUP_COMMAND_ENV}
if [[ "\${__orca_emit_ready_marker-1}" == 1 ]]; then
builtin printf "${spec.readyMarkerEscaped}"
__orca_emit_ready_marker=""
fi
builtin print -r -- "$__orca_command"
eval "$__orca_command"
fi`
: ''
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

In non-ZLE zsh, the restored command runs with Orca's function scope and options.

The eval "$__orca_command" runs inside __orca_deferred_init. That function starts with builtin emulate -L zsh. This causes three problems:

  • Options: the command runs with zsh default options, not the options the user's .zshrc set (for example extendedglob, nonomatch, ksharrays). LOCAL_OPTIONS then discards any option the command changes.
  • Scope: the command runs in function scope. A local, typeset, or return inside the command, or inside a staged script it sources, acts on __orca_deferred_init instead of the top-level shell.
  • Lifecycle markers: the command runs from a precmd or sched hook, so preexec_functions (including __orca_osc133_preexec) never fire. No 133;C is emitted, and no 133;D follows.

The ZLE path (BUFFER + zle accept-line) does not have these problems. Only the unsetopt zle case is affected. The PR review notes this as P1/P2.

Fix: do not run eval inside the emulated function.

  1. In __orca_deferred_init, register a one-shot precmd function that is defined outside emulate -L.
  2. That function prints 133;C, sets __orca_in_command with typeset -g, and runs the command at the user's option state.
  3. The next __orca_osc133_precmd then prints D.

Then update daemon-zsh-zshenv.txt and local-zsh-zshenv.txt.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant