agent: Release completed terminal PTY resources - #62978
Conversation
6501548 to
b83d95a
Compare
93f7cb1 to
c012a0a
Compare
|
FYI this is taking down Zed (and my entire OS) pretty hard every 1-2 hours on a normal working day. It would be great to close up this hole. Happy to iterate on the solution or do feel free to take this in a completely different direction. |
c012a0a to
58255ba
Compare
Completed agent terminal entities remain alive to render conversation history, which also retained the PTY sender and its kqueue descriptor. Release live PTY resources after capturing final output while preserving the terminal buffer, process metadata, and working-directory behavior. Strengthen terminal lifecycle types so task terminals always have paired completion state and released PTY resources cannot be treated as live. This makes the invalid states behind the regression unrepresentable. Closes zed-industries#62972
58255ba to
3e32407
Compare
SomeoneToIgnore
left a comment
There was a problem hiding this comment.
Thank you for the nice find!
If I understand correctly, this change also moves the killpg(SIGTERM) from entity drop to the moment the command completes: background children left in the shell's process group (e.g. nohup server &) are now terminated as soon as the foreground command exits, where before they survived for the whole session.
Seems legit — it matches the terminal tool's "no indefinite commands" contract and the existing timeout/cancel kill paths — but wanted to raise it in the comments in case we need to come back to this decision.
Closes zed-industries#62972 Completed Agent terminal cards remain alive so conversation history can render their output. Each retained terminal also kept its Alacritty PTY sender alive, which retained a poller and one kqueue descriptor after the command exited. Terminal lifecycle state is now more strongly typed to prevent this class of regression. Task terminals are created through a sealed `TerminalMode` API that pairs their running state with the correct completion channel, while PTY resources use explicit active and released states. This makes invalid lifecycle combinations unrepresentable instead of relying on callers to coordinate independent `Option` values correctly. - Release the live PTY sender after final terminal output is captured. - Preserve PTY process metadata and working-directory behavior for historical terminal cards. - Represent PTY resources as explicit active or released lifecycle states. - Construct interactive and task terminals through typed modes so a task cannot omit its running state or use a mismatched completion channel. - Keep resource release idempotent for later ACP release and entity destruction. - Add a macOS regression test that retains the historical terminal entity while exercising the complete sandboxed terminal tool path. Release Notes: - Fixed Agent terminal tool calls leaking file descriptors on macOS. Co-authored-by: Kirill Bulatov <kirill@zed.dev>
Closes zed-industries#62972 Completed Agent terminal cards remain alive so conversation history can render their output. Each retained terminal also kept its Alacritty PTY sender alive, which retained a poller and one kqueue descriptor after the command exited. Terminal lifecycle state is now more strongly typed to prevent this class of regression. Task terminals are created through a sealed `TerminalMode` API that pairs their running state with the correct completion channel, while PTY resources use explicit active and released states. This makes invalid lifecycle combinations unrepresentable instead of relying on callers to coordinate independent `Option` values correctly. - Release the live PTY sender after final terminal output is captured. - Preserve PTY process metadata and working-directory behavior for historical terminal cards. - Represent PTY resources as explicit active or released lifecycle states. - Construct interactive and task terminals through typed modes so a task cannot omit its running state or use a mismatched completion channel. - Keep resource release idempotent for later ACP release and entity destruction. - Add a macOS regression test that retains the historical terminal entity while exercising the complete sandboxed terminal tool path. Release Notes: - Fixed Agent terminal tool calls leaking file descriptors on macOS. Co-authored-by: Kirill Bulatov <kirill@zed.dev>
Closes #62972
Completed Agent terminal cards remain alive so conversation history can render their output. Each retained terminal also kept its Alacritty PTY sender alive, which retained a poller and one kqueue descriptor after the command exited.
Terminal lifecycle state is now more strongly typed to prevent this class of regression. Task terminals are created through a sealed
TerminalModeAPI that pairs their running state with the correct completion channel, while PTY resources use explicit active and released states. This makes invalid lifecycle combinations unrepresentable instead of relying on callers to coordinate independentOptionvalues correctly.Release Notes: