Skip to content

fix(tui): route warning logs to status bar instead of stderr#2210

Merged
TaylorMutch merged 3 commits into
NVIDIA:mainfrom
r3v5:fix-tui-warning-log
Jul 10, 2026
Merged

fix(tui): route warning logs to status bar instead of stderr#2210
TaylorMutch merged 3 commits into
NVIDIA:mainfrom
r3v5:fix-tui-warning-log

Conversation

@r3v5

@r3v5 r3v5 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Replace all 25 tracing::warn!/info!/debug!() calls in openshell-tui with app.status_text assignments to prevent stderr writes from corrupting ratatui's alternate-screen layout
  • Add ForwardWarnings event variant to decouple port-forward warning delivery from sandbox name in CreateResult, preventing downstream gRPC lookup failures in handle_exec_command
  • Remove tracing dependency from the TUI crate as a compile-time guard against reintroduction (14 other crates retain it)

Related Issue

Closes #2120

Changes

crates/openshell-tui/src/lib.rs

  • 18 direct-access tracing::*!() calls → app.status_text assignments
  • 7 spawned-task calls in start_port_forwards()Vec<String> accumulation returned to caller
  • 2 tracing::debug!() calls in refresh_draft_chunks() silently dropped (not user-actionable)
  • start_port_forwards() return type changed from () to Vec<String>
  • Forward warnings sent via Event::ForwardWarnings, not embedded in sandbox name

crates/openshell-tui/src/event.rs

  • Added ForwardWarnings(Vec<String>) variant to Event enum

crates/openshell-tui/Cargo.toml

  • Removed tracing = { workspace = true }

Before / After

Before (stderr spam corrupting TUI layout)

Before - stderr spam

After — error-state sandbox with gateway down (errors in title bar, clean layout)

After - errors in title bar After - cycling errors

After — reproduced issue #2120 scenario (error-phase sandbox, gateway stopped mid-session)

After - issue 2120 repro

Manual Verification

Prerequisites

  • A running OpenShell gateway (brew services start openshell or equivalent)
  • At least one error-phase sandbox (openshell sandbox create --name test-error will create one that enters Error phase)

Test 1: Error-state sandbox (issue #2120 repro)

openshell term
# Navigate to the error-phase sandbox, press Enter
# Wait 10+ seconds (5 ticks)
# Expected: clean TUI layout, no stderr spam
# Expected: title bar may show error text in parentheses if gRPC calls fail

Test 2: Gateway down mid-session

# Tab 1:
openshell term
# Navigate to any sandbox

# Tab 2:
brew services stop openshell

# Tab 1 (wait 4-6 seconds):
# Expected: title bar shows "(failed to refresh sandbox policy: tcp connect error)" or similar
# Expected: TUI layout remains intact, no stderr corruption
# Expected: errors cycle in title bar each tick (policy, sandboxes, providers)

# Cleanup:
brew services start openshell

Test 3: Compile-time guard

grep -rn 'tracing::' crates/openshell-tui/src/
# Expected: zero results (exit code 1)

cargo clippy -p openshell-tui
# Expected: no warnings

Testing

  • cargo build -p openshell-tui — compiles clean
  • cargo clippy -p openshell-tui — no warnings
  • grep -rn 'tracing::' crates/openshell-tui/src/ — zero results
  • mise run pre-commit — passes
  • Manual: openshell term → error sandbox → clean layout, no stderr corruption
  • Manual: gateway stopped mid-session → errors display in title bar, TUI intact
  • Reproduced exact scenario from issue [term] Warning Log Messed Up the TUI #2120 (error-phase sandbox detail view)

r3v5 added 3 commits July 7, 2026 12:58
tracing::warn/info/debug calls in the TUI crate write to stderr via
the global tracing subscriber. In ratatui's alternate-screen/raw-mode,
stderr writes corrupt the terminal layout. Error-state sandboxes
amplify this as background gRPC polls fail every 2s tick.

Replace all 25 tracing calls with app.status_text assignments for
direct-access sites, and Vec<String> accumulation for the spawned
start_port_forwards task. Add ForwardWarnings event variant to
decouple forward warning delivery from the sandbox name in
CreateResult, preventing downstream gRPC lookup failures.

Closes NVIDIA#2120

Signed-off-by: Ian Miller <milleryan2003@gmail.com>
New event type for non-fatal port-forward warnings during sandbox
creation. Keeps warning delivery separate from the sandbox name
in CreateResult to avoid corrupting downstream gRPC lookups.

Signed-off-by: Ian Miller <milleryan2003@gmail.com>
Compile-time guard against reintroducing stderr-writing tracing
calls in the TUI crate. 14 other crates retain the dependency.

Signed-off-by: Ian Miller <milleryan2003@gmail.com>
@r3v5 r3v5 requested review from a team, derekwaynecarr, maxamillion and mrunalp as code owners July 10, 2026 10:51
@copy-pr-bot

copy-pr-bot Bot commented Jul 10, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@r3v5

r3v5 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

I have read the DCO document and I hereby sign the DCO.

@r3v5

r3v5 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

recheck

@TaylorMutch

Copy link
Copy Markdown
Collaborator

/ok to test 97862a8

@TaylorMutch TaylorMutch left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good to me!

@TaylorMutch TaylorMutch enabled auto-merge (squash) July 10, 2026 15:33
@TaylorMutch TaylorMutch merged commit 5f38b7c into NVIDIA:main Jul 10, 2026
30 checks passed
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.

[term] Warning Log Messed Up the TUI

2 participants