feat(neo-tui): full pi-tui rewrite — fixes all 4 reported bugs (shift+enter, Korean wrap, silent failure, idle/answering)#14
Conversation
…Keys + OSC52 Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
…p, truncate, slice Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
…ys + OSC52 Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
…, wrap, slice) Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
…aste markers, history nav Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
…th completion Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
… completion Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
…history navigation Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
…t, model, branch dirty
…l cards, thinking toggle
…idle/answering ambiguity)
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
…history nav, autocomplete, mouse scroll Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
…riants Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
…ory nav + mouse scroll into app loop Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
…er error state (fixes Bug 3 silent failure) Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
…color audit verified clean
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
There was a problem hiding this comment.
13 issues found across 49 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/neo-tui/src/components/select_list.rs">
<violation number="1" location="packages/neo-tui/src/components/select_list.rs:203">
P2: Picker filtering is not reachable from keyboard input because unhandled key events are ignored and swallowed by overlay dispatch. Add char/backspace handling in `SelectList::handle_event` to mutate `filter` and recompute matches.</violation>
</file>
<file name="packages/neo-tui/src/components/markdown.rs">
<violation number="1" location="packages/neo-tui/src/components/markdown.rs:129">
P2: Nested markdown inline styles are not combined; only the most recent style is applied, so mixed formatting (e.g. bold+italic) renders incorrectly.</violation>
<violation number="2" location="packages/neo-tui/src/components/markdown.rs:323">
P2: `is_comment` incorrectly classifies `*`-prefixed tokens as comments, which mis-highlights common operators and expressions.</violation>
</file>
<file name="packages/neo-tui/src/anim/mod.rs">
<violation number="1" location="packages/neo-tui/src/anim/mod.rs:26">
P2: `Spinner::custom` allows `interval_ms = 0`, which can panic in `next_frame` when dividing by `self.interval_ms`.</violation>
</file>
<file name="packages/neo-tui/src/text/mod.rs">
<violation number="1" location="packages/neo-tui/src/text/mod.rs:147">
P1: `slice_by_column` incorrectly shifts slice bounds when ANSI appears before `start`, causing off-by-one visible slices on styled text.</violation>
</file>
<file name="packages/neo-tui/src/components/settings_list.rs">
<violation number="1" location="packages/neo-tui/src/components/settings_list.rs:75">
P2: This component duplicates core `SelectList` logic instead of reusing it, creating a maintenance/drift risk.</violation>
</file>
<file name="packages/neo-tui/src/theme/opencode_palette.rs">
<violation number="1" location="packages/neo-tui/src/theme/opencode_palette.rs:183">
P2: Status background blends are reversed, making bg colors too close to status foreground colors and hurting footer label contrast.</violation>
</file>
<file name="packages/neo-tui/src/overlay/mod.rs">
<violation number="1" location="packages/neo-tui/src/overlay/mod.rs:227">
P1: Picker confirmation dispatches `neo.model.set:*` / `neo.theme.set:*` IDs that `execute_action` never handles, so selecting an item closes the overlay but does not apply the choice.</violation>
</file>
<file name="packages/neo-tui/src/app/mod.rs">
<violation number="1" location="packages/neo-tui/src/app/mod.rs:266">
P2: Autocomplete selection can drift to off-screen entries when there are more than 6 results, so Tab may apply a different item than the highlighted row.</violation>
<violation number="2" location="packages/neo-tui/src/app/mod.rs:598">
P2: Header connection indicator is never synchronized with inbound connection state, so the new header status dot can display stale/incorrect connectivity.</violation>
<violation number="3" location="packages/neo-tui/src/app/mod.rs:819">
P2: Startup error paths after `init_writes()` do not restore terminal capability state, which can leave tmux key mode altered after failed launch.</violation>
</file>
<file name="packages/neo-tui/src/components/input.rs">
<violation number="1" location="packages/neo-tui/src/components/input.rs:171">
P2: History-next at the newest entry leaves stale history text in the input instead of returning to a blank compose buffer.</violation>
<violation number="2" location="packages/neo-tui/src/components/input.rs:413">
P2: Stale paste segments are never removed, so undoing large pastes can retain large hidden buffers and degrade performance over time.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| let ansi_start = start; | ||
| let ansi_end = end_exclusive; | ||
| let ansi_skip_offset = usize::from(has_ansi_before_column(input, start)); | ||
| let start = start.saturating_sub(ansi_skip_offset); |
There was a problem hiding this comment.
P1: slice_by_column incorrectly shifts slice bounds when ANSI appears before start, causing off-by-one visible slices on styled text.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/neo-tui/src/text/mod.rs, line 147:
<comment>`slice_by_column` incorrectly shifts slice bounds when ANSI appears before `start`, causing off-by-one visible slices on styled text.</comment>
<file context>
@@ -0,0 +1,547 @@
+ let ansi_start = start;
+ let ansi_end = end_exclusive;
+ let ansi_skip_offset = usize::from(has_ansi_before_column(input, start));
+ let start = start.saturating_sub(ansi_skip_offset);
+ let end_exclusive = end_exclusive.saturating_sub(ansi_skip_offset);
+
</file context>
| return OverlayResult::Continue; | ||
| } | ||
| if let Some((label, _idx)) = select.take_selection() { | ||
| return OverlayResult::Selected(format!("{action_prefix}:{label}")); |
There was a problem hiding this comment.
P1: Picker confirmation dispatches neo.model.set:* / neo.theme.set:* IDs that execute_action never handles, so selecting an item closes the overlay but does not apply the choice.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/neo-tui/src/overlay/mod.rs, line 227:
<comment>Picker confirmation dispatches `neo.model.set:*` / `neo.theme.set:*` IDs that `execute_action` never handles, so selecting an item closes the overlay but does not apply the choice.</comment>
<file context>
@@ -65,8 +90,146 @@ impl Overlay {
+ return OverlayResult::Continue;
+ }
+ if let Some((label, _idx)) = select.take_selection() {
+ return OverlayResult::Selected(format!("{action_prefix}:{label}"));
+ }
+ if select.was_cancelled() {
</file context>
| self.cancelled = true; | ||
| EventResult::Consumed | ||
| } | ||
| _ => EventResult::Ignored, |
There was a problem hiding this comment.
P2: Picker filtering is not reachable from keyboard input because unhandled key events are ignored and swallowed by overlay dispatch. Add char/backspace handling in SelectList::handle_event to mutate filter and recompute matches.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/neo-tui/src/components/select_list.rs, line 203:
<comment>Picker filtering is not reachable from keyboard input because unhandled key events are ignored and swallowed by overlay dispatch. Add char/backspace handling in `SelectList::handle_event` to mutate `filter` and recompute matches.</comment>
<file context>
@@ -0,0 +1,206 @@
+ self.cancelled = true;
+ EventResult::Consumed
+ }
+ _ => EventResult::Ignored,
+ }
+ }
</file context>
| } | ||
|
|
||
| fn is_comment(text: &str) -> bool { | ||
| text.starts_with("//") || text.starts_with("/*") || text.starts_with('*') |
There was a problem hiding this comment.
P2: is_comment incorrectly classifies *-prefixed tokens as comments, which mis-highlights common operators and expressions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/neo-tui/src/components/markdown.rs, line 323:
<comment>`is_comment` incorrectly classifies `*`-prefixed tokens as comments, which mis-highlights common operators and expressions.</comment>
<file context>
@@ -0,0 +1,346 @@
+}
+
+fn is_comment(text: &str) -> bool {
+ text.starts_with("//") || text.starts_with("/*") || text.starts_with('*')
+}
+
</file context>
| text.starts_with("//") || text.starts_with("/*") || text.starts_with('*') | |
| text.starts_with("//") || text.starts_with("/*") |
| self.render_code_block(&lang, &body); | ||
| } | ||
| Event::Code(text) => { | ||
| let style = self |
There was a problem hiding this comment.
P2: Nested markdown inline styles are not combined; only the most recent style is applied, so mixed formatting (e.g. bold+italic) renders incorrectly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/neo-tui/src/components/markdown.rs, line 129:
<comment>Nested markdown inline styles are not combined; only the most recent style is applied, so mixed formatting (e.g. bold+italic) renders incorrectly.</comment>
<file context>
@@ -0,0 +1,346 @@
+ self.render_code_block(&lang, &body);
+ }
+ Event::Code(text) => {
+ let style = self
+ .token_style(Token::MarkdownCode)
+ .bg(self.theme.token(Token::BackgroundElement));
</file context>
| fn init_terminal() -> Result<Terminal<CrosstermBackend<Stdout>>> { | ||
| enable_raw_mode()?; | ||
| let caps = TerminalCaps::detect(); | ||
| write_terminal_bytes(&caps.init_writes())?; |
There was a problem hiding this comment.
P2: Startup error paths after init_writes() do not restore terminal capability state, which can leave tmux key mode altered after failed launch.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/neo-tui/src/app/mod.rs, line 819:
<comment>Startup error paths after `init_writes()` do not restore terminal capability state, which can leave tmux key mode altered after failed launch.</comment>
<file context>
@@ -602,7 +815,12 @@ fn synthesise_select_event(action_id: &str) -> Option<KeyEvent> {
fn init_terminal() -> Result<Terminal<CrosstermBackend<Stdout>>> {
- enable_raw_mode()?;
+ let caps = TerminalCaps::detect();
+ write_terminal_bytes(&caps.init_writes())?;
+ if let Err(err) = enable_raw_mode() {
+ let _ = write_terminal_bytes(&caps.cleanup_writes());
</file context>
| if items.is_empty() { | ||
| self.autocomplete_index = 0; | ||
| } else { | ||
| self.autocomplete_index = (self.autocomplete_index + 1) % items.len(); |
There was a problem hiding this comment.
P2: Autocomplete selection can drift to off-screen entries when there are more than 6 results, so Tab may apply a different item than the highlighted row.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/neo-tui/src/app/mod.rs, line 266:
<comment>Autocomplete selection can drift to off-screen entries when there are more than 6 results, so Tab may apply a different item than the highlighted row.</comment>
<file context>
@@ -165,6 +181,127 @@ impl App {
+ if items.is_empty() {
+ self.autocomplete_index = 0;
+ } else {
+ self.autocomplete_index = (self.autocomplete_index + 1) % items.len();
+ }
+ }
</file context>
| match msg { | ||
| Inbound::Event(event) => self.apply_event(event), | ||
| Inbound::Event(event) => { | ||
| self.footer.connected = true; |
There was a problem hiding this comment.
P2: Header connection indicator is never synchronized with inbound connection state, so the new header status dot can display stale/incorrect connectivity.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/neo-tui/src/app/mod.rs, line 598:
<comment>Header connection indicator is never synchronized with inbound connection state, so the new header status dot can display stale/incorrect connectivity.</comment>
<file context>
@@ -406,8 +594,39 @@ impl App {
match msg {
- Inbound::Event(event) => self.apply_event(event),
+ Inbound::Event(event) => {
+ self.footer.connected = true;
+ self.apply_event(event);
+ }
</file context>
| pub fn recall_next_history(&mut self) -> Option<String> { | ||
| let current = self.history.cursor?; | ||
| let next = current + 1; | ||
| if next >= self.history.entries.len() { |
There was a problem hiding this comment.
P2: History-next at the newest entry leaves stale history text in the input instead of returning to a blank compose buffer.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/neo-tui/src/components/input.rs, line 171:
<comment>History-next at the newest entry leaves stale history text in the input instead of returning to a blank compose buffer.</comment>
<file context>
@@ -37,9 +65,120 @@ pub struct InputState {
+ pub fn recall_next_history(&mut self) -> Option<String> {
+ let current = self.history.cursor?;
+ let next = current + 1;
+ if next >= self.history.entries.len() {
+ self.history.cursor = None;
+ return None;
</file context>
| fn refresh_paste_ranges(&mut self) { | ||
| let buffer = self.buffer.as_str(); | ||
| for segment in &mut self.paste_segments { | ||
| let sentinel = paste_sentinel(segment.id); | ||
| segment.byte_range = buffer | ||
| .find(&sentinel) | ||
| .map_or(0..0, |start| start..start + sentinel.len()); | ||
| } |
There was a problem hiding this comment.
P2: Stale paste segments are never removed, so undoing large pastes can retain large hidden buffers and degrade performance over time.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/neo-tui/src/components/input.rs, line 413:
<comment>Stale paste segments are never removed, so undoing large pastes can retain large hidden buffers and degrade performance over time.</comment>
<file context>
@@ -220,10 +396,127 @@ impl InputState {
+ self.paste_segments.clear();
+ }
+
+ fn refresh_paste_ranges(&mut self) {
+ let buffer = self.buffer.as_str();
+ for segment in &mut self.paste_segments {
</file context>
| fn refresh_paste_ranges(&mut self) { | |
| let buffer = self.buffer.as_str(); | |
| for segment in &mut self.paste_segments { | |
| let sentinel = paste_sentinel(segment.id); | |
| segment.byte_range = buffer | |
| .find(&sentinel) | |
| .map_or(0..0, |start| start..start + sentinel.len()); | |
| } | |
| fn refresh_paste_ranges(&mut self) { | |
| let buffer = self.buffer.as_str(); | |
| self.paste_segments.retain_mut(|segment| { | |
| let sentinel = paste_sentinel(segment.id); | |
| if let Some(start) = buffer.find(&sentinel) { | |
| segment.byte_range = start..start + sentinel.len(); | |
| true | |
| } else { | |
| false | |
| } | |
| }); | |
| } |
Cover --theme / --list-themes / --backend-bin / --backend-args / --demo / --demo-seconds, list the 16 bundled themes, and surface the non-obvious key actions (Shift+Enter in tmux, autocomplete triggers, overlay shortcuts, mouse wheel). Touches up the module layout to match the post-rewrite tree. Refs #14
There was a problem hiding this comment.
2 issues found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/neo-tui/README.md">
<violation number="1" location="packages/neo-tui/README.md:38">
P2: The `--backend-bin` docs claim `--mode rpc` is added automatically, but the code only forwards args from `--backend-args`/`SENPI_NEO_BACKEND_ARGS`.</violation>
<violation number="2" location="packages/neo-tui/README.md:42">
P2: The README uses `opencode/dracula` as a theme id example, but valid bundled ids are `dracula`, `tokyonight`, etc. (`opencode/dracula` is treated as a file path and fails).</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| | Flag | Env | Description | | ||
| |------|-----|-------------| | ||
| | `--backend-bin <PATH>` | `SENPI_NEO_BACKEND_BIN` | Path to the senpi backend binary. Spawned with `--mode rpc` on startup; if unset, the TUI runs offline (demo mode or empty session). | |
There was a problem hiding this comment.
P2: The --backend-bin docs claim --mode rpc is added automatically, but the code only forwards args from --backend-args/SENPI_NEO_BACKEND_ARGS.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/neo-tui/README.md, line 38:
<comment>The `--backend-bin` docs claim `--mode rpc` is added automatically, but the code only forwards args from `--backend-args`/`SENPI_NEO_BACKEND_ARGS`.</comment>
<file context>
@@ -16,56 +16,87 @@ A TUI needs exclusive ownership of the terminal: raw mode, alternate screen, Kit
+
+| Flag | Env | Description |
+|------|-----|-------------|
+| `--backend-bin <PATH>` | `SENPI_NEO_BACKEND_BIN` | Path to the senpi backend binary. Spawned with `--mode rpc` on startup; if unset, the TUI runs offline (demo mode or empty session). |
+| `--backend-args <JSON>` | `SENPI_NEO_BACKEND_ARGS` | JSON array of extra args forwarded to the backend, e.g. `'["--mode","rpc"]'`. |
+| `--demo` | `SENPI_NEO_DEMO` | Render the canned demo scene used for screenshots. |
</file context>
| | `--backend-bin <PATH>` | `SENPI_NEO_BACKEND_BIN` | Path to the senpi backend binary. Spawned with `--mode rpc` on startup; if unset, the TUI runs offline (demo mode or empty session). | | |
| | `--backend-bin <PATH>` | `SENPI_NEO_BACKEND_BIN` | Path to the backend binary. Spawned with args from `--backend-args` / `SENPI_NEO_BACKEND_ARGS`; if unset, the TUI runs offline (demo mode or empty session). | |
| | `--backend-args <JSON>` | `SENPI_NEO_BACKEND_ARGS` | JSON array of extra args forwarded to the backend, e.g. `'["--mode","rpc"]'`. | | ||
| | `--demo` | `SENPI_NEO_DEMO` | Render the canned demo scene used for screenshots. | | ||
| | `--demo-seconds <N>` | — | Exit after `N` seconds in demo mode. `0` = until Ctrl-C. | | ||
| | `--theme <ID\|PATH>` | `SENPI_NEO_THEME` | Override the theme by bundled id (`senpi-neo-dark`, `opencode/dracula`, …) or by JSON file path. | |
There was a problem hiding this comment.
P2: The README uses opencode/dracula as a theme id example, but valid bundled ids are dracula, tokyonight, etc. (opencode/dracula is treated as a file path and fails).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/neo-tui/README.md, line 42:
<comment>The README uses `opencode/dracula` as a theme id example, but valid bundled ids are `dracula`, `tokyonight`, etc. (`opencode/dracula` is treated as a file path and fails).</comment>
<file context>
@@ -16,56 +16,87 @@ A TUI needs exclusive ownership of the terminal: raw mode, alternate screen, Kit
+| `--backend-args <JSON>` | `SENPI_NEO_BACKEND_ARGS` | JSON array of extra args forwarded to the backend, e.g. `'["--mode","rpc"]'`. |
+| `--demo` | `SENPI_NEO_DEMO` | Render the canned demo scene used for screenshots. |
+| `--demo-seconds <N>` | — | Exit after `N` seconds in demo mode. `0` = until Ctrl-C. |
+| `--theme <ID\|PATH>` | `SENPI_NEO_THEME` | Override the theme by bundled id (`senpi-neo-dark`, `opencode/dracula`, …) or by JSON file path. |
+| `--list-themes` | — | Print bundled theme ids and exit. |
+
</file context>
| | `--theme <ID\|PATH>` | `SENPI_NEO_THEME` | Override the theme by bundled id (`senpi-neo-dark`, `opencode/dracula`, …) or by JSON file path. | | |
| | `--theme <ID\|PATH>` | `SENPI_NEO_THEME` | Override the theme by bundled id (`senpi-neo-dark`, `dracula`, …) or by JSON file path. | |
Oracle review on PR #14 flagged two real defects in the bundled assets: 1. senpi-neo-dark.json defined 70/72 semantic tokens, missing `diffAddedText` and `diffRemovedText`. The resolver is lenient and silently falls back to `Color::Reset` for missing tokens, so the diff text renderer was inheriting whatever the terminal's default foreground happened to be instead of the theme's intended red/green. Added the two missing tokens and locked the contract with a new exhaustive test (`bundled_dark_theme_resolves_every_token_in_token_all`) that scans the resolved theme for `Color::Reset` and fails if any `Token::ALL` member is missing. 2. CI `neo-tui-keymap-parity` regression test was failing because the neo keymap shipped three non-legacy bindings outside the `neo.*` namespace: - `tui.editor.newLine`: redundant with the legacy `tui.input.newLine` from `TUI_KEYBINDINGS` (both bound to `shift+enter`, both insert a newline; the main key dispatcher was already handling the legacy one). Removed. - `tui.input.historyPrev` / `tui.input.historyNext`: legitimately neo-only (the legacy senpi TUI has no history navigation), so renamed to `neo.input.historyPrev` / `neo.input.historyNext` where neo-only bindings belong. Updated the Rust app dispatcher match arms and the existing Rust-side parity test to match. Refs #14
Oracle review on PR #14 flagged that `senpi --neo` was spawning the Rust TUI with an empty arg vector while quietly routing the user's original argv to the senpi backend RPC. That made the README's `senpi --neo --theme opencode/dracula` line a lie: senpi's own CLI parser eats `--theme <path>` first, and the value never reaches the neo TUI's `--theme <id|path>` flag. Introduce a `--` sentinel split: everything before `--` (minus the `--neo` flag itself) stays on the senpi-backend side, everything after `--` is forwarded verbatim to the `senpi-neo-tui` binary which already has a clap parser for `--theme`, `--list-themes`, `--demo`, `--demo-seconds`, `--backend-bin`, and `--backend-args`. Examples that now work end-to-end: senpi --neo -- --theme opencode/dracula senpi --neo -- --list-themes senpi --neo -- --demo --demo-seconds 5 senpi --neo --provider anthropic -- --theme nord The split lives in an exported `splitNeoArgs` so it is unit-testable without spawning a child process; the new `neo-tui-arg-forwarding.test.ts` regression suite covers the six forwarding shapes (no sentinel, sentinel only, mixed before+after, `--list-themes`, empty after sentinel, `--neo` mid-argv). README updated with the new invocation contract. Refs #14
|
Merged. Squash commit on main is Final delta vs main:
Stats: 298+ Rust tests, 2300+ TS tests, fmt + clippy + biome + tsgo all clean. Out-of-scope follow-ups left for future PRs: |
Oracle round-4 review of PR #15 (`cbcc9331`) found two more silent no-op paths in the picker plumbing: 1. `Ctrl+L` returned `AppAction::OpenModelPicker` and fired `Command::GetAvailableModels` to the backend, but never actually set `self.overlay = Some(Overlay::ModelPicker(_))`, so the advertised model picker just never appeared. Now opens the overlay immediately with the bundled `MODELS` constant; the backend response can refresh it later but the user sees a usable list right away. 2. The theme picker overlay emits `OverlayResult::Selected("neo.theme.set:<id>")` when the user confirms a row, but `execute_action` had no match arm for that prefix - so the dispatcher silently fell into the catch-all `Consumed` no-op. The overlay closed, no theme changed, no error was shown. Classic Bug-3 silent failure. Added an `apply_theme_selection` helper (kept `execute_action` under the 100-line clippy threshold) that calls `theme::load_by_id`, swaps `self.theme` on success, or pushes a `Role::Error` chat bubble plus footer `Status::Error` / `theme load failed` on failure (e.g. typo, stale config). Either way the user sees the result. Tests added (`packages/neo-tui/tests/app_loop.rs`): - `ctrl_l_actually_opens_the_model_picker_overlay` - locks the overlay-open contract. - `theme_picker_selection_applies_the_chosen_theme` - drives the `neo.theme.set:dracula` action and verifies the resolved `Token::Background` color flips to the dracula registry entry (display name comparison would be brittle - JSON `name` is "Dracula", registry id is `dracula`). - `theme_picker_selection_with_unknown_id_pushes_a_chat_error` - locks the no-silent-fail contract for unknown registry ids. Plus a new `#[doc(hidden)] pub fn execute_action_for_tests` lets integration tests drive a specific action id without re-implementing the keymap dispatch chain. Used only by the two new theme-set tests. Test count is now 308 passing (was 305 in #15), 0 failing, 1 ignored. `cargo clippy --package senpi-neo-tui --all-targets -- -D warnings` and `cargo fmt --package senpi-neo-tui -- --check` both green. Refs #14
…agged Oracle round-5 audit of PR #15 found four more leak vectors of the user's original Bug 3 contract ("if there is an error, say so"): 1. `Inbound::Response { success: false }` and `Inbound::ParseError` were already wired in the earlier rounds, but the model picker (`Selected("neo.model.set:<id>")`) silently consumed the selection. Picking a model closed the overlay with no visible change and no backend traffic. Added `apply_model_selection` that pushes a Role::System chat note pointing the user at legacy senpi for runtime model switching until the SetModel/provider plumbing lands. 2. The slash menu, command palette, and /hotkeys overlay all advertised actions the neo TUI never wired - `app.session.*`, `app.tree.*`, `app.tree.filter.*`, `app.models.*`, `app.editor.external`, `app.message.followUp`/`dequeue`, `app.clipboard.pasteImage`, `neo.sidebar.toggle`, `neo.compact`, `neo.toggle_animations`. Selecting any of them was a silent no-op. Added `ADVERTISED_BUT_UNIMPLEMENTED_ACTIONS` set + a `note_unimplemented_action` helper that pushes a one-line "not yet wired in --neo" chat-system note. The chord is visibly accounted for. 3. `maybe_spawn_backend()` swallowed `RpcClient::spawn` failures with `.ok()`. A misconfigured `SENPI_NEO_BACKEND_BIN` booted as if no backend were attached - identical to demo mode - and the user only saw a generic "No backend process is connected." message after their first command, with the underlying `ClientError` text lost. Now returns `Result<Option<RpcClient>, String>`, and `drive()` immediately feeds the failure to `apply_inbound(Inbound::Error { ... })` before the first render so the user sees the actual spawn error in the chat + footer. 4. `spawn_writer` and `spawn_stdout_reader` exited silently on I/O errors (`write_all`, `flush`, `lines.next_line` returning `Err`). The user only saw the eventual child exit much later - the actual transport failure point was lost. Both now own a clone of `inbound_tx` and send `Inbound::Error` with the io error text before exiting. `spawn_writer` also now reports serialization failures via the same channel instead of continuing silently. Tests added (TDD red-then-green): - `model_picker_selection_pushes_visible_feedback` - locks the chat-note contract for `neo.model.set:<id>`. - `unimplemented_slash_command_visibly_notifies_user` - locks the not-yet-wired note for `app.session.new` (representative of the whole `ADVERTISED_BUT_UNIMPLEMENTED_ACTIONS` set). Test count: 310 passing (was 308 after round 4), 0 failing, 1 ignored. `cargo fmt --check`, `cargo clippy --all-targets -- -D warnings` clean. Refactored two helpers (`apply_theme_selection`, `apply_model_selection`, `note_unimplemented_action`) out of `execute_action` to keep it under the clippy 100-line threshold. Refs #14, #15
…ound 6 Oracle's sixth review of PR #15 head (`35bbdece`) confirmed the spawn / writer / stdout-reader / picker / unimplemented-action fixes from round 5 but flagged six additional places where the "if there's an error, say so" contract still leaked through: 1. `app.editor.external` (Ctrl+G) returned `AppAction::ExternalEditor`, but `action_to_command` maps that variant to `None` and the run loop has no other handler - the keystroke produced zero visible effect. The dispatcher now also pushes a chat-system note explaining the in-buffer external editor is not yet wired in `senpi --neo`, while still returning the typed variant so the existing parity test continues to lock the dispatch contract. Extracted as `App::apply_external_editor_action` to keep `execute_action` under clippy's 100-line ceiling. 2. `app.suspend` (Ctrl+Z) was advertised in `assets/keymaps/default.json:45` and exposed via the slash menu / command palette, but missing from `ADVERTISED_BUT_UNIMPLEMENTED_ACTIONS`. Falling through to the catch-all `_ => AppAction::Consumed(...)` made it a silent no-op. Added to the list so the dispatcher now routes it through `note_unimplemented_action` and surfaces a "not yet wired" chat note. 3. `RpcEvent::MessageEnd { message }` ignored the `message.errorMessage` field. When the assistant turn fails, `agent-loop.ts::buildErrorAssistantMessage` attaches the provider error string to the final assistant message and ships it through `message_end`. The previous arm only dropped the empty bubble and flipped footer idle, silently discarding the error. Extracted `apply_message_end` helper that pops the empty placeholder bubble (existing behavior), then checks for `errorMessage` - if present, pushes `Role::Error` with footer `Status::Error` (`assistant error` label); otherwise falls back to the clean idle path. 4. `RpcEvent::CompactionEnd { aborted, error_message, will_retry, .. }` failure variants were dropped by the catch-all `_ => {}`. Compaction failures typically mean the summarization LLM rejected the request - the user has to know so they can manually compact, fork, or accept the larger context. Added a guarded match arm `if aborted || error_message.is_some()` that calls a new `apply_compaction_failure` helper. Successful compactions stay silent (would flood chat on every routine compaction). 5. `RpcEvent::AutoRetryEnd { success: false, attempt, final_error }` exhausted retries used to silently flip back to idle. Added a destructure-guard match arm `success: false` that calls `apply_auto_retry_failure`, which pushes a `Role::Error` message naming the attempt count and final error and flips the footer to `retry exhausted`. Successful retries stay silent so transient recoveries do not flood chat. 6. `crossterm::EventStream::next()` returning `Some(Err(_))` (terminal input pipe I/O error) and `None` (stream exhausted / TTY closed) were both silently consumed by `_ => {}` in `drive`'s match arm. The user got a silent freeze with no chat / footer indication of why their keystrokes stopped landing. Extracted `handle_terminal_event` helper plus a `TerminalEventOutcome` enum (Continue / Quit / Disconnected / BackendChannelClosed) so the drive loop can surface input-pipe errors as `Inbound::Error` and treat stream exhaustion as `Inbound::Disconnected` before breaking. The helper extraction also keeps `drive` under clippy's per-fn line ceiling. 7. `spawn_stderr_reader` used `while let Ok(Some(line))`, silently dropping any `Err(_)` from `lines.next_line()`. The child watcher eventually surfaces the exit, but until then the user had no idea why the diagnostic stream went quiet. The function now takes an `inbound_tx` clone and emits `Inbound::Error { stderr_tail: "backend stderr read failed: ..." }` immediately on transport read failure, matching the round-5 `spawn_writer` / `spawn_stdout_reader` shape. Refactor: extracted `apply_message_update_delta`, `apply_message_end`, `apply_compaction_failure`, `apply_auto_retry_failure`, `apply_external_editor_action`, `handle_terminal_event` helpers + the `TerminalEventOutcome` enum to keep `apply_event`, `execute_action`, and `drive` under clippy's `too_many_lines` ceiling and to express the new arms as typed unit-test surfaces. Tests added (TDD red-then-green): six regression tests in `tests/app_loop.rs` for Defects 1-5, plus two inverse-contract tests (`apply_inbound_compaction_end_success_does_not_disturb_chat`, `apply_inbound_auto_retry_end_success_does_not_disturb_chat`) that lock the "no chat noise on success" guarantee against future over-firing. Defects 6 and 7 live in spawned tokio tasks driven by external transports and are verified by code review + the helper extraction (`handle_terminal_event` returns a typed outcome enum, so the drive loop's match exhaustively names every failure path, and the stderr-reader refactor mirrors the round-5 stdout-reader / writer patterns). Existing tests untouched. Total Rust test count is now 318 passing (was 310 after round 5), 0 failing, 1 ignored. `cargo clippy --package senpi-neo-tui --all-targets -- -D warnings`, `cargo fmt --package senpi-neo-tui -- --check`, and `npm run check` (897 files) all green. Refs #14, #15.
Oracle's round-7 review of HEAD `f789df30` confirmed every round-6
fix and the three quality gates, but flagged one last Bug-3 silent
path: `tui.select.{up,down,pageUp,pageDown,confirm,cancel}` are
bound in the bundled keymap and exposed by the command palette, but
they only have a useful effect while an overlay is open. The
compositor's `synthesise_select_event` translates them to a
synthetic `KeyEvent` that the active overlay's raw handler consumes;
with no overlay open the dispatcher fell into the catch-all silent
`Consumed` arm and produced zero feedback. Selecting
`tui.select.up` from the palette closed it with nothing visible.
Bug 3 contract: every advertised chord that lands must produce
visible feedback. `note_unimplemented_action` would be misleading
here (these actions ARE wired - just only inside an overlay), so
the fix introduces a separate path.
Added:
- `OVERLAY_SCOPED_SELECT_ACTIONS` module-level constant with the
six `tui.select.*` ids.
- `is_overlay_scoped_select_action(id) -> bool` predicate.
- `App::note_overlay_only_action(action_id) -> AppAction` that
pushes a `Role::System` chat note explaining that the action
only takes effect while an overlay (slash menu, command palette,
model / theme picker, help) is open.
- Dispatcher arm in `execute_action` ordered BEFORE the existing
`is_advertised_unimplemented_action` predicate so the
overlay-scoping message wins over the generic "not yet wired"
message.
Regression test: `tui_select_action_outside_overlay_visibly_notifies_user`
in `tests/app_loop.rs` (asserts `AppAction::Consumed` plus a
`Role::System` chat message that names the action AND mentions
"overlay").
Total Rust test count is now 319 passing (was 318 after round 6),
0 failing, 1 ignored. `cargo clippy --package senpi-neo-tui --all-targets -- -D warnings`,
`cargo fmt --package senpi-neo-tui -- --check`, and `npm run check`
(897 files) all green.
Refs #14, #15
Oracle's eighth review of HEAD `8ac34409` confirmed every prior fix (rounds 1-7) and the three quality gates, then surfaced two more silent paths: 1. `neo.slash.open` (`/`) is bound in the bundled keymap and exposed by the command palette. The raw key path in `handle_key` opens the slash overlay only when the user types `/` with an empty Input-focus buffer (so mid-prompt `/` inserts literally). When the action was dispatched THROUGH `execute_action` (the palette path), there was no arm and it fell into the catch-all silent consume. Selecting `/help` from the palette closed the palette with no slash overlay and no chat note. New `App::open_slash_overlay` helper opens the overlay unconditionally - the palette explicitly picked the action, so the buffer-empty precondition no longer applies. Regression: `neo_slash_open_dispatched_from_palette_opens_slash_overlay`. 2. `tui.input.tab` (`tab`) is bound in the keymap and surfaced by the palette. `try_autocomplete_action` consumes it ONLY when an autocomplete popup is visible; outside that context it returned `None` and the dispatcher fell into the catch-all silent consume. New `App::note_autocomplete_only_action` helper pushes a chat-system note: "`tui.input.tab` only takes effect while an autocomplete popup is showing. Type `@` for path completion or `/` for slash commands first." Mirrors the `note_overlay_only_action` shape from round 7. Regression: `tui_input_tab_outside_autocomplete_visibly_notifies_user`. Cleanup (per Oracle's audit recommendation): removed the dead `"app.message.followUp"` entry from `ADVERTISED_BUT_UNIMPLEMENTED_ACTIONS`. The explicit `execute_action` arm at line 645 has always shadowed it - the list entry was audit-misleading. Replaced with a comment explaining the precedence so future auditors do not re-add it. Refactor (under clippy's 100-line ceiling): extracted `App::open_slash_overlay`, `App::apply_follow_up_action`, `App::apply_submit_action`, and `App::note_autocomplete_only_action` from `execute_action`. 1:1 with their inline equivalents, no behavior change. Oracle's exhaustive walk of `assets/keymaps/default.json` confirmed the only remaining class-(E) defect (raw-handler-only path) was `neo.slash.open`. No class-(F) orphans found. Every advertised default keybinding now routes to one of: real behavior, `note_unimplemented_action`, `note_overlay_only_action`, `note_autocomplete_only_action`, or an `apply_*` / `open_*` overlay/message helper. Total Rust test count is now 321 passing (was 319 after round 7), 0 failing, 1 ignored. `cargo clippy --package senpi-neo-tui --all-targets -- -D warnings`, `cargo fmt --package senpi-neo-tui -- --check`, and `npm run check` (897 files) all green. Refs #14, #15.
Oracle's ninth review of HEAD `1e08e6b7` flagged the last
remaining Bug-3 silent path: `app.exit` with a non-empty input
buffer returned `AppAction::Consumed("tui.editor.deleteCharForward")`
WITHOUT actually performing a delete-char-forward. The label
string was emitted but the buffer never mutated. Selecting
`/quit` from the slash menu / command palette while drafting a
message silently closed the menu with no quit, no delete, no
chat feedback - a textbook Bug 3 violation.
The old branch was trying to mimic legacy senpi's
`Ctrl+D-on-non-empty-buffer = delete-char-forward` semantics,
but the implementation never actually performed the delete.
Tested behavior was empty-buffer Ctrl+D = Quit only; the
non-empty branch was unreached in tests and silently broken.
Fix: `app.exit` now always returns `AppAction::Quit`. The user
explicitly invoked exit - whether via Ctrl+D, /quit from the
slash menu, or selecting "app.exit" from the palette - and we
respect that. Users wanting the legacy keystroke semantic
(Ctrl+D mid-text deletes a character) can rebind Ctrl+D to
`tui.editor.deleteCharForward` in their keymap; that arm
already exists with the correct delete behavior.
Regression: `app_exit_dispatched_from_palette_quits_even_with_nonempty_buffer`
in `tests/app_loop.rs` drives `execute_action_for_tests("app.exit")`
with a populated input buffer and asserts `AppAction::Quit`.
The existing `ctrl_d_with_empty_input_resolves_to_app_exit`
continues to pass.
Existing tests untouched. Total Rust test count is now 322
passing (was 321 after round 8), 0 failing, 1 ignored.
`cargo clippy --package senpi-neo-tui --all-targets -- -D warnings`,
`cargo fmt --package senpi-neo-tui -- --check`, and
`npm run check` (897 files) all green.
Refs #14, #15
#15) * fix(neo-tui): surface every silent backend failure + wire alt+t + accept opencode/ id prefix Oracle round-3 review of merged PR #14 (commit 45dbb57) caught four silent regressions that all violate the original Bug 3 contract ('if there is an error, say so'): 1. `Inbound::Response { success: false }` was matched to `{}` and dropped. Every backend command rejection - submit, cycle-model, abort, get-available-models - vanished without UI feedback. Now surfaces as a Role::Error chat bubble + Status::Error footer with the original `response.error` string when present, or a command-name fallback when the backend omitted the message. 2. `Inbound::ParseError { line, source }` only emitted a `tracing::warn!` line, which is invisible to a user running `senpi --neo` in a real terminal. The previous regression test `app_inbound_parse_error_does_not_disrupt_ui` actively locked the bad behavior in. Renamed to `app_inbound_parse_error_surfaces_to_chat_and_footer` and inverted the assertions; `apply_inbound` now pushes a Role::Error bubble with a truncated frame preview + flips the footer to 'protocol error' while still keeping the tracing line for log aggregation. 3. `senpi-neo-tui --theme opencode/dracula` (the exact form the README documents) failed with `reading theme json opencode/dracula: No such file or directory`. Root cause: `looks_like_theme_path` used `value.contains('/')` as the path heuristic, which ate every namespaced bundled id. Replaced the heuristic with an explicit path-indicator list (`Path::is_file` || starts with `~/`, `./`, `../`, or `/`) and added a registry-side `strip_prefix("opencode/")` so both `dracula` and `opencode/dracula` resolve to the same theme. 4. `Alt+T` was bound to `neo.theme.picker` in the default keymap and advertised in the README + help overlay, but the dispatcher had no match arm for it, so the chord silently fell through to the `Consumed` catch-all and the overlay never opened. Added a `neo.theme.picker` arm that builds a `ThemePickerOverlay` seeded with the active theme's name and returns the new `AppAction::OpenThemePicker` variant. TDD: each fix is locked by a new failing-then-passing test in `tests/app_loop.rs` (4 added covering Response success+failure shapes, ParseError surfacing, and Alt+T overlay open) and `tests/theme.rs` (`opencode_themes_are_resolvable_by_both_flat_id_and_opencode_prefix`). Total: 305 Rust tests passing (was 298), 0 failing, 1 ignored. `cargo clippy --all-targets -- -D warnings` and `cargo fmt -- --check` green, `npm run check` clean, 9 TS regression tests still pass. Refs #14 * fix(neo-tui): wire ctrl+l + alt+t picker selection paths end-to-end Oracle round-4 review of PR #15 (`cbcc9331`) found two more silent no-op paths in the picker plumbing: 1. `Ctrl+L` returned `AppAction::OpenModelPicker` and fired `Command::GetAvailableModels` to the backend, but never actually set `self.overlay = Some(Overlay::ModelPicker(_))`, so the advertised model picker just never appeared. Now opens the overlay immediately with the bundled `MODELS` constant; the backend response can refresh it later but the user sees a usable list right away. 2. The theme picker overlay emits `OverlayResult::Selected("neo.theme.set:<id>")` when the user confirms a row, but `execute_action` had no match arm for that prefix - so the dispatcher silently fell into the catch-all `Consumed` no-op. The overlay closed, no theme changed, no error was shown. Classic Bug-3 silent failure. Added an `apply_theme_selection` helper (kept `execute_action` under the 100-line clippy threshold) that calls `theme::load_by_id`, swaps `self.theme` on success, or pushes a `Role::Error` chat bubble plus footer `Status::Error` / `theme load failed` on failure (e.g. typo, stale config). Either way the user sees the result. Tests added (`packages/neo-tui/tests/app_loop.rs`): - `ctrl_l_actually_opens_the_model_picker_overlay` - locks the overlay-open contract. - `theme_picker_selection_applies_the_chosen_theme` - drives the `neo.theme.set:dracula` action and verifies the resolved `Token::Background` color flips to the dracula registry entry (display name comparison would be brittle - JSON `name` is "Dracula", registry id is `dracula`). - `theme_picker_selection_with_unknown_id_pushes_a_chat_error` - locks the no-silent-fail contract for unknown registry ids. Plus a new `#[doc(hidden)] pub fn execute_action_for_tests` lets integration tests drive a specific action id without re-implementing the keymap dispatch chain. Used only by the two new theme-set tests. Test count is now 308 passing (was 305 in #15), 0 failing, 1 ignored. `cargo clippy --package senpi-neo-tui --all-targets -- -D warnings` and `cargo fmt --package senpi-neo-tui -- --check` both green. Refs #14 * fix(neo-tui): close every Bug-3 silent-failure path Oracle round 5 flagged Oracle round-5 audit of PR #15 found four more leak vectors of the user's original Bug 3 contract ("if there is an error, say so"): 1. `Inbound::Response { success: false }` and `Inbound::ParseError` were already wired in the earlier rounds, but the model picker (`Selected("neo.model.set:<id>")`) silently consumed the selection. Picking a model closed the overlay with no visible change and no backend traffic. Added `apply_model_selection` that pushes a Role::System chat note pointing the user at legacy senpi for runtime model switching until the SetModel/provider plumbing lands. 2. The slash menu, command palette, and /hotkeys overlay all advertised actions the neo TUI never wired - `app.session.*`, `app.tree.*`, `app.tree.filter.*`, `app.models.*`, `app.editor.external`, `app.message.followUp`/`dequeue`, `app.clipboard.pasteImage`, `neo.sidebar.toggle`, `neo.compact`, `neo.toggle_animations`. Selecting any of them was a silent no-op. Added `ADVERTISED_BUT_UNIMPLEMENTED_ACTIONS` set + a `note_unimplemented_action` helper that pushes a one-line "not yet wired in --neo" chat-system note. The chord is visibly accounted for. 3. `maybe_spawn_backend()` swallowed `RpcClient::spawn` failures with `.ok()`. A misconfigured `SENPI_NEO_BACKEND_BIN` booted as if no backend were attached - identical to demo mode - and the user only saw a generic "No backend process is connected." message after their first command, with the underlying `ClientError` text lost. Now returns `Result<Option<RpcClient>, String>`, and `drive()` immediately feeds the failure to `apply_inbound(Inbound::Error { ... })` before the first render so the user sees the actual spawn error in the chat + footer. 4. `spawn_writer` and `spawn_stdout_reader` exited silently on I/O errors (`write_all`, `flush`, `lines.next_line` returning `Err`). The user only saw the eventual child exit much later - the actual transport failure point was lost. Both now own a clone of `inbound_tx` and send `Inbound::Error` with the io error text before exiting. `spawn_writer` also now reports serialization failures via the same channel instead of continuing silently. Tests added (TDD red-then-green): - `model_picker_selection_pushes_visible_feedback` - locks the chat-note contract for `neo.model.set:<id>`. - `unimplemented_slash_command_visibly_notifies_user` - locks the not-yet-wired note for `app.session.new` (representative of the whole `ADVERTISED_BUT_UNIMPLEMENTED_ACTIONS` set). Test count: 310 passing (was 308 after round 4), 0 failing, 1 ignored. `cargo fmt --check`, `cargo clippy --all-targets -- -D warnings` clean. Refactored two helpers (`apply_theme_selection`, `apply_model_selection`, `note_unimplemented_action`) out of `execute_action` to keep it under the clippy 100-line threshold. Refs #14, #15 * fix(neo-tui): surface six more Bug 3 silent paths flagged by Oracle round 6 Oracle's sixth review of PR #15 head (`35bbdece`) confirmed the spawn / writer / stdout-reader / picker / unimplemented-action fixes from round 5 but flagged six additional places where the "if there's an error, say so" contract still leaked through: 1. `app.editor.external` (Ctrl+G) returned `AppAction::ExternalEditor`, but `action_to_command` maps that variant to `None` and the run loop has no other handler - the keystroke produced zero visible effect. The dispatcher now also pushes a chat-system note explaining the in-buffer external editor is not yet wired in `senpi --neo`, while still returning the typed variant so the existing parity test continues to lock the dispatch contract. Extracted as `App::apply_external_editor_action` to keep `execute_action` under clippy's 100-line ceiling. 2. `app.suspend` (Ctrl+Z) was advertised in `assets/keymaps/default.json:45` and exposed via the slash menu / command palette, but missing from `ADVERTISED_BUT_UNIMPLEMENTED_ACTIONS`. Falling through to the catch-all `_ => AppAction::Consumed(...)` made it a silent no-op. Added to the list so the dispatcher now routes it through `note_unimplemented_action` and surfaces a "not yet wired" chat note. 3. `RpcEvent::MessageEnd { message }` ignored the `message.errorMessage` field. When the assistant turn fails, `agent-loop.ts::buildErrorAssistantMessage` attaches the provider error string to the final assistant message and ships it through `message_end`. The previous arm only dropped the empty bubble and flipped footer idle, silently discarding the error. Extracted `apply_message_end` helper that pops the empty placeholder bubble (existing behavior), then checks for `errorMessage` - if present, pushes `Role::Error` with footer `Status::Error` (`assistant error` label); otherwise falls back to the clean idle path. 4. `RpcEvent::CompactionEnd { aborted, error_message, will_retry, .. }` failure variants were dropped by the catch-all `_ => {}`. Compaction failures typically mean the summarization LLM rejected the request - the user has to know so they can manually compact, fork, or accept the larger context. Added a guarded match arm `if aborted || error_message.is_some()` that calls a new `apply_compaction_failure` helper. Successful compactions stay silent (would flood chat on every routine compaction). 5. `RpcEvent::AutoRetryEnd { success: false, attempt, final_error }` exhausted retries used to silently flip back to idle. Added a destructure-guard match arm `success: false` that calls `apply_auto_retry_failure`, which pushes a `Role::Error` message naming the attempt count and final error and flips the footer to `retry exhausted`. Successful retries stay silent so transient recoveries do not flood chat. 6. `crossterm::EventStream::next()` returning `Some(Err(_))` (terminal input pipe I/O error) and `None` (stream exhausted / TTY closed) were both silently consumed by `_ => {}` in `drive`'s match arm. The user got a silent freeze with no chat / footer indication of why their keystrokes stopped landing. Extracted `handle_terminal_event` helper plus a `TerminalEventOutcome` enum (Continue / Quit / Disconnected / BackendChannelClosed) so the drive loop can surface input-pipe errors as `Inbound::Error` and treat stream exhaustion as `Inbound::Disconnected` before breaking. The helper extraction also keeps `drive` under clippy's per-fn line ceiling. 7. `spawn_stderr_reader` used `while let Ok(Some(line))`, silently dropping any `Err(_)` from `lines.next_line()`. The child watcher eventually surfaces the exit, but until then the user had no idea why the diagnostic stream went quiet. The function now takes an `inbound_tx` clone and emits `Inbound::Error { stderr_tail: "backend stderr read failed: ..." }` immediately on transport read failure, matching the round-5 `spawn_writer` / `spawn_stdout_reader` shape. Refactor: extracted `apply_message_update_delta`, `apply_message_end`, `apply_compaction_failure`, `apply_auto_retry_failure`, `apply_external_editor_action`, `handle_terminal_event` helpers + the `TerminalEventOutcome` enum to keep `apply_event`, `execute_action`, and `drive` under clippy's `too_many_lines` ceiling and to express the new arms as typed unit-test surfaces. Tests added (TDD red-then-green): six regression tests in `tests/app_loop.rs` for Defects 1-5, plus two inverse-contract tests (`apply_inbound_compaction_end_success_does_not_disturb_chat`, `apply_inbound_auto_retry_end_success_does_not_disturb_chat`) that lock the "no chat noise on success" guarantee against future over-firing. Defects 6 and 7 live in spawned tokio tasks driven by external transports and are verified by code review + the helper extraction (`handle_terminal_event` returns a typed outcome enum, so the drive loop's match exhaustively names every failure path, and the stderr-reader refactor mirrors the round-5 stdout-reader / writer patterns). Existing tests untouched. Total Rust test count is now 318 passing (was 310 after round 5), 0 failing, 1 ignored. `cargo clippy --package senpi-neo-tui --all-targets -- -D warnings`, `cargo fmt --package senpi-neo-tui -- --check`, and `npm run check` (897 files) all green. Refs #14, #15. * fix(neo-tui): surface tui.select.* outside an overlay (Oracle round 7) Oracle's round-7 review of HEAD `f789df30` confirmed every round-6 fix and the three quality gates, but flagged one last Bug-3 silent path: `tui.select.{up,down,pageUp,pageDown,confirm,cancel}` are bound in the bundled keymap and exposed by the command palette, but they only have a useful effect while an overlay is open. The compositor's `synthesise_select_event` translates them to a synthetic `KeyEvent` that the active overlay's raw handler consumes; with no overlay open the dispatcher fell into the catch-all silent `Consumed` arm and produced zero feedback. Selecting `tui.select.up` from the palette closed it with nothing visible. Bug 3 contract: every advertised chord that lands must produce visible feedback. `note_unimplemented_action` would be misleading here (these actions ARE wired - just only inside an overlay), so the fix introduces a separate path. Added: - `OVERLAY_SCOPED_SELECT_ACTIONS` module-level constant with the six `tui.select.*` ids. - `is_overlay_scoped_select_action(id) -> bool` predicate. - `App::note_overlay_only_action(action_id) -> AppAction` that pushes a `Role::System` chat note explaining that the action only takes effect while an overlay (slash menu, command palette, model / theme picker, help) is open. - Dispatcher arm in `execute_action` ordered BEFORE the existing `is_advertised_unimplemented_action` predicate so the overlay-scoping message wins over the generic "not yet wired" message. Regression test: `tui_select_action_outside_overlay_visibly_notifies_user` in `tests/app_loop.rs` (asserts `AppAction::Consumed` plus a `Role::System` chat message that names the action AND mentions "overlay"). Total Rust test count is now 319 passing (was 318 after round 6), 0 failing, 1 ignored. `cargo clippy --package senpi-neo-tui --all-targets -- -D warnings`, `cargo fmt --package senpi-neo-tui -- --check`, and `npm run check` (897 files) all green. Refs #14, #15 * fix(neo-tui): close two last Bug 3 leaks Oracle round 8 flagged Oracle's eighth review of HEAD `8ac34409` confirmed every prior fix (rounds 1-7) and the three quality gates, then surfaced two more silent paths: 1. `neo.slash.open` (`/`) is bound in the bundled keymap and exposed by the command palette. The raw key path in `handle_key` opens the slash overlay only when the user types `/` with an empty Input-focus buffer (so mid-prompt `/` inserts literally). When the action was dispatched THROUGH `execute_action` (the palette path), there was no arm and it fell into the catch-all silent consume. Selecting `/help` from the palette closed the palette with no slash overlay and no chat note. New `App::open_slash_overlay` helper opens the overlay unconditionally - the palette explicitly picked the action, so the buffer-empty precondition no longer applies. Regression: `neo_slash_open_dispatched_from_palette_opens_slash_overlay`. 2. `tui.input.tab` (`tab`) is bound in the keymap and surfaced by the palette. `try_autocomplete_action` consumes it ONLY when an autocomplete popup is visible; outside that context it returned `None` and the dispatcher fell into the catch-all silent consume. New `App::note_autocomplete_only_action` helper pushes a chat-system note: "`tui.input.tab` only takes effect while an autocomplete popup is showing. Type `@` for path completion or `/` for slash commands first." Mirrors the `note_overlay_only_action` shape from round 7. Regression: `tui_input_tab_outside_autocomplete_visibly_notifies_user`. Cleanup (per Oracle's audit recommendation): removed the dead `"app.message.followUp"` entry from `ADVERTISED_BUT_UNIMPLEMENTED_ACTIONS`. The explicit `execute_action` arm at line 645 has always shadowed it - the list entry was audit-misleading. Replaced with a comment explaining the precedence so future auditors do not re-add it. Refactor (under clippy's 100-line ceiling): extracted `App::open_slash_overlay`, `App::apply_follow_up_action`, `App::apply_submit_action`, and `App::note_autocomplete_only_action` from `execute_action`. 1:1 with their inline equivalents, no behavior change. Oracle's exhaustive walk of `assets/keymaps/default.json` confirmed the only remaining class-(E) defect (raw-handler-only path) was `neo.slash.open`. No class-(F) orphans found. Every advertised default keybinding now routes to one of: real behavior, `note_unimplemented_action`, `note_overlay_only_action`, `note_autocomplete_only_action`, or an `apply_*` / `open_*` overlay/message helper. Total Rust test count is now 321 passing (was 319 after round 7), 0 failing, 1 ignored. `cargo clippy --package senpi-neo-tui --all-targets -- -D warnings`, `cargo fmt --package senpi-neo-tui -- --check`, and `npm run check` (897 files) all green. Refs #14, #15. * fix(neo-tui): app.exit always quits (Oracle round 9) Oracle's ninth review of HEAD `1e08e6b7` flagged the last remaining Bug-3 silent path: `app.exit` with a non-empty input buffer returned `AppAction::Consumed("tui.editor.deleteCharForward")` WITHOUT actually performing a delete-char-forward. The label string was emitted but the buffer never mutated. Selecting `/quit` from the slash menu / command palette while drafting a message silently closed the menu with no quit, no delete, no chat feedback - a textbook Bug 3 violation. The old branch was trying to mimic legacy senpi's `Ctrl+D-on-non-empty-buffer = delete-char-forward` semantics, but the implementation never actually performed the delete. Tested behavior was empty-buffer Ctrl+D = Quit only; the non-empty branch was unreached in tests and silently broken. Fix: `app.exit` now always returns `AppAction::Quit`. The user explicitly invoked exit - whether via Ctrl+D, /quit from the slash menu, or selecting "app.exit" from the palette - and we respect that. Users wanting the legacy keystroke semantic (Ctrl+D mid-text deletes a character) can rebind Ctrl+D to `tui.editor.deleteCharForward` in their keymap; that arm already exists with the correct delete behavior. Regression: `app_exit_dispatched_from_palette_quits_even_with_nonempty_buffer` in `tests/app_loop.rs` drives `execute_action_for_tests("app.exit")` with a populated input buffer and asserts `AppAction::Quit`. The existing `ctrl_d_with_empty_input_resolves_to_app_exit` continues to pass. Existing tests untouched. Total Rust test count is now 322 passing (was 321 after round 8), 0 failing, 1 ignored. `cargo clippy --package senpi-neo-tui --all-targets -- -D warnings`, `cargo fmt --package senpi-neo-tui -- --check`, and `npm run check` (897 files) all green. Refs #14, #15 * fix(neo-tui): close three more Bug-3 silent paths Oracle round 10 flagged 1. `app.thinking.toggle` (Ctrl+T) and `app.tools.expand` (Ctrl+O) flipped `self.thinking_visible` / `self.tools_expanded` but no render path consumed those fields, so the chord produced zero visible effect. New `apply_thinking_visibility_toggle` / `apply_tools_expanded_toggle` helpers preserve the field mutation (so wiring renderer hookups later is a one-line change) and push a chat-system "not yet wired to chat rendering" note so the chord visibly lands now. 2. Successful `cycle_model` / `set_model` / `cycle_thinking_level` responses were silently dropped by `apply_inbound` (only the `success: false` branch was wired). Pressing Ctrl+P or Shift+Tab fired the RPC, the backend changed the model / level, but the user saw no header / footer / chat update. New `apply_response` extracted helper routes successful responses to `apply_model_change_response` and `apply_thinking_change_response`. Both update header + footer AND push a chat note. Null data ("no other model configured") now pushes an explanatory note so the chord still produces visible feedback. 3. `app.model.cycleBackward` (Shift+Ctrl+P) silently cycled FORWARD because `action_to_command` mapped both directions to the same forward-only `Command::CycleModel` (the wire protocol is next-only today). Now routes through `note_unimplemented_action` so the user sees an explicit "not yet wired" note instead of getting the wrong model. Refactor: simplified `AppAction::CycleModel { forward: bool }` to `AppAction::CycleModel` since the discriminator was always discarded by `action_to_command` and only `forward: true` is reachable. Backward cycling lands at `note_unimplemented_action` in `execute_action` before producing the variant. Test count: 322 -> 329 passing. Seven new tests cover the three defects. Four existing tests updated to assert the new behavior. One test replaced (action_to_command_maps_cycle_model_regardless_of_direction -> action_to_command_maps_cycle_model_to_cycle_model_command). `cargo fmt --check`, `cargo clippy --all-targets -- -D warnings`, `cargo test -j 1`, and `npm run check` (897 files) all green. * fix(neo-tui): surface extension_ui_request notifications + dialogs (Oracle round 11) Extensions emit `extension_ui_request` JSONL frames on stdout to drive user-facing notifications (`method: "notify"`) and modal dialogs (`select`, `confirm`, `input`, `editor`). The neo-tui `Event` enum had no variant for them, so the `#[serde(other)]` arm caught them as `Event::Other` and `apply_event` silently discarded them. An extension could emit `notifyType: "error"` with body "Command blocked" and the user saw absolutely nothing. Added typed `RpcEvent::ExtensionUiRequest { method, message, notify_type, title }` variant and new `apply_extension_ui_request` helper that routes: - `notify` → Role::Error + footer error state for `notifyType: "error"`, Role::System otherwise. - Dialog methods → Role::System "not yet wired" note naming the method + title; the agent-side timeout auto-resolves until dialog overlays land in a future iteration. - Per-extension UI updates (`setStatus`, `setWidget`, `setTitle`, `set_editor_text`) stay silent. These are per-extension self-management traffic, not user-facing errors. The inverse contract is locked by a regression test. Test count: 329 -> 333 passing. Four new tests cover the four routing cases (error notify, warning notify, dialog method, silent setStatus). See packages/coding-agent/docs/rpc.md → "Extension UI Requests" for the wire contract. `cargo fmt --check`, `cargo clippy --all-targets -- -D warnings`, `cargo test -j 1`, and `npm run check` (897 files) all green.
Summary
Throw-it-out-and-rewrite of
senpi --neowith 39 TDD-locked atomic commits. Portspackages/tui(TypeScript pi-tui, 11,470 LOC) to native Rust+ratatui with patterns from../codex/codex-rs/tuiand../opencode's palette + overrides theme schema. Fixes all 4 user-reported bugs and adds 6 new features.Bugs fixed (verified end-to-end in tmux)
Shift+Enterin tmux submits instead of inserting newlineDISAMBIGUATE_ESCAPE_CODESalone doesn't ask tmux to emit modifyOtherKeys CSI-uterm::TerminalCapsemits\x1b[>4;2mon startup whenTMUXenv detected, plusREPORT_ALL_KEYS_AS_ESCAPE_CODESflag. Default keymap bindsshift+entertotui.editor.newLine.text::wrap_text_with_ansi+ grapheme-awareInputState::display_lines/cursor_visual_positionproduce true multi-line word-wrap. CJK double-width handled viaunicode_width.Inbound::{Error, Disconnected, ParseError}variants;app::apply_inboundconsumes them, pushes chat error bubble, flips footer toStatus::Error + connected: false + label "backend disconnected".right_widthalways reserved the metrics clusterStatusbackground-tint tokens (StatusIdleBg/BusyBg/StreamingBg/ToolBg/ErrorBg), hide metrics when all-zero, dedicated× backend disconnectedline on disconnect.Each fix is locked by a failing-test-first commit pair. tmux QA evidence saved under
.tmp/qa-baseline/during development.New features delivered
@pathautocomplete popup (Tab to apply) and/slash menu (popup overlay) via newAutocompleteengine +SelectListcomponent.SelectList.Token::*).anim::Spinner / Scanner / Pulse) for state-aware footer/cursor breathing.Foundation modules added
text/term/TerminalCapsdetection, tmuxmodifyOtherKeys, OSC 52 clipboard helpercompositor/Componenttrait + stack dispatch (top-down events, bottom-up render) with focus + cursor delegationanim/components/markdown.rsTokencomponents/select_list.rscomponents/settings_list.rsSelectListcomponents/autocomplete.rs/-slash +@-path completion engine vianucleo-matcheroverlay/{ModelPicker,ThemePicker}SelectListrpc/Inbound::{Error,Disconnected,ParseError}Tests
tests/term.rstests/text.rstests/compositor.rstests/editor.rs+tests/app_loop.rs(editor)tests/markdown.rstests/select_list.rstests/autocomplete.rstests/chat_view.rstests/footer.rstests/header.rstests/settings_list.rstests/app_loop.rs(integration)tests/rpc_client.rstests/overlay_pickers.rstests/anim.rstests/app_loop.rs(RPC consumption)298 tests passing (was 156 at branch start; +142).
cargo fmt --check,cargo clippy --package senpi-neo-tui --all-targets -- -D warnings,cargo test --package senpi-neo-tui -j 1all green.Theme audit
ast-grep --pattern 'Color::Rgb($_, $_, $_)' --lang rust packages/neo-tui/src/:theme/mod.rs(resolver) andtheme/opencode_palette.rs(RGB blending). Expected.components/,overlay/,app/,term/,anim/,compositor/,layout/,main.rs. Every render-time color routes throughtheme.token(Token::...).Process discipline
unwrap/expectoutside test modules.unsafewithout SAFETY proof.--no-verifyon commits.git add -A— every commit was file-explicit.tmux kill-serverin any QA harness.ad5c9c22 fix(coding-agent): bound remote compaction attemptsand0f76f18c fix(ai): strip cloudflare anthropic computer tools— out of scope but already committed by another agent, left in place rather than rewriting history).Known follow-ups (deliberately deferred)
/themeslash command dispatcher.GetAvailableModelsplumbing for model picker (currently hardcoded list).cc @code-yeongyu
Summary by cubic
Rewrote the
senpi --neoTUI in Rust (packages/neo-tui) for a faster, markdown-capable chat UI and fixed all 4 reported input/render/state bugs. Added--arg forwarding so neo flags reach the Rust binary; hardenedpackages/aiandpackages/coding-agentfor Cloudflare Anthropic native tool quirks and OpenAI remote compaction timeouts.New Features
/) and@pathautocomplete with fuzzy ranking; Tab to apply.Bug Fixes
neo.input.historyPrev/Next; removed redundanttui.editor.newLine.senpi-neo-darkcompletes all tokens (incl.diffAddedText/diffRemovedText) with an exhaustive test.packages/ai: Cloudflare Anthropic routes strip hook-injected nativecomputer_*tools afteronPayload; computer-use beta headers removed only where unsupported.packages/coding-agent: Remote OpenAI compaction times out cleanly and falls back to local with aremote_fallbackevent.Migration
--sentinel to pass neo flags to the Rust TUI, e.g.senpi --neo -- --theme opencode/dracula,senpi --neo -- --list-themes, or mix backends and neo flags:senpi --neo --provider anthropic -- --demo.Written for commit 159b752. Summary will update on new commits. Review in cubic