Skip to content

Fix namespace transport after background process exit - #588

Merged
jdchawla29 merged 1 commit into
mainfrom
codex/namespace-pipe-lifecycle
Aug 16, 2026
Merged

Fix namespace transport after background process exit#588
jdchawla29 merged 1 commit into
mainfrom
codex/namespace-pipe-lifecycle

Conversation

@jdchawla29

@jdchawla29 jdchawla29 commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Route namespace-host standard streams through AsyncSSH-owned raw pipe transports.
  • Make descriptor and process-group cleanup explicit for non-TTY, PTY, redirect failure, and cancellation paths.
  • Add regressions for inherited EOF, delayed output, stdin-only detachment, fully redirected background jobs, and standard stream forwarding.

Root cause

Namespace-host commands used asyncio subprocess streams as AsyncSSH redirect sources. ProcessGroup.wait() intentionally returns when the process leader exits, while a background descendant may still hold those streams. HUD then closes the SSH channel. When the descendant later writes or closes its inherited stream, AsyncSSH's delayed stream task targets an already-closed channel and closes the shared internal SSH connection. All later workspace commands then fail without an exit status.

Raw descriptors select AsyncSSH's pipe transport path, whose lifetime is owned by the SSH channel and safely closed when the channel exits. Models do not need to add </dev/null or rewrite daemon grammar to keep later workspace commands healthy.

Validation

  • uv run --python 3.11 --with='.[dev]' pytest -q hud/environment/tests/test_workspace.py hud/utils/tests/test_process.py — 78 passed
  • uv run --python 3.12 --with='.[dev]' pytest -q — 1028 passed, 9 deselected
  • uv run --python 3.12 --extra dev --extra train ty check --error-on-warning
  • uv run --with='.[dev]' ruff format hud/environment/namespace.py hud/environment/tests/test_workspace.py --check
  • uv run --with='.[dev]' ruff check hud/environment/namespace.py hud/environment/tests/test_workspace.py
  • Exact Modal base-image reproduction: the background wrapper completed and eight later commands succeeded on the same cached connection.

Note

High Risk
Changes core namespace-host process spawning and AsyncSSH I/O wiring; a bug could break all workspace commands or leak FDs, though regressions target the reported connection-death scenario.

Overview
Fixes workspace sessions dying after a persistent namespace command leaves background children holding inherited stdio.

namespace.py no longer wires AsyncSSH to asyncio subprocess pipe objects. Non-TTY spawns use os.pipe() and hand the SSH channel raw read ends via redirect_stdin / redirect_stdout / redirect_stderr (send_eof=False), so channel teardown owns transport lifetime even when the process leader exits early. PTY paths use the same redirect API with explicit dup/close handling. Spawn and wait paths now terminate the process group on errors or cancellation and close every FD in finally.

test_workspace.py adds _connected_namespace_host to exercise the real namespace host without full bubblewrap/nsenter, plus regressions for background jobs (inherited EOF, late output, stdin detachment, full redirection) and stdin/stdout/stderr forwarding.

Reviewed by Cursor Bugbot for commit 5bfc986. Bugbot is set up for automated code reviews on this repo. Configure here.

@jdchawla29
jdchawla29 marked this pull request as ready for review August 16, 2026 00:00

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 5bfc986. Configure here.

Comment thread hud/environment/namespace.py
@jdchawla29
jdchawla29 merged commit 5c6194b into main Aug 16, 2026
8 checks passed
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