Skip to content

fix(staged): detach env-capture shell from user TTY#756

Merged
matt2e merged 2 commits into
mainfrom
terminal-killed
May 28, 2026
Merged

fix(staged): detach env-capture shell from user TTY#756
matt2e merged 2 commits into
mainfrom
terminal-killed

Conversation

@matt2e
Copy link
Copy Markdown
Contributor

@matt2e matt2e commented May 28, 2026

Summary

The interactive $SHELL -ils spawned for environment capture inherited the staged binary's controlling TTY (the user's real /dev/tty). On exit, the shell could tcsetpgrp or mutate terminal modes against that TTY, leaving the foreground process group pointing at a dead PGID — which caused the outer zsh to later die with EIO on read ("terminal killed").

Fix: call setsid() in pre_exec to detach the spawned shell into its own session with no controlling terminal. Applied to both the async (capture_shell_env) and blocking (capture_shell_env_blocking) paths.

Test plan

  • Run staged from a terminal, trigger env capture (e.g. add a repo / open a project), then continue using the parent shell — confirm it no longer dies with EIO.
  • Verify env capture still returns the expected variables.

matt2e and others added 2 commits May 28, 2026 13:26
`capture_shell_env`/`capture_shell_env_blocking` spawned `$SHELL -ils`
inheriting the parent's controlling terminal. The interactive shell
then `tcsetpgrp`'d the user's real /dev/tty and, on exit, left the
foreground process group pointing at a dead PGID — causing the outer
zsh to die with EIO on TTY read after `just dev` had been running.

Add `setsid()` via `pre_exec` to both spawn paths so the child runs in
a new session with no controlling terminal, matching the canonical
pattern in `crates/builderbot-actions/src/executor.rs`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
`tokio::process::Command` exposes `pre_exec` as an inherent unsafe
method on Unix, so the `use std::os::unix::process::CommandExt;` line
added in 4c51a61 was dead. The sibling sync path still needs the import
because `std::process::Command::pre_exec` only exists via the trait.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
@matt2e matt2e requested review from baxen and wesbillman as code owners May 28, 2026 07:02
@matt2e matt2e merged commit 6ee2eb8 into main May 28, 2026
5 checks passed
@matt2e matt2e deleted the terminal-killed branch May 28, 2026 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant