feat: add CLI tab activity indicator - #36698
Conversation
mxnstrexgl
left a comment
There was a problem hiding this comment.
🤖 Automated PR Review
Security Scan
- ✓ No hardcoded secrets, injection sinks, unsafe deserialization, or dependency red flags found by this automated scan.
Code Quality
- ✓ No blocking code-quality issues found by this automated scan.
Summary
Status: APPROVE — security findings: 0, quality suggestions: 0.
Automated review; raw diff content intentionally omitted.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused, opt-in classic-CLI implementation. The premise remains valid: current cli.py:5356-5362 and cli.py:11046-11073 update the in-terminal spinner/tool state but do not update a tab title. The existing Ink TUI has a related implementation at ui-tui/src/app/useMainApp.ts:572-582, but that does not provide the PR's classic-CLI config surface.
Problems
cli.py:3818always writes OSC 0. Current terminal-title handling explicitly takes a Windows path atui-tui/packages/hermes-ink/src/ink/hooks/use-terminal-title.ts:28-31(process.titleonwin32) rather than relying on OSC. This PR needs equivalent classic-CLI Windows handling or an explicit unsupported-platform gate with coverage.
Suggested changes
- Add a tested Windows-native title path (or platform gate) before salvage; preserve the existing TTY and sanitization behavior for OSC-capable terminals.
Automated hermes-sweeper review.
| try: | ||
| if not (hasattr(stream, "isatty") and stream.isatty()): | ||
| return | ||
| stream.write(f"\033]0;{title}\a") |
There was a problem hiding this comment.
This emits OSC 0 on every platform. Current ui-tui/packages/hermes-ink/src/ink/hooks/use-terminal-title.ts:28-31 uses a dedicated win32 path instead of OSC; please add equivalent classic-CLI Windows handling (and coverage), or explicitly gate this feature where unsupported.
# Conflicts: # cli.py
…-indicator # Conflicts: # hermes_cli/config.py
|
Addressed the Windows title handling review in
Validation on the refreshed branch:
|
Review note (overlap + Windows tab visibility)Opt-in spinner templates are closer to a “status board” title than a static idle string — useful for multi-tab workflows. Two notes from a Windows daily-driver angle:
Not blocking on the opt-in design itself. |
Summary
Test Plan
venv/bin/python -m pytest tests/cli/test_cli_status_bar.py tests/cli/test_cli_background_status_indicator.py tests/cli/test_cli_tab_activity_indicator.py tests/hermes_cli/test_config*.py -q -o 'addopts='Notes
display.tab_activity_indicator: false.