fix: debug output capturing for TUI / panics - #827
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR introduces a Fatal event severity throughout the event system, refactors tracing output normalization in the host runtime, enables TUI rendering of fatal events with distinct badges, and installs a global panic hook to emit panic information as fatal events with source location context. ChangesFatal event system and panic handling
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/mesh-llm-tui/src/output/mod.rs`:
- Around line 8487-8490: The panic-time TUI restore always emits terminal escape
sequences; change force_restore_tui_after_panic so it first checks an “TUI
active” condition before calling force_restore_tui_terminal() and
disable_raw_mode(). Locate the function force_restore_tui_after_panic and add a
guard that returns early when the TUI has not been entered (either consult an
existing session-mode check or a dedicated boolean like tui_entered flag), and
ensure the flag is set when the alternate screen / raw mode is actually enabled
elsewhere so only active interactive sessions trigger the restore on panic.
- Around line 7044-7046: startup_history_summary() is currently including
OutputEvent::Fatal in the branch that returns Some(event.summary_line()), which
causes fatal events to consume startup-history slots; update the match arm so
Fatal is excluded (only keep Error and Warning returning
Some(event.summary_line())), ensuring OutputEvent::Fatal is still handled
elsewhere to influence failure state but not produce startup-summary entries;
reference the startup_history_summary function and the OutputEvent::Fatal and
summary_line() symbols when making the change.
In `@crates/mesh-llm/src/lib.rs`:
- Around line 54-78: The panic-hook implementation (install_terminal_panic_hook)
and its helpers (panic_message, panic_context) were added directly in
crates/mesh-llm/src/lib.rs and must be moved into the owning crate
(mesh_llm_tui) so mesh-llm remains a thin shim; remove these functions from
lib.rs and expose a simple API in mesh_llm_tui (e.g., pub fn
install_terminal_panic_hook() that calls force_restore_tui_after_panic and
emit_fatal_panic with formatted panic_message/panic_context), then call that new
API from mesh-llm’s wiring code; ensure the helper logic for extracting payload
and location is implemented inside mesh_llm_tui so mesh-llm only invokes
mesh_llm_tui::install_terminal_panic_hook() and contains no
panic-formatting/domain logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6d9f90ce-558c-4014-b99b-1552ba5f964f
📒 Files selected for processing (4)
crates/mesh-llm-events/src/lib.rscrates/mesh-llm-host-runtime/src/runtime/mod.rscrates/mesh-llm-tui/src/output/mod.rscrates/mesh-llm/src/lib.rs
ndizazzo
force-pushed
the
fix/proto-stderr
branch
from
June 11, 2026 05:57
7340fc9 to
da492bb
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
crates/mesh-llm-tui/src/lib.rs (1)
16-17: ⚡ Quick winCall the owning module directly in the new panic path.
These calls currently resolve through
pub use output::*, which makes new code depend on the transitional root re-export. Point them atoutput::force_restore_tui_after_panic()/output::emit_fatal_panic(...)instead so the shim can be removed cleanly later.Suggested change
- force_restore_tui_after_panic(); - let _ = emit_fatal_panic(panic_message(info), panic_context(info)); + output::force_restore_tui_after_panic(); + let _ = output::emit_fatal_panic(panic_message(info), panic_context(info));As per coding guidelines,
crates/*/src/lib.rs: "Root re-exports are acceptable as temporary compatibility shims during refactors. New code should prefer importing from the owning module directly. Remove transitional re-exports once call sites have been updated."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/mesh-llm-tui/src/lib.rs` around lines 16 - 17, Update the two calls to use the owning module directly: replace force_restore_tui_after_panic() with output::force_restore_tui_after_panic() and replace emit_fatal_panic(panic_message(info), panic_context(info)) with output::emit_fatal_panic(panic_message(info), panic_context(info)); this removes reliance on the root re-export and lets the transitional shim be removed later.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/mesh-llm-tui/src/lib.rs`:
- Around line 49-89: This test currently installs the global panic hook via
install_terminal_panic_hook() and never restores the previous global hook,
leaking process-global state; fix by calling std::panic::take_hook() before
install_terminal_panic_hook() (store into a variable like previous_hook), run
the test (the existing panic::catch_unwind block), and then restore the original
hook with std::panic::set_hook(previous_hook) at the end (ensure restoration
runs after the catch_unwind so the previous hook is always set back).
In `@crates/mesh-llm-tui/src/output/mod.rs`:
- Line 357: The level column is too narrow so the "FATAL" badge collides with
the message; update the layout to reserve an extra column by increasing
PRETTY_TUI_EVENT_LEVEL_WIDTH from 5 to 6 or, alternatively, ensure the renderer
appends an explicit single space after the level badge where
PRETTY_TUI_EVENT_LEVEL_WIDTH is used (the badge formatting/rendering code that
produces the TUI event line). This change will restore the gap between the level
badge and the message across all render paths that rely on
PRETTY_TUI_EVENT_LEVEL_WIDTH.
- Around line 2482-2485: The match arm that treats OutputEvent::Error and
OutputEvent::Fatal the same is incorrectly marking running_models.last_mut() as
Error for process-level Fatal events; change the match so only
OutputEvent::Error { .. } updates the last running model's status
(RuntimeStatus::Error) and do not map OutputEvent::Fatal { .. } onto
running_models.last_mut(); leave Fatal unhandled here until Fatal includes a
model identity (also note upsert_model() can reorder running_models, so avoid
assuming last_mut() is the failing model).
---
Nitpick comments:
In `@crates/mesh-llm-tui/src/lib.rs`:
- Around line 16-17: Update the two calls to use the owning module directly:
replace force_restore_tui_after_panic() with
output::force_restore_tui_after_panic() and replace
emit_fatal_panic(panic_message(info), panic_context(info)) with
output::emit_fatal_panic(panic_message(info), panic_context(info)); this removes
reliance on the root re-export and lets the transitional shim be removed later.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 696dc75f-551b-43bd-915a-cd424131307f
📒 Files selected for processing (5)
crates/mesh-llm-events/src/lib.rscrates/mesh-llm-host-runtime/src/runtime/mod.rscrates/mesh-llm-tui/src/lib.rscrates/mesh-llm-tui/src/output/mod.rscrates/mesh-llm/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/mesh-llm-host-runtime/src/runtime/mod.rs
- crates/mesh-llm-events/src/lib.rs
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/mesh-llm-tui/src/output/mod.rs (1)
8522-8531:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftPanic-time restore doesn’t deactivate the dashboard worker.
This helper restores the terminal out-of-band, but it never clears the worker/formatter state that still drives
render_if_dirty(). If the panic unwinds a background task instead of terminating the process, the redraw loop can keep painting after the fatal line and default panic diagnostics. Clear the shared TUI-active state as part of emergency restore, or have the interactive formatter observe a shared “panic restored” flag before redrawing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/mesh-llm-tui/src/output/mod.rs` around lines 8522 - 8531, force_restore_tui_after_panic restores the terminal but doesn’t deactivate the background dashboard worker or clear the TUI-active state, so render_if_dirty can continue repainting after a panic; update this function to, after confirming GLOBAL_OUTPUT_MANAGER.get().is_some_and(...), explicitly clear the TUI-active/dashboard state on the manager (e.g. call the manager method that unsets tui_entered / stops the dashboard worker and disables the interactive formatter) before calling force_restore_tui_terminal() and disable_raw_mode(), and also ensure render_if_dirty / the interactive formatter observes a shared “panic restored” flag on GLOBAL_OUTPUT_MANAGER to skip redraws when set.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/mesh-llm-tui/src/output/mod.rs`:
- Around line 2482-2485: The handler for OutputEvent::Error must not guess which
model failed by mutating running_models.last_mut(); instead either include model
identity on OutputEvent::Error or stop per-model mutations here—update the match
arm for OutputEvent::Error to remove the code that sets model.status =
RuntimeStatus::Error (and optionally record the error in a global/session-level
error field or log it), and if you choose to propagate identity, modify the
OutputEvent::Error variant and the producers to carry the model id and then set
the specific model’s RuntimeStatus::Error; key symbols: OutputEvent::Error,
running_models, upsert_model(), RuntimeStatus::Error.
---
Outside diff comments:
In `@crates/mesh-llm-tui/src/output/mod.rs`:
- Around line 8522-8531: force_restore_tui_after_panic restores the terminal but
doesn’t deactivate the background dashboard worker or clear the TUI-active
state, so render_if_dirty can continue repainting after a panic; update this
function to, after confirming GLOBAL_OUTPUT_MANAGER.get().is_some_and(...),
explicitly clear the TUI-active/dashboard state on the manager (e.g. call the
manager method that unsets tui_entered / stops the dashboard worker and disables
the interactive formatter) before calling force_restore_tui_terminal() and
disable_raw_mode(), and also ensure render_if_dirty / the interactive formatter
observes a shared “panic restored” flag on GLOBAL_OUTPUT_MANAGER to skip redraws
when set.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2dcc9a9e-99fc-4df2-bebc-21f0fe187aa0
📒 Files selected for processing (2)
crates/mesh-llm-tui/src/lib.rscrates/mesh-llm-tui/src/output/mod.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/mesh-llm-tui/src/lib.rs
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/mesh-llm-tui/src/output/mod.rs (1)
8522-8531:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftPanic-time restore doesn’t deactivate the dashboard worker.
This helper restores the terminal out-of-band, but it never clears the worker/formatter state that still drives
render_if_dirty(). If the panic unwinds a background task instead of terminating the process, the redraw loop can keep painting after the fatal line and default panic diagnostics. Clear the shared TUI-active state as part of emergency restore, or have the interactive formatter observe a shared “panic restored” flag before redrawing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/mesh-llm-tui/src/output/mod.rs` around lines 8522 - 8531, force_restore_tui_after_panic restores the terminal but doesn’t deactivate the background dashboard worker or clear the TUI-active state, so render_if_dirty can continue repainting after a panic; update this function to, after confirming GLOBAL_OUTPUT_MANAGER.get().is_some_and(...), explicitly clear the TUI-active/dashboard state on the manager (e.g. call the manager method that unsets tui_entered / stops the dashboard worker and disables the interactive formatter) before calling force_restore_tui_terminal() and disable_raw_mode(), and also ensure render_if_dirty / the interactive formatter observes a shared “panic restored” flag on GLOBAL_OUTPUT_MANAGER to skip redraws when set.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/mesh-llm-tui/src/output/mod.rs`:
- Around line 2482-2485: The handler for OutputEvent::Error must not guess which
model failed by mutating running_models.last_mut(); instead either include model
identity on OutputEvent::Error or stop per-model mutations here—update the match
arm for OutputEvent::Error to remove the code that sets model.status =
RuntimeStatus::Error (and optionally record the error in a global/session-level
error field or log it), and if you choose to propagate identity, modify the
OutputEvent::Error variant and the producers to carry the model id and then set
the specific model’s RuntimeStatus::Error; key symbols: OutputEvent::Error,
running_models, upsert_model(), RuntimeStatus::Error.
---
Outside diff comments:
In `@crates/mesh-llm-tui/src/output/mod.rs`:
- Around line 8522-8531: force_restore_tui_after_panic restores the terminal but
doesn’t deactivate the background dashboard worker or clear the TUI-active
state, so render_if_dirty can continue repainting after a panic; update this
function to, after confirming GLOBAL_OUTPUT_MANAGER.get().is_some_and(...),
explicitly clear the TUI-active/dashboard state on the manager (e.g. call the
manager method that unsets tui_entered / stops the dashboard worker and disables
the interactive formatter) before calling force_restore_tui_terminal() and
disable_raw_mode(), and also ensure render_if_dirty / the interactive formatter
observes a shared “panic restored” flag on GLOBAL_OUTPUT_MANAGER to skip redraws
when set.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2dcc9a9e-99fc-4df2-bebc-21f0fe187aa0
📒 Files selected for processing (2)
crates/mesh-llm-tui/src/lib.rscrates/mesh-llm-tui/src/output/mod.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/mesh-llm-tui/src/lib.rs
🛑 Comments failed to post (1)
crates/mesh-llm-tui/src/output/mod.rs (1)
2482-2485:
⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftStop guessing the failing model for generic
OutputEvent::Error.
OutputEvent::Errorstill has no model identity, andupsert_model()keepsrunning_modelssorted by name, so Line 2483 will flip an arbitrary model toErrorin multi-model sessions. Either carry model identity on the event or stop mutating per-model state here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/mesh-llm-tui/src/output/mod.rs` around lines 2482 - 2485, The handler for OutputEvent::Error must not guess which model failed by mutating running_models.last_mut(); instead either include model identity on OutputEvent::Error or stop per-model mutations here—update the match arm for OutputEvent::Error to remove the code that sets model.status = RuntimeStatus::Error (and optionally record the error in a global/session-level error field or log it), and if you choose to propagate identity, modify the OutputEvent::Error variant and the producers to carry the model id and then set the specific model’s RuntimeStatus::Error; key symbols: OutputEvent::Error, running_models, upsert_model(), RuntimeStatus::Error.
i386
approved these changes
Jun 13, 2026
michaelneale
added a commit
that referenced
this pull request
Jun 13, 2026
* origin/main: Add transport-aware Skippy stage ordering (#814) Share Skippy stage wire byte accounting (#818) Report Skippy artifact cold-start costs (#815) fix: debug output capturing for TUI / panics (#827) fix(hero): visual corrections for iPhone SE size devices (#838) Add Skippy stage role metadata (#816) Add Skippy request cache epoch telemetry (#817) Consolidate agent skills and fix stale docs (Windows deploy, repo map, design docs) (#836) feature(version): normalize version markers for different build types (#831) fix(website): fix visual regressions (#835) fix(gh): change micn to michaelneale in auto_assign.yml Revert "fix(gh): replace micn with IvGolovach in auto_assign.yml (not a collaborator)" fix(gh): replace micn with IvGolovach in auto_assign.yml (not a collaborator)
michaelneale
added a commit
that referenced
this pull request
Jun 14, 2026
* origin/main: (29 commits) MoA: don't let small-model consensus pre-empt a still-running large model (#837) fix(console): render thinking traces as markdown Add bounded direct path repair (#846) Fix skippy smoke PR gate (#850) Stabilize skippy smoke chain startup (#849) fix(ci): switch back to auto-assign workflow fix(website): polish longform visual explainer (#843) fix: gemma thinking Carry GLM llama MTP patches (#840) Refresh llama.cpp canary patch queue (#839) Add transport-aware Skippy stage ordering (#814) Share Skippy stage wire byte accounting (#818) Report Skippy artifact cold-start costs (#815) fix: debug output capturing for TUI / panics (#827) fix(hero): visual corrections for iPhone SE size devices (#838) Add Skippy stage role metadata (#816) Add Skippy request cache epoch telemetry (#817) Consolidate agent skills and fix stale docs (Windows deploy, repo map, design docs) (#836) feature(version): normalize version markers for different build types (#831) fix(website): fix visual regressions (#835) ... # Conflicts: # AGENTS.md
This was referenced Aug 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This fixes two terminal-output issues:
noq_prototransport warnings are now normalized into normal mesh events instead of surfacing as raw ANSI-formattedstderr:lines in the TUI.fatalevent.What changed
Clean
noq_protoevent routingMeshTracingStderrWriternow detects tracing targets beginning withnoq_proto, strips ANSI escape sequences, normalizes formatted tracing output, and re-emits it with atransportcontext.Example normalized output:
Fatal event support
Added a first-class fatal output level and event shape:
Fatal events now render through the JSON, pretty, and TUI event paths.
Panic-safe terminal recovery
Installed a process panic hook after output initialization. On panic, mesh-llm now:
This avoids panic text being painted over the alternate-screen TUI.
Validation
Manual QA
Verified the terminal surface in tmux:
Summary by CodeRabbit