Repository navigation
fix(terminal): complete OMP stale cwd recovery - #17154
Conversation
Greptile SummaryThe PR completes POSIX OMP stale-working-directory recovery while preserving the parent shell’s state.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| src/main/pty/omp-shell-wrapper.ts | Extends stale-cwd validation to all wrapped POSIX OMP commands and verifies physical cwd paths before direct invocation. |
| src/main/pty/omp-shell-wrapper.node-pty.test.ts | Adds real Bash and Zsh PTY scenarios covering stale inodes, removed symlinks, unset PWD, fallback selection, parent-shell isolation, argv, and status propagation. |
| config/reliability-gates.jsonc | Updates the terminal cwd recovery invariant, oracle, evidence, and known coverage gaps to reflect the expanded behavior. |
| src/main/fixtures/shell-wrapper-snapshots/local-bash-rcfile.txt | Regenerates the local Bash wrapper fixture with the shared cwd validation and recovery logic. |
| src/main/fixtures/shell-wrapper-snapshots/local-zsh-zshenv.txt | Regenerates the local Zsh wrapper fixture with the shared cwd validation and recovery logic. |
| src/main/fixtures/shell-wrapper-snapshots/daemon-bash-rcfile.txt | Regenerates the daemon and SSH Bash fixture with the shared wrapper behavior. |
| src/main/fixtures/shell-wrapper-snapshots/daemon-zsh-zshenv.txt | Regenerates the daemon and SSH Zsh fixture with the shared wrapper behavior. |
| src/main/fixtures/shell-wrapper-snapshots/relay-bash-rcfile.txt | Regenerates the relay Bash fixture with the shared wrapper behavior. |
| src/main/fixtures/shell-wrapper-snapshots/relay-zsh-zshenv.txt | Regenerates the relay Zsh fixture with the shared wrapper behavior. |
Reviews (4): Last reviewed commit: "fix(terminal): complete OMP stale cwd re..." | Re-trigger Greptile
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between fd027a5deafe6f1297b08b5b527b39a2014c5f3d and 4294ae474c260b39d5512ad6822efaeba5a41483. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughShell-wrapper cwd validation now checks logical and physical paths for directory existence and inode identity. Recovery now runs for all OMP invocations, including extension-skipping commands. PTY coverage adds symlink, stale-directory, unset- Merge Risk: ⚪ Minimal · up to This change completes stale working-directory recovery for wrapped terminal commands, with reported coverage for the affected Bash and Zsh cases and passing checks. No actionable merge-blocking risk remains beyond normal review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (1 skipped: 1 unsupported.) Full details: Out of Scope Changes checkExplanation The changes remain focused on POSIX OMP stale-cwd recovery, related shell fixtures, reliability gates, and PTY tests. No unrelated product, protocol, dependency, or native PowerShell changes are included.
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. Comment |
4294ae4 to
d387f3e
Compare
d387f3e to
834b8cf
Compare
ELI5
An Orca terminal can keep standing in an old folder object after the project path is deleted and recreated. The prompt still shows the familiar path, but OMP cannot determine its current directory and exits before doing any work.
Orca now checks the shell's real directory before every wrapped OMP command. If the path was replaced, OMP runs from the live replacement in a temporary subshell; the user's parent shell stays untouched. If only a logical symlink disappeared but the physical directory is still usable, Orca leaves it alone.
What Changed
omp --versionthat intentionally skip the status extension.pwd -Poutput unless the path still exists and names the shell's current directory.PWDis unset or names a removed logical symlink.Why
The original recovery in #17128 left two real failure modes: extension-skipping commands bypassed recovery, and shell builtins could report a cached deleted path as though the cwd were usable. A narrow cwd predicate plus a recovery subshell closes those gaps without changing OMP argv, extension selection, exit status, or parent-shell state.
Linked Issue
Fixes #16457
Follow-up to #17128.
Visual Proof
N/A — this changes terminal cwd/process behavior, not product layout or visual design. The real rendered terminal flow is covered under Testing.
Testing
I manually tested these changes locally
Automated tests added/updated, or explained why not below
Dedicated Orca Electron instance attached to this branch:
omp/18.0.10 --versionexited 0 from a replaced cwd while the parent shell remained on the stale inodePWD+ no workspace/root fallback showed the actionable Orca error and returned 1 without starting OMP10 focused test files: 151/151 tests passed.
Bash 3.2 and Zsh 5.9 real-PTY wrapper tests: 20/20 passed.
All six generated local/daemon/relay Bash/Zsh fixtures passed native syntax checks; the shared wrapper passed ShellCheck.
pnpm tc:nodepnpm run check:code-quality:changedpnpm run check:max-lines-ratchetpnpm run check:reliability-gatesFull macOS, Windows, shell-contract, typecheck, static-analysis, and eight-shard test coverage is enforced by PR CI.
AI Disclosure
Review
terminal-session.startup-cwd-missing-dir-recovery— recovery may rebind only the child OMP invocation; parent cwd/PWD, argv, extension behavior, and exit status remain authoritative.PWD, skipped and extension-enabled argv, nonzero status propagation, parent-shell isolation, and visible failure without invocation when no recovery path exists.Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Residual execution gaps are explicit in the experimental reliability gate: no live Linux/WSL or SSH/relay stale-inode journey was run. Those topologies consume the same execution-host POSIX wrapper pinned by generated-file snapshots. Native Windows/PowerShell does not use this cwd predicate.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)