diff --git a/src/sdk/src/config/mod.rs b/src/sdk/src/config/mod.rs index d88675d3e..ebddd55f0 100644 --- a/src/sdk/src/config/mod.rs +++ b/src/sdk/src/config/mod.rs @@ -57,11 +57,12 @@ pub use persist::{ }; pub use types::{ wire_value, AttributionConfig, BackendConfig, BudgetConfig, ControlStyle, CoreConfig, - EvolveSettings, FieldPlacement, FieldVisibility, FleetConfig, HarnessNameStyle, HarnessSection, - HookDefaultsConfig, HostSection, HubSection, HubWorkerConfig, LinkConfig, LoadedConfig, - McpSection, MedullaConfig, OnboardingConfig, OpencodeConfig, PathStyle, Peer, - ProviderBudgetConfig, RouterConfig, RouterProviderConfig, StatusLineConfig, ThemeConfig, - TuiConfig, UpdateConfig, WorkflowConfig, WorkflowsConfig, DEFAULT_CONTEXT_WINDOW_TOKENS, + EvolveSettings, FavoriteWorkspace, FieldPlacement, FieldVisibility, FleetConfig, + HarnessNameStyle, HarnessSection, HookDefaultsConfig, HostSection, HubSection, HubWorkerConfig, + LinkConfig, LoadedConfig, McpSection, MedullaConfig, OnboardingConfig, OpencodeConfig, + PathStyle, Peer, ProviderBudgetConfig, RouterConfig, RouterProviderConfig, StatusLineConfig, + ThemeConfig, TuiConfig, UpdateConfig, WorkflowConfig, WorkflowsConfig, + DEFAULT_CONTEXT_WINDOW_TOKENS, }; pub use urls::{ default_backend_base_url, display_host, is_staging, resolve_backend_base_url, diff --git a/src/sdk/src/config/types/mod.rs b/src/sdk/src/config/types/mod.rs index 3660b21a2..9b6e3e880 100644 --- a/src/sdk/src/config/types/mod.rs +++ b/src/sdk/src/config/types/mod.rs @@ -74,6 +74,8 @@ mod mcp_tests; mod orchestration; mod presentation; mod status_line; +#[cfg(test)] +mod tests; pub use connections::*; pub use document::*; diff --git a/src/sdk/src/config/types/orchestration.rs b/src/sdk/src/config/types/orchestration.rs index 6260f21d9..cb83e66a1 100644 --- a/src/sdk/src/config/types/orchestration.rs +++ b/src/sdk/src/config/types/orchestration.rs @@ -251,6 +251,24 @@ pub struct HarnessSection { /// orchestrated tasks may run. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub recent_workspaces: Vec, + /// Saved directory shortcuts for the manual harness launcher. + /// + /// Favorites are deliberately separate from recent history: a favorite is + /// an operator's named destination and remains useful even after it has not + /// been used for a while. Like history, it does not grant the orchestrator + /// access to the directory. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub favorite_workspaces: Vec, +} + +/// A durable, operator-chosen name for a directory used by the manual launcher. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "camelCase")] +pub struct FavoriteWorkspace { + /// The short name shown in the directory picker and matched by search. + pub name: String, + /// The directory the favorite opens. + pub path: String, } impl Default for HarnessSection { @@ -259,6 +277,7 @@ impl Default for HarnessSection { handback: "ask".to_string(), skip_permissions: false, recent_workspaces: Vec::new(), + favorite_workspaces: Vec::new(), } } } diff --git a/src/sdk/src/config/types/tests.rs b/src/sdk/src/config/types/tests.rs new file mode 100644 index 000000000..48c8aa1b1 --- /dev/null +++ b/src/sdk/src/config/types/tests.rs @@ -0,0 +1,18 @@ +//! Unit tests for the config data-model types. Serde defaults, camelCase +//! parsing, and round-trip behaviour for each `[section]` the TUI reads. + +use super::*; + +#[test] +fn harness_favorite_workspaces_round_trip_with_their_names() { + let cfg: TuiConfig = serde_json::from_str( + r#"{"harness":{"favoriteWorkspaces":[{"name":"Medulla","path":"/work/medulla"}]}}"#, + ) + .unwrap(); + + assert_eq!(cfg.harness.favorite_workspaces[0].name, "Medulla"); + assert_eq!(cfg.harness.favorite_workspaces[0].path, "/work/medulla"); + assert!(serde_json::to_string(&cfg) + .unwrap() + .contains("\"favoriteWorkspaces\"")); +} diff --git a/src/tui/src/ui/app/commands/dispatch.rs b/src/tui/src/ui/app/commands/dispatch.rs index 029ad9437..0befdb0e0 100644 --- a/src/tui/src/ui/app/commands/dispatch.rs +++ b/src/tui/src/ui/app/commands/dispatch.rs @@ -108,6 +108,10 @@ impl App { self.add_workspace(&text); None } + PromptKind::FavoriteWorkspaceAdd(workspace) => { + self.save_favorite_workspace(&text, &workspace); + None + } PromptKind::CustomHarnessAdd => { self.save_custom_harness(None, &text); None diff --git a/src/tui/src/ui/app/harness_workspace.rs b/src/tui/src/ui/app/harness_workspace.rs index 638c2c6e1..70a66d683 100644 --- a/src/tui/src/ui/app/harness_workspace.rs +++ b/src/tui/src/ui/app/harness_workspace.rs @@ -12,6 +12,20 @@ const MAX_RECENT_WORKSPACES: usize = 12; const FOLDER_SCORE_OFFSET: usize = 5; const KNOWN_FUZZY_SCORE_OFFSET: usize = 20; +/// A known workspace before ranking: the resolved path, where it came from, and +/// the favorite label when it is a saved shortcut. +/// +/// Named fields rather than a `(String, String, Option)` tuple so a +/// path and its provenance cannot be swapped silently at a push site, and the +/// provenance is a `&'static str` because the candidates are rebuilt on every +/// keystroke — only the rows that survive filtering to become +/// [`WorkspaceChoice`]s need their own `String`. +struct WorkspaceCandidate { + path: String, + source: &'static str, + label: Option, +} + impl App { /// Advance the launcher to its workspace step and populate the first list. pub(super) fn open_harness_workspace_step(&mut self, edit_default: bool) { @@ -145,6 +159,113 @@ impl App { .map_err(|error| format!("workspace history was not saved ({error})")) } + /// Persist `workspace` as a named shortcut for the manual launcher. + /// + /// A name replaces any older favorite with the same spelling, while the + /// path is de-duplicated so one directory cannot occupy several top-ranked + /// rows under different aliases. + pub(in crate::ui::app) fn save_favorite_workspace(&mut self, name: &str, workspace: &str) { + let name = name.trim(); + if name.is_empty() { + self.set_status("Favorite name cannot be empty"); + return; + } + let Some(harnesses) = &self.local_sessions else { + self.set_status("This device is not hosting, so it has no workspace favorites"); + return; + }; + let path = harnesses.resolve_workspace(workspace); + if !Path::new(&path).is_dir() { + self.set_status("Favorites must point to an existing directory"); + return; + } + // Build the candidate list without touching the live one: persistence + // is the commit point, so a write failure must not leave a favorite in + // memory that the config file never recorded — the picker would then + // offer a save that is not there, and a later successful save could + // silently persist it. + let mut favorites = self.loaded.config.harness.favorite_workspaces.clone(); + // Compare effective resolved paths rather than the stored spellings: a + // favorite saved from relative input (e.g. `repo` against the host + // workspace) must not survive beside the same directory re-saved under + // its resolved absolute path — a re-alias, not a second directory. + favorites.retain(|favorite| { + !favorite.name.eq_ignore_ascii_case(name) + && harnesses.resolve_workspace(&favorite.path) != path + }); + favorites.insert( + 0, + medulla::config::FavoriteWorkspace { + name: name.to_string(), + path: path.clone(), + }, + ); + let Some(config_path) = &self.config_path else { + self.loaded.config.harness.favorite_workspaces = favorites; + self.set_status(format!( + "Saved favorite {name} · this run only — no config file" + )); + self.after_saving_favorite(&path); + return; + }; + // A favourite is two strings, so serialization cannot realistically + // fail — but the value has to come from somewhere the handler controls, + // so a serialization error reports a status instead of panicking inside + // the UI command. + let Ok(value) = toml::Value::try_from(favorites.clone()) else { + self.set_status("Could not serialize favorite workspaces"); + return; + }; + match medulla::config::persist_setting(config_path, "harness", "favoriteWorkspaces", value) + { + Ok(()) => { + self.loaded.config.harness.favorite_workspaces = favorites; + self.set_status(format!("Saved favorite {name} · {path}")); + self.after_saving_favorite(&path); + } + Err(error) => self.set_status(format!("Could not save favorite ({error})")), + } + } + + /// Re-anchor the launcher on the workspace that was just favorited. + /// + /// Saving the favorite is the operator's way of saying Enter should start + /// the harness there, so the arrowed cursor — which pointed at the saved + /// row beforehand — must follow the saved workspace to the row it now + /// occupies rather than keep its old index and silently select a + /// *different* directory parked in that row. The ranking orders by match + /// score first and insertion order second, so the promoted favorite only + /// leads the list when it also scores best against the active query; + /// otherwise it lands at a non-zero row. Re-run the completions and place + /// the cursor on the actual row of the saved workspace, so the highlight + /// and Enter both follow the favorite. + /// + /// When the rename happens under a filter whose query only matched the old + /// label, the saved favorite is absent from the refreshed list: the refresh + /// dropped the old match while the new name does not match the unchanged + /// query, leaving the list empty or leading with an unrelated row. Forcing + /// the cursor onto row 0 there would make Enter reject the workspace or + /// start the harness in that row, so the query is re-pointed at the saved + /// workspace instead — the one row Enter must still land on. + fn after_saving_favorite(&mut self, saved: &str) { + self.refresh_harness_workspace_choices(); + if let Some(picker) = &mut self.session_picker { + if let Some(index) = picker + .workspace_choices + .iter() + .position(|choice| choice.path == saved) + { + picker.workspace_index = index; + picker.workspace_picked = true; + } else { + picker.workspace_query = saved.to_string(); + picker.workspace_index = 0; + picker.workspace_picked = false; + self.refresh_harness_workspace_choices(); + } + } + } + /// Rank recent, configured, and filesystem-derived workspace suggestions. fn workspace_choices(&self, query: &str) -> Vec { let Some(harnesses) = &self.local_sessions else { @@ -154,25 +275,53 @@ impl App { let process_dir = std::env::current_dir().unwrap_or_else(|_| base.to_path_buf()); let resolved_query = harnesses.resolve_workspace(query); let mut known = Vec::new(); + for favorite in &self.loaded.config.harness.favorite_workspaces { + known.push(WorkspaceCandidate { + path: absolute(&favorite.path, base), + source: "favorite", + label: Some(favorite.name.clone()), + }); + } for path in &self.loaded.config.harness.recent_workspaces { - known.push((absolute(path, base), "recent")); + known.push(WorkspaceCandidate { + path: absolute(path, base), + source: "recent", + label: None, + }); } - known.push((harnesses.workspace.clone(), "default")); + known.push(WorkspaceCandidate { + path: harnesses.workspace.clone(), + source: "default", + label: None, + }); if !self.loaded.config.host.workspace.trim().is_empty() { - known.push(( - absolute(&self.loaded.config.host.workspace, &process_dir), - "registered", - )); + known.push(WorkspaceCandidate { + path: absolute(&self.loaded.config.host.workspace, &process_dir), + source: "registered", + label: None, + }); } for path in &self.loaded.config.host.workspaces { - known.push((absolute(path, &process_dir), "registered")); + known.push(WorkspaceCandidate { + path: absolute(path, &process_dir), + source: "registered", + label: None, + }); } for host in &self.loaded.config.hosts { if !host.workspace.trim().is_empty() { - known.push((absolute(&host.workspace, &process_dir), "registered")); + known.push(WorkspaceCandidate { + path: absolute(&host.workspace, &process_dir), + source: "registered", + label: None, + }); } for path in &host.workspaces { - known.push((absolute(path, &process_dir), "registered")); + known.push(WorkspaceCandidate { + path: absolute(path, &process_dir), + source: "registered", + label: None, + }); } } @@ -180,10 +329,21 @@ impl App { let mut ranked = known .into_iter() .enumerate() - .filter(|(_, (path, _))| Path::new(path).is_dir()) - .filter_map(|(order, (path, source))| { - match_score(&path, query) - .map(|score| (score, order, WorkspaceChoice { path, source })) + .filter(|(_, candidate)| Path::new(&candidate.path).is_dir()) + .filter_map(|(order, candidate)| { + workspace_match_score(&candidate.path, candidate.label.as_deref(), query).map( + |score| { + ( + score, + order, + WorkspaceChoice { + path: candidate.path, + source: candidate.source.to_string(), + label: candidate.label, + }, + ) + }, + ) }) .collect::>(); @@ -198,7 +358,8 @@ impl App { folder_order + index, WorkspaceChoice { path, - source: "folder", + source: "folder".to_string(), + label: None, }, ) }), @@ -222,6 +383,23 @@ impl App { } } +/// Match a saved name and its path, keeping whichever scores better. +/// +/// A favorite is searchable by both spellings an operator can use, and the two +/// can disagree: a query that names the directory exactly is a strictly better +/// match than one that only loosely resembles the label. `or_else` would skip +/// the path score whenever the label scored at all, so a favorite could rank +/// behind a plain filesystem completion for the same directory and lose the +/// row to path de-duplication — the exact case the name was added to fix. +pub(super) fn workspace_match_score(path: &str, label: Option<&str>, query: &str) -> Option { + let label_score = label.and_then(|label| match_score(label, query)); + let path_score = match_score(path, query); + match (label_score, path_score) { + (Some(label_score), Some(path_score)) => Some(label_score.min(path_score)), + (label_score, path_score) => label_score.or(path_score), + } +} + /// Make a configured path absolute against its owning resolution directory. pub(super) fn absolute(path: &str, base: &Path) -> String { let path = Path::new(path); diff --git a/src/tui/src/ui/app/harness_workspace_tests.rs b/src/tui/src/ui/app/harness_workspace_tests.rs index f9cd9e4d1..0ec6f1447 100644 --- a/src/tui/src/ui/app/harness_workspace_tests.rs +++ b/src/tui/src/ui/app/harness_workspace_tests.rs @@ -1,7 +1,7 @@ //! Focused tests for bounded folder completion and fuzzy ranking. use super::harness_workspace::{ - absolute, folder_completions, fuzzy_subsequence_score, match_score, + absolute, folder_completions, fuzzy_subsequence_score, match_score, workspace_match_score, }; #[test] @@ -35,6 +35,35 @@ fn known_workspace_basename_prefixes_beat_filesystem_duplicates() { assert_eq!(match_score("/work/project-beta", "project-b"), Some(1)); } +#[test] +fn favorite_names_are_searchable_as_well_as_their_paths() { + assert_eq!( + workspace_match_score("/work/medulla-public", Some("Primary Medulla"), "primary"), + Some(1) + ); + assert!( + workspace_match_score("/work/medulla-public", Some("Primary Medulla"), "medulla").is_some() + ); +} + +#[test] +fn an_exact_path_match_outranks_a_loose_label_match_for_a_favorite() { + // A query that names the directory exactly must rank the favorite at the + // path score rather than the loose label score, or a plain filesystem + // completion for the same directory would outrank it and win the row after + // path de-duplication. + assert_eq!( + workspace_match_score("/work/medulla", Some("primary medulla"), "medulla"), + Some(0) + ); + // The label's own exact match is still honoured when the path does not + // match at all. + assert_eq!( + workspace_match_score("/work/medulla-public", Some("Primary Medulla"), "primary"), + Some(1) + ); +} + #[test] fn loose_known_matches_do_not_beat_concrete_folder_matches() { let random_parent = match_score("/tmp/.tmpbQM6Hg", "pb").unwrap(); @@ -164,3 +193,306 @@ fn arrowing_onto_a_completion_still_wins_over_the_typed_query() { "a deliberately chosen completion is still what Enter uses" ); } + +#[test] +fn saving_a_named_favorite_persists_it_and_makes_its_name_searchable() { + let root = tempfile::tempdir().unwrap(); + let workspace = root.path().join("medulla"); + std::fs::create_dir(&workspace).unwrap(); + let config = root.path().join("config.toml"); + std::fs::write(&config, "[harness]\n").unwrap(); + let mut app = picker_on_workspace_step(&workspace); + app.set_config_path(config.clone()); + + app.save_favorite_workspace("Daily Medulla", workspace.to_str().unwrap()); + + assert_eq!(app.loaded.config.harness.favorite_workspaces.len(), 1); + assert_eq!( + app.loaded.config.harness.favorite_workspaces[0].name, + "Daily Medulla" + ); + assert_eq!( + app.loaded.config.harness.favorite_workspaces[0].path, + workspace.to_string_lossy() + ); + assert!(std::fs::read_to_string(config) + .unwrap() + .contains("favoriteWorkspaces")); + + let picker = app.session_picker.as_mut().unwrap(); + picker.workspace_query = "daily".into(); + picker.workspace_picked = false; + app.refresh_harness_workspace_choices(); + let choice = app + .session_picker + .as_ref() + .unwrap() + .workspace_choices + .first() + .unwrap(); + assert_eq!(choice.label.as_deref(), Some("Daily Medulla")); + assert_eq!(choice.path, workspace.to_string_lossy()); +} + +#[test] +fn saving_a_favorite_reanchors_the_cursor_on_the_saved_workspace() { + // Saving promotes the new favorite to the head of the list, so the arrowed + // cursor must follow it (index 0) rather than keep pointing at the row it + // covered before — otherwise Enter starts the harness in whatever directory + // landed in that row instead of the workspace just favorited. + let root = tempfile::tempdir().unwrap(); + let workspace = root.path().join("medulla"); + std::fs::create_dir(&workspace).unwrap(); + let config = root.path().join("config.toml"); + std::fs::write(&config, "[harness]\n").unwrap(); + let mut app = picker_on_workspace_step(&workspace); + app.set_config_path(config.clone()); + + // The operator arrows onto a non-first completion before saving. + let picker = app.session_picker.as_mut().unwrap(); + picker.workspace_index = 3; + picker.workspace_picked = true; + + app.save_favorite_workspace("Daily Medulla", workspace.to_str().unwrap()); + + let picker = app.session_picker.as_ref().unwrap(); + assert_eq!( + picker.workspace_index, 0, + "the cursor follows the promoted favorite to the head row" + ); + assert!( + picker.workspace_picked, + "Enter must still honour the highlighted row after the refresh" + ); + assert_eq!( + app.selected_picker_workspace() + .map(std::path::PathBuf::from), + Some(workspace.clone()), + "Enter starts in the workspace that was just favorited" + ); +} + +#[test] +fn saving_a_favorite_reanchors_the_cursor_on_its_actual_row_when_filtered_behind_another_match() { + // The ranking orders by match score first, so a favorite whose path does + // not score best against the active query does not lead the list: a + // different entry outranks it and the saved workspace lands at a non-zero + // row. Hard-coding the cursor onto row 0 there would make Enter start the + // harness in that outranking directory instead of the workspace just + // favorited — the cursor must follow the saved workspace to its real row. + let root = tempfile::tempdir().unwrap(); + let zap = root.path().join("zap"); + let zap_mark = root.path().join("zap-mark"); + std::fs::create_dir(&zap).unwrap(); + std::fs::create_dir(&zap_mark).unwrap(); + let config = root.path().join("config.toml"); + std::fs::write(&config, "[harness]\n").unwrap(); + let mut app = picker_on_workspace_step(root.path()); + app.set_config_path(config.clone()); + // An existing favorite matches the query exactly and therefore outranks the + // newly saved one, whose name only prefixes the query. + app.loaded.config.harness.favorite_workspaces = vec![medulla::config::FavoriteWorkspace { + name: "daily".into(), + path: zap.to_string_lossy().into_owned(), + }]; + let picker = app.session_picker.as_mut().unwrap(); + picker.workspace_query = "zap".into(); + app.refresh_harness_workspace_choices(); + + app.save_favorite_workspace("mark", zap_mark.to_str().unwrap()); + + let picker = app.session_picker.as_ref().unwrap(); + let saved_index = picker + .workspace_choices + .iter() + .position(|choice| choice.path == zap_mark.to_string_lossy()) + .expect("the saved workspace is still offered as a completion"); + assert!( + saved_index > 0, + "the pre-existing exact-match favorite must outrank the freshly saved one" + ); + assert_eq!( + picker.workspace_index, saved_index, + "the cursor follows the saved workspace to its actual ranked row" + ); + assert!( + picker.workspace_picked, + "Enter must still honour the highlighted row after the refresh" + ); + assert_eq!( + app.selected_picker_workspace() + .map(std::path::PathBuf::from), + Some(zap_mark.clone()), + "Enter must start in the workspace that was just favorited, not the outranking row" + ); +} + +#[test] +fn a_failed_favorite_save_does_not_replace_the_in_memory_favorites() { + let root = tempfile::tempdir().unwrap(); + let workspace = root.path().join("medulla"); + std::fs::create_dir(&workspace).unwrap(); + let mut app = picker_on_workspace_step(&workspace); + // An unparsable config file makes persistence fail before any write — a + // favorite the disk never recorded must not be visible in memory either, + // or a later successful save could silently persist it. + let config = root.path().join("config.toml"); + std::fs::write(&config, "not [valid toml {{{").unwrap(); + app.set_config_path(config); + + app.save_favorite_workspace("Daily Medulla", workspace.to_str().unwrap()); + + assert!( + app.loaded.config.harness.favorite_workspaces.is_empty(), + "a favorite that could not be persisted must not appear to be saved" + ); + assert!(app.status().contains("Could not save favorite")); +} + +#[test] +fn saving_under_a_resolved_absolute_path_replaces_a_relative_favorite() { + // A favorite persisted from relative input (`repo` against the host + // workspace) resolves to the same directory as an absolute re-save of it, + // so the de-duplication must compare effective resolved paths — otherwise + // the old entry survives under its stale spelling and the same directory + // occupies two ranked rows. + let root = tempfile::tempdir().unwrap(); + let repo = root.path().join("repo"); + std::fs::create_dir(&repo).unwrap(); + let config = root.path().join("config.toml"); + std::fs::write(&config, "[harness]\n").unwrap(); + let mut app = picker_on_workspace_step(root.path()); + app.set_config_path(config.clone()); + app.loaded.config.harness.favorite_workspaces = vec![medulla::config::FavoriteWorkspace { + name: "old alias".into(), + path: "repo".into(), + }]; + + app.save_favorite_workspace("new alias", repo.to_str().unwrap()); + + assert_eq!( + app.loaded.config.harness.favorite_workspaces.len(), + 1, + "a relative entry must not survive beside the same directory re-saved absolutely" + ); + assert_eq!( + app.loaded.config.harness.favorite_workspaces[0].name, + "new alias" + ); + assert_eq!( + app.loaded.config.harness.favorite_workspaces[0].path, + repo.to_string_lossy() + ); +} + +#[test] +fn renaming_a_filtered_favorite_cannot_fall_through_to_an_unrelated_row() { + // Shift+F on a favorite found only through a query matching its old label, + // renamed so the new label no longer matches the unchanged query: the + // refresh drops the old row and the promoted favorite is absent, leaving a + // list that is empty or leads with an unrelated match. Forcing the cursor + // onto row 0 would make Enter reject the workspace or launch the unrelated + // row, so the picker must keep the saved workspace selected. + let root = tempfile::tempdir().unwrap(); + let workspace = root.path().join("medulla"); + std::fs::create_dir(&workspace).unwrap(); + let other = root.path().join("other-medulla"); + std::fs::create_dir(&other).unwrap(); + let config = root.path().join("config.toml"); + std::fs::write(&config, "[harness]\n").unwrap(); + let mut app = picker_on_workspace_step(&workspace); + app.set_config_path(config.clone()); + app.loaded.config.harness.favorite_workspaces = vec![ + medulla::config::FavoriteWorkspace { + name: "daily".into(), + path: workspace.to_string_lossy().into_owned(), + }, + medulla::config::FavoriteWorkspace { + name: "dailies".into(), + path: other.to_string_lossy().into_owned(), + }, + ]; + let picker = app.session_picker.as_mut().unwrap(); + picker.workspace_query = "daily".into(); + app.refresh_harness_workspace_choices(); + assert_eq!( + app.session_picker + .as_ref() + .unwrap() + .workspace_choices + .first() + .unwrap() + .label + .as_deref(), + Some("daily"), + "the filtered favorite leads the list before the rename" + ); + + app.save_favorite_workspace("zap", workspace.to_str().unwrap()); + + let picker = app.session_picker.as_ref().unwrap(); + assert_eq!( + picker.workspace_query, + workspace.to_string_lossy(), + "a rename that outlives its filter re-points the query at the saved workspace" + ); + assert_eq!( + app.selected_picker_workspace() + .map(std::path::PathBuf::from), + Some(workspace.clone()), + "Enter must not launch the unrelated row that now leads the old filter" + ); +} + +#[test] +fn saving_a_favorite_without_a_config_path_keeps_it_in_memory_only() { + // No config file → the favorite is committed in memory for the run, the + // status says so, and the launcher still re-anchors on the saved workspace + // even though nothing will persist. + let root = tempfile::tempdir().unwrap(); + let workspace = root.path().join("medulla"); + std::fs::create_dir(&workspace).unwrap(); + let mut app = picker_on_workspace_step(&workspace); + + app.save_favorite_workspace("mark", workspace.to_str().unwrap()); + + assert_eq!( + app.loaded.config.harness.favorite_workspaces, + vec![medulla::config::FavoriteWorkspace { + name: "mark".into(), + path: workspace.to_string_lossy().into_owned(), + }] + ); + assert!(app.status().contains("this run only"), "{}", app.status()); + let picker = app.session_picker.as_ref().unwrap(); + assert!( + picker.workspace_picked, + "the cursor still follows the saved workspace without a config file" + ); + assert_eq!( + app.selected_picker_workspace() + .map(std::path::PathBuf::from), + Some(workspace.clone()), + "Enter starts in the workspace that was just favorited" + ); +} + +#[test] +fn saving_a_favorite_without_hosting_is_rejected() { + // A device that hosts nothing cannot have workspace favorites at all, and + // the save says so instead of failing deeper into the command. + let mut loaded = medulla::config::LoadedConfig::defaults("medulla.tui.json".into()); + loaded.config.link = Some(medulla::config::LinkConfig::default()); + let mut app = super::types::App::new( + std::sync::Arc::new(medulla::runtime::mock::MockRuntime::empty()), + loaded, + ); + + app.save_favorite_workspace("mark", "/nowhere"); + + assert!(app.status().contains("not hosting"), "{}", app.status()); + assert!( + app.loaded.config.harness.favorite_workspaces.is_empty(), + "no favorite can be recorded without a hosted workspace" + ); +} diff --git a/src/tui/src/ui/app/input/mouse.rs b/src/tui/src/ui/app/input/mouse.rs index 7b4e55d21..7068ccf8f 100644 --- a/src/tui/src/ui/app/input/mouse.rs +++ b/src/tui/src/ui/app/input/mouse.rs @@ -81,6 +81,15 @@ impl App { } // Everything else the question is over is still swallowed, below. } + // The inline prompt owns the pointer over the picker exactly as it owns + // the keyboard (see `on_key`, which routes the prompt before the picker): + // it is an edit on top of a modal, and a click that replayed Enter on a + // picker row behind it would start a harness while the favorite-name + // prompt was still on screen. The prompt itself is a text entry with no + // click targets, so every pointer event is swallowed while it is open. + if self.prompt.is_some() { + return None; + } // The picker's rows are click targets too, and its list takes the wheel. // Same reasoning as the question above: it is opened from a rail row the // operator clicked (`+ New session`) or from `Ctrl-T`, so they arrive diff --git a/src/tui/src/ui/app/overlays_tests.rs b/src/tui/src/ui/app/overlays_tests.rs index 0643b2fc3..50dfa70a2 100644 --- a/src/tui/src/ui/app/overlays_tests.rs +++ b/src/tui/src/ui/app/overlays_tests.rs @@ -19,7 +19,7 @@ use medulla::runtime::mock::MockRuntime; use super::types::{ tab_pos, App, HandbackPrompt, Overlay, PromptKind, ResumePicker, SessionPicker, - SessionPickerStep, RP_TEMPLATES, + SessionPickerStep, WorkspaceChoice, RP_TEMPLATES, }; use crate::ui::composer::{Draft, TextPrompt}; @@ -137,6 +137,72 @@ fn handback_prompt_swallows_clicks_behind_it() { ); } +#[test] +fn inline_prompt_swallows_clicks_that_would_reach_the_picker_behind_it() { + // The favorite-add prompt opens *on top of* the workspace picker (Shift+F). + // Keyboard routing gives the prompt precedence over the picker, and the + // pointer must not come to disagree: a click on a picker row behind the + // prompt would replay Enter and start a harness while the favorite-name + // edit is still on screen. + let mut app = app(); + app.session_picker = Some(SessionPicker { + choices: Vec::new(), + index: 0, + step: SessionPickerStep::Workspace, + cwd: "/".into(), + workspace_query: "x".into(), + workspace_choices: vec![WorkspaceChoice { + path: "/tmp".into(), + source: "folder".into(), + label: None, + }], + workspace_index: 0, + workspace_picked: false, + }); + app.prompt = Some(TextPrompt::new( + PromptKind::FavoriteWorkspaceAdd("/tmp".into()), + "Save favorite for /tmp", + )); + app.hit_session_picker = Some(( + ratatui::layout::Rect { + x: 0, + y: 0, + width: 60, + height: 18, + }, + vec![( + ratatui::layout::Rect { + x: 0, + y: 5, + width: 60, + height: 1, + }, + 0, + )], + )); + + let _ = app.on_mouse(MouseEvent { + kind: MouseEventKind::Down(MouseButton::Left), + column: 10, + row: 5, + modifiers: KeyModifiers::NONE, + }); + + assert!( + app.prompt.is_some(), + "the click must not dismiss the favorite prompt" + ); + let picker = app.session_picker.as_ref().unwrap(); + assert!( + !picker.workspace_picked, + "the click must not be read as choosing a workspace row" + ); + assert_eq!( + picker.workspace_index, 0, + "the picker selection must be left where it was" + ); +} + #[test] fn pointer_input_cancels_an_armed_harness_close() { let mut app = app(); diff --git a/src/tui/src/ui/app/render/session_modals.rs b/src/tui/src/ui/app/render/session_modals.rs index f43d7b3c1..786f2d3bc 100644 --- a/src/tui/src/ui/app/render/session_modals.rs +++ b/src/tui/src/ui/app/render/session_modals.rs @@ -17,6 +17,10 @@ use super::super::types::{App, SessionPickerStep}; const HARNESS_TRAILER_LINES: usize = 3; +/// Width allotted to a workspace row's path (or favorite label and path) +/// before the dim provenance suffix is appended. +const WORKSPACE_ROW_WIDTH: usize = 43; + impl App { /// Draw the "start a session" picker. pub(super) fn draw_harness_picker(&mut self, f: &mut Frame, area: Rect) { @@ -26,11 +30,11 @@ impl App { let (rows, title) = match picker.step { SessionPickerStep::Harness => ( picker.choices.len(), - "Choose a harness type — ↑/↓ · Enter workspace · Esc cancel", + "Choose a harness type — Enter workspace · Esc cancel", ), SessionPickerStep::Workspace => ( picker.workspace_choices.len(), - "Choose workspace — type to filter · Tab complete · Enter start · Esc back", + "Choose workspace — type to filter · Enter start · Esc back", ), }; let height = (rows as u16).saturating_add(7).clamp(8, 18); @@ -139,14 +143,23 @@ impl App { } else { Style::default() }; - TLine::from(vec![ - Span::styled( - format!( - "{marker}{}", - medulla::ui::util::clip_left(&choice.path, 43) - ), - style, + // A bare path is clipped from the left, so the tail + // that identifies the directory survives. A named + // favorite instead clips from both ends: `★ name ·` + // is the distinguishing part the operator added, and + // clipping from the left would delete it whenever + // the path below runs long. + let display = match &choice.label { + Some(label) => medulla::ui::util::clip_middle( + &format!("★ {label} · {}", choice.path), + WORKSPACE_ROW_WIDTH, ), + None => { + medulla::ui::util::clip_left(&choice.path, WORKSPACE_ROW_WIDTH) + } + }; + TLine::from(vec![ + Span::styled(format!("{marker}{display}"), style), Span::styled( format!(" {}", choice.source), Style::default().add_modifier(Modifier::DIM), @@ -166,12 +179,15 @@ impl App { Style::default().add_modifier(Modifier::DIM), ))); } - // Said here as well as in the status line, because it is the one fact - // that makes this different from every other way to start a session — - // and it is now a statement rather than a question, so it is said on - // both steps and never asked. + // The one fact that makes this picker different from every other way + // to start a session is that the session is unmanaged, so that is + // stated on both steps. The keyboard verbs beside it are step-specific: + // Tab complete and Shift+F save favorite need the workspace step's + // text field and chosen directory, and advertising them on the harness + // step — where neither key is bound — would send an operator pressing + // the hint into silence. lines.push(TLine::from(Span::styled( - " unmanaged · the orchestrator will not dispatch into it", + harness_picker_hint(picker.step), Style::default().add_modifier(Modifier::DIM), ))); f.render_widget(Paragraph::new(Text::from(lines)), inner); @@ -339,6 +355,22 @@ impl App { } } +/// The footer hint under the picker, stated per step. +/// +/// `↑/↓ choose` applies to both steps, and the "unmanaged" statement is true +/// of any hand-started session, so both are said everywhere. `Tab complete` +/// and `Shift+F save favorite` are only meaningful on the workspace step — the +/// one with a text field to complete and a chosen directory to remember — and +/// the keys are not bound on the harness step, so the hint is not shown there. +pub(super) fn harness_picker_hint(step: SessionPickerStep) -> &'static str { + match step { + SessionPickerStep::Harness => " ↑/↓ choose · unmanaged", + SessionPickerStep::Workspace => { + " ↑/↓ choose · Tab complete · Shift+F save favorite · unmanaged" + } + } +} + /// Window a long harness list so the selected row always remains visible. pub(super) fn harness_choice_window( total: usize, diff --git a/src/tui/src/ui/app/render/tests.rs b/src/tui/src/ui/app/render/tests.rs index 42f337904..1835f4f48 100644 --- a/src/tui/src/ui/app/render/tests.rs +++ b/src/tui/src/ui/app/render/tests.rs @@ -46,6 +46,35 @@ fn harness_choice_window_keeps_the_selection_visible() { ); assert_eq!(super::session_modals::harness_choice_window(2, 1, 13), 0..2); } + +#[test] +fn harness_picker_hint_only_advertises_bound_keys() { + use crate::ui::app::types::SessionPickerStep; + + let harness = super::session_modals::harness_picker_hint(SessionPickerStep::Harness); + assert!( + !harness.contains("Tab complete"), + "Tab is not bound on the harness step, so the hint must not advertise it: {harness}" + ); + assert!( + !harness.contains("Shift+F"), + "Shift+F is not bound on the harness step, so the hint must not advertise it: {harness}" + ); + assert!( + harness.contains("unmanaged"), + "the harness step still states that a hand-started session is unmanaged" + ); + + let workspace = super::session_modals::harness_picker_hint(SessionPickerStep::Workspace); + assert!( + workspace.contains("Tab complete"), + "the workspace step advertises the completion key it actually binds" + ); + assert!( + workspace.contains("Shift+F save favorite"), + "the workspace step advertises the favorite key it actually binds" + ); +} #[test] fn leaving_the_agents_tab_takes_the_keyboard_back_from_an_attached_harness() { // The bug this pins: `release_session` was only reached from diff --git a/src/tui/src/ui/app/session_control/picker.rs b/src/tui/src/ui/app/session_control/picker.rs index a7ef67e59..a6bf388c6 100644 --- a/src/tui/src/ui/app/session_control/picker.rs +++ b/src/tui/src/ui/app/session_control/picker.rs @@ -7,7 +7,8 @@ use medulla::protocol::HarnessProvider; use crate::ui::harness_pane::HarnessChoice; -use super::super::types::{tab_pos, App, SessionPicker, SessionPickerStep}; +use super::super::types::{tab_pos, App, Prompt, PromptKind, SessionPicker, SessionPickerStep}; +use crate::ui::composer::Draft; impl App { /// Open the "start a session" picker, or spawn directly when the command @@ -156,6 +157,18 @@ impl App { } } KeyCode::Tab => self.complete_harness_workspace(), + KeyCode::Char('F') if event.modifiers == KeyModifiers::SHIFT => { + let Some(workspace) = self.selected_picker_workspace() else { + self.set_status("Choose an existing directory before saving a favorite"); + return; + }; + self.prompt = Some(Prompt { + kind: PromptKind::FavoriteWorkspaceAdd(workspace.clone()), + title: format!("Save favorite for {workspace}"), + draft: Draft::new(), + }); + self.set_status("Favorite name · Enter save · Esc cancel"); + } KeyCode::Backspace => { if let Some(picker) = &mut self.session_picker { picker.workspace_query.pop(); diff --git a/src/tui/src/ui/app/tests/mod.rs b/src/tui/src/ui/app/tests/mod.rs index e2736bc73..d658a9f0b 100644 --- a/src/tui/src/ui/app/tests/mod.rs +++ b/src/tui/src/ui/app/tests/mod.rs @@ -171,7 +171,8 @@ fn enter_answers_the_harness_picker_not_the_harness_behind_it() { if let Some(picker) = &mut a.session_picker { picker.workspace_choices = vec![WorkspaceChoice { path: ".".into(), - source: "recent", + source: "recent".into(), + label: None, }]; picker.workspace_index = 0; } diff --git a/src/tui/src/ui/app/types/mod.rs b/src/tui/src/ui/app/types/mod.rs index 94fc1be28..d2f2bedc7 100644 --- a/src/tui/src/ui/app/types/mod.rs +++ b/src/tui/src/ui/app/types/mod.rs @@ -1,13 +1,16 @@ //! The data model for the interactive TUI screen. //! //! This module is deliberately wiring only. Screen state and its supporting -//! model types live in [`model`], while the compact rendered rail hit map lives -//! in [`rail_hit`]. Keeping those responsibilities separate lets the app's +//! model types live in [`model`], the manual-launcher picker and its prompt +//! overlays in [`picker`], and the compact rendered rail hit map in +//! [`rail_hit`]. Keeping those responsibilities separate lets the app's //! sibling input, rendering, and command modules share the model without //! turning this directory module into another monolithic source file. mod model; +mod picker; mod rail_hit; pub use model::*; +pub(in crate::ui::app) use picker::*; pub(in crate::ui::app) use rail_hit::{RailHit, RailHitTarget}; diff --git a/src/tui/src/ui/app/types/model.rs b/src/tui/src/ui/app/types/model.rs index 586caffbb..f73148670 100644 --- a/src/tui/src/ui/app/types/model.rs +++ b/src/tui/src/ui/app/types/model.rs @@ -13,12 +13,13 @@ use std::sync::Arc; use ratatui::layout::Rect; -use crate::ui::composer::{Draft, TextPrompt}; +use crate::ui::composer::Draft; use crate::ui::theme::Theme; use medulla::client::{FeedbackComment, FeedbackItem, FeedbackQuery, FeedbackType}; use medulla::config::LoadedConfig; use medulla::runtime::{ContextItem, Runtime, RuntimeSnapshot, WorkerOp}; +use super::picker::*; use super::rail_hit::RailHit; /// The ordered top-level tab names. The tab index selects into this array. @@ -527,288 +528,6 @@ pub enum Cmd { }, } -/// The modal state for the "resume a chat" picker overlay. -pub(in crate::ui::app) struct ResumePicker { - /// The resumable chats to choose from. - pub(in crate::ui::app) chats: Vec, - /// The highlighted row. - pub(in crate::ui::app) index: usize, -} - -/// An overlay the app can draw over the content pane. -/// -/// Ordered as they stack, back to front: the two that float over the content, -/// then the session picker, then the question asked about a session being -/// released, and finally the two that claim a row of their own below it. -/// -/// Produced by [`App::visible_overlays`], which is the single source of truth -/// for what is in front of the content — see [`super::super::overlays`]. -#[derive(Clone, Copy, PartialEq, Eq, Debug)] -pub(in crate::ui::app) enum Overlay { - /// The prepared-decision board. - Decisions, - /// The agent-template detail popup. - TemplatePopup, - /// The "start a session" picker. - SessionPicker, - /// The question asked when the operator lets go of a session. - HandbackPrompt, - /// The shared single-line prompt (Workers add/edit, Agents answer). - InlinePrompt, - /// The saved-chat resume picker. - ResumePicker, -} - -/// The modal state for the harness-type/workspace picker overlay. -/// -/// It answers exactly one question — which CLI, in which directory — and -/// confirming the second step starts the session. It used to carry a -/// `PickerPurpose` because the same two steps also declared an agent, so the end -/// of the flow depended on which door had opened it; the declaration flow is -/// gone, and with one ending there is nothing left to carry. -pub(in crate::ui::app) struct SessionPicker { - /// Installed providers and registered presets, in offer order. - pub(in crate::ui::app) choices: Vec, - /// The highlighted row. - pub(in crate::ui::app) index: usize, - /// Which half of the two-step picker owns the keyboard. - pub(in crate::ui::app) step: SessionPickerStep, - /// Default directory used to seed the editable workspace query. - pub(in crate::ui::app) cwd: String, - /// Inline fuzzy-completion text on the workspace step. - pub(in crate::ui::app) workspace_query: String, - /// Cached workspace rows, refreshed only when the query changes. - pub(in crate::ui::app) workspace_choices: Vec, - /// Highlighted workspace completion. - pub(in crate::ui::app) workspace_index: usize, - /// Whether the operator has deliberately picked one of the completions. - /// - /// Distinct from `workspace_index != 0`, which cannot express it: a query - /// that offers a single completion leaves the cursor on row zero however - /// deliberately it was moved there. Set by the arrows, cleared whenever the - /// query changes, and read by - /// [`selected_picker_workspace`](App::selected_picker_workspace) to decide - /// whether an entered directory outranks the completions listed under it. - pub(in crate::ui::app) workspace_picked: bool, -} - -/// Active stage of the manual session launcher. -/// -/// There is deliberately no "managed or unmanaged?" stage. A session the -/// operator starts by hand is theirs — that is what starting it by hand *means* -/// — and the orchestrator spawns its own sessions managed without asking -/// anybody. So the question only ever had one sensible answer, and asking it -/// bought a keystroke, an extra screen, and a freshly started session the -/// operator then had to take back from the orchestrator before typing into it. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(in crate::ui::app) enum SessionPickerStep { - /// Choose an installed CLI or registered preset. - Harness, - /// Choose or complete the working directory. - Workspace, -} - -/// One cached workspace completion and why it was suggested. -#[derive(Debug, Clone, PartialEq, Eq)] -pub(in crate::ui::app) struct WorkspaceChoice { - /// Absolute directory path. - pub(in crate::ui::app) path: String, - /// Short operator-facing provenance such as `recent` or `folder`. - pub(in crate::ui::app) source: &'static str, -} - -/// A pointer gesture a harness owns until the button comes back up. -/// -/// Terminals grab the pointer on press: every drag and the release belong to -/// whoever took the press, regardless of where the pointer has moved to since. -/// The embedded pane has to do the same, because the alternatives are both -/// visible failures — a release that lands outside the pane, or one swallowed -/// by the hand-back question the click itself opened, leaves the child holding -/// a button nobody is pressing. Claude Code and Codex then read every later -/// motion as a drag and anchor their popups to a press the operator has long -/// since let go of. -#[derive(Clone)] -pub(in crate::ui::app) struct PointerGrab { - /// The session that received the press. - pub(in crate::ui::app) session: String, - /// The button that went down, so a second button's events are not stolen. - pub(in crate::ui::app) button: crate::ui::harness_pane::mouse::Button, - /// Where that session's pane was when the press landed. - /// - /// Carried rather than re-read from `hit_session` because the grab has to - /// outlive the pane: the click that opened a modal, detached the harness, - /// or scrolled the rail can move or remove the rect before the release - /// arrives, and the release still has to be encoded against the geometry - /// the child believes it has. - pub(in crate::ui::app) rect: Rect, -} - -/// The "you still hold this session" confirmation shown on release. -/// -/// Modelled on an unsaved-changes prompt, and for the same reason: an operator -/// who took a session over and walked away has left the orchestrator locked out -/// of it, and the moment they release the keyboard is the only moment they are -/// certainly thinking about it. Silently handing it back would be worse — it -/// would resume dispatch into a session mid-thought. -pub(in crate::ui::app) struct HandbackPrompt { - /// The session the question is about. - /// - /// Every answer acts on this, never on whatever the rail last resolved: the - /// question can outlive the frame that raised it, and a `y` that moved - /// control of a *different* session is the worst outcome this whole flow - /// has. - pub(in crate::ui::app) session: String, - /// Whether attaching is what took control, as opposed to an explicit - /// `/takecontrol`. An explicit take is a decision, so the prompt says so - /// rather than implying the operator got here by accident. - pub(in crate::ui::app) took_control: bool, - /// What the operator wants continued, typed into the prompt. - /// - /// This is the moment they actually have the context — they are leaving the - /// session *now* — so it is the one place worth asking. `/handoff ` - /// exists for the operator who already knows; this is for the one who is - /// only reminded by being asked. - pub(in crate::ui::app) note: crate::ui::composer::Draft, - /// Whether keystrokes are going into the note rather than answering. - /// - /// Modal because `y`/`n` have to keep meaning yes and no: an operator who - /// starts typing a note that begins with "no, ..." must not have the first - /// letter answer the question for them. - pub(in crate::ui::app) editing_note: bool, - /// Which direction the question is about: `true` asks whether to take the - /// session from the orchestrator, `false` whether to hand it back. - /// - /// One prompt for both because they are the same decision seen from either - /// side, and the answer is the same keystroke — but the sentence has to say - /// which way control is about to move, or the operator confirms the - /// opposite of what they meant. - pub(in crate::ui::app) is_takeover: bool, -} - -/// How the operator came to hold a session the orchestrator had. -/// -/// Only the wording of the release question turns on this — both origins ask, -/// because both locked dispatch out of a workspace. What does *not* appear here -/// is "started it myself": that session was never taken from anyone, so it is -/// absent from [`App::sessions_taken`] rather than being a third variant. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(in crate::ui::app) enum TakeOrigin { - /// Focusing in took it, which the operator may not have realised. - Focus, - /// `/takecontrol`, `Ctrl-G`, or answering the takeover question — a decision. - Explicit, -} - -/// What to do when the operator releases a session they took. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] -pub enum HandbackPolicy { - /// Ask, every time. - #[default] - Ask, - /// Always hand back without asking. - Always, - /// Never hand back; releasing the keyboard keeps control. - Never, -} - -impl HandbackPolicy { - /// Parse the `[harness].handback` config value, falling back to - /// [`Ask`](Self::Ask) for anything unrecognized — a typo in a config file - /// should not silently change who controls a session. - pub fn from_config(value: &str) -> Self { - match value.trim().to_ascii_lowercase().as_str() { - "always" => HandbackPolicy::Always, - "never" => HandbackPolicy::Never, - _ => HandbackPolicy::Ask, - } - } -} - -/// The action a small inline prompt (Hosts add/edit, Agents answer) submits. -pub(in crate::ui::app) enum PromptKind { - /// Select an arbitrary Git revision as the Changes comparison baseline. - ChangesBaseline, - /// Attach a session-local review comment to a file, hunk, or patch line. - ChangesComment { - /// Repository-relative path being reviewed. - path: std::path::PathBuf, - /// Position within that file's patch the note is bound to. - anchor: medulla::ui::git_review::CommentAnchor, - }, - /// Add a worker from an address/@handle line. - HostAdd, - /// Edit the label of the worker with the given id. - HostEditLabel(String), - /// Declare another directory this device may work in. - WorkspaceAdd, - /// Add a named OpenRouter-backed coding harness. - CustomHarnessAdd, - /// Edit the custom harness with the given stable id. - CustomHarnessEdit(String), - /// Declare a lifecycle hook for every harness Medulla launches. - HookAdd, - /// Edit the hook at the given row of the Hooks page. - HookEdit(usize), - /// Reject a workflow proposal with the operator's explanation. - RejectProposal { - /// The workflow the proposal belongs to. - workflow: String, - /// The proposal awaiting the decision. - proposal_id: String, - }, - /// Answer a pending sub-agent question. - AnswerQuestion { - /// The cycle the question belongs to. - cycle_id: String, - /// The pending question's id. - question_id: String, - }, - /// Answer a prepared decision and dismiss it locally once routed. - DecisionAnswer { - /// Stable decision id. - decision_id: String, - /// Cycle that owns the question. - cycle_id: String, - /// Harness question id. - question_id: String, - }, - /// Comment on the given feedback board item. - FeedbackComment { - /// The item being commented on. - id: String, - }, - /// Step one of submitting feedback: the title. Submitting advances to - /// [`PromptKind::FeedbackBody`] rather than sending anything. - FeedbackTitle { - /// Feature request or bug report, chosen by which key opened the prompt. - kind: FeedbackType, - }, - /// Step two of submitting feedback: the body. Submitting sends it. - FeedbackBody { - /// Feature request or bug report. - kind: FeedbackType, - /// The title captured in step one. - title: String, - }, - /// One field of a workflow's declared inputs, collected before the run - /// starts. Submitting either opens the prompt for the next field or, when - /// this was the last, dispatches the run. - /// - /// The whole set is carried on the prompt rather than parked in `App` - /// state, so cancelling with `Esc` abandons the collected values with it — - /// a half-filled set cannot leak into the next run. - WorkflowInput { - /// The workflow the values are being collected for. - workflow_id: String, - /// Whether to dispatch a dry run rather than a real one. - dry_run: bool, - /// The fields still to ask about; the head is the one on screen. - remaining: Vec, - /// What has been collected so far, keyed by input name. - collected: serde_json::Map, - }, -} - /// The Feedback surface's state: the loaded page, the selected row, that row's /// comments, and the active query. pub(in crate::ui::app) struct FeedbackState { @@ -849,9 +568,6 @@ impl Default for FeedbackState { } } -/// A single-line inline input overlay shared with daemon controls. -pub(in crate::ui::app) type Prompt = TextPrompt; - /// Cached credential-presence flags displayed by Routing's Manage Keys pane. #[derive(Default)] pub(in crate::ui::app) struct CredentialStatus { diff --git a/src/tui/src/ui/app/types/picker.rs b/src/tui/src/ui/app/types/picker.rs new file mode 100644 index 000000000..53f725287 --- /dev/null +++ b/src/tui/src/ui/app/types/picker.rs @@ -0,0 +1,305 @@ +//! Modal state for the manual-launcher picker and the small prompt/overlay +//! surfaces the launcher and session control raise. +//! +//! The harness/workspace launcher ([`SessionPicker`], [`ResumePicker`], +//! [`SessionPickerStep`], [`WorkspaceChoice`]) plus the single-line prompt +//! overlays ([`Prompt`]/[`PromptKind`]) and the session-release question +//! ([`HandbackPrompt`], [`TakeOrigin`], [`HandbackPolicy`]) sit here rather +//! than in [`super::model`], which holds the screen model itself and is at its +//! size ceiling. All are re-exported through [`super`] as `types::*`. + +use ratatui::layout::Rect; + +use crate::ui::composer::TextPrompt; +use medulla::client::FeedbackType; + +/// The modal state for the "resume a chat" picker overlay. +pub(in crate::ui::app) struct ResumePicker { + /// The resumable chats to choose from. + pub(in crate::ui::app) chats: Vec, + /// The highlighted row. + pub(in crate::ui::app) index: usize, +} + +/// An overlay the app can draw over the content pane. +/// +/// Ordered as they stack, back to front: the two that float over the content, +/// then the session picker, then the question asked about a session being +/// released, and finally the two that claim a row of their own below it. +/// +/// Produced by [`App::visible_overlays`](crate::ui::app::App::visible_overlays), +/// which is the single source of truth for what is in front of the content — +/// see [`super::super::overlays`]. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub(in crate::ui::app) enum Overlay { + /// The prepared-decision board. + Decisions, + /// The agent-template detail popup. + TemplatePopup, + /// The "start a session" picker. + SessionPicker, + /// The question asked when the operator lets go of a session. + HandbackPrompt, + /// The shared single-line prompt (Workers add/edit, Agents answer). + InlinePrompt, + /// The saved-chat resume picker. + ResumePicker, +} + +/// The modal state for the harness-type/workspace picker overlay. +/// +/// It answers exactly one question — which CLI, in which directory — and +/// confirming the second step starts the session. It used to carry a +/// `PickerPurpose` because the same two steps also declared an agent, so the end +/// of the flow depended on which door had opened it; the declaration flow is +/// gone, and with one ending there is nothing left to carry. +pub(in crate::ui::app) struct SessionPicker { + /// Installed providers and registered presets, in offer order. + pub(in crate::ui::app) choices: Vec, + /// The highlighted row. + pub(in crate::ui::app) index: usize, + /// Which half of the two-step picker owns the keyboard. + pub(in crate::ui::app) step: SessionPickerStep, + /// Default directory used to seed the editable workspace query. + pub(in crate::ui::app) cwd: String, + /// Inline fuzzy-completion text on the workspace step. + pub(in crate::ui::app) workspace_query: String, + /// Cached workspace rows, refreshed only when the query changes. + pub(in crate::ui::app) workspace_choices: Vec, + /// Highlighted workspace completion. + pub(in crate::ui::app) workspace_index: usize, + /// Whether the operator has deliberately picked one of the completions. + /// + /// Distinct from `workspace_index != 0`, which cannot express it: a query + /// that offers a single completion leaves the cursor on row zero however + /// deliberately it was moved there. Set by the arrows, cleared whenever the + /// query changes, and read by + /// [`selected_picker_workspace`](crate::ui::app::App::selected_picker_workspace) + /// to decide whether an entered directory outranks the completions listed + /// under it. + pub(in crate::ui::app) workspace_picked: bool, +} + +/// Active stage of the manual session launcher. +/// +/// There is deliberately no "managed or unmanaged?" stage. A session the +/// operator starts by hand is theirs — that is what starting it by hand *means* +/// — and the orchestrator spawns its own sessions managed without asking +/// anybody. So the question only ever had one sensible answer, and asking it +/// bought a keystroke, an extra screen, and a freshly started session the +/// operator then had to take back from the orchestrator before typing into it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(in crate::ui::app) enum SessionPickerStep { + /// Choose an installed CLI or registered preset. + Harness, + /// Choose or complete the working directory. + Workspace, +} + +/// One cached workspace completion and why it was suggested. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(in crate::ui::app) struct WorkspaceChoice { + /// Absolute directory path. + pub(in crate::ui::app) path: String, + /// Short operator-facing provenance such as `favorite`, `recent`, or `folder`. + pub(in crate::ui::app) source: String, + /// An operator-defined favorite name, when this is a saved shortcut. + pub(in crate::ui::app) label: Option, +} + +/// A pointer gesture a harness owns until the button comes back up. +/// +/// Terminals grab the pointer on press: every drag and the release belong to +/// whoever took the press, regardless of where the pointer has moved to since. +/// The embedded pane has to do the same, because the alternatives are both +/// visible failures — a release that lands outside the pane, or one swallowed +/// by the hand-back question the click itself opened, leaves the child holding +/// a button nobody is pressing. Claude Code and Codex then read every later +/// motion as a drag and anchor their popups to a press the operator has long +/// since let go of. +#[derive(Clone)] +pub(in crate::ui::app) struct PointerGrab { + /// The session that received the press. + pub(in crate::ui::app) session: String, + /// The button that went down, so a second button's events are not stolen. + pub(in crate::ui::app) button: crate::ui::harness_pane::mouse::Button, + /// Where that session's pane was when the press landed. + /// + /// Carried rather than re-read from `hit_session` because the grab has to + /// outlive the pane: the click that opened a modal, detached the harness, + /// or scrolled the rail can move or remove the rect before the release + /// arrives, and the release still has to be encoded against the geometry + /// the child believes it has. + pub(in crate::ui::app) rect: Rect, +} + +/// The "you still hold this session" confirmation shown on release. +/// +/// Modelled on an unsaved-changes prompt, and for the same reason: an operator +/// who took a session over and walked away has left the orchestrator locked out +/// of it, and the moment they release the keyboard is the only moment they are +/// certainly thinking about it. Silently handing it back would be worse — it +/// would resume dispatch into a session mid-thought. +pub(in crate::ui::app) struct HandbackPrompt { + /// The session the question is about. + /// + /// Every answer acts on this, never on whatever the rail last resolved: the + /// question can outlive the frame that raised it, and a `y` that moved + /// control of a *different* session is the worst outcome this whole flow + /// has. + pub(in crate::ui::app) session: String, + /// Whether attaching is what took control, as opposed to an explicit + /// `/takecontrol`. An explicit take is a decision, so the prompt says so + /// rather than implying the operator got here by accident. + pub(in crate::ui::app) took_control: bool, + /// What the operator wants continued, typed into the prompt. + /// + /// This is the moment they actually have the context — they are leaving the + /// session *now* — so it is the one place worth asking. `/handoff ` + /// exists for the operator who already knows; this is for the one who is + /// only reminded by being asked. + pub(in crate::ui::app) note: crate::ui::composer::Draft, + /// Whether keystrokes are going into the note rather than answering. + /// + /// Modal because `y`/`n` have to keep meaning yes and no: an operator who + /// starts typing a note that begins with "no, ..." must not have the first + /// letter answer the question for them. + pub(in crate::ui::app) editing_note: bool, + /// Which direction the question is about: `true` asks whether to take the + /// session from the orchestrator, `false` whether to hand it back. + /// + /// One prompt for both because they are the same decision seen from either + /// side, and the answer is the same keystroke — but the sentence has to say + /// which way control is about to move, or the operator confirms the + /// opposite of what they meant. + pub(in crate::ui::app) is_takeover: bool, +} + +/// How the operator came to hold a session the orchestrator had. +/// +/// Only the wording of the release question turns on this — both origins ask, +/// because both locked dispatch out of a workspace. What does *not* appear here +/// is "started it myself": that session was never taken from anyone, so it is +/// absent from [`App::sessions_taken`](crate::ui::app::App::sessions_taken) +/// rather than being a third variant. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(in crate::ui::app) enum TakeOrigin { + /// Focusing in took it, which the operator may not have realised. + Focus, + /// `/takecontrol`, `Ctrl-G`, or answering the takeover question — a decision. + Explicit, +} + +/// What to do when the operator releases a session they took. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum HandbackPolicy { + /// Ask, every time. + #[default] + Ask, + /// Always hand back without asking. + Always, + /// Never hand back; releasing the keyboard keeps control. + Never, +} + +impl HandbackPolicy { + /// Parse the `[harness].handback` config value, falling back to + /// [`Ask`](Self::Ask) for anything unrecognized — a typo in a config file + /// should not silently change who controls a session. + pub fn from_config(value: &str) -> Self { + match value.trim().to_ascii_lowercase().as_str() { + "always" => HandbackPolicy::Always, + "never" => HandbackPolicy::Never, + _ => HandbackPolicy::Ask, + } + } +} + +/// The action a small inline prompt (Hosts add/edit, Agents answer) submits. +pub(in crate::ui::app) enum PromptKind { + /// Select an arbitrary Git revision as the Changes comparison baseline. + ChangesBaseline, + /// Attach a session-local review comment to a file, hunk, or patch line. + ChangesComment { + /// Repository-relative path being reviewed. + path: std::path::PathBuf, + /// Position within that file's patch the note is bound to. + anchor: medulla::ui::git_review::CommentAnchor, + }, + /// Add a worker from an address/@handle line. + HostAdd, + /// Edit the label of the worker with the given id. + HostEditLabel(String), + /// Declare another directory this device may work in. + WorkspaceAdd, + /// Save the selected manual-launcher directory under an operator-chosen name. + FavoriteWorkspaceAdd(String), + /// Add a named OpenRouter-backed coding harness. + CustomHarnessAdd, + /// Edit the custom harness with the given stable id. + CustomHarnessEdit(String), + /// Declare a lifecycle hook for every harness Medulla launches. + HookAdd, + /// Edit the hook at the given row of the Hooks page. + HookEdit(usize), + /// Reject a workflow proposal with the operator's explanation. + RejectProposal { + /// The workflow the proposal belongs to. + workflow: String, + /// The proposal awaiting the decision. + proposal_id: String, + }, + /// Answer a pending sub-agent question. + AnswerQuestion { + /// The cycle the question belongs to. + cycle_id: String, + /// The pending question's id. + question_id: String, + }, + /// Answer a prepared decision and dismiss it locally once routed. + DecisionAnswer { + /// Stable decision id. + decision_id: String, + /// Cycle that owns the question. + cycle_id: String, + /// Harness question id. + question_id: String, + }, + /// Comment on the given feedback board item. + FeedbackComment { + /// The item being commented on. + id: String, + }, + /// Step one of submitting feedback: the title. Submitting advances to + /// [`PromptKind::FeedbackBody`] rather than sending anything. + FeedbackTitle { + /// Feature request or bug report, chosen by which key opened the prompt. + kind: FeedbackType, + }, + /// Step two of submitting feedback: the body. Submitting sends it. + FeedbackBody { + /// Feature request or bug report. + kind: FeedbackType, + /// The title captured in step one. + title: String, + }, + /// One field of a workflow's declared inputs, collected before the run + /// starts. Submitting either opens the prompt for the next field or, when + /// this was the last, dispatches the run. + /// + /// The whole set is carried on the prompt rather than parked in `App` + /// state, so cancelling with `Esc` abandons the collected values with it — + /// a half-filled set cannot leak into the next run. + WorkflowInput { + /// The workflow the values are being collected for. + workflow_id: String, + /// Whether to dispatch a dry run rather than a real one. + dry_run: bool, + /// The fields still to ask about; the head is the one on screen. + remaining: Vec, + /// What has been collected so far, keyed by input name. + collected: serde_json::Map, + }, +} +/// A single-line inline input overlay shared with daemon controls. +pub(in crate::ui::app) type Prompt = TextPrompt;