fix: drop -i flag from login shell PATH resolution to prevent SIGTTOU#8836
fix: drop -i flag from login shell PATH resolution to prevent SIGTTOU#8836michaelneale wants to merge 1 commit into
Conversation
The resolve_login_shell_path() function spawns a shell with -l -i -c to recover the user's PATH. The -i (interactive) flag causes zsh to call tcsetpgrp(), stealing the foreground process group from the parent goose process. When rustyline later calls tcsetattr to enter raw mode, goose is in a background process group and receives SIGTTOU, suspending the CLI immediately after the banner prints. Since commit b7aea3d ('warm shell PATH lookup during session init'), this shell is spawned eagerly at startup rather than lazily on first shell command, making the race condition much more likely to trigger. The -l (login) flag alone is sufficient to source .zprofile/.bash_profile where PATH modifications are typically configured. Signed-off-by: Michael Neale <[email protected]>
|
@jamadeo I am not sure if this is just my pathological env, but was consistently an issue for me just today until -i was removed (as would time out and leave goose orphaned somehow each time) - might not be right solution though. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bcc2955009
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // `-l` (login) is sufficient to source profile files where PATH is set. | ||
| std::process::Command::new(&shell) | ||
| .args(["-l", "-i", "-c", "echo $PATH"]) | ||
| .args(["-l", "-c", "echo $PATH"]) |
There was a problem hiding this comment.
Switching the probe to -l -c only means shells that split startup files by mode (notably zsh) no longer load interactive rc files like .zshrc, where many users keep PATH additions for tools such as nvm/pyenv/asdf; in GUI/desktop launches with a minimal inherited PATH, this can make MCP subprocesses fail to find expected binaries after this commit. Please keep the SIGTTOU fix but add a safe fallback path-resolution strategy that preserves interactive PATH entries when login-only output is incomplete.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
yeah this is true - so I am confused as to what we should do.
|
My first experience with Goose was it just crashes on start up and even takes down my shell (Konsole on KDE), so I came across your PR. Edit: I think it might possibly be due to fish shell being my login shell and the |
- QwenLM/qwen-code#3642: BackgroundShellRegistry + /bashes (merge-after-nits) - cline/cline#10384: maxRetryAfter cap with test-cleanup hazard (request-changes) - aaif-goose/goose#8836: drop -i flag SIGTTOU fix (merge-as-is)
|
merged #8804 which should fix this |
3 participants
Category: fix
User Impact: The CLI no longer randomly suspends with "suspended (tty output)" when starting a session.
Problem: Since #8631 ("warm shell PATH lookup during session init"), goose eagerly spawns
zsh -l -i -c "echo $PATH"during startup to resolve the user's PATH. The-i(interactive) flag causes zsh to calltcsetpgrp(), stealing the foreground process group from goose. When rustyline then callstcsetattrto enter raw mode for the input prompt, goose is in a background process group and the kernel sends SIGTTOU, immediately suspending the CLI.Solution: Drop the
-iflag from both call sites. The-l(login) flag alone is sufficient to source.zprofile/.bash_profilewhere PATH is configured. Interactive mode is unnecessary and actively harmful when the spawned shell shares a controlling terminal with the parent process.File changes
crates/goose/src/agents/platform_extensions/developer/shell.rs
Removed
-ifrom the shell args inresolve_login_shell_path()for both the flatpak and normal code paths. Added a comment explaining why interactive mode must not be used.crates/goose-mcp/src/subprocess.rs
Same fix for the equivalent
resolve_login_shell_path()used by MCP extension subprocesses.Reproduction Steps
cargo build --release -p goose-cli./target/release/goosesuspended (tty output)