fix: redirect daemon subprocess stdout/stderr on POSIX to prevent TUI corruption#1380
Merged
nicoloboschi merged 1 commit intoMay 4, 2026
Conversation
… corruption On POSIX, the daemon subprocess previously inherited the parent process's stdout/stderr file descriptors. When running inside a TUI (e.g. Hermes terminal UI) that uses stdio pipes for JSON-RPC communication, any output from the daemon subprocess (uvx download progress, Python library init messages, Rich UI frames) would leak into the parent's terminal, corrupting the Ink UI rendering. This change makes POSIX behavior consistent with Windows (which already redirected to daemon_log) and the existing UI-spawn path, by always passing a log_handle to _detach_popen_kwargs. Fixes: daemon output leaking into TUI, causing input bar misalignment and timer display corruption.
3 tasks
nicoloboschi
added a commit
that referenced
this pull request
May 4, 2026
…1418) * chore(embed): tidy detach-popen helper and close log fds in parent Follow-up to #1380. With the POSIX inherit-fd path gone, `log_handle` is always supplied — drop the dead `None` branch in `_detach_popen_kwargs`, type the parameter, and refresh the docstring. Wrap the daemon and UI log opens in `with` blocks so the parent's copy of the fd is released once Popen has dup'd it into the child. Add a regression test that locks down POSIX stdout/stderr redirection so future refactors don't silently re-introduce the TUI-corruption regression. * chore: apply pending lint formatter and uv.lock sync - Drop trailing commas in api.ts that the project formatter rewrites. - Refresh uv.lock to resolve opentelemetry-* against the raised floors introduced in #1373 (`1.41.0` / `0.62b1`). Both fall out of running `./scripts/hooks/lint.sh` on a clean checkout and are unrelated to the embed-detach cleanup in this PR — bundling them so the working tree stays clean after lint.
liling
pushed a commit
to liling/hindsight
that referenced
this pull request
May 5, 2026
… corruption (vectorize-io#1380) On POSIX, the daemon subprocess previously inherited the parent process's stdout/stderr file descriptors. When running inside a TUI (e.g. Hermes terminal UI) that uses stdio pipes for JSON-RPC communication, any output from the daemon subprocess (uvx download progress, Python library init messages, Rich UI frames) would leak into the parent's terminal, corrupting the Ink UI rendering. This change makes POSIX behavior consistent with Windows (which already redirected to daemon_log) and the existing UI-spawn path, by always passing a log_handle to _detach_popen_kwargs. Fixes: daemon output leaking into TUI, causing input bar misalignment and timer display corruption. Co-authored-by: Li Lao <[email protected]>
liling
pushed a commit
to liling/hindsight
that referenced
this pull request
May 5, 2026
…ectorize-io#1418) * chore(embed): tidy detach-popen helper and close log fds in parent Follow-up to vectorize-io#1380. With the POSIX inherit-fd path gone, `log_handle` is always supplied — drop the dead `None` branch in `_detach_popen_kwargs`, type the parameter, and refresh the docstring. Wrap the daemon and UI log opens in `with` blocks so the parent's copy of the fd is released once Popen has dup'd it into the child. Add a regression test that locks down POSIX stdout/stderr redirection so future refactors don't silently re-introduce the TUI-corruption regression. * chore: apply pending lint formatter and uv.lock sync - Drop trailing commas in api.ts that the project formatter rewrites. - Refresh uv.lock to resolve opentelemetry-* against the raised floors introduced in vectorize-io#1373 (`1.41.0` / `0.62b1`). Both fall out of running `./scripts/hooks/lint.sh` on a clean checkout and are unrelated to the embed-detach cleanup in this PR — bundling them so the working tree stays clean after lint.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On POSIX, the daemon subprocess spawned by
DaemonEmbedManager._start_daemon_locked()inherited the parent process's stdout/stderr file descriptors. When running inside a TUI (e.g. Hermes terminal UI) that uses stdio pipes for JSON-RPC communication, any output from the daemon subprocess leaks into the parent's terminal, corrupting the Ink UI rendering.Symptoms observed:
Root Cause
The code had platform-specific behavior:
On POSIX, the child process inherits the parent's file descriptors. When the parent is a gateway process communicating via stdio pipes (TUI mode), any write from the child to stdout/stderr goes through those pipes and corrupts the JSON-RPC protocol or TUI rendering.
Fix
Make POSIX behavior consistent with Windows by always passing a log_handle to
_detach_popen_kwargs(). The daemon's stdout/stderr are now redirected to daemon_log on both platforms.This matches the existing UI-spawn behavior (line 625), which already uses
_detach_popen_kwargs(log_file).Changes
hindsight-embed/hindsight_embed/daemon_embed_manager.py:_start_daemon_locked()_detach_popen_kwargs()Testing