chore(embed): tidy detach-popen helper and close log fds in parent#1418
Merged
nicoloboschi merged 2 commits intomainfrom May 4, 2026
Merged
chore(embed): tidy detach-popen helper and close log fds in parent#1418nicoloboschi merged 2 commits intomainfrom
nicoloboschi merged 2 commits intomainfrom
Conversation
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.
- 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.
2 tasks
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
1 participant
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.
Summary
Follow-up cleanup to #1380 (which made the daemon redirect stdout/stderr to a log file on POSIX, fixing TUI corruption).
log_handle is Nonebranch in_detach_popen_kwargs. After fix: redirect daemon subprocess stdout/stderr on POSIX to prevent TUI corruption #1380, both call sites always pass a handle, so the inherit-fd branch is unreachable. Type the parameter asIO[bytes]and trim the docstring to match reality._start_daemon_locked) and the UI spawn (start_ui) inwith open(...)blocks. Popen dups the fd into the child during spawn, so the parent's handle is dead weight after Popen returns — closing it stops the small per-spawn fd leak that pre-existed both PRs.test_posix_popen_redirects_stdout_stderr_to_logasserts that on POSIX the daemon's stdout/stderr go to a real file handle (notNone/inherited). The previous regression had no test guarding it.No behavior change for end users — purely tidy-up plus a guardrail.
Test plan
uv run pytest tests/test_profile_daemon_config.py tests/test_daemon_client.py(26 passed)./scripts/hooks/lint.sh(clean)