mirror of
https://github.com/zed-industries/zed.git
synced 2026-08-24 00:16:18 +00:00
8e18ab0cd7
# Objective This is a follow-up to my own PR #59270 which fixed the hang it targeted (#55042) only partially and broke `ShellBuilder::build` for fish. **The hang is only half fixed.** The agent's terminal tool wraps commands as `sh -i -c 'exec </dev/null; <command>'`. The redirect sits on the same line as the command, but POSIX shells parse an entire line before executing any of it. So when `<command>` has a syntax error, the whole line is discarded — including the `exec` that was supposed to close stdin. Interactive `dash` then recovers from the error, finds stdin still attached to the PTY, and sits at a `$` prompt until the user presses Ctrl+D. That means #59270 only helped when the redirect had already run, i.e. multi-line commands whose error is on a later line. A single-line command still hangs, including `cat <(echo hi)`, the example from that PR's own description. Probably prompts or tool definitions changed since that PR landed which now make it more likely again to run into this issue, at least with Anthropic models. **fish runs no command at all.** #59270 also applied the `exec </dev/null; …` form to fish, replacing its `begin; …; end </dev/null` wrapper. But fish's `exec` requires a command to replace the shell with, so a bare `exec </dev/null` is a usage error: fish prints `exec`'s help text, skips the rest of the command, and exits 0. ## Solution For POSIX shells, put the redirect on its own line: ```sh sh -i -c 'exec </dev/null <command>' ``` That makes it a separate parse-and-execute unit, so it runs before the command line is parsed. Any syntax error is then reported by a shell that already has `/dev/null` on stdin, and it exits instead of prompting. For fish, go back to `begin; <command>; end </dev/null`. fish exits on a parse error even when interactive, so it never needed the `exec` form to begin with. Only the `Posix` and `Fish` arms change; the other shells' redirects are untouched. Note this is still a targeted fix for `dash`. The underlying reason a syntax error becomes an interactive prompt is that agent terminals are built with `-i` at all — non-interactive `dash` exits cleanly even with no redirect. Dropping `-i` would remove the whole class of hang, but it would also stop sourcing users' interactive shell config in agent terminals. ## Testing `cargo test -p util --lib shell_builder` covers the changed command assembly. Reviewers can check the shell behavior directly on a PTY, without building Zed: ```sh # hangs at a `$` prompt until Ctrl+D script -qec "sh -i -c 'exec </dev/null; echo hi ;;'" /dev/null # exits immediately with just the syntax error script -qec "sh -i -c 'exec </dev/null echo hi ;;'" /dev/null ``` ```sh fish -i -c 'exec </dev/null; echo test' # prints exec's help, no "test", exit 0 fish -i -c 'begin; echo test; end </dev/null' # prints "test" fish -i -c 'if true' # exit 127, no hang ``` macOS is unaffected in practice because `/bin/sh` there is bash, which exits on a syntax error even with `-i`; Windows goes through the `Cmd`/`PowerShell` arms, which this doesn't touch. A `zsh`-as-`/bin/sh` setup is likewise unaffected. ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable --- Release Notes: - Fixed agent terminal commands hanging at a shell prompt after a syntax error |
||
|---|---|---|
| .. | ||
| src | ||
| Cargo.toml | ||
| LICENSE-APACHE | ||