fix(pty): avoid exiting ordinary fish shells - #1102
Conversation
📝 WalkthroughWalkthroughThe Fish initialization script now returns from sourced execution when guards fail. Tests verify that ordinary shells survive before running terminal-specific prompt checks. ChangesFish initialization behavior
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/src/modules/pty/scripts/init.test.fish`:
- Around line 3-6: Update the child Fish command in the ordinary-shell check
around `init.fish` so it exits immediately when `source $argv[1]` fails, before
emitting `ordinary-shell-survived`; keep the existing output assertion and
environment setup unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1f13c3d5-1650-4752-a3b5-b6eaa0ac8271
📒 Files selected for processing (2)
src-tauri/src/modules/pty/scripts/init.fishsrc-tauri/src/modules/pty/scripts/init.test.fish
| set -l ordinary (env -u TERAX_TERMINAL fish --no-config -c 'source $argv[1]; echo ordinary-shell-survived' -- "$script_dir/init.fish") | ||
| test "$ordinary" = ordinary-shell-survived; or exit 1 | ||
|
|
||
| set -gx TERAX_TERMINAL 1 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fail the child when source fails.
The assertion checks only the captured output. Fish can continue to echo ordinary-shell-survived after source returns a non-zero status, so a broken init.fish can pass this check. Make the child exit before printing the sentinel:
Proposed test hardening
- set -l ordinary (env -u TERAX_TERMINAL fish --no-config -c 'source $argv[1]; echo ordinary-shell-survived' -- "$script_dir/init.fish")
+ set -l ordinary (env -u TERAX_TERMINAL fish --no-config -c 'source $argv[1]; or exit 1; echo ordinary-shell-survived' -- "$script_dir/init.fish")📝 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.
| set -l ordinary (env -u TERAX_TERMINAL fish --no-config -c 'source $argv[1]; echo ordinary-shell-survived' -- "$script_dir/init.fish") | |
| test "$ordinary" = ordinary-shell-survived; or exit 1 | |
| set -gx TERAX_TERMINAL 1 | |
| set -l ordinary (env -u TERAX_TERMINAL fish --no-config -c 'source $argv[1]; or exit 1; echo ordinary-shell-survived' -- "$script_dir/init.fish") | |
| test "$ordinary" = ordinary-shell-survived; or exit 1 | |
| set -gx TERAX_TERMINAL 1 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src-tauri/src/modules/pty/scripts/init.test.fish` around lines 3 - 6, Update
the child Fish command in the ordinary-shell check around `init.fish` so it
exits immediately when `source $argv[1]` fails, before emitting
`ordinary-shell-survived`; keep the existing output assertion and environment
setup unchanged.
1 participant
What
Return from the globally sourced Fish integration file instead of exiting the whole shell when Terax integration is not active.
Why
Opening a Terax Fish terminal installs
terax.fishin Fish's globalconf.ddirectory. A later ordinary Fish shell sources that file withoutTERAX_TERMINAL; the formerexit 0terminates that shell before it can run user commands or configuration.How
Use Fish's
returnin the two guard branches, then exercise an ordinaryfish --no-configprocess in the existing Fish harness.Testing
pnpm lintexits successfully; the repository reports 103 existing warnings in untouched frontend filespnpm check-typespnpm test(547 tests)cargo clippy --all-targets --locked -- -D warningscargo nextest run --locked(276 tests)git diff --checkScreenshots / GIFs
Not applicable. No UI change.
Notes for reviewer
The guards still skip all Terax prompt setup outside a Terax terminal. They now leave the calling Fish shell running.
Summary by CodeRabbit
Bug Fixes
Tests