Skip to content

fix(tui): route mid-session diagnostics through the TUI, never raw stderr - #140

Open
Emasoft wants to merge 2 commits into
mlhher:mainfrom
Emasoft:fix/tui-diagnostics-sink
Open

Emasoft wants to merge 2 commits into
mlhher:mainfrom
Emasoft:fix/tui-diagnostics-sink

Conversation

@Emasoft

@Emasoft Emasoft commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

Any mid-session fmt.Fprintf(os.Stderr) write was painting raw text over bubbletea's alt-screen. Plugin hook warnings like [plugin/onToolCall/...] hook timed out and bash-gate hints garbled and duplicated the footer, and visually replaced the agent-name row with whatever stderr printed last. Confirmed by screenshot + OCR — the burned-in garbage matches the stderr text exactly.

The fix

Everything that can fire mid-session now goes through a diagnostic sink instead of stderr:

  • PluginManager — hook warnings report through the sink
  • BaseOrchestrator — SetDiagnostics/reportf, and the sink propagates to spawned children in the spawn runner
  • tool package — bash-gate hints route through the sink

The TUI installs one that renders a warning toast via a new DiagnosticMsg (6s expiry, standard clear tick, truncated to terminal width with the status bar's ellipsis helper). stderr stays the fallback for headless/CLI runs, where there's no alt-screen to paint over.

reportf is guarded by its own diagMu instead of the orchestrator's main lock, so it stays callable while mu is held — the naive version deadlocked.

TUI fixes this surfaced

  • Expiry-aware toast clear — clearToastMsg checks ToastExpireTime, so a clear tick scheduled by a previous toast can't kill a newer one early, and stale toasts can't be cleared twice.
  • McpStatusMsg clears on a real 3s tea.Tick — the old immediate-clear command ran the moment Bubble Tea processed it, erasing the toast after a single rendered frame even though expiry said 3s.
  • Global KeyReleaseMsg swallow — nothing consumes key-release events, so updateInternal drops them at the top; an esc release can no longer cancel a pending prompt.

Ported onto upstream/main's shapes: BaseOrchestrator carries only SetDiagnostics/reportf + diagMu (upstream has no dropped-events machinery), and the child sink installs in the actual spawn runner.

Testing

$ go test -count=1 -race ./...
ok  	late/cmd/late	2.097s
?  	late/cmd/mcp-run	[no test files]
ok  	late/internal/agent	1.869s
?  	late/internal/assets	[no test files]
ok  	late/internal/client	2.227s
ok  	late/internal/common	2.461s
ok  	late/internal/config	1.626s
ok  	late/internal/executor	9.919s
ok  	late/internal/git	2.950s
ok  	late/internal/mcp	2.932s
ok  	late/internal/orchestrator	3.215s
?  	late/internal/pathutil	[no test files]
ok  	late/internal/plugin	13.033s
ok  	late/internal/session	3.802s
ok  	late/internal/skill	3.169s
ok  	late/internal/tool	4.272s
ok  	late/internal/tool/ast	3.971s
ok  	late/internal/tui	9.057s

exit 0 — all 18 packages green (15 ok, 3 without test files).

…derr

Mid-session fmt.Fprintf(os.Stderr) writes (event-drop reports, plugin
hook warnings, bash-gate hints) paint raw text over the bubbletea
alt-screen, garbling/duplicating the footer and visually replacing the
agent-name row. PluginManager, BaseOrchestrator (propagated to spawned
children in the spawn runner) and the tool package now carry a
diagnostic sink; the TUI installs one that renders a warning toast
(DiagnosticMsg). stderr remains the fallback for headless/CLI runs.

Includes the TUI review fixes the sink feature exposed and depends on:
stale-toast expiry-aware clear (clearToastMsg handler), the
McpStatusMsg proper 3s clear tick, the global KeyReleaseMsg swallow at
the top of updateInternal (esc release no longer cancels prompts), and
the orchestrator's dedicated diagMu so reportf stays callable under mu.

Ported onto upstream/main's shapes: BaseOrchestrator carries only
SetDiagnostics/reportf + diagMu (upstream has no dropped-events
machinery), and the child sink installs in the actual spawn runner.
golangci-lint (unused) flags internal/tool/diagnostics.go:35:
func diagnosticsSink is unused. reportf already reads diagnosticsFuncs
inline under the RLock, so the accessor is dead code. Remove it.

Reproduced locally with golangci-lint v2.13.2 (same version as CI's
golangci-lint-action@v9); full go test ./... -race -count=1 green.
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.

1 participant