From 9db319dc06cab48bddde9045ddbfe95b3709726e Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 29 Aug 2026 02:27:44 -0700 Subject: [PATCH] fix(terminal): recover OMP from stale working directories (#17128) * fix(terminal): recover OMP from stale cwd * fix(terminal): harden OMP cwd recovery --- config/reliability-gates.jsonc | 70 +++++++--- .../scripts/pr-workflow-parallelism.test.mjs | 2 +- .../daemon-bash-rcfile.txt | 35 ++++- .../daemon-zsh-zshenv.txt | 35 ++++- .../local-bash-rcfile.txt | 35 ++++- .../local-zsh-zshenv.txt | 35 ++++- .../relay-bash-rcfile.txt | 35 ++++- .../relay-zsh-zshenv.txt | 35 ++++- .../pty/omp-shell-wrapper.node-pty.test.ts | 130 ++++++++++++++++-- src/main/pty/omp-shell-wrapper.ts | 35 ++++- 10 files changed, 398 insertions(+), 49 deletions(-) diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index a4b6bd10f07..10143d9019c 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -6321,36 +6321,48 @@ }, { "id": "terminal-session.startup-cwd-missing-dir-recovery", - "title": "Fresh local terminal creation cannot be bricked by a deleted startup cwd", + "title": "Terminal working-directory recovery stays scoped and provider-safe", "maturity": "experimental", "protection": "partial", "owner": "terminal-runtime", - "layer": "shared-main-renderer-contract", - "surfaces": ["terminal lifecycle", "tab creation", "PTY spawn", "startup cwd persistence"], + "layer": "shared-main-renderer-and-posix-shell-contract", + "surfaces": [ + "terminal lifecycle", + "tab creation", + "PTY spawn", + "startup cwd persistence", + "long-lived Bash and Zsh OMP launches" + ], "platforms": ["macos", "linux", "windows", "mobile"], "providers": ["local", "daemon", "ssh", "wsl", "remote-runtime"], "coveredPlatforms": ["macos"], "coveredProviders": ["local", "ssh", "remote-runtime"], - "coverageNotes": "Local macOS evidence covers the shared missing-dir fallback policy, main pty:spawn recovery and metadata, no-flag and reattach strictness, renderer IPC flag routing, SSH-tagged and remote-runtime omission, and the visibility-gated terminal notice. Daemon shares the same pre-provider main cwd decision but lacks a live daemon-provider run; WSL UNC paths are exempt from the probe by design and lack a live run; Linux/Windows and mobile/API strictness are gaps.", + "coverageNotes": "Local macOS evidence covers the shared missing-dir fallback policy, main pty:spawn recovery and metadata, no-flag and reattach strictness, renderer IPC flag routing, SSH-tagged and remote-runtime omission, and the visibility-gated terminal notice. Real Bash and Zsh node-pty tests replace the active cwd inode and exercise the shared POSIX OMP wrapper; generated-file snapshots pin identical local, daemon/SSH, and relay wrappers. Daemon shares the same pre-provider main cwd decision but lacks a live daemon-provider run; WSL UNC paths are exempt from the spawn probe and lack a live run; native PowerShell is unaffected; Linux/Windows and mobile/API strictness remain gaps.", "motivatingLinks": [ "https://github.com/stablyai/orca/issues/7239", "https://github.com/stablyai/orca/pull/7750", - "https://github.com/stablyai/orca/pull/7678" + "https://github.com/stablyai/orca/pull/7678", + "https://github.com/stablyai/orca/issues/16457", + "https://github.com/stablyai/orca/pull/17128" ], - "invariant": "A fresh local renderer terminal spawn may recover from a saved startup cwd whose directory no longer exists only by spawning at the selected workspace root and printing a generic in-terminal notice; existing directories — including ones outside the worktree (#7685) — spawn as requested, and reattach, SSH, remote-runtime, runtime/API, and mobile callers keep exact cwd semantics.", - "oracle": "The shared resolver falls back to the workspace root only when the injected existence probe reports the resolved cwd missing and the workspace root present, and never probes floating terminals or a cwd equal to the root. The renderer sends cwdFallback only for fresh local IPC spawns, main honors it only when connectionId and sessionId are absent, WSL UNC paths never engage the probe-based fallback, main returns fallback metadata only after an actual fallback, the IPC transport preserves that metadata, and the connection layer writes a generic notice that omits the missing path.", + "invariant": "A fresh local renderer terminal spawn may recover from a saved startup cwd whose directory no longer exists only by spawning at the selected workspace root and printing a generic in-terminal notice; existing directories — including ones outside the worktree (#7685) — spawn as requested, and reattach, SSH, remote-runtime, runtime/API, and mobile callers keep exact cwd semantics. A long-lived POSIX shell whose cwd inode was deleted and replaced may run an extension-enabled OMP launch from the live path named by its logical PWD only in a subshell; the parent shell cwd, OMP argv, status extension, and exit status remain unchanged, while a genuinely unavailable path fails visibly without invoking OMP. A usable current directory remains authoritative when PWD is unset instead of being remapped to a workspace fallback.", + "oracle": "The shared resolver falls back to the workspace root only when the injected existence probe reports the resolved cwd missing and the workspace root present, and never probes floating terminals or a cwd equal to the root. The renderer sends cwdFallback only for fresh local IPC spawns, main honors it only when connectionId and sessionId are absent, WSL UNC paths never engage the probe-based fallback, main returns fallback metadata only after an actual fallback, the IPC transport preserves that metadata, and the connection layer writes a generic notice that omits the missing path. In real interactive Bash and Zsh PTYs, unset PWD in a usable nested directory and require OMP to stay there rather than remap to ORCA_WORKTREE_PATH; then delete and recreate the active project path, require OMP to observe the replacement inode with byte-exact extension argv and its nonzero status preserved, require the parent shell to remain on the stale inode, and finally delete the replacement and require an actionable failure before the fake OMP binary runs again.", "commands": [ "pnpm exec vitest run --config config/vitest.config.ts src/shared/terminal-startup-cwd.test.ts", "pnpm exec vitest run --config config/vitest.config.ts src/main/ipc/pty-spawn-cwd-fallback.test.ts src/main/ipc/pty-wsl-cwd-validation.test.ts", "pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/components/terminal-pane/pty-transport-connect-spawn.test.ts", - "pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/components/terminal-pane/pty-connection-runtime-owner-spawn-routing.test.ts" + "pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/components/terminal-pane/pty-connection-runtime-owner-spawn-routing.test.ts", + "pnpm exec vitest run --config config/vitest.config.ts src/main/pty/omp-shell-wrapper.node-pty.test.ts src/main/pty/omp-shell-wrapper-alias-safety.test.ts src/main/shell-wrapper-generated-file-snapshot.test.ts" ], "testFiles": [ "src/shared/terminal-startup-cwd.test.ts", "src/main/ipc/pty-spawn-cwd-fallback.test.ts", "src/main/ipc/pty-wsl-cwd-validation.test.ts", "src/renderer/src/components/terminal-pane/pty-transport-connect-spawn.test.ts", - "src/renderer/src/components/terminal-pane/pty-connection-runtime-owner-spawn-routing.test.ts" + "src/renderer/src/components/terminal-pane/pty-connection-runtime-owner-spawn-routing.test.ts", + "src/main/pty/omp-shell-wrapper.node-pty.test.ts", + "src/main/pty/omp-shell-wrapper-alias-safety.test.ts", + "src/main/shell-wrapper-generated-file-snapshot.test.ts" ], "assertionRefs": [ { @@ -6392,6 +6404,21 @@ "startup cwd fallback metadata prints a generic in-terminal notice", "remote-runtime worktree spawns are not marked with cwdFallback" ] + }, + { + "file": "src/main/pty/omp-shell-wrapper.node-pty.test.ts", + "assertions": [ + "real Bash and Zsh PTYs rebind OMP to a recreated cwd inode without changing the parent shell", + "an unset PWD with a usable cwd stays unset and does not remap OMP to ORCA_WORKTREE_PATH", + "extension argv and a nonzero OMP exit status survive the recovery subshell", + "a subsequently missing logical cwd prints an actionable error and does not invoke OMP again" + ] + }, + { + "file": "src/main/shell-wrapper-generated-file-snapshot.test.ts", + "assertions": [ + "local, daemon/SSH, and relay Bash/Zsh files contain the byte-identical shared OMP wrapper" + ] } ], "evidenceRuns": [ @@ -6430,11 +6457,20 @@ "result": "passed", "durationSeconds": 6.5, "summary": "1 test file passed, 341 tests passed; covers local IPC marking, the generic terminal fallback notice, and remote-runtime omission." + }, + { + "date": "2026-08-29", + "runner": "local", + "platform": "macos", + "command": "pnpm exec vitest run --config config/vitest.config.ts src/main/pty/omp-shell-wrapper.node-pty.test.ts src/main/pty/omp-shell-wrapper-alias-safety.test.ts src/main/shell-wrapper-generated-file-snapshot.test.ts", + "result": "passed", + "durationSeconds": 3, + "summary": "3 test files passed; real Bash and Zsh PTYs covered unset-PWD authority, stale-inode recovery, parent-shell isolation, extension argv, exit status, missing-path visibility, alias safety, and generated local/daemon/relay parity." } ], "runtimeBudget": { - "p95Seconds": 30, - "scope": "focused unit and IPC contract tests" + "p95Seconds": 40, + "scope": "focused unit, IPC, generated-wrapper, and real Bash/Zsh PTY contracts" }, "flakeHistory": { "status": "unknown", @@ -6442,23 +6478,25 @@ }, "redGreenEvidence": { "status": "partial", - "evidence": "The main IPC missing-cwd tests fail with the provider's 'Working directory ... does not exist.' error when the fallback is removed and pass with it. Full live Electron reproduction from a production persisted session is not captured." + "evidence": "The main IPC missing-cwd tests fail with the provider's 'Working directory ... does not exist.' error when the spawn fallback is removed and pass with it. Before the OMP recovery, the stale-inode reproduction makes the child process fail to resolve its cwd; the candidate rebinds to the recreated path in real Bash and Zsh PTYs. Full live Electron reproduction from a production persisted session is not captured." }, "performanceBudget": { "required": true, - "evidence": "The runtime change adds at most two statSync probes on the fresh-local spawn path (the provider already stats the same paths during validation) and one bounded terminal write only when fallback actually occurs; no polling, provider listing, hidden-pane work, startup awaits, subprocesses, or render-loop work was added." + "evidence": "The spawn recovery adds at most two statSync probes on the fresh-local spawn path (the provider already stats the same paths during validation) and one bounded terminal write only when fallback actually occurs. Each extension-enabled OMP launch adds a constant number of shell filesystem predicates; only a detected stale/unusable cwd creates one short recovery subshell. No polling, retry, provider listing, hidden-pane work, startup awaits, extra OMP subprocesses, or render-loop work was added." }, "promotionCriteria": [ "Attach CI evidence for all declared test files.", "Add a live Electron regression that opens a local terminal whose persisted startupCwd was deleted and proves visible shell input/output at the workspace root.", - "Add WSL/mobile/API provider-contract coverage or explicitly narrow their risk scope." + "Add a live stale-inode OMP run on Linux or WSL and through an SSH/relay-generated wrapper.", + "Add WSL/mobile/API spawn provider-contract coverage or explicitly narrow their risk scope." ], "knownGaps": [ "No live Electron fixture seeds a persisted tab whose startupCwd directory was deleted.", "Daemon coverage is via the shared pre-provider main cwd decision, not a live daemon provider spawn.", - "WSL UNC paths bypass the probe by design and have no live existence-recovery run; Linux, Windows, and mobile/API strictness are not directly exercised." + "The stale-inode OMP oracle is local macOS Bash/Zsh plus shared generated-file parity; it does not execute through live SSH, relay, Linux, or WSL providers.", + "WSL UNC paths bypass the spawn probe by design and have no live existence-recovery run; Linux, Windows, and mobile/API spawn strictness are not directly exercised." ], - "demotionRule": "Demote or quarantine if the gate flakes without a product bug, if an existing directory is ever remapped away from the requested cwd, or if a reattach/remote/API caller can engage the fallback." + "demotionRule": "Demote or quarantine if the gate flakes without a product bug, if an existing directory is ever remapped away from the requested cwd, if a reattach/remote/API caller can engage the spawn fallback, or if OMP recovery remaps a usable cwd with unset PWD, changes argv/status/parent cwd, or invokes OMP when the logical path remains unavailable." }, { "id": "agent-status.pi-hook-liveness", diff --git a/config/scripts/pr-workflow-parallelism.test.mjs b/config/scripts/pr-workflow-parallelism.test.mjs index 29f532b9436..c781373b005 100644 --- a/config/scripts/pr-workflow-parallelism.test.mjs +++ b/config/scripts/pr-workflow-parallelism.test.mjs @@ -14,6 +14,7 @@ const shellContractFiles = [ 'src/main/daemon/shell-ready.test.ts', 'src/main/providers/local-pty-shell-ready-zsh-launch-environment.test.ts', 'src/main/providers/__tests__/shell-ready-framework-example.test.ts', + 'src/main/pty/omp-shell-wrapper.node-pty.test.ts', 'src/main/shell-startup-feature-channel.test.ts', 'src/main/zsh-scoped-histfile.live-shell.test.ts', 'src/main/zsh-startup-hook-user-config-equivalence.live-shell.test.ts', @@ -22,7 +23,6 @@ const shellContractFiles = [ ] const patchedNodePtyContractFiles = [ 'src/main/daemon/node-pty-fd-leak.test.ts', - 'src/main/pty/omp-shell-wrapper.node-pty.test.ts', 'src/shared/fish-query-reply-child-stdin.node-pty.test.ts' ] const nativeShellContractFiles = [...shellContractFiles, ...patchedNodePtyContractFiles] diff --git a/src/main/__fixtures__/shell-wrapper-snapshots/daemon-bash-rcfile.txt b/src/main/__fixtures__/shell-wrapper-snapshots/daemon-bash-rcfile.txt index 63748ee93af..b91f1065f84 100644 --- a/src/main/__fixtures__/shell-wrapper-snapshots/daemon-bash-rcfile.txt +++ b/src/main/__fixtures__/shell-wrapper-snapshots/daemon-bash-rcfile.txt @@ -44,9 +44,17 @@ __orca_omp_should_skip_extension() { esac return 1 } -__orca_omp() { - local __orca_use_extension=1 - __orca_omp_should_skip_extension "${1:-}" && __orca_use_extension=0 +__orca_omp_cwd_is_usable() { + [[ -x . ]] || return 1 + if [[ -n "${PWD:-}" ]]; then + [[ -d "${PWD}" && "${PWD}" -ef . ]] + else + builtin pwd -P >/dev/null 2>&1 + fi +} +__orca_omp_invoke() { + local __orca_use_extension="$1" + shift if [[ $__orca_use_extension -eq 1 && -n "${ORCA_OMP_STATUS_EXTENSION:-}" && -f "${ORCA_OMP_STATUS_EXTENSION}" ]]; then if [[ "${1:-}" == "launch" ]]; then shift @@ -58,6 +66,27 @@ __orca_omp() { command omp "$@" fi } +__orca_omp() { + local __orca_use_extension=1 + __orca_omp_should_skip_extension "${1:-}" && __orca_use_extension=0 + if [[ $__orca_use_extension -eq 1 ]] && ! __orca_omp_cwd_is_usable; then + local __orca_logical_cwd="${PWD:-${ORCA_WORKTREE_PATH:-${ORCA_ROOT_PATH:-}}}" + # Why: a restored shell can retain the deleted directory inode after its path is recreated. + ( + if [[ -z "$__orca_logical_cwd" ]]; then + printf 'Orca: OMP cannot start because no terminal working directory is available. Open a new terminal in an existing directory.\n' >&2 + return 1 + fi + if ! builtin cd -P -- "$__orca_logical_cwd" 2>/dev/null; then + printf 'Orca: OMP cannot access the terminal working directory "%s". Open a new terminal in an existing directory.\n' "$__orca_logical_cwd" >&2 + return 1 + fi + __orca_omp_invoke "$__orca_use_extension" "$@" + ) + else + __orca_omp_invoke "$__orca_use_extension" "$@" + fi +} if [[ -n "${ORCA_OMP_STATUS_EXTENSION:-}" ]]; then # Why the function reserved word: it suppresses alias expansion of the name, which # an `alias omp` otherwise rewrites at parse time, aborting the rest of the file. diff --git a/src/main/__fixtures__/shell-wrapper-snapshots/daemon-zsh-zshenv.txt b/src/main/__fixtures__/shell-wrapper-snapshots/daemon-zsh-zshenv.txt index e734f8c1867..bce09f52ff0 100644 --- a/src/main/__fixtures__/shell-wrapper-snapshots/daemon-zsh-zshenv.txt +++ b/src/main/__fixtures__/shell-wrapper-snapshots/daemon-zsh-zshenv.txt @@ -82,9 +82,17 @@ __orca_deferred_init() { esac return 1 } - __orca_omp() { - local __orca_use_extension=1 - __orca_omp_should_skip_extension "${1:-}" && __orca_use_extension=0 + __orca_omp_cwd_is_usable() { + [[ -x . ]] || return 1 + if [[ -n "${PWD:-}" ]]; then + [[ -d "${PWD}" && "${PWD}" -ef . ]] + else + builtin pwd -P >/dev/null 2>&1 + fi + } + __orca_omp_invoke() { + local __orca_use_extension="$1" + shift if [[ $__orca_use_extension -eq 1 && -n "${ORCA_OMP_STATUS_EXTENSION:-}" && -f "${ORCA_OMP_STATUS_EXTENSION}" ]]; then if [[ "${1:-}" == "launch" ]]; then shift @@ -96,6 +104,27 @@ __orca_deferred_init() { command omp "$@" fi } + __orca_omp() { + local __orca_use_extension=1 + __orca_omp_should_skip_extension "${1:-}" && __orca_use_extension=0 + if [[ $__orca_use_extension -eq 1 ]] && ! __orca_omp_cwd_is_usable; then + local __orca_logical_cwd="${PWD:-${ORCA_WORKTREE_PATH:-${ORCA_ROOT_PATH:-}}}" + # Why: a restored shell can retain the deleted directory inode after its path is recreated. + ( + if [[ -z "$__orca_logical_cwd" ]]; then + printf 'Orca: OMP cannot start because no terminal working directory is available. Open a new terminal in an existing directory.\n' >&2 + return 1 + fi + if ! builtin cd -P -- "$__orca_logical_cwd" 2>/dev/null; then + printf 'Orca: OMP cannot access the terminal working directory "%s". Open a new terminal in an existing directory.\n' "$__orca_logical_cwd" >&2 + return 1 + fi + __orca_omp_invoke "$__orca_use_extension" "$@" + ) + else + __orca_omp_invoke "$__orca_use_extension" "$@" + fi + } if [[ -n "${ORCA_OMP_STATUS_EXTENSION:-}" ]]; then # Why the function reserved word: it suppresses alias expansion of the name, which # an `alias omp` otherwise rewrites at parse time, aborting the rest of the file. diff --git a/src/main/__fixtures__/shell-wrapper-snapshots/local-bash-rcfile.txt b/src/main/__fixtures__/shell-wrapper-snapshots/local-bash-rcfile.txt index 9d1e2922b45..ee9cb7975ec 100644 --- a/src/main/__fixtures__/shell-wrapper-snapshots/local-bash-rcfile.txt +++ b/src/main/__fixtures__/shell-wrapper-snapshots/local-bash-rcfile.txt @@ -47,9 +47,17 @@ __orca_omp_should_skip_extension() { esac return 1 } -__orca_omp() { - local __orca_use_extension=1 - __orca_omp_should_skip_extension "${1:-}" && __orca_use_extension=0 +__orca_omp_cwd_is_usable() { + [[ -x . ]] || return 1 + if [[ -n "${PWD:-}" ]]; then + [[ -d "${PWD}" && "${PWD}" -ef . ]] + else + builtin pwd -P >/dev/null 2>&1 + fi +} +__orca_omp_invoke() { + local __orca_use_extension="$1" + shift if [[ $__orca_use_extension -eq 1 && -n "${ORCA_OMP_STATUS_EXTENSION:-}" && -f "${ORCA_OMP_STATUS_EXTENSION}" ]]; then if [[ "${1:-}" == "launch" ]]; then shift @@ -61,6 +69,27 @@ __orca_omp() { command omp "$@" fi } +__orca_omp() { + local __orca_use_extension=1 + __orca_omp_should_skip_extension "${1:-}" && __orca_use_extension=0 + if [[ $__orca_use_extension -eq 1 ]] && ! __orca_omp_cwd_is_usable; then + local __orca_logical_cwd="${PWD:-${ORCA_WORKTREE_PATH:-${ORCA_ROOT_PATH:-}}}" + # Why: a restored shell can retain the deleted directory inode after its path is recreated. + ( + if [[ -z "$__orca_logical_cwd" ]]; then + printf 'Orca: OMP cannot start because no terminal working directory is available. Open a new terminal in an existing directory.\n' >&2 + return 1 + fi + if ! builtin cd -P -- "$__orca_logical_cwd" 2>/dev/null; then + printf 'Orca: OMP cannot access the terminal working directory "%s". Open a new terminal in an existing directory.\n' "$__orca_logical_cwd" >&2 + return 1 + fi + __orca_omp_invoke "$__orca_use_extension" "$@" + ) + else + __orca_omp_invoke "$__orca_use_extension" "$@" + fi +} if [[ -n "${ORCA_OMP_STATUS_EXTENSION:-}" ]]; then # Why the function reserved word: it suppresses alias expansion of the name, which # an `alias omp` otherwise rewrites at parse time, aborting the rest of the file. diff --git a/src/main/__fixtures__/shell-wrapper-snapshots/local-zsh-zshenv.txt b/src/main/__fixtures__/shell-wrapper-snapshots/local-zsh-zshenv.txt index af7cab85587..222cab93896 100644 --- a/src/main/__fixtures__/shell-wrapper-snapshots/local-zsh-zshenv.txt +++ b/src/main/__fixtures__/shell-wrapper-snapshots/local-zsh-zshenv.txt @@ -82,9 +82,17 @@ __orca_deferred_init() { esac return 1 } - __orca_omp() { - local __orca_use_extension=1 - __orca_omp_should_skip_extension "${1:-}" && __orca_use_extension=0 + __orca_omp_cwd_is_usable() { + [[ -x . ]] || return 1 + if [[ -n "${PWD:-}" ]]; then + [[ -d "${PWD}" && "${PWD}" -ef . ]] + else + builtin pwd -P >/dev/null 2>&1 + fi + } + __orca_omp_invoke() { + local __orca_use_extension="$1" + shift if [[ $__orca_use_extension -eq 1 && -n "${ORCA_OMP_STATUS_EXTENSION:-}" && -f "${ORCA_OMP_STATUS_EXTENSION}" ]]; then if [[ "${1:-}" == "launch" ]]; then shift @@ -96,6 +104,27 @@ __orca_deferred_init() { command omp "$@" fi } + __orca_omp() { + local __orca_use_extension=1 + __orca_omp_should_skip_extension "${1:-}" && __orca_use_extension=0 + if [[ $__orca_use_extension -eq 1 ]] && ! __orca_omp_cwd_is_usable; then + local __orca_logical_cwd="${PWD:-${ORCA_WORKTREE_PATH:-${ORCA_ROOT_PATH:-}}}" + # Why: a restored shell can retain the deleted directory inode after its path is recreated. + ( + if [[ -z "$__orca_logical_cwd" ]]; then + printf 'Orca: OMP cannot start because no terminal working directory is available. Open a new terminal in an existing directory.\n' >&2 + return 1 + fi + if ! builtin cd -P -- "$__orca_logical_cwd" 2>/dev/null; then + printf 'Orca: OMP cannot access the terminal working directory "%s". Open a new terminal in an existing directory.\n' "$__orca_logical_cwd" >&2 + return 1 + fi + __orca_omp_invoke "$__orca_use_extension" "$@" + ) + else + __orca_omp_invoke "$__orca_use_extension" "$@" + fi + } if [[ -n "${ORCA_OMP_STATUS_EXTENSION:-}" ]]; then # Why the function reserved word: it suppresses alias expansion of the name, which # an `alias omp` otherwise rewrites at parse time, aborting the rest of the file. diff --git a/src/main/__fixtures__/shell-wrapper-snapshots/relay-bash-rcfile.txt b/src/main/__fixtures__/shell-wrapper-snapshots/relay-bash-rcfile.txt index 6ac463f5770..9d90929085d 100644 --- a/src/main/__fixtures__/shell-wrapper-snapshots/relay-bash-rcfile.txt +++ b/src/main/__fixtures__/shell-wrapper-snapshots/relay-bash-rcfile.txt @@ -36,9 +36,17 @@ __orca_omp_should_skip_extension() { esac return 1 } -__orca_omp() { - local __orca_use_extension=1 - __orca_omp_should_skip_extension "${1:-}" && __orca_use_extension=0 +__orca_omp_cwd_is_usable() { + [[ -x . ]] || return 1 + if [[ -n "${PWD:-}" ]]; then + [[ -d "${PWD}" && "${PWD}" -ef . ]] + else + builtin pwd -P >/dev/null 2>&1 + fi +} +__orca_omp_invoke() { + local __orca_use_extension="$1" + shift if [[ $__orca_use_extension -eq 1 && -n "${ORCA_OMP_STATUS_EXTENSION:-}" && -f "${ORCA_OMP_STATUS_EXTENSION}" ]]; then if [[ "${1:-}" == "launch" ]]; then shift @@ -50,6 +58,27 @@ __orca_omp() { command omp "$@" fi } +__orca_omp() { + local __orca_use_extension=1 + __orca_omp_should_skip_extension "${1:-}" && __orca_use_extension=0 + if [[ $__orca_use_extension -eq 1 ]] && ! __orca_omp_cwd_is_usable; then + local __orca_logical_cwd="${PWD:-${ORCA_WORKTREE_PATH:-${ORCA_ROOT_PATH:-}}}" + # Why: a restored shell can retain the deleted directory inode after its path is recreated. + ( + if [[ -z "$__orca_logical_cwd" ]]; then + printf 'Orca: OMP cannot start because no terminal working directory is available. Open a new terminal in an existing directory.\n' >&2 + return 1 + fi + if ! builtin cd -P -- "$__orca_logical_cwd" 2>/dev/null; then + printf 'Orca: OMP cannot access the terminal working directory "%s". Open a new terminal in an existing directory.\n' "$__orca_logical_cwd" >&2 + return 1 + fi + __orca_omp_invoke "$__orca_use_extension" "$@" + ) + else + __orca_omp_invoke "$__orca_use_extension" "$@" + fi +} if [[ -n "${ORCA_OMP_STATUS_EXTENSION:-}" ]]; then # Why the function reserved word: it suppresses alias expansion of the name, which # an `alias omp` otherwise rewrites at parse time, aborting the rest of the file. diff --git a/src/main/__fixtures__/shell-wrapper-snapshots/relay-zsh-zshenv.txt b/src/main/__fixtures__/shell-wrapper-snapshots/relay-zsh-zshenv.txt index 6e791812e07..335b54c60c3 100644 --- a/src/main/__fixtures__/shell-wrapper-snapshots/relay-zsh-zshenv.txt +++ b/src/main/__fixtures__/shell-wrapper-snapshots/relay-zsh-zshenv.txt @@ -56,9 +56,17 @@ __orca_deferred_init() { esac return 1 } - __orca_omp() { - local __orca_use_extension=1 - __orca_omp_should_skip_extension "${1:-}" && __orca_use_extension=0 + __orca_omp_cwd_is_usable() { + [[ -x . ]] || return 1 + if [[ -n "${PWD:-}" ]]; then + [[ -d "${PWD}" && "${PWD}" -ef . ]] + else + builtin pwd -P >/dev/null 2>&1 + fi + } + __orca_omp_invoke() { + local __orca_use_extension="$1" + shift if [[ $__orca_use_extension -eq 1 && -n "${ORCA_OMP_STATUS_EXTENSION:-}" && -f "${ORCA_OMP_STATUS_EXTENSION}" ]]; then if [[ "${1:-}" == "launch" ]]; then shift @@ -70,6 +78,27 @@ __orca_deferred_init() { command omp "$@" fi } + __orca_omp() { + local __orca_use_extension=1 + __orca_omp_should_skip_extension "${1:-}" && __orca_use_extension=0 + if [[ $__orca_use_extension -eq 1 ]] && ! __orca_omp_cwd_is_usable; then + local __orca_logical_cwd="${PWD:-${ORCA_WORKTREE_PATH:-${ORCA_ROOT_PATH:-}}}" + # Why: a restored shell can retain the deleted directory inode after its path is recreated. + ( + if [[ -z "$__orca_logical_cwd" ]]; then + printf 'Orca: OMP cannot start because no terminal working directory is available. Open a new terminal in an existing directory.\n' >&2 + return 1 + fi + if ! builtin cd -P -- "$__orca_logical_cwd" 2>/dev/null; then + printf 'Orca: OMP cannot access the terminal working directory "%s". Open a new terminal in an existing directory.\n' "$__orca_logical_cwd" >&2 + return 1 + fi + __orca_omp_invoke "$__orca_use_extension" "$@" + ) + else + __orca_omp_invoke "$__orca_use_extension" "$@" + fi + } if [[ -n "${ORCA_OMP_STATUS_EXTENSION:-}" ]]; then # Why the function reserved word: it suppresses alias expansion of the name, which # an `alias omp` otherwise rewrites at parse time, aborting the rest of the file. diff --git a/src/main/pty/omp-shell-wrapper.node-pty.test.ts b/src/main/pty/omp-shell-wrapper.node-pty.test.ts index e75fc3c6cdb..6ab30dfdbae 100644 --- a/src/main/pty/omp-shell-wrapper.node-pty.test.ts +++ b/src/main/pty/omp-shell-wrapper.node-pty.test.ts @@ -1,5 +1,13 @@ import { spawnSync } from 'node:child_process' -import { chmodSync, mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { + chmodSync, + mkdtempSync, + mkdirSync, + readFileSync, + realpathSync, + rmSync, + writeFileSync +} from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import * as pty from 'node-pty' @@ -8,7 +16,11 @@ import { getPosixOmpShellWrapper } from './omp-shell-wrapper' const describePosix = process.platform === 'win32' ? describe.skip : describe const hasBash = process.platform !== 'win32' && spawnSync('bash', ['--version']).status === 0 +const hasZsh = process.platform !== 'win32' && spawnSync('zsh', ['--version']).status === 0 const itWithBash = hasBash ? it : it.skip +const itWithZsh = hasZsh ? it : it.skip + +type PosixShell = 'bash' | 'zsh' const tempDirs: string[] = [] @@ -31,33 +43,38 @@ fi { printf 'PI=%s\\n' "$PI_CODING_AGENT_DIR" printf 'EFFECTIVE=%s\\n' "$agent_dir" + printf 'CWD=%s\\n' "$(pwd -P)" i=0 for arg in "$@"; do i=$((i + 1)) printf 'ARG%s=%s\\n' "$i" "$arg" done } > "$ORCA_CAPTURE_FILE" +exit "\${ORCA_TEST_FAKE_OMP_EXIT_CODE:-0}" `, { mode: 0o755 } ) chmodSync(ompPath, 0o755) } -async function runInteractiveBashPty(args: { +async function runInteractivePosixPty(args: { rcfileContent: string env: Record input: string cwd: string + shell?: PosixShell }): Promise { const rcfile = join(args.cwd, 'rcfile') writeFileSync(rcfile, args.rcfileContent) + const shell = args.shell ?? 'bash' + const shellArgs = shell === 'bash' ? ['--noprofile', '--rcfile', rcfile, '-i'] : ['-f', '-i'] - const proc = pty.spawn('bash', ['--noprofile', '--rcfile', rcfile, '-i'], { + const proc = pty.spawn(shell, shellArgs, { name: 'xterm-256color', cols: 100, rows: 30, cwd: args.cwd, - env: args.env + env: { ...args.env, ORCA_TEST_RCFILE: rcfile } }) let output = '' @@ -72,13 +89,14 @@ async function runInteractiveBashPty(args: { let timeout: ReturnType | null = null const timeoutPromise = new Promise((_resolve, reject) => { timeout = setTimeout( - () => reject(new Error(`timed out waiting for bash PTY output:\n${output}`)), + () => reject(new Error(`timed out waiting for ${shell} PTY output:\n${output}`)), 5000 ) }) try { - proc.write(args.input.replace(/\n/g, '\r')) + const input = shell === 'zsh' ? `source "$ORCA_TEST_RCFILE"\n${args.input}` : args.input + proc.write(input.replace(/\n/g, '\r')) const { exitCode } = await Promise.race([exitPromise, timeoutPromise]) expect(exitCode).toBe(0) return output @@ -130,7 +148,7 @@ describePosix('OMP shell wrapper node-pty reproduction', () => { const unwrappedCapture = join(tempDir, 'unwrapped-capture') const unwrappedAfterPi = join(tempDir, 'unwrapped-after-pi') - await runInteractiveBashPty({ + await runInteractivePosixPty({ cwd: tempDir, rcfileContent: '', env: makeEnv(unwrappedCapture, unwrappedAfterPi), @@ -148,7 +166,7 @@ exit 0 const wrappedCapture = join(tempDir, 'wrapped-capture') const wrappedAfterPi = join(tempDir, 'wrapped-after-pi') - const wrappedOutput = await runInteractiveBashPty({ + const wrappedOutput = await runInteractivePosixPty({ cwd: tempDir, rcfileContent: getPosixOmpShellWrapper(), env: makeEnv(wrappedCapture, wrappedAfterPi), @@ -182,7 +200,7 @@ exit 0 writeFakeOmp(binDir) const captureFile = join(tempDir, 'config-capture') - await runInteractiveBashPty({ + await runInteractivePosixPty({ cwd: tempDir, rcfileContent: getPosixOmpShellWrapper(), env: { @@ -237,7 +255,7 @@ exit 0 writeFakeOmp(binDir) const captureFile = join(tempDir, `${subcommand}-capture`) - await runInteractiveBashPty({ + await runInteractivePosixPty({ cwd: tempDir, rcfileContent: getPosixOmpShellWrapper(), env: { @@ -280,7 +298,7 @@ exit 0 writeFakeOmp(binDir) const captureFile = join(tempDir, 'default-config-capture') - await runInteractiveBashPty({ + await runInteractivePosixPty({ cwd: tempDir, rcfileContent: getPosixOmpShellWrapper(), env: { @@ -308,4 +326,94 @@ exit 0 ) } ) + + async function expectStaleCwdRecovery(shell: PosixShell): Promise { + const tempDir = makeTempDir() + const workspaceDir = join(tempDir, 'workspace') + const projectDir = join(workspaceDir, 'project') + const homeDir = join(tempDir, 'home') + const binDir = join(tempDir, 'bin') + const extensionDir = join(tempDir, 'extensions') + mkdirSync(projectDir, { recursive: true }) + mkdirSync(homeDir) + mkdirSync(binDir) + mkdirSync(extensionDir) + const expectedProjectDir = realpathSync(projectDir) + const statusExtension = join(extensionDir, 'orca-agent-status.ts') + writeFileSync(statusExtension, 'export default {}') + writeFakeOmp(binDir) + + const unsetPwdCaptureFile = join(tempDir, 'unset-pwd-capture') + const staleCaptureFile = join(tempDir, 'stale-cwd-capture') + const resultFile = join(tempDir, 'stale-cwd-result') + const output = await runInteractivePosixPty({ + shell, + cwd: projectDir, + rcfileContent: `cd() { return 97; } +${getPosixOmpShellWrapper()}`, + env: { + INPUTRC: '/dev/null', + PROMPT_COMMAND: '', + ORCA_STALE_PROJECT_DIR: projectDir, + ORCA_UNSET_PWD_CAPTURE_FILE: unsetPwdCaptureFile, + ORCA_STALE_CAPTURE_FILE: staleCaptureFile, + ORCA_WORKTREE_PATH: workspaceDir, + HOME: homeDir, + PATH: `${binDir}:/usr/bin:/bin:/usr/sbin:/sbin`, + ORCA_OMP_STATUS_EXTENSION: statusExtension, + ORCA_CAPTURE_FILE: staleCaptureFile, + ORCA_RESULT_FILE: resultFile, + ORCA_TEST_FAKE_OMP_EXIT_CODE: '23', + TERM: 'xterm-256color' + }, + input: `ORCA_CAPTURE_FILE="$ORCA_UNSET_PWD_CAPTURE_FILE" +unset PWD +omp +__orca_test_unset_status=$? +if [[ -z "\${PWD+x}" ]]; then + __orca_test_pwd_state=unset +else + __orca_test_pwd_state=set +fi +builtin cd -P -- "$ORCA_STALE_PROJECT_DIR" +ORCA_CAPTURE_FILE="$ORCA_STALE_CAPTURE_FILE" +/bin/rm -rf -- "$ORCA_STALE_PROJECT_DIR" +/bin/mkdir -p -- "$ORCA_STALE_PROJECT_DIR" +omp +__orca_test_first_status=$? +if [[ "$PWD" -ef . ]]; then + __orca_test_parent_state=live +else + __orca_test_parent_state=stale +fi +/bin/rm -rf -- "$ORCA_STALE_PROJECT_DIR" +omp +__orca_test_missing_status=$? +printf 'UNSET=%s\nPWD=%s\nFIRST=%s\nPARENT=%s\nMISSING=%s\n' "$__orca_test_unset_status" "$__orca_test_pwd_state" "$__orca_test_first_status" "$__orca_test_parent_state" "$__orca_test_missing_status" > "$ORCA_RESULT_FILE" +exit 0 +` + }) + + const unsetPwdCapture = readFileSync(unsetPwdCaptureFile, 'utf8') + expect(unsetPwdCapture).toContain(`CWD=${expectedProjectDir}`) + const staleCapture = readFileSync(staleCaptureFile, 'utf8') + expect(staleCapture).toContain(`CWD=${expectedProjectDir}`) + expect(staleCapture.split('\n').filter((line) => line.startsWith('ARG'))).toEqual([ + 'ARG1=--extension', + `ARG2=${statusExtension}` + ]) + expect(staleCapture).not.toContain('--cwd') + expect(readFileSync(resultFile, 'utf8')).toBe( + 'UNSET=23\nPWD=unset\nFIRST=23\nPARENT=stale\nMISSING=1\n' + ) + expect(output).toContain('Orca: OMP cannot access the terminal working directory') + } + + itWithBash('rebinds a stale Bash cwd before launching OMP', async () => { + await expectStaleCwdRecovery('bash') + }) + + itWithZsh('rebinds a stale Zsh cwd before launching OMP', async () => { + await expectStaleCwdRecovery('zsh') + }) }) diff --git a/src/main/pty/omp-shell-wrapper.ts b/src/main/pty/omp-shell-wrapper.ts index be94a9c088f..fb6d274dc57 100644 --- a/src/main/pty/omp-shell-wrapper.ts +++ b/src/main/pty/omp-shell-wrapper.ts @@ -51,9 +51,17 @@ __orca_omp_should_skip_extension() { esac return 1 } -__orca_omp() { - local __orca_use_extension=1 - __orca_omp_should_skip_extension "\${1:-}" && __orca_use_extension=0 +__orca_omp_cwd_is_usable() { + [[ -x . ]] || return 1 + if [[ -n "\${PWD:-}" ]]; then + [[ -d "\${PWD}" && "\${PWD}" -ef . ]] + else + builtin pwd -P >/dev/null 2>&1 + fi +} +__orca_omp_invoke() { + local __orca_use_extension="$1" + shift if [[ $__orca_use_extension -eq 1 && -n "\${ORCA_OMP_STATUS_EXTENSION:-}" && -f "\${ORCA_OMP_STATUS_EXTENSION}" ]]; then if [[ "\${1:-}" == "launch" ]]; then shift @@ -65,6 +73,27 @@ __orca_omp() { command omp "$@" fi } +__orca_omp() { + local __orca_use_extension=1 + __orca_omp_should_skip_extension "\${1:-}" && __orca_use_extension=0 + if [[ $__orca_use_extension -eq 1 ]] && ! __orca_omp_cwd_is_usable; then + local __orca_logical_cwd="\${PWD:-\${ORCA_WORKTREE_PATH:-\${ORCA_ROOT_PATH:-}}}" + # Why: a restored shell can retain the deleted directory inode after its path is recreated. + ( + if [[ -z "$__orca_logical_cwd" ]]; then + printf 'Orca: OMP cannot start because no terminal working directory is available. Open a new terminal in an existing directory.\\n' >&2 + return 1 + fi + if ! builtin cd -P -- "$__orca_logical_cwd" 2>/dev/null; then + printf 'Orca: OMP cannot access the terminal working directory "%s". Open a new terminal in an existing directory.\\n' "$__orca_logical_cwd" >&2 + return 1 + fi + __orca_omp_invoke "$__orca_use_extension" "$@" + ) + else + __orca_omp_invoke "$__orca_use_extension" "$@" + fi +} if [[ -n "\${ORCA_OMP_STATUS_EXTENSION:-}" ]]; then # Why the function reserved word: it suppresses alias expansion of the name, which # an \`alias omp\` otherwise rewrites at parse time, aborting the rest of the file.