Skip to content

fix: redirect daemon subprocess stdout/stderr on POSIX to prevent TUI corruption#1380

Merged
nicoloboschi merged 1 commit into
vectorize-io:mainfrom
laoli-no1:fix/redirect-daemon-stdout-posix
May 4, 2026
Merged

fix: redirect daemon subprocess stdout/stderr on POSIX to prevent TUI corruption#1380
nicoloboschi merged 1 commit into
vectorize-io:mainfrom
laoli-no1:fix/redirect-daemon-stdout-posix

Conversation

@laoli-no1

Copy link
Copy Markdown
Contributor

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:

  • Python package download/install progress lines appear during daemon startup (uvx, pip)
  • Python library init messages (torch, sentence-transformers, etc.)
  • Rich UI frames from daemon manager
  • These outputs contain ANSI escape sequences that interfere with terminal rendering
  • TUI input bar layout corruption, timer display misalignment

Root Cause

The code had platform-specific behavior:

  • Windows: Redirected stdout/stderr to daemon_log (mandatory, since DETACHED_PROCESS removes the console)
  • POSIX: Inherited parent's fds ("legacy daemon-spawn 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:
    • Removed the Windows/POSIX if-else branch in _start_daemon_locked()
    • Always open daemon_log and pass it to _detach_popen_kwargs()
    • Updated docstring to reflect that log_handle=None is no longer the default for daemon spawns
    • Updated code comment to explain the rationale

Testing

  • Manual test on macOS with Hermes TUI: daemon startup no longer causes UI corruption
  • The daemon log file captures all subprocess output (uvx progress, Python init, Rich UI)
  • Existing behavior unchanged: Python's own logging (via HINDSIGHT_API_DAEMON_LOG) still appends to the same file

… 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.
@nicoloboschi
nicoloboschi merged commit 4c28e66 into vectorize-io:main May 4, 2026
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.
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.

2 participants