-
Notifications
You must be signed in to change notification settings - Fork 3
fix(tui): consolidated TUI fixes for overflow, positioning, and panic prevention #69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… prevention This PR consolidates the following fixes: - #38: Prevent usize to u16 overflow in interactive renderer - #42: Prevent usize to u16 overflow in card count displays - #58: Fix cursor positioning and underflow in selection list - #59: Fix mention popup positioning and Unicode width calculation - #60: Improve autocomplete popup positioning and width calculation - #64: Prevent underflow in dropdown navigation and scroll calculations - #66: Prevent panic in HelpBrowserState when sections empty All changes target the TUI components to improve robustness: - Added saturating casts for u16 conversions - Fixed cursor positioning calculations - Added bounds checking for empty sections - Improved Unicode width handling for popups
Greptile OverviewGreptile SummaryThis PR consolidates 7 individual TUI fixes addressing overflow, underflow, and panic issues across dropdown, scroll, and popup components. Key Changes:
All changes use defensive programming patterns (saturating arithmetic, bounds checking, early returns) consistently applied across 13 files. Test coverage includes the new empty sections case. Confidence Score: 5/5
|
| Filename | Overview |
|---|---|
| src/cortex-tui-components/src/scroll.rs | Added zero visible items guard and double saturating_sub to prevent underflow in scroll calculations |
| src/cortex-tui-components/src/selection_list.rs | Fixed cursor positioning with character count and saturating arithmetic for disabled reason alignment |
| src/cortex-tui/src/widgets/autocomplete.rs | Changed width calculation to use visible items instead of all items, and improved popup positioning fallback logic |
| src/cortex-tui/src/widgets/help_browser/state.rs | Changed current_section() return type to Option<&HelpSection> with empty sections guard |
| src/cortex-tui/src/widgets/scrollable_dropdown.rs | Added comprehensive zero max_visible guards and bounds checking with double saturating_sub throughout |
Sequence Diagram
sequenceDiagram
participant User
participant TUI as TUI Component
participant State as Component State
participant Render as Render Logic
User->>TUI: Navigate/Scroll Action
TUI->>State: Update selection/scroll
alt Empty or Zero Visible Items
State->>State: Check max_visible == 0 || items.is_empty()
State-->>TUI: Return early (prevent underflow)
else Normal Case
State->>State: Calculate new position
State->>State: Use saturating_sub(max_visible.saturating_sub(1))
State-->>TUI: Safe position value
end
TUI->>Render: Calculate dimensions
alt Large Item Count
Render->>Render: u16::try_from(count).unwrap_or(u16::MAX)
Render->>Render: saturating_add for height components
Render-->>TUI: Clamped u16 dimensions
else Unicode/Wide Characters
Render->>Render: Use chars().count() for width
Render-->>TUI: Accurate Unicode-aware dimensions
end
alt Popup Positioning
Render->>Render: Check if area.y >= height
alt Fits Above
Render->>Render: Position above input
else No Room Above
Render->>Render: Position below input
end
end
TUI->>User: Display UI (no panic/overflow)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5 files reviewed, no comments
|
Closing this PR to consolidate into a single mega-PR combining all bug fixes. The changes will be included in a new consolidated PR. |
This PR consolidates all bug fixes and security improvements from PRs #69-88 into a single cohesive change. ## Categories ### Security Fixes - Path traversal prevention in MCP and session storage - Shell injection prevention in restore scripts - Secure random temp files for external editor - TOCTOU race condition fixes ### TUI Improvements - Overflow prevention for u16 conversions - Cursor positioning fixes in selection lists - Unicode width handling for popups - Empty section handling in help browser ### Error Handling - Graceful semaphore and init failure handling - Improved error propagation in middleware - Better client access error handling - SystemTime operation safety ### Memory and Storage - Cache size limits to prevent unbounded growth - File lock cleanup for memory leak prevention - fsync after critical writes for durability - Bounded ToolResponseStore with automatic cleanup ### Protocol Robustness - Buffer size limits for StreamProcessor - ToolState transition validation - State machine documentation ### Numeric Safety - Saturating operations to prevent overflow/underflow - Safe UTF-8 string slicing throughout codebase ### Tools - Parameter alias support for backward compatibility - Handler name consistency fixes ## Files Modified Multiple files across cortex-tui, cortex-engine, cortex-exec, cortex-common, cortex-protocol, cortex-storage, cortex-mcp-server, and other crates. Closes #69, #70, #71, #73, #75, #80, #82, #87, #88
This PR consolidates all bug fixes and security improvements from PRs #69-88 into a single cohesive change. ## Categories ### Security Fixes - Path traversal prevention in MCP and session storage - Shell injection prevention in restore scripts - Secure random temp files for external editor - TOCTOU race condition fixes ### TUI Improvements - Overflow prevention for u16 conversions - Cursor positioning fixes in selection lists - Unicode width handling for popups - Empty section handling in help browser ### Error Handling - Graceful semaphore and init failure handling - Improved error propagation in middleware - Better client access error handling - SystemTime operation safety ### Memory and Storage - Cache size limits to prevent unbounded growth - File lock cleanup for memory leak prevention - fsync after critical writes for durability - Bounded ToolResponseStore with automatic cleanup ### Protocol Robustness - Buffer size limits for StreamProcessor - ToolState transition validation - State machine documentation ### Numeric Safety - Saturating operations to prevent overflow/underflow - Safe UTF-8 string slicing throughout codebase ### Tools - Parameter alias support for backward compatibility - Handler name consistency fixes ## Files Modified Multiple files across cortex-tui, cortex-engine, cortex-exec, cortex-common, cortex-protocol, cortex-storage, cortex-mcp-server, and other crates. Closes #69, #70, #71, #73, #75, #80, #82, #87, #88
Summary
This PR consolidates 7 individual TUI fixes into a single, cohesive change:
Included PRs:
Changes:
Files Modified:
src/cortex-tui-components/src/dropdown.rssrc/cortex-tui-components/src/scroll.rssrc/cortex-tui-components/src/selection_list.rssrc/cortex-tui/src/cards/commands.rssrc/cortex-tui/src/cards/models.rssrc/cortex-tui/src/cards/sessions.rssrc/cortex-tui/src/interactive/renderer.rssrc/cortex-tui/src/widgets/autocomplete.rssrc/cortex-tui/src/widgets/help_browser/render.rssrc/cortex-tui/src/widgets/help_browser/state.rssrc/cortex-tui/src/widgets/help_browser/tests.rssrc/cortex-tui/src/widgets/mention_popup.rssrc/cortex-tui/src/widgets/scrollable_dropdown.rsCloses #38, #42, #58, #59, #60, #64, #66