From 455f74fcb4e60a31a61ef3d451545cd6603da3f6 Mon Sep 17 00:00:00 2001 From: Edwin Date: Wed, 19 Aug 2026 19:26:20 -0700 Subject: [PATCH] Fix TUI channel editor save UX --- crates/cli/src/app.rs | 89 ++++++++++++++++++- crates/cli/src/app/operator_dialog.rs | 59 ++++++++++-- crates/cli/src/ui.rs | 12 ++- .../0175-operator-view-focus-is-edit-mode.md | 8 ++ 4 files changed, 155 insertions(+), 13 deletions(-) diff --git a/crates/cli/src/app.rs b/crates/cli/src/app.rs index e4677174..73ba8d3f 100644 --- a/crates/cli/src/app.rs +++ b/crates/cli/src/app.rs @@ -45492,7 +45492,7 @@ mod tests { app.open_edit_operator_view("assistant"); app.open_new_operator_channel(); assert_eq!(channel_editor(&app).channel.id, ""); - app.on_key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)) + app.on_key(KeyEvent::new(KeyCode::Char('s'), KeyModifiers::CONTROL)) .await; // Whatever the save round-trip did, the untyped ID became the // suggested one before validation ran — never an empty-ID error. @@ -45505,6 +45505,61 @@ mod tests { server.abort(); } + #[tokio::test] + async fn channel_editor_marks_unsaved_edits_and_names_ctrl_s_save() { + let (mut app, _dir, server) = captured_app().await; + app.operators.push(operator_summary_for_test("assistant")); + app.operator_channel_catalog = app.operators[0].channels.clone(); + app.open_edit_operator_view("assistant"); + app.open_edit_operator_channel(0); + assert!( + !channel_editor(&app).is_dirty(), + "saved channel starts clean" + ); + + let backend = ratatui::backend::TestBackend::new(120, 24); + let mut term = ratatui::Terminal::new(backend).expect("terminal"); + let mut draw = |app: &mut App| { + let editor = channel_editor(app).clone(); + term.draw(|f| { + crate::ui::render_operator_channel_editor(f, f.area(), app, &editor); + }) + .expect("draw"); + rendered_text(term.backend().buffer()) + }; + + let text = draw(&mut app); + assert!(text.contains("Channel · assistant"), "{text}"); + assert!( + !text.contains("Channel · assistant*"), + "clean channel is unmarked" + ); + assert!(text.contains("C-s save"), "save hint is explicit: {text}"); + assert!( + !text.contains("Enter/C-s save"), + "Enter is not advertised as save: {text}" + ); + + app.on_key(KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE)) + .await; + assert!( + channel_editor(&app).is_dirty(), + "editing the port marks the channel dirty" + ); + let text = draw(&mut app); + assert!( + text.contains("Channel · assistant*"), + "dirty channel carries the marker: {text}" + ); + + app.open_new_operator_channel(); + assert!( + channel_editor(&app).is_dirty(), + "a channel that has never been saved is dirty" + ); + server.abort(); + } + #[tokio::test] async fn channel_editor_shows_the_id_placeholder_dimmed_and_underlines_the_input() { let (mut app, _dir, server) = captured_app().await; @@ -45817,8 +45872,24 @@ mod tests { editor.response_mode_overrides = "C-sensitive=auto-after,D-Private=draft".into(); } + assert!(channel_editor(&app).is_dirty()); app.on_key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)) .await; + assert!( + matches!( + request_rx.try_recv(), + Err(tokio::sync::mpsc::error::TryRecvError::Empty) + ), + "Enter must not send a channel save" + ); + assert_eq!( + channel_editor(&app).mode, + OperatorChannelDialogMode::Create, + "Enter leaves the unsaved draft in place" + ); + + app.on_key(KeyEvent::new(KeyCode::Char('s'), KeyModifiers::CONTROL)) + .await; let params = request_rx.recv().await.expect("put request"); assert_eq!( params.channel.response_mode_overrides, @@ -45832,6 +45903,22 @@ mod tests { "C-sensitive=auto-after,D-Private=draft", "the daemon summary is formatted back into the editor" ); + assert!( + !channel_editor(&app).is_dirty(), + "the daemon-confirmed save clears dirty state" + ); + let backend = ratatui::backend::TestBackend::new(120, 24); + let mut term = ratatui::Terminal::new(backend).expect("terminal"); + let editor = channel_editor(&app).clone(); + term.draw(|f| { + crate::ui::render_operator_channel_editor(f, f.area(), &mut app, &editor); + }) + .expect("draw"); + let text = rendered_text(term.backend().buffer()); + assert!( + !text.contains("Channel · assistant*"), + "saved state clears the rendered dirty marker: {text}" + ); server.abort(); } diff --git a/crates/cli/src/app/operator_dialog.rs b/crates/cli/src/app/operator_dialog.rs index ab9fd8f0..64f80930 100644 --- a/crates/cli/src/app/operator_dialog.rs +++ b/crates/cli/src/app/operator_dialog.rs @@ -176,6 +176,10 @@ pub struct OperatorChannelDialog { pub mode: OperatorChannelDialogMode, pub operator_name: String, pub channel: OperatorChannelSummary, + /// The editable channel fields as the daemon last confirmed them. + /// Runtime-only publication state is intentionally ignored by + /// [`Self::is_dirty`]. + pub saved: OperatorChannelSummary, pub selected_field: usize, pub note: Option, pub new_secret: Option, @@ -185,11 +189,53 @@ pub struct OperatorChannelDialog { /// Editable `CHANNEL=MODE` list for slack-personal overrides. Kept as /// text so partially typed entries survive until save-time validation. pub response_mode_overrides: String, + pub saved_response_mode_overrides: String, /// Suggested channel ID, shown dimmed while the created channel's ID is /// still empty and adopted by a save that never typed one. pub id_placeholder: String, } +fn same_editable_channel(left: &OperatorChannelSummary, right: &OperatorChannelSummary) -> bool { + left.id == right.id + && left.kind == right.kind + && left.enabled == right.enabled + && left.port == right.port + && left.allowed_workspaces == right.allowed_workspaces + && left.allowed_channels == right.allowed_channels + && left.progress == right.progress + && left.follow_up == right.follow_up + && left.thread_context == right.thread_context + && left.trigger == right.trigger + && left.response_mode == right.response_mode + && left.auto_after_secs == right.auto_after_secs + && left.disclosure == right.disclosure + && left.poll_interval_secs == right.poll_interval_secs +} + +impl OperatorChannelDialog { + /// A created channel has not been saved yet. Existing channels are dirty + /// when any editable field or write-only credential differs from the last + /// daemon-confirmed state. + pub fn is_dirty(&self) -> bool { + self.mode == OperatorChannelDialogMode::Create + || !same_editable_channel(&self.channel, &self.saved) + || self.response_mode_overrides != self.saved_response_mode_overrides + || !self.app_token.is_empty() + || !self.bot_token.is_empty() + } + + fn adopt_saved(&mut self, channel: OperatorChannelSummary) { + self.response_mode_overrides = + format_response_mode_overrides(channel.response_mode_overrides.as_ref()); + self.saved_response_mode_overrides = self.response_mode_overrides.clone(); + self.saved = channel.clone(); + self.channel = channel; + self.mode = OperatorChannelDialogMode::Edit; + self.app_token.clear(); + self.bot_token.clear(); + } +} + /// Address-level actions exposed by a channel publication. Keeping this typed /// prevents the TUI from assuming every future ingress protocol produces a /// browser URL: URLs can be opened and copied, while socket endpoints can only @@ -901,6 +947,7 @@ impl App { attached_to: Some(dialog.operator.name.clone()), publication: None, }, + saved: OperatorChannelSummary::default(), selected_field: 0, note: Some("HTTP channels bind on loopback as soon as they are saved.".to_string()), new_secret: None, @@ -908,6 +955,7 @@ impl App { app_token: String::new(), bot_token: String::new(), response_mode_overrides: String::new(), + saved_response_mode_overrides: String::new(), id_placeholder: id, }); true @@ -937,6 +985,7 @@ impl App { dialog.channel_editor = Some(OperatorChannelDialog { mode: OperatorChannelDialogMode::Edit, operator_name: dialog.operator.name.clone(), + saved: channel.clone(), channel, selected_field: 2, note: Some(if attached_here { @@ -948,6 +997,7 @@ impl App { confirm_delete: false, app_token: String::new(), bot_token: String::new(), + saved_response_mode_overrides: response_mode_overrides.clone(), response_mode_overrides, id_placeholder: String::new(), }); @@ -1470,13 +1520,7 @@ impl App { parent.adopt_saved(operator); } if let Some(editor) = parent.channel_editor.as_mut() { - editor.mode = OperatorChannelDialogMode::Edit; - editor.response_mode_overrides = format_response_mode_overrides( - result.channel.response_mode_overrides.as_ref(), - ); - editor.channel = result.channel; - editor.app_token.clear(); - editor.bot_token.clear(); + editor.adopt_saved(result.channel); let has_new_secret = new_secret.is_some(); editor.new_secret = new_secret; editor.confirm_delete = false; @@ -2171,7 +2215,6 @@ impl App { dialog.channel_editor = None; } } - KeyCode::Enter => self.save_operator_channel(false).await, KeyCode::Tab | KeyCode::Down => { if let Some(dialog) = self.operator_dialog.as_mut() { if let Some(editor) = dialog.channel_editor.as_mut() { diff --git a/crates/cli/src/ui.rs b/crates/cli/src/ui.rs index 441da088..e3718c30 100644 --- a/crates/cli/src/ui.rs +++ b/crates/cli/src/ui.rs @@ -9298,7 +9298,11 @@ pub(crate) fn render_operator_channel_editor( }; let mut fields = vec![ Line::from(Span::styled( - format!("Channel · {}", editor.operator_name), + format!( + "Channel · {}{}", + editor.operator_name, + if editor.is_dirty() { "*" } else { "" } + ), Style::default() .fg(app.theme.accent) .add_modifier(Modifier::BOLD), @@ -9421,11 +9425,11 @@ pub(crate) fn render_operator_channel_editor( } f.render_widget(Paragraph::new(help).wrap(Wrap { trim: false }), columns[2]); let footer = if editor.mode == crate::app::OperatorChannelDialogMode::Create { - "Enter/C-s save · Esc back" + "C-s save · Esc back" } else if editor.channel.kind == "http" { - "Enter/C-s save · C-r rotate credential · C-d delete · Esc back" + "C-s save · C-r rotate credential · C-d delete · Esc back" } else { - "Enter/C-s save · C-d delete · Esc back" + "C-s save · C-d delete · Esc back" }; let footer_y = area.bottom().saturating_sub(1); f.render_widget( diff --git a/specs/0175-operator-view-focus-is-edit-mode.md b/specs/0175-operator-view-focus-is-edit-mode.md index eaeff786..6e2728f9 100644 --- a/specs/0175-operator-view-focus-is-edit-mode.md +++ b/specs/0175-operator-view-focus-is-edit-mode.md @@ -38,6 +38,12 @@ The pane title marks unsaved state with a trailing `*` on the operator name. confirmed. Channel attachment and detachment are applied straight to the daemon rather than staged in the editor, so they never contribute to unsaved state. +The dedicated editor for an attached channel follows the same explicit-save +language independently of the surrounding definition: `C-s` is its only save +key, `Enter` never saves, and a trailing `*` marks a new or modified channel +until the daemon confirms the save. Its footer always names `C-s save` so the +save boundary is visible while editing. + The view is one continuous navigable list: the definition fields, then the channel catalog rows, then the routed session rows, wrapping back to the first field. Channels are a section, not a definition field. Next/previous @@ -77,6 +83,8 @@ be modelled as a sub-mode of a field. the per-field help, and the field count in step. - Anything that adopts a daemon-confirmed definition into the editor must also refresh the saved baseline, or the title will keep claiming unsaved work. +- A successful channel save must likewise refresh the channel editor's saved + baseline and clear its dirty marker. - Global chord prefixes must keep working over an open editor. The editor no longer closes to make room for them, so it has to stand aside for chord continuation keys.