Skip to content

fix(cli): prevent TTY hang during command delegation (#1396)#1422

Open
Han5991 wants to merge 7 commits intovoidzero-dev:mainfrom
Han5991:fix/1396-lefthook-tty-hang
Open

fix(cli): prevent TTY hang during command delegation (#1396)#1422
Han5991 wants to merge 7 commits intovoidzero-dev:mainfrom
Han5991:fix/1396-lefthook-tty-hang

Conversation

@Han5991
Copy link
Copy Markdown
Contributor

@Han5991 Han5991 commented Apr 20, 2026

Resolves #1396

Description

Commands like vp run or vp test running under background/reporter environments (e.g., lefthook with certain Node.js versions) would hang due to query_terminal_colors() blocking on /dev/tty.

The root cause was apply_custom_help() eagerly evaluating the vite_plus_header() (and its OSC queries) to build the clap help template during argument parsing, prior to command dispatch.

This commit:

  • Removes the eager header evaluation from clap help templates.
  • Prints the dynamic vite_plus_header() lazily only on ErrorKind::DisplayHelp.
  • Explicitly passes dynamic_colors flags into print_runtime_header(), allowing delegate commands to safely use vite_plus_header_static() right before execution without blocking on TTY queries.

@netlify
Copy link
Copy Markdown

netlify bot commented Apr 20, 2026

Deploy Preview for viteplus-preview canceled.

Name Link
🔨 Latest commit 22978f1
🔍 Latest deploy log https://app.netlify.com/projects/viteplus-preview/deploys/69e63fa5ea28f50008abdcbe

@Han5991 Han5991 force-pushed the fix/1396-lefthook-tty-hang branch from 3dc99eb to e965560 Compare April 20, 2026 07:11
Comment thread packages/cli/snap-tests-global/cli-helper-message/snap.txt Outdated
@fengmk2
Copy link
Copy Markdown
Member

fengmk2 commented Apr 20, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9a5ccb10a8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/vite_global_cli/src/cli.rs Outdated
@Han5991 Han5991 requested a review from fengmk2 April 20, 2026 10:06
@fengmk2
Copy link
Copy Markdown
Member

fengmk2 commented Apr 20, 2026

@codex review

@fengmk2
Copy link
Copy Markdown
Member

fengmk2 commented Apr 20, 2026

@Han5991 The amount of changes currently is a bit too large. Why wasn't the solution mentioned here #1396 (comment) considered?

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@Han5991
Copy link
Copy Markdown
Contributor Author

Han5991 commented Apr 20, 2026

@Han5991 The amount of changes currently is a bit too large. Why wasn't the solution mentioned here #1396 (comment) considered?

You're right that the earlier change set was broader than it needed to be.

I did try the Option 1 approach from #1396 by adding the foreground-process-group check in
crates/vite_shared/src/header.rs, but it did not cover this repro. In the child-PTY setup
here, lefthook spawns vp on a separate TTY, and from vp's perspective it is still
attached to a terminal and is also the foreground process group for that child TTY. In
other words, stdin/stdout are terminals and tcgetpgrp(stdin) == getpgrp(), so the
process-group guard does not fire even though the OSC /dev/tty round-trip can still
block.

I also should have evaluated Option 2 from that comment earlier. I did not give that path
enough weight up front, and instead spent time on CLI-side workaround changes that made the
diff larger than necessary.

I've since dropped those CLI-side header workarounds and kept the fix localized to crates/ vite_shared/src/header.rs.

The current change uses the Option 2-style approach instead: the terminal color query now
runs in a worker thread, and the main path only waits for a short wall-clock timeout before
falling back to the default header colors. That way, even if the /dev/tty query gets
stuck under a lefthook PTY, command dispatch is no longer blocked by it.

Han5991 added 7 commits April 21, 2026 00:00
…r header evaluation (voidzero-dev#1396)

Commands like `vp run` or `vp test` running under background/reporter environments (e.g., lefthook with certain Node.js versions) would hang due to `query_terminal_colors()` blocking on `/dev/tty`.

The root cause was `apply_custom_help()` eagerly evaluating the `vite_plus_header()` (and its OSC queries) to build the clap help template during argument parsing, prior to command dispatch.

This commit:

- Removes the eager header evaluation from clap help templates.

- Prints the dynamic `vite_plus_header()` lazily only on `ErrorKind::DisplayHelp`.

- Explicitly passes `dynamic_colors` flags into `print_runtime_header()`, allowing delegate commands to safely use `vite_plus_header_static()` right before execution without blocking on TTY queries.
@Han5991 Han5991 force-pushed the fix/1396-lefthook-tty-hang branch from c5a03ba to 22978f1 Compare April 20, 2026 15:00
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.

vp hangs when spawned by lefthook (opens /dev/tty)

2 participants