Skip to content

fix: detect statusline width from controlling terminal#510

Open
linziyanleo wants to merge 2 commits into
jarrodwatts:mainfrom
linziyanleo:fix/tty-width-detection
Open

fix: detect statusline width from controlling terminal#510
linziyanleo wants to merge 2 commits into
jarrodwatts:mainfrom
linziyanleo:fix/tty-width-detection

Conversation

@linziyanleo
Copy link
Copy Markdown

Summary

  • When the statusline process's stdout is piped to Claude Code, process.stdout.columns is undefined and terminal width detection fails
  • Falls back to a hardcoded default (120) even when the real terminal is narrower or wider, causing broken wrapping or wasted space
  • Adds /dev/tty fallback: opens the controlling terminal directly to query its column count when stdout/stderr streams and COLUMNS env are all unavailable
  • Skipped on Windows (no /dev/tty) and when CLAUDE_HUD_DISABLE_TTY_WIDTH=1 is set (for testing)

Test plan

  • Updated tests/terminal.test.js to set CLAUDE_HUD_DISABLE_TTY_WIDTH=1 in the test harness so unit tests don't accidentally read the real terminal
  • Full test suite passes (508/508)
  • Manual verification: statusline correctly detects terminal width in Ghostty, iTerm2, and Terminal.app when stdout is piped

Copy link
Copy Markdown
Owner

@jarrodwatts jarrodwatts left a comment

Choose a reason for hiding this comment

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

This targets a real piped-stdout width problem, but I am holding it because it adds synchronous /dev/tty probing to the hot statusline path. The HUD can run every few hundred milliseconds, so this needs more proof before it belongs in core width detection.

Required fixes:

  • Add a positive mocked test for the /dev/tty fallback and a failure/unavailable-tty test.
  • Demonstrate or avoid repeated open/destroy cost in the hot path. A lightweight cache or setup-level width propagation may be better if repeated probing is measurable.
  • Confirm Node and Bun behavior for the tty.WriteStream lifecycle, including close/destroy behavior after openSync.

Security review found no token/network issue. The hold is runtime risk and testability.

@jarrodwatts
Copy link
Copy Markdown
Owner

Thanks for adding the cache and mocked tty-probe coverage. I am leaving this open for a maintainer width-strategy decision after the forceMaxWidth merge. The remaining question is whether core width detection should probe /dev/tty in the statusline process at all, even with per-process caching.\n\nNo credential or network issue was found. The open risk is runtime behavior in the hot statusline path, plus overlap with the setup-level COLUMNS work in #513 and the broader row-budget work in #511.

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