From 43ac8d9f93d88895ebfa397a77aeedd088c11692 Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Fri, 14 Aug 2026 22:55:00 +0100 Subject: [PATCH] Edit voro.toml settings by selection on the Config screen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Config screen bound a letter per value — `V` for the default viewer, `A` for the default agent — and `max_running`, the one queue option with no TUI surface at all, was next in line for a third. That does not scale, and it is the wrong model: a config screen is a list of settings you select, with one key that edits whatever is under the selection. The screen now carries a Settings list above the viewers — the default agent, the default viewer, and the dispatch cap — each showing the value in force and whether it came from `voro.toml` or is Voro's own default, which the resolved value alone cannot say when the operator's number happens to equal the default. A flat `config_rows` list holds the settings and the viewers together, the way `cockpit_rows` does, so one selection runs over both and ⏎ (or `e`) opens each row's own editor: the pickers `V`/`A` used to open, the viewer form, and a new numeric entry for the cap. `a` and `d` stay keys — `a` acts with nothing selected and `d` is destructive, neither being an edit of the selected value — and `d` on a setting is refused pointing at ⏎. `V` and `A` are unbound, which shrinks the case-convention exception list from seven entries to five. The cap is written by a new `config_edit::set_max_running` through the same comment-preserving `toml_edit` pair as the rest, refusing a negative count in the loader's own sentence (now factored into one place), and the save refreshes, so raising the cap while the queue reads `⏸ dispatch at capacity` restores the dispatch rows on that keypress. `AgentsConfig` keeps `max_running` as the raw `Option` and exposes the three from-file accessors the provenance column reads. Verified with `cargo test --workspace`, clippy and fmt clean, and driven live in tmux against a scratch store and config: selection crossing the two lists with one highlight, both pickers and the cap entry opening on ⏎, a negative and a non-numeric entry refused with the form still open, esc leaving the file untouched, a saved cap keeping the file's comments and re-gating the queue in place, `V`/`A` inert, and the parse-error branch unchanged. --- CHANGELOG.md | 15 + crates/voro-core/src/agent.rs | 63 +++- crates/voro-core/src/config_edit.rs | 63 ++++ crates/voro/src/app.rs | 540 ++++++++++++++++++++++++++-- crates/voro/src/ui.rs | 255 +++++++++++-- docs/DESIGN.md | 253 +++++++------ 6 files changed, 983 insertions(+), 206 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5161db3..4e68e59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -174,6 +174,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **The Config screen edits settings you select, starting with the dispatch + cap.** It used to bind a letter per value — `V` for the default viewer, `A` + for the default agent — and `max_running`, the one queue option with no TUI + surface at all, was next in line for a third. Instead the screen has grown a + **Settings** list above the viewers: the default agent, the default viewer, + and the dispatch cap, each showing the value in force and whether it came from + your `voro.toml` or is Voro's own default. One selection runs over the + settings and the viewers together, and ⏎ (or `e`) edits whatever it is on — + the same pickers `V` and `A` used to open, and a numeric entry for the cap. + Raising the cap while the queue reads `⏸ dispatch at capacity` brings the + dispatch rows back on that keypress. `a` adds a viewer and `d` deletes one as + before; `V` and `A` are unbound, and the `?` map no longer lists them. Every + write still goes through the comment-preserving writer, so your file's + formatting and comments survive it. + - **A quick propose is named for its project, like the planning session beside it.** `n` used to launch its agent under `voro-propose-` while `N` on the same project opened `voro-plan-`, so the two read in diff --git a/crates/voro-core/src/agent.rs b/crates/voro-core/src/agent.rs index 1b0c245..f47b6db 100644 --- a/crates/voro-core/src/agent.rs +++ b/crates/voro-core/src/agent.rs @@ -859,6 +859,17 @@ impl RawCosts { } } +/// Why a negative dispatch cap is refused, in one place: the file and the +/// Config screen's editor (DESIGN.md §5) both write the cap, and an operator +/// who meets the refusal at one surface should meet the same sentence at the +/// other. +pub(crate) fn negative_max_running(n: i64) -> String { + format!( + "max_running is {n} — it counts dispatches in flight, so it cannot be negative (0 stops \ + the queue offering dispatches at all)" + ) +} + /// Validate one agent's verb templates, shared by the built-ins and the user /// file. `dispatch` (or its alias `cmd`) must be present and carry the /// prompt-file placeholder; the session verbs carry their placeholders when @@ -1102,8 +1113,11 @@ pub struct AgentsConfig { /// The attention price band the queue ranks by (DESIGN.md §7), defaults /// with any `[costs]` overrides layered on. costs: AttentionCosts, - /// How many dispatches ride at once before the queue stops offering more. - max_running: i64, + /// How many dispatches ride at once before the queue stops offering more, + /// as the file spells it — `None` when the key is absent, which is what + /// lets the Config screen say whether the cap in force is the operator's + /// or Voro's own (DESIGN.md §5). + max_running: Option, path: PathBuf, } @@ -1159,7 +1173,7 @@ impl AgentsConfig { viewers: BTreeMap::new(), default_viewer: None, costs: AttentionCosts::default(), - max_running: DEFAULT_MAX_RUNNING, + max_running: None, path: path.to_path_buf(), } } @@ -1190,19 +1204,14 @@ impl AgentsConfig { provenance.insert(name.clone(), prov); agents.insert(name, agent); } - let max_running = match raw.max_running { - None => DEFAULT_MAX_RUNNING, - Some(n) if n >= 0 => n, - Some(n) => { - return Err(Error::AgentConfigInvalid { - path: path.to_path_buf(), - message: format!( - "max_running is {n} — it counts dispatches in flight, so it cannot be \ - negative (0 stops the queue offering dispatches at all)" - ), - }); - } - }; + if let Some(n) = raw.max_running + && n < 0 + { + return Err(Error::AgentConfigInvalid { + path: path.to_path_buf(), + message: negative_max_running(n), + }); + } let costs = match raw.costs { Some(costs) => costs.resolve(path)?, None => AttentionCosts::default(), @@ -1215,7 +1224,7 @@ impl AgentsConfig { viewers: raw.viewers, default_viewer: raw.default_viewer, costs, - max_running, + max_running: raw.max_running, path: path.to_path_buf(), }) } @@ -1228,9 +1237,29 @@ impl AgentsConfig { /// The dispatch WIP cap (DESIGN.md §7): how many tasks may be running /// before the queue stops offering dispatches. pub fn max_running(&self) -> i64 { + self.max_running.unwrap_or(DEFAULT_MAX_RUNNING) + } + + /// The cap as `voro.toml` spells it, `None` when the file names none — the + /// provenance the Config screen's settings list shows beside the value + /// (DESIGN.md §5), which the resolved [`max_running`](Self::max_running) + /// alone cannot tell apart from a cap that happens to equal the default. + pub fn max_running_from_file(&self) -> Option { self.max_running } + /// `default_agent` as the file spells it, before the PATH probe that + /// [`default_name`](Self::default_name) falls back to. + pub fn default_agent_from_file(&self) -> Option<&str> { + self.default.as_deref() + } + + /// `default_viewer` as the file spells it, before the resolution rules + /// [`default_viewer_name`](Self::default_viewer_name) falls back to. + pub fn default_viewer_from_file(&self) -> Option<&str> { + self.default_viewer.as_deref() + } + /// Every agent name defined in the config, for the TUI's dispatch picker /// (DESIGN.md §8/§9). `agents` is a `BTreeMap`, so this is already sorted. pub fn agent_names(&self) -> Vec { diff --git a/crates/voro-core/src/config_edit.rs b/crates/voro-core/src/config_edit.rs index 8e0cdf4..e01f32d 100644 --- a/crates/voro-core/src/config_edit.rs +++ b/crates/voro-core/src/config_edit.rs @@ -118,6 +118,19 @@ pub fn set_default_agent(path: &Path, name: &str) -> Result<()> { write_doc(path, &doc) } +/// Set `max_running`, the dispatch WIP cap (DESIGN.md §7), refusing a negative +/// count in the same words [`crate::agent::AgentsConfig::load`] refuses one +/// read from the file. `0` is legal and means what it means there: the queue +/// offers no dispatches at all. +pub fn set_max_running(path: &Path, n: i64) -> Result<()> { + if n < 0 { + return Err(invalid(crate::agent::negative_max_running(n))); + } + let mut doc = load_doc(path)?; + doc["max_running"] = value(n); + write_doc(path, &doc) +} + /// Whether a viewer command lacks the `{path}` placeholder — a warning, not an /// error (DESIGN.md §5): such a command runs in the checkout's own directory, /// which is occasionally what a `git difftool -d` wants but usually a mistake. @@ -425,6 +438,56 @@ cmd = \"git -C {path} difftool -d {base}...{branch}\" # inline note std::fs::remove_dir_all(&dir).unwrap(); } + /// The dispatch cap is written like any other key: comments and the + /// operator's own content survive it, an absent file is created by it, `0` + /// is legal, and a negative count is refused in the words the loader uses + /// (DESIGN.md §5/§7). + #[test] + fn set_max_running_writes_the_cap_and_refuses_a_negative_one() { + let dir = scratch("cap"); + let path = dir.join("voro/voro.toml"); + assert!(!path.exists()); + + set_max_running(&path, 3).unwrap(); + assert_eq!(AgentsConfig::load(&path).unwrap().max_running(), 3); + assert_eq!( + AgentsConfig::load(&path).unwrap().max_running_from_file(), + Some(3) + ); + + // rewriting the file leaves what the operator wrote around it alone + let original = "\ +# how many at once +max_running = 3 # was 5 + +[viewers.zed] +cmd = \"zed {path}\" +"; + std::fs::write(&path, original).unwrap(); + set_max_running(&path, 9).unwrap(); + let text = std::fs::read_to_string(&path).unwrap(); + assert!(text.contains("# how many at once"), "{text}"); + assert!(text.contains("[viewers.zed]"), "{text}"); + assert!(text.contains("max_running = 9"), "{text}"); + assert_eq!(AgentsConfig::load(&path).unwrap().max_running(), 9); + + // 0 is a cap, not an absence: the queue offers nothing + set_max_running(&path, 0).unwrap(); + assert_eq!(AgentsConfig::load(&path).unwrap().max_running(), 0); + + // and the refusal is the loader's own sentence, not a second wording + let refused = set_max_running(&path, -1).unwrap_err().to_string(); + assert!(refused.contains("cannot be negative"), "{refused}"); + assert_eq!(AgentsConfig::load(&path).unwrap().max_running(), 0); + + let hand_written = dir.join("by-hand.toml"); + std::fs::write(&hand_written, "max_running = -1\n").unwrap(); + let from_file = AgentsConfig::load(&hand_written).unwrap_err().to_string(); + assert!(from_file.contains(&refused), "{from_file} vs {refused}"); + + std::fs::remove_dir_all(&dir).unwrap(); + } + #[test] fn set_defaults_validate_and_record() { let dir = scratch("defaults"); diff --git a/crates/voro/src/app.rs b/crates/voro/src/app.rs index 48289df..282b9ac 100644 --- a/crates/voro/src/app.rs +++ b/crates/voro/src/app.rs @@ -89,6 +89,37 @@ pub struct ConfigViewerRow { pub editable: bool, } +/// Which `voro.toml` value a settings row edits (DESIGN.md §5). The two +/// defaults carry the [`DefaultKind`] their shared picker is keyed on. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SettingKind { + Default(DefaultKind), + MaxRunning, +} + +/// One setting row on the Config screen (DESIGN.md §5): a `voro.toml` value the +/// operator selects and edits with one key, showing what is in force and where +/// it came from — the file, or the rule Voro fell back to. Derived once per +/// refresh, like every other row on the screen. +#[derive(Debug, Clone)] +pub struct ConfigSettingRow { + pub name: &'static str, + pub value: String, + /// Where `value` came from, parenthesised on screen: `voro.toml` when the + /// key is set, else the resolution rule that produced it. + pub source: String, + pub kind: SettingKind, +} + +/// One selectable row on the Config screen: the settings list and the viewers +/// list share a single selection, so both index into one flat row list the way +/// the cockpit's [`CockpitRow`] does. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ConfigRow { + Setting(usize), + Viewer(usize), +} + /// One selectable row on the cockpit; indices point into the App caches. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum CockpitRow { @@ -201,6 +232,13 @@ pub enum Mode { task_id: i64, buffer: String, }, + /// Collecting the dispatch WIP cap from the Config screen's settings list + /// (DESIGN.md §5/§7). A one-line entry like [`LinkPr`](Mode::LinkPr), and + /// like it it names nothing else: the setting it writes is the one the + /// selection was on when it opened. + EditMaxRunning { + buffer: String, + }, /// Collecting the one line `n` expands into a task (DESIGN.md §6/§8). Like /// `LinkPr` and unlike `Prompt` it names no task — there is none yet — so it /// carries the project the proposal will land in instead. @@ -565,11 +603,15 @@ pub struct App { /// The Config screen's view of `voro.toml` (DESIGN.md §5), reloaded every /// refresh so an edit — from either this screen or a dispatch — is reflected - /// immediately. Agents are read-only; the viewers are what `config_sel` - /// selects, and the ones a `voro.toml` table backs are what edit/delete act - /// on — a built-in row is selectable but refuses both. + /// immediately. Agents are read-only; the settings and the viewers are what + /// `config_sel` runs over, through the flat `config_rows`, and ⏎ edits + /// whichever it lands on — a built-in viewer row is selectable but refuses. pub config_agents: Vec, + pub config_settings: Vec, pub config_viewers: Vec, + /// The settings and the viewers as one selectable list, which is the space + /// `config_sel` counts in. + pub config_rows: Vec, /// The legacy anonymous `[viewer]` table's command, shown read-only. pub config_anon_viewer: Option, /// A `voro.toml` that failed to parse, surfaced on the screen rather than @@ -670,7 +712,9 @@ impl App { tasks_sel: 0, projects_sel: 0, config_agents: Vec::new(), + config_settings: Vec::new(), config_viewers: Vec::new(), + config_rows: Vec::new(), config_anon_viewer: None, config_error: None, config_sel: 0, @@ -856,7 +900,7 @@ impl App { self.projects_sel = self.projects_sel.min(self.projects.len().saturating_sub(1)); self.config_sel = self .config_sel - .min(self.config_viewers.len().saturating_sub(1)); + .min(self.config_rows.len().saturating_sub(1)); Ok(()) } @@ -934,7 +978,9 @@ impl App { Ok(config) => config, Err(e) => { self.config_agents.clear(); + self.config_settings.clear(); self.config_viewers.clear(); + self.config_rows.clear(); self.config_anon_viewer = None; self.config_error = Some(e.to_string()); return; @@ -975,9 +1021,98 @@ impl App { }) .collect(); self.config_anon_viewer = config.anonymous_viewer_cmd().map(str::to_string); + self.config_settings = Self::settings_rows(&config, default_agent, default_viewer); + self.config_rows = (0..self.config_settings.len()) + .map(ConfigRow::Setting) + .chain((0..self.config_viewers.len()).map(ConfigRow::Viewer)) + .collect(); self.config_error = None; } + /// The settings rows in the order the screen lists them, each carrying the + /// value in force and where it came from (DESIGN.md §5). A value the file + /// names reads `voro.toml`; one it does not names the rule that resolved it, + /// so a fresh install can see that the cap and the defaults it is running + /// under are Voro's own rather than something it was never asked about. + fn settings_rows( + config: &AgentsConfig, + default_agent: Option, + default_viewer: Option, + ) -> Vec { + // Nothing resolves at all: no agent on PATH, or no viewer anywhere. + const NONE: &str = "—"; + + let (agent_value, agent_source) = match config.default_agent_from_file() { + Some(name) => (name.to_string(), "voro.toml".to_string()), + None => match default_agent { + Some(name) => (name, "first agent found on PATH".to_string()), + None => (NONE.into(), "no agent found on PATH".to_string()), + }, + }; + // The same order `AgentsConfig::default_viewer_name` resolves in: the + // key, then the anonymous table, then a sole named viewer, then the + // built-ins probed against PATH. + let (viewer_value, viewer_source) = match config.default_viewer_from_file() { + Some(name) => (name.to_string(), "voro.toml".to_string()), + None if config.anonymous_viewer_cmd().is_some() => { + ("[viewer]".into(), "the anonymous table".to_string()) + } + None => match default_viewer { + Some(name) if config.viewer_names().len() == 1 => { + (name, "the only viewer configured".to_string()) + } + Some(name) => (name, "first viewer found on PATH".to_string()), + None => (NONE.into(), "no viewer found on PATH".to_string()), + }, + }; + let cap_source = match config.max_running_from_file() { + Some(_) => "voro.toml", + None => "voro's default", + }; + vec![ + ConfigSettingRow { + name: "default agent", + value: agent_value, + source: agent_source, + kind: SettingKind::Default(DefaultKind::Agent), + }, + ConfigSettingRow { + name: "default viewer", + value: viewer_value, + source: viewer_source, + kind: SettingKind::Default(DefaultKind::Viewer), + }, + ConfigSettingRow { + name: "dispatch cap", + value: config.max_running().to_string(), + source: cap_source.to_string(), + kind: SettingKind::MaxRunning, + }, + ] + } + + /// What the Config screen's selection is on, if anything. + pub fn selected_config_row(&self) -> Option { + self.config_rows.get(self.config_sel).copied() + } + + /// The selected viewer, `None` when the selection is on a setting — so the + /// viewer actions read the row they act on rather than indexing blind. + pub fn selected_viewer(&self) -> Option<&ConfigViewerRow> { + match self.selected_config_row()? { + ConfigRow::Viewer(i) => self.config_viewers.get(i), + ConfigRow::Setting(_) => None, + } + } + + /// The selected setting, `None` when the selection is on a viewer. + pub fn selected_setting(&self) -> Option<&ConfigSettingRow> { + match self.selected_config_row()? { + ConfigRow::Setting(i) => self.config_settings.get(i), + ConfigRow::Viewer(_) => None, + } + } + pub fn selected_task_id(&self) -> Option { match self.screen { Screen::Cockpit => match self.cockpit_rows.get(self.cockpit_sel)? { @@ -1208,7 +1343,7 @@ impl App { Screen::Cockpit => (&mut self.cockpit_sel, self.cockpit_rows.len()), Screen::Tasks => (&mut self.tasks_sel, self.all.len()), Screen::Projects => (&mut self.projects_sel, self.projects.len()), - Screen::Config => (&mut self.config_sel, self.config_viewers.len()), + Screen::Config => (&mut self.config_sel, self.config_rows.len()), }; if len == 0 { return; @@ -1226,7 +1361,7 @@ impl App { Screen::Cockpit => (&mut self.cockpit_sel, self.cockpit_rows.len()), Screen::Tasks => (&mut self.tasks_sel, self.all.len()), Screen::Projects => (&mut self.projects_sel, self.projects.len()), - Screen::Config => (&mut self.config_sel, self.config_viewers.len()), + Screen::Config => (&mut self.config_sel, self.config_rows.len()), }; if index >= len { return; @@ -1321,11 +1456,16 @@ impl App { pub fn enter_hint(&self) -> Option<&'static str> { match self.screen { Screen::Projects => None, - Screen::Config => self - .config_viewers - .get(self.config_sel) - .filter(|v| v.editable) - .map(|_| "⏎ edit"), + // Every row with an editor behind it; a built-in viewer has none — + // it is overridden by `a`, not edited. + Screen::Config => match self.selected_config_row()? { + ConfigRow::Setting(_) => Some("⏎ edit"), + ConfigRow::Viewer(i) => self + .config_viewers + .get(i) + .filter(|v| v.editable) + .map(|_| "⏎ edit"), + }, Screen::Tasks => self.all.get(self.tasks_sel).map(|_| "⏎ view"), Screen::Cockpit => match self.cockpit_rows.get(self.cockpit_sel)? { CockpitRow::Queue(i) => match self.queue.rows.get(*i)? { @@ -1419,6 +1559,7 @@ impl App { buffer, } => self.key_prompt(key, task_id, kind, buffer), Mode::LinkPr { task_id, buffer } => self.key_link_pr(key, task_id, buffer), + Mode::EditMaxRunning { buffer } => self.key_max_running(key, buffer), Mode::QuickCreate { project_id, buffer } => { self.key_quick_create(key, project_id, buffer) } @@ -1477,7 +1618,7 @@ impl App { Hit::CockpitRow(i) if self.screen == Screen::Cockpit => self.select_index(i), Hit::TaskRow(i) if self.screen == Screen::Tasks => self.select_index(i), Hit::ProjectRow(i) if self.screen == Screen::Projects => self.select_index(i), - Hit::ViewerRow(i) if self.screen == Screen::Config => self.select_index(i), + Hit::ConfigRow(i) if self.screen == Screen::Config => self.select_index(i), Hit::PickerOption(i) => self.click_picker_option(i), _ => {} } @@ -2809,6 +2950,61 @@ impl App { self.mode = Mode::LinkPr { task_id, buffer }; } + /// Drive the dispatch-cap entry (DESIGN.md §5/§7). Enter saves, esc cancels, + /// backspace edits. Only the characters a whole number is spelled with are + /// taken, so a stray letter never lands in the field — what it cannot refuse + /// at the keystroke (a lone `-`, a count past `i64`, a negative cap) the + /// save refuses with the form still open. + fn key_max_running(&mut self, key: KeyEvent, mut buffer: String) { + match key.code { + KeyCode::Esc => return, + KeyCode::Enter => { + self.save_max_running(&buffer); + return; + } + KeyCode::Backspace => { + buffer.pop(); + } + KeyCode::Char(c) if c.is_ascii_digit() || c == '-' => buffer.push(c), + _ => {} + } + self.mode = Mode::EditMaxRunning { buffer }; + } + + /// Write the dispatch cap and rebuild the queue on the same keypress, so a + /// raised cap restores the dispatch rows the capacity line had replaced + /// (DESIGN.md §7) rather than waiting for the next unrelated refresh. A + /// refusal — from the parse or from the writer — keeps the entry open with + /// what was typed intact, the way the link-a-PR prompt does. + fn save_max_running(&mut self, raw: &str) { + let trimmed = raw.trim(); + let reopen = |app: &mut App, message: String| { + app.status = Some(message); + app.mode = Mode::EditMaxRunning { + buffer: raw.to_string(), + }; + }; + let Ok(n) = trimmed.parse::() else { + reopen( + self, + format!("'{trimmed}' is not a whole number — the dispatch cap counts tasks"), + ); + return; + }; + match voro_core::config_edit::set_max_running(&self.dispatch_ctx.agents_path, n) { + Ok(()) => { + self.status = Some(if n == 0 { + "dispatch cap -> 0 — the queue will offer no dispatches".into() + } else { + format!("dispatch cap -> {n}") + }); + let result = self.refresh(); + self.report(result); + } + Err(e) => reopen(self, e.to_string()), + } + } + /// Drive the quick-create prompt (DESIGN.md §6/§8). Enter hands the typed /// line to a background agent, esc cancels. The buffer is one line — the /// terse intent the agent expands — so this stays a simple line editor. @@ -3205,20 +3401,22 @@ impl App { }; } - /// The Config screen's local keys (DESIGN.md §5): `a` adds a viewer, `e`/⏎ - /// edits the selected one's command, `d` deletes it, `V`/`A` pick the default - /// viewer/agent. Movement and the alt-digit screen jumps are `key_normal`'s. + /// The Config screen's local keys (DESIGN.md §5): `e`/⏎ edits whatever the + /// selection is on — a setting or a viewer — while `a` adds a viewer and `d` + /// deletes one. The two list operations keep letters of their own because + /// neither is an edit of the selected value: `a` acts with nothing selected, + /// and `d` is destructive. `J`/`K` and the page keys scroll the agents pane, + /// which has no selection of its own. Movement and the alt-digit screen + /// jumps are `key_normal`'s. fn key_config(&mut self, key: KeyEvent) { match key.code { KeyCode::Char('a') => self.open_viewer_form(None, None), - KeyCode::Char('e') | KeyCode::Enter => self.edit_selected_viewer(), + KeyCode::Char('e') | KeyCode::Enter => self.activate_config_row(), KeyCode::Char('d') => self.delete_selected_viewer(), - KeyCode::Char('V') => self.open_default_picker(DefaultKind::Viewer), - KeyCode::Char('A') => self.open_default_picker(DefaultKind::Agent), // The agents pane takes the cockpit card's scroll keys for the same - // reason it has there: `j`/`k` are the list's — here the viewers' - // — so the pane below them is driven by the shifted pair and the - // page keys (DESIGN.md §9). + // reason it has there: `j`/`k` are the list's — here the settings + // and the viewers' — so the pane above them is driven by the shifted + // pair and the page keys (DESIGN.md §9). KeyCode::Char('J') => self.scroll_config_agents(1), KeyCode::Char('K') => self.scroll_config_agents(-1), KeyCode::PageDown => self.scroll_config_agents(DETAIL_PAGE_STEP), @@ -3227,6 +3425,33 @@ impl App { } } + /// Edit whatever the Config screen's selection is on: a setting opens its + /// own editor — the picker for the two defaults, the numeric entry for the + /// dispatch cap — and a viewer row opens the viewer form. + fn activate_config_row(&mut self) { + match self.selected_config_row() { + Some(ConfigRow::Setting(i)) => match self.config_settings.get(i).map(|s| s.kind) { + Some(SettingKind::Default(kind)) => self.open_default_picker(kind), + Some(SettingKind::MaxRunning) => self.open_max_running_entry(), + None => {} + }, + Some(ConfigRow::Viewer(_)) => self.edit_selected_viewer(), + None => self.status = Some("nothing selected — press a to add a viewer".into()), + } + } + + /// Open the dispatch-cap entry pre-filled with the cap in force, so raising + /// or lowering it starts from the number the screen just showed. + fn open_max_running_entry(&mut self) { + let buffer = self + .config_settings + .iter() + .find(|s| s.kind == SettingKind::MaxRunning) + .map(|s| s.value.clone()) + .unwrap_or_default(); + self.mode = Mode::EditMaxRunning { buffer }; + } + /// Open the add/edit-viewer form. `existing` pre-fills it for an edit (name /// locked); `review_project` threads through the quick path so a viewer /// created from the viewer picker becomes that project's viewer. @@ -3268,7 +3493,7 @@ impl App { /// overriding it is an *add* of the same name — so it is refused with that /// named rather than opening a form whose write would fail. fn edit_selected_viewer(&mut self) { - match self.config_viewers.get(self.config_sel) { + match self.selected_viewer() { Some(v) if !v.editable => { self.status = Some(format!( "'{}' is built into voro — press a and name it '{}' to override it", @@ -3287,7 +3512,16 @@ impl App { /// (DESIGN.md §5) — the same refusal as `voro viewer remove`, with /// the offending projects named. Deleting the default clears `default_viewer`. fn delete_selected_viewer(&mut self) { - let Some(viewer) = self.config_viewers.get(self.config_sel) else { + // A setting is changed, never removed: there is no state in which + // voro has no dispatch cap, so `d` on one has nothing to do. + if let Some(setting) = self.selected_setting() { + self.status = Some(format!( + "'{}' is a setting, not a list — press ⏎ to change it", + setting.name + )); + return; + } + let Some(viewer) = self.selected_viewer() else { self.status = Some("no viewer selected".into()); return; }; @@ -3517,7 +3751,7 @@ impl App { // Land the selection on what was just written, so `e`/`d` act on it // rather than on whichever row the list happens to sort first. if let Some(i) = self.config_viewers.iter().position(|v| v.name == trimmed) { - self.config_sel = i; + self.config_sel = self.config_settings.len() + i; } } @@ -5055,8 +5289,8 @@ mod tests { } } - /// The Config screen's row index for a viewer. The built-ins share the - /// list, so a test never assumes where a name sorts. + /// The Config screen's viewer-list index for a viewer. The built-ins share + /// the list, so a test never assumes where a name sorts. fn row(app: &App, name: &str) -> usize { app.config_viewers .iter() @@ -5064,6 +5298,19 @@ mod tests { .unwrap_or_else(|| panic!("no viewer row named {name}")) } + /// …and the selection that lands on it, past the settings rows above. + fn viewer_sel(app: &App, name: &str) -> usize { + app.config_settings.len() + row(app, name) + } + + /// The selection that lands on a named setting. + fn setting_sel(app: &App, name: &str) -> usize { + app.config_settings + .iter() + .position(|s| s.name == name) + .unwrap_or_else(|| panic!("no setting row named {name}")) + } + /// `o` with no viewer set up anywhere raises the add-viewer form rather /// than only complaining, and says why on the status line; every /// other way opening can fail still just reports. Driven through @@ -5238,7 +5485,7 @@ mod tests { assert!(app.config_viewers.iter().all(|v| !v.editable)); // e/d on a built-in row refuse, naming the override that replaces it - app.config_sel = row(&app, "code"); + app.config_sel = viewer_sel(&app, "code"); for k in ['d', 'e'] { key(&mut app, KeyCode::Char(k)); let status = app.status.clone().unwrap_or_default(); @@ -5260,9 +5507,12 @@ mod tests { key(&mut app, KeyCode::Enter); assert!(matches!(app.mode, Mode::Normal)); // the selection lands on what was just written - assert_eq!(app.config_viewers[app.config_sel].name, "mine"); - assert_eq!(app.config_viewers[app.config_sel].cmd, "mine {path}"); - assert_eq!(app.config_viewers[app.config_sel].provenance, "user"); + let selected = app + .selected_viewer() + .expect("the selection lands on a viewer"); + assert_eq!(selected.name, "mine"); + assert_eq!(selected.cmd, "mine {path}"); + assert_eq!(selected.provenance, "user"); assert_eq!( AgentsConfig::load(&path) .unwrap() @@ -5284,10 +5534,11 @@ mod tests { "mine {path} --wait" ); - // default: V opens the picker over every viewer, built-ins included; - // walk to the top and back down to `mine`, since where the cursor - // starts depends on what is installed - key(&mut app, KeyCode::Char('V')); + // default: ⏎ on the default-viewer setting opens the picker over every + // viewer, built-ins included; walk to the top and back down to `mine`, + // since where the cursor starts depends on what is installed + app.config_sel = setting_sel(&app, "default viewer"); + key(&mut app, KeyCode::Enter); let names = match &app.mode { Mode::DefaultPicker { names, .. } => names.clone(), _ => panic!("expected the default picker to open"), @@ -5311,7 +5562,7 @@ mod tests { ); // delete: d removes it and clears the now-dangling default - app.config_sel = row(&app, "mine"); + app.config_sel = viewer_sel(&app, "mine"); key(&mut app, KeyCode::Char('d')); assert!(app.config_viewers.iter().all(|v| v.name != "mine")); let config = AgentsConfig::load(&path).unwrap(); @@ -5325,6 +5576,221 @@ mod tests { let _ = std::fs::remove_dir_all(path.parent().unwrap()); } + /// One selection runs over both lists (DESIGN.md §5): `j`/`k` cross from the + /// settings into the viewers and back without a focus key, ⏎ edits whichever + /// kind it is on, and `d` — a list operation, not an edit — is refused on a + /// setting with what to press instead. + #[test] + fn one_selection_runs_over_the_settings_and_the_viewers() { + let toml = "[viewers.zed]\ncmd = \"zed {path}\"\n"; + let (store, ctx, _project) = scratch_env("config-rows", Some(toml)); + let path = ctx.agents_path.clone(); + let mut app = App::new(store, ctx).unwrap(); + alt_key(&mut app, KeyCode::Char('4')); + + // the settings come first, in the order the screen lists them + let names: Vec<&str> = app.config_settings.iter().map(|s| s.name).collect(); + assert_eq!(names, ["default agent", "default viewer", "dispatch cap"]); + assert_eq!( + app.config_rows.len(), + app.config_settings.len() + app.config_viewers.len() + ); + + // k at the top stays put; j walks off the last setting onto the first + // viewer and back + app.config_sel = 0; + key(&mut app, KeyCode::Char('k')); + assert_eq!( + app.selected_setting().map(|s| s.name), + Some("default agent") + ); + for _ in 0..app.config_settings.len() { + key(&mut app, KeyCode::Char('j')); + } + assert_eq!(app.selected_config_row(), Some(ConfigRow::Viewer(0))); + key(&mut app, KeyCode::Char('k')); + assert_eq!(app.selected_setting().map(|s| s.name), Some("dispatch cap")); + + // ⏎ routes by row kind: picker, picker, numeric entry, viewer form + for (setting, opens) in [ + ("default agent", "agent picker"), + ("default viewer", "viewer picker"), + ("dispatch cap", "cap entry"), + ] { + app.config_sel = setting_sel(&app, setting); + key(&mut app, KeyCode::Enter); + match (&app.mode, opens) { + (Mode::DefaultPicker { kind, .. }, "agent picker") => { + assert_eq!(*kind, DefaultKind::Agent) + } + (Mode::DefaultPicker { kind, .. }, "viewer picker") => { + assert_eq!(*kind, DefaultKind::Viewer) + } + (Mode::EditMaxRunning { .. }, "cap entry") => {} + _ => panic!("{setting} did not open the {opens}"), + } + key(&mut app, KeyCode::Esc); + } + app.config_sel = viewer_sel(&app, "zed"); + key(&mut app, KeyCode::Enter); + assert!(matches!( + app.mode, + Mode::ViewerForm(ViewerFormState { editing: true, .. }) + )); + key(&mut app, KeyCode::Esc); + + // d on a setting is refused: a setting is changed, never removed + app.config_sel = setting_sel(&app, "dispatch cap"); + key(&mut app, KeyCode::Char('d')); + let status = app.status.clone().unwrap_or_default(); + assert!(status.contains("dispatch cap"), "{status}"); + assert!(status.contains("⏎"), "{status}"); + assert!(matches!(app.mode, Mode::Normal)); + assert!(app.config_viewers.iter().any(|v| v.name == "zed")); + + let _ = std::fs::remove_dir_all(path.parent().unwrap()); + } + + /// Each settings row says what is in force and where it came from + /// (DESIGN.md §5), so a fresh install can tell a value it chose from one + /// Voro fell back to — the distinction the resolved value alone cannot make + /// when the operator's number happens to equal the default. + #[test] + fn the_settings_rows_carry_their_value_and_its_provenance() { + let (store, ctx, _project) = scratch_env("config-provenance", None); + let path = ctx.agents_path.clone(); + let mut app = App::new(store, ctx).unwrap(); + alt_key(&mut app, KeyCode::Char('4')); + + let source = |app: &App, name: &str| { + app.config_settings + .iter() + .find(|s| s.name == name) + .map(|s| (s.value.clone(), s.source.clone())) + .unwrap() + }; + // with no file at all, every value is Voro's own and says so + let (value, from) = source(&app, "dispatch cap"); + assert_eq!(value, scheduler::DEFAULT_MAX_RUNNING.to_string()); + assert_eq!(from, "voro's default"); + assert_ne!(source(&app, "default agent").1, "voro.toml"); + assert_ne!(source(&app, "default viewer").1, "voro.toml"); + + // and once the file names them, they are the operator's + voro_core::config_edit::set_max_running(&path, 5).unwrap(); + voro_core::config_edit::set_default_agent(&path, "codex").unwrap(); + voro_core::config_edit::set_default_viewer(&path, "code").unwrap(); + app.refresh().unwrap(); + assert_eq!( + source(&app, "dispatch cap"), + ("5".into(), "voro.toml".into()) + ); + assert_eq!( + source(&app, "default agent"), + ("codex".into(), "voro.toml".into()) + ); + assert_eq!( + source(&app, "default viewer"), + ("code".into(), "voro.toml".into()) + ); + + let _ = std::fs::remove_dir_all(path.parent().unwrap()); + } + + /// The dispatch cap is edited where it is shown (DESIGN.md §5/§7): the entry + /// opens on the cap in force, a bad number is refused with the form still + /// open, and a saved one is written *and* re-gates the queue on the same + /// keypress — so raising it while the capacity line has replaced the + /// dispatch rows brings them back without restarting the TUI. + #[test] + fn saving_the_dispatch_cap_writes_it_and_re_gates_the_queue() { + let (mut store, ctx, _project) = + scratch_env("config-cap", Some("# mine\nmax_running = 1\n")); + let path = ctx.agents_path.clone(); + let project = store.create_project("demo", "/tmp/demo").unwrap(); + let mut ready = |title: &str| { + store + .create_task(NewTask { + project_id: project.id, + repo_id: None, + title: title.into(), + body: String::new(), + priority: Priority::P1, + state: TaskState::Ready, + agent: None, + human: false, + deep: false, + }) + .unwrap() + }; + let running = ready("in flight"); + let waiting = ready("waiting for room"); + store.apply(running.id, Action::Start).unwrap(); + + let mut app = App::new(store, ctx).unwrap(); + alt_key(&mut app, KeyCode::Char('4')); + // one running against a cap of one: the queue offers no dispatch + assert!(app.queue.at_capacity.is_some()); + assert!(!app.queue_task_ids().contains(&waiting.id)); + + // ⏎ opens the entry on the cap in force + app.config_sel = setting_sel(&app, "dispatch cap"); + key(&mut app, KeyCode::Enter); + match &app.mode { + Mode::EditMaxRunning { buffer } => assert_eq!(buffer, "1"), + _ => panic!("expected the cap entry"), + } + + // a number it cannot save is refused with what was typed still there + key(&mut app, KeyCode::Backspace); + type_str(&mut app, "-2"); + key(&mut app, KeyCode::Enter); + let status = app.status.clone().unwrap_or_default(); + assert!(status.contains("cannot be negative"), "{status}"); + match &app.mode { + Mode::EditMaxRunning { buffer } => assert_eq!(buffer, "-2"), + _ => panic!("the refusal should keep the entry open"), + } + assert_eq!(AgentsConfig::load(&path).unwrap().max_running(), 1); + + // esc leaves the file exactly as it was + key(&mut app, KeyCode::Esc); + assert!(matches!(app.mode, Mode::Normal)); + assert_eq!(AgentsConfig::load(&path).unwrap().max_running(), 1); + + // and a good one writes, keeps the operator's comment, and re-gates the + // queue on the same keypress + key(&mut app, KeyCode::Enter); + key(&mut app, KeyCode::Backspace); + type_str(&mut app, "4"); + key(&mut app, KeyCode::Enter); + assert!(matches!(app.mode, Mode::Normal)); + assert_eq!(AgentsConfig::load(&path).unwrap().max_running(), 4); + assert!( + std::fs::read_to_string(&path).unwrap().contains("# mine"), + "the writer preserves what was already in the file" + ); + assert!(app.queue.at_capacity.is_none()); + assert!(app.queue_task_ids().contains(&waiting.id)); + assert_eq!( + app.config_settings + .iter() + .find(|s| s.name == "dispatch cap") + .map(|s| (s.value.as_str(), s.source.as_str())), + Some(("4", "voro.toml")) + ); + + // 0 is a cap, not an absence — the queue offers nothing at all + key(&mut app, KeyCode::Enter); + key(&mut app, KeyCode::Backspace); + type_str(&mut app, "0"); + key(&mut app, KeyCode::Enter); + assert_eq!(AgentsConfig::load(&path).unwrap().max_running(), 0); + assert!(app.queue.at_capacity.is_some()); + + let _ = std::fs::remove_dir_all(path.parent().unwrap()); + } + /// Deleting a viewer a project still names is refused on the /// Config screen too, naming the project (DESIGN.md §5). #[test] @@ -5337,8 +5803,8 @@ mod tests { let mut app = App::new(store, ctx).unwrap(); alt_key(&mut app, KeyCode::Char('4')); - app.config_sel = row(&app, "zed"); - assert!(app.config_viewers[app.config_sel].editable); + app.config_sel = viewer_sel(&app, "zed"); + assert!(app.selected_viewer().is_some_and(|v| v.editable)); key(&mut app, KeyCode::Char('d')); assert!( app.status.as_deref().unwrap_or("").contains("demo2"), diff --git a/crates/voro/src/ui.rs b/crates/voro/src/ui.rs index 4bd0c3b..d1e2fad 100644 --- a/crates/voro/src/ui.rs +++ b/crates/voro/src/ui.rs @@ -28,7 +28,9 @@ pub enum Hit { CockpitRow(usize), TaskRow(usize), ProjectRow(usize), - ViewerRow(usize), + /// A Config screen row, settings list and viewers list alike: an index into + /// `App::config_rows`, the one space `config_sel` counts in. + ConfigRow(usize), /// An option of whichever modal picker is open. PickerOption(usize), } @@ -198,6 +200,12 @@ fn draw_mode(frame: &mut Frame, app: &App, hits: &mut HitMap) { "Link PR (URL or owner/repo#n) — ⏎ to submit, esc to cancel".to_string(), buffer, ), + Mode::EditMaxRunning { buffer } => draw_text_entry_popup( + frame, + "Dispatch cap — how many run at once, 0 for none — ⏎ to save, esc to cancel" + .to_string(), + buffer, + ), Mode::QuickCreate { project_id, buffer } => { let project = app .projects @@ -1760,12 +1768,20 @@ fn draw_config(frame: &mut Frame, app: &App, hits: &mut HitMap) { ))); } - // The pane takes the height its rows need, yielding only what the viewers - // list below needs for a border and a row: both panes scroll, so the split - // is about which one is read whole without a keypress, and that is this one. - let agents_h = (agent_lines.len() as u16 + 2).clamp(3, main.height.saturating_sub(3).max(3)); - let [agents_area, viewers_area] = - Layout::vertical([Constraint::Length(agents_h), Constraint::Min(3)]).areas(main); + // The pane takes the height its rows need, yielding to the settings list — + // which is exactly its rows, neither scrolling nor growing — and to what the + // viewers list below needs for a border and a row: every pane here is + // reachable whole, so the split is about which is read without a keypress, + // and that is this one. + let settings_h = app.config_settings.len() as u16 + 2; + let agents_h = + (agent_lines.len() as u16 + 2).clamp(3, main.height.saturating_sub(settings_h + 3).max(3)); + let [agents_area, settings_area, viewers_area] = Layout::vertical([ + Constraint::Length(agents_h), + Constraint::Length(settings_h), + Constraint::Min(3), + ]) + .areas(main); // The rows the pane cannot fit are reached with `J`/`K` and the page keys, // the cockpit card's gesture: the pane carries no selection to scroll with, @@ -1790,6 +1806,39 @@ fn draw_config(frame: &mut Frame, app: &App, hits: &mut HitMap) { agents_area, ); + // Settings: the `voro.toml` values this screen edits, each with the value in + // force and where it came from — the file, or the rule Voro fell back to + // (DESIGN.md §5). They share the selection with the viewers below, so + // exactly one of the two lists ever shows a highlight. + let settings_items: Vec = app + .config_settings + .iter() + .map(|s| { + ListItem::new(Line::from(vec![ + Span::raw(format!(" {:<16}", s.name)), + Span::styled(format!("{:<14}", s.value), Style::new().bold()), + Span::styled(format!("({})", s.source), Style::new().dim()), + ])) + }) + .collect(); + let settings_count = app.config_settings.len(); + let mut settings_state = ListState::default() + .with_selected((app.config_sel < settings_count).then_some(app.config_sel)); + let settings = List::new(settings_items) + .block( + Block::default() + .borders(Borders::ALL) + .title("Settings — ⏎ edit"), + ) + .highlight_style(SELECTED); + frame.render_stateful_widget(settings, settings_area, &mut settings_state); + hits.push_list( + settings_area, + settings_state.offset(), + settings_count, + Hit::ConfigRow, + ); + // Viewers: every viewer `open` can run — the built-ins with the user's // tables layered over them, each carrying its provenance like the agents // pane above, the default starred — then the anonymous [viewer] as a @@ -1820,18 +1869,18 @@ fn draw_config(frame: &mut Frame, app: &App, hits: &mut HitMap) { ), ]))); } - // The selection only ever lands on a named viewer, never the anonymous note. - let selected = if named == 0 { - None - } else { - Some(app.config_sel) + // The selection only ever lands on a named viewer, never the anonymous note + // — and only while it is past the settings rows above. + let selected = match app.config_sel.checked_sub(settings_count) { + Some(i) if i < named => Some(i), + _ => None, }; let mut state = ListState::default().with_selected(selected); let viewers = List::new(viewer_items) .block( Block::default() .borders(Borders::ALL) - .title("Viewers — a add · e edit · d delete · V default · A default agent") + .title("Viewers — a add · ⏎ edit · d delete") .title_bottom( Line::from(format!(" {} ", app.config_path().display())).right_aligned(), ), @@ -1840,7 +1889,9 @@ fn draw_config(frame: &mut Frame, app: &App, hits: &mut HitMap) { frame.render_stateful_widget(viewers, viewers_area, &mut state); // Same rule as the selection: the anonymous note below the named viewers is // not selectable, so it is not clickable either. - hits.push_list(viewers_area, state.offset(), named, Hit::ViewerRow); + hits.push_list(viewers_area, state.offset(), named, |i| { + Hit::ConfigRow(settings_count + i) + }); draw_status(frame, app, status); } @@ -2045,7 +2096,12 @@ fn hint_candidates(app: &App) -> Vec<(&'static str, &'static str, bool)> { ("q", "quit", true), ], Screen::Config => { - let viewers = !app.config_viewers.is_empty(); + // Deleting is the viewer list's own operation, so it appears only + // while the selection is in that list; editing is every row's. + let on_viewer = matches!( + app.selected_config_row(), + Some(crate::app::ConfigRow::Viewer(_)) + ); // While the gate holds (DESIGN.md §9) `tab` cycles the shorter // Projects ↔ Config ring, so the slot has to name where it lands. let next = if app.projects.is_empty() { @@ -2054,11 +2110,9 @@ fn hint_candidates(app: &App) -> Vec<(&'static str, &'static str, bool)> { "cockpit" }; vec![ + enter, ("a", "add viewer", true), - ("e", "edit", viewers), - ("d", "delete", viewers), - ("V", "default viewer", viewers), - ("A", "default agent", true), + ("d", "delete", on_viewer), ("?", "keys", true), ("tab", next, true), ("q", "quit", true), @@ -2104,20 +2158,17 @@ const NEW_KEYS: [(&str, &str); 2] = [ /// The uppercase keys DESIGN.md §9 names as standing outside the case /// convention, because none is the shifted half of a pair: `C` and the projects -/// screen's `A` share a letter with an unrelated action, `J`/`K` scroll a pane -/// that has no selection to scroll with — the cockpit's card and the Config -/// screen's agents — and the Config screen's `V`/`A` pick defaults. Every other -/// uppercase binding has to be the interactive half of a pair, which the test -/// below enforces screen by screen. +/// screen's `A` share a letter with an unrelated action, and `J`/`K` scroll a +/// pane that has no selection to scroll with — the cockpit's card and the Config +/// screen's agents. Every other uppercase binding has to be the interactive half +/// of a pair, which the test below enforces screen by screen. #[cfg(test)] -const CASE_EXCEPTIONS: [(Screen, &str); 9] = [ +const CASE_EXCEPTIONS: [(Screen, &str); 7] = [ (Screen::Cockpit, "C"), (Screen::Cockpit, "J"), (Screen::Cockpit, "K"), (Screen::Tasks, "C"), (Screen::Projects, "A"), - (Screen::Config, "V"), - (Screen::Config, "A"), (Screen::Config, "J"), (Screen::Config, "K"), ]; @@ -2250,11 +2301,9 @@ fn key_map(screen: Screen, no_projects: bool) -> Vec { ( "Actions", vec![ + ("⏎/e", "edit the selected setting or viewer"), ("a", "add a viewer"), - ("⏎/e", "edit the selected viewer's command"), ("d", "delete the selected viewer"), - ("V", "pick the default viewer"), - ("A", "pick the default agent"), ], ), ( @@ -2995,8 +3044,10 @@ mod tests { } /// The agents pane sizes to the rows it has: an operator with several - /// model-carrying agents, each now three lines tall, still sees every one - /// of them, and the viewers list below keeps a row (DESIGN.md §9). + /// model-carrying agents, each now three lines tall, sees every one of them + /// without touching the scroll, and the lists below keep their rows + /// (DESIGN.md §9). What happens when even that is not enough is the scroll + /// test below. #[test] fn config_screen_shows_every_agent_it_has() { use crate::app::App; @@ -3034,7 +3085,12 @@ mod tests { let mut app = App::new(store, ctx).unwrap(); alt_screen(&mut app, '4'); - let mut terminal = Terminal::new(TestBackend::new(100, 24)).unwrap(); + // Tall enough for the settings pane the agents now share the screen + // with: it is a fixed height that neither scrolls nor grows, so it is + // subtracted before the paragraph takes what its rows need. + let mut terminal = Terminal::new(TestBackend::new(100, 29)).unwrap(); + // …and it is what the pane fits, not what it can be scrolled to. + assert_eq!(app.config_agents_scroll, 0); terminal .draw(|f| { draw(f, &app); @@ -3148,12 +3204,129 @@ mod tests { "{rendered}" ); - // `K` walks back, and the viewers list keeps its own `j`/`k`. + // `K` walks back, and the two lists below keep their own `j`/`k`. app.on_key(KeyEvent::from(KeyCode::PageUp)); assert!(app.config_agents_scroll < hidden); let before = app.config_sel; app.on_key(KeyEvent::from(KeyCode::Char('j'))); - assert_ne!(app.config_sel, before, "j still moves the viewer selection"); + assert_ne!(app.config_sel, before, "j still moves the row selection"); + + std::fs::remove_dir_all(&dir).unwrap(); + } + + /// The settings pane at an ordinary terminal (DESIGN.md §5/§9): every + /// setting shows its value and where it came from, the viewers list below + /// still has its border and a row, and the selection the two lists share + /// highlights exactly one line whichever list it is in. + #[test] + fn config_settings_pane_reads_at_80x24() { + use crate::app::App; + use ratatui::Terminal; + use ratatui::backend::TestBackend; + use voro_core::Store; + + let dir = std::env::temp_dir().join(format!( + "voro-ui-config-settings-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + let agents_path = dir.join("voro.toml"); + std::fs::create_dir_all(&dir).unwrap(); + // the cap and the default agent are the operator's; the default viewer + // is whatever voro resolves, so the pane shows both provenances at once + std::fs::write( + &agents_path, + "default_agent = \"claude\"\nmax_running = 2\n\n[viewers.zed]\ncmd = \"zed {path}\"\n", + ) + .unwrap(); + + let store = Store::open_in_memory().unwrap(); + let ctx = crate::dispatch::DispatchCtx { + db_path: dir.join("voro.db"), + agents_path, + runtime_dir: dir.join("sessions"), + ref_capture_timeout: std::time::Duration::ZERO, + message_grace: std::time::Duration::from_millis(300), + }; + let mut app = App::new(store, ctx).unwrap(); + alt_screen(&mut app, '4'); + + let mut terminal = Terminal::new(TestBackend::new(80, 24)).unwrap(); + // The lines of the frame, and the ones the selection has reversed. + let read = |terminal: &Terminal| -> (Vec, Vec) { + let buffer = terminal.backend().buffer(); + let mut lines = Vec::new(); + let mut highlighted = Vec::new(); + for y in 0..buffer.area.height { + let mut line = String::new(); + let mut reversed = false; + for x in 0..buffer.area.width { + let cell = &buffer[(x, y)]; + line.push_str(cell.symbol()); + reversed |= cell.modifier.contains(Modifier::REVERSED); + } + if reversed { + highlighted.push(y as usize); + } + lines.push(line); + } + (lines, highlighted) + }; + let draw_now = |terminal: &mut Terminal, app: &App| { + terminal + .draw(|f| { + draw(f, app); + }) + .unwrap(); + }; + + draw_now(&mut terminal, &app); + let (lines, highlighted) = read(&terminal); + let find = |needle: &str| { + lines + .iter() + .find(|l| l.contains(needle)) + .unwrap_or_else(|| panic!("no line with {needle}:\n{}", lines.join("\n"))) + .clone() + }; + assert!(find("Settings").contains("⏎ edit")); + let cap = find("dispatch cap"); + assert!(cap.contains(" 2 ") && cap.contains("(voro.toml)"), "{cap}"); + let agent = find("default agent"); + assert!( + agent.contains("claude") && agent.contains("(voro.toml)"), + "{agent}" + ); + // resolved rather than chosen: a sole `[viewers.*]` table is the default + let viewer = find("default viewer"); + assert!( + viewer.contains("zed") && viewer.contains("(the only viewer configured)"), + "{viewer}" + ); + // the viewers list keeps its border and a row of its own + assert!(find("Viewers").contains("a add · ⏎ edit · d delete")); + assert!(lines.iter().any(|l| l.contains("zed {path}"))); + + // the selection starts on the first setting, and moving it into the + // viewers list moves the one highlight there — never two at once + assert_eq!(highlighted.len(), 1, "{highlighted:?}"); + let first = highlighted[0]; + assert!(read(&terminal).0[first].contains("default agent")); + + for _ in 0..app.config_settings.len() { + app.move_selection(1); + } + draw_now(&mut terminal, &app); + let (lines, highlighted) = read(&terminal); + assert_eq!(highlighted.len(), 1, "{highlighted:?}"); + assert!( + lines[highlighted[0]].contains(&app.config_viewers[0].name), + "{}", + lines[highlighted[0]] + ); std::fs::remove_dir_all(&dir).unwrap(); } @@ -6204,7 +6377,17 @@ mod tests { let target = app.config_viewers[1].name.clone(); let (x, y) = point_of(&terminal, &target); app.on_mouse(x, y, &hits); - assert_eq!(app.config_viewers[app.config_sel].name, target); + assert_eq!( + app.selected_viewer().map(|v| v.name.clone()), + Some(target.clone()) + ); + + // The settings list above clicks the same way — the two lists share one + // row space, so a click in either lands the single selection. + let (x, y) = point_of(&terminal, "dispatch cap"); + app.on_mouse(x, y, &hits); + assert_eq!(app.selected_setting().map(|s| s.name), Some("dispatch cap")); + assert!(app.selected_viewer().is_none()); std::fs::remove_dir_all(&dir).unwrap(); } diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 792d6c1..37c088b 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -561,28 +561,44 @@ and one it cannot (§8). `voro viewer list` does the same for viewers, flagging the default. The file is writable by Voro as well as read. The TUI's Config screen (§9) and -the `voro viewer add`/`viewer remove` verbs edit it in place — adding, -changing, and deleting `[viewers.]` tables and setting -`default_viewer`/`default_agent` — through a single write helper +the `voro viewer add`/`viewer remove` verbs edit it in place — adding, changing, +and deleting `[viewers.]` tables and setting `default_viewer`, +`default_agent` and `max_running` — through a single write helper (`voro-core::config_edit`) built on `toml_edit`, so a machine write preserves the file's existing content, formatting, and comments and touches only the key -it changes. A missing file is created on first edit. The command is optional at -both surfaces and defaults to ` {path}`, which is what nearly every -editor CLI wants; naming a *built-in* defaults to that built-in's own command -instead, so overriding one starts from what it replaces. In the form the -command does not merely default but *follows* the name field, rewritten on -every keystroke and rendered dim until the operator types in it, at which point -it is theirs; deleting it back to empty hands it to the name again. The -user-owned surface is all that is writable this way: agents stay read-only in -the TUI (editing a built-in means writing a wholesale override table, a sharper -knife deferred here), and deleting a viewer that a project's `viewer` still -names is refused with the projects named, while deleting the default viewer -clears `default_viewer`. A built-in viewer is read-only for the same reason an -agent is — it lives in the binary, not the file — so the Config screen lists -the built-ins with their provenance but refuses to edit or delete one, and -`voro viewer remove code` says the same, naming the *add* of that name that -overrides it. Choosing one is not writing one, so both a `default_viewer` and a -project's own viewer may name a built-in with no table defining it. +it changes. A missing file is created on first edit. + +The Config screen edits those keys from a **settings list** — the default agent, +the default viewer, and the dispatch cap — sitting above the viewers. Each row +carries the value in force and where it came from: `voro.toml` where the file +names the key, and otherwise the rule that resolved it (the first agent found on +PATH, the only viewer configured, Voro's own default cap), since the value alone +cannot tell a cap the operator chose from one that happens to equal the default. +A setting is edited, never deleted — there is no state in which Voro runs under +no cap — so `d` on one is refused pointing at ⏎. The cap means here what it +means in the file: `0` is legal and stops the queue offering dispatches at all +(§7), and a negative one is refused in the same sentence whichever surface +writes it. Saving rebuilds the queue on that keypress, so a raised cap restores +the dispatch rows the capacity line had replaced without restarting the TUI. + +The add/edit-viewer form asks for the one thing a first-time operator cannot +answer, the command line. It is optional at both surfaces and defaults to +` {path}`, which is what nearly every editor CLI wants; naming a +*built-in* defaults to that built-in's own command instead, so overriding one +starts from what it replaces. In the form the command does not merely default +but *follows* the name field, rewritten on every keystroke and rendered dim +until the operator types in it, at which point it is theirs; deleting it back to +empty hands it to the name again. The user-owned surface is all that is writable +this way: agents stay read-only in the TUI (editing a built-in means writing a +wholesale override table, a sharper knife deferred here), and deleting a viewer +that a project's `viewer` still names is refused with the projects named, while +deleting the default viewer clears `default_viewer`. A built-in viewer is +read-only for the same reason an agent is — it lives in the binary, not the file +— so the Config screen lists the built-ins with their provenance but refuses to +edit or delete one, and `voro viewer remove code` says the same, naming the +*add* of that name that overrides it. Choosing one is not writing one, so both a +`default_viewer` and a project's own viewer may name a built-in with no table +defining it. ```toml # ~/.config/voro/voro.toml — all optional; extends/overrides the built-in claude @@ -974,18 +990,19 @@ operator attention, so they cannot tell a review from an idle afternoon. Dispatch is the one action the divisor prices wrongly, and it is shaped rather than priced. Handing a task to an agent costs the operator a keypress, so on -attention alone it would outrank everything — but every dispatch manufactures -a future review and loads the fleet, so its real cost is a *concurrency slot*. -It is therefore metered: `max_running` (`voro.toml`, default 5) caps how many -dispatches ride at once, and once `running >= max_running` every row whose -action would open a session — *dispatch* and *redispatch* alike — leaves the -queue, replaced by a single capacity line naming the counts (`⏸ dispatch at -capacity (5/5 running)`). The running tally is the one `stats` already reports. -Suppressing the rows rather than demoting them is the point: at the cap they are -not cheap actions, they are unavailable ones, and the capacity line says so -where an empty queue would have implied there was nothing to start. A `do` row -is untouched by the gate — a human task spends the operator's hands, not a -slot — and the counts (§12) keep the suppressed backlog felt. +attention alone it would outrank everything — but every dispatch manufactures a +future review and loads the fleet, so its real cost is a *concurrency slot*. It +is therefore metered: `max_running` (default 5, set in `voro.toml` or on the +Config screen's settings list, §5) caps how many dispatches ride at once, and +once `running >= max_running` every row whose action would open a session — +*dispatch* and *redispatch* alike — leaves the queue, replaced by a single +capacity line naming the counts (`⏸ dispatch at capacity (5/5 running)`). The +running tally is the one `stats` already reports. Suppressing the rows rather +than demoting them is the point: at the cap they are not cheap actions, they are +unavailable ones, and the capacity line says so where an empty queue would have +implied there was nothing to start. A `do` row is untouched by the gate — a +human task spends the operator's hands, not a slot — and the counts (§12) keep +the suppressed backlog felt. Cheap actions need one further guard, or the pricing swaps one swamping for another: forty proposals at 0.8 would fill the queue with triage. Proposals @@ -2313,34 +2330,42 @@ picker entirely and creates straight into the live one, and a store whose every project is archived opens no picker at all, refusing with a status line pointing at the projects screen. -Beyond the cockpit, the TUI cycles (Tab, or `alt-1`–`alt-4`, subject to the -gate above while no project is registered) through three further full-screen -views: the **task browser**, the **projects screen** (weights, archive, and the +Beyond the cockpit, the TUI cycles (Tab, or `alt-1`–`alt-4`, subject to the gate +above while no project is registered) through three further full-screen views: +the **task browser**, the **projects screen** (weights, archive, and the per-project viewer), and a **Config screen** that renders and edits the `voro.toml` surface (§5) — the effective agents read-only with provenance and -the default marked, and the named viewers editable in place (add, change -command, delete, and pick `default_viewer`/`default_agent`) through the -comment-preserving write helper. An agent occupies more than a row there: under -its name, provenance and verb list sit dim continuation lines carrying the -dispatch command it runs and, where it names one, what `{model}` resolves to — -a line of its own rather than a tail on the name row, since the verb list is -long enough to clip a tail off an ordinary terminal's width, and the annotation -exists precisely to be read beside the placeholder in the command above it. The -pane sizes to the rows it has rather than to a fixed cap, yielding height only -where the viewers list below would otherwise lose its last row. Where even that -is not enough — a short terminal, several agents — the pane **scrolls** -rather than silently dropping what falls past its border. It scrolls with -`J`/`K` and the page keys, the cockpit card's gesture and for the same reason: -the pane carries no selection of its own, `j`/`k` on this screen belonging to -the viewers list below, and a second selection is a heavier thing to add to a -screen than a scroll. Like the card it advertises the scroll only when there is -one, on its bottom border, so a pane holding everything says nothing and a pane -hiding rows says how many and which keys move them. DB-backed configuration -(projects, weights, viewers) stays on the projects screen; the Config screen is -the voro.toml view. The projects screen's viewer picker also offers a "new -viewer…" entry that opens the same add-viewer form and selects the new viewer -for that project, so first-time viewer setup needs no detour through the Config -screen. +the default marked, then the settings list of §5, then the named viewers +editable in place (add, change command, delete) through the comment-preserving +write helper. A setting is *selected*, not bound to a letter of its own: one +selection runs over the settings rows and the viewer rows alike, and ⏎ (or `e`) +edits whichever it is on — the pick-from-list for the default agent and the +default viewer, a one-line numeric entry for the dispatch cap. A screen that +spends an uppercase letter per value does not scale, and it is the wrong model: +a config screen is a list of settings you select. Only `a` (add a viewer) and +`d` (delete one) stay keys, because neither is an edit of the selected value: +`a` acts with nothing selected, and `d` is destructive. An agent occupies more +than a row there: under its name, provenance and verb list sit dim continuation +lines carrying the dispatch command it runs and, where it names one, what +`{model}` resolves to — a line of its own rather than a tail on the name row, +since the verb list is long enough to clip a tail off an ordinary terminal's +width, and the annotation exists precisely to be read beside the placeholder in +the command above it. The pane sizes to the rows it has rather than to a fixed +cap, yielding height to the settings list — which is exactly its rows, neither +scrolling nor growing — and to what the viewers list below needs for a border +and a row. Where even that is not enough — a short terminal, several agents — +the pane **scrolls** rather than silently dropping what falls past its border. +It scrolls with `J`/`K` and the page keys, the cockpit card's gesture and for +the same reason: the pane carries no selection of its own, `j`/`k` on this +screen belonging to the two lists below, and a second selection is a heavier +thing to add to a screen than a scroll. Like the card it advertises the scroll +only when there is one, on its bottom border, so a pane holding everything says +nothing and a pane hiding rows says how many and which keys move them. DB-backed +configuration (projects, weights, viewers) stays on the projects screen; the +Config screen is the voro.toml view. The projects screen's viewer picker also +offers a "new viewer…" entry that opens the same add-viewer form and selects the +new viewer for that project, so first-time viewer setup needs no detour through +the Config screen. **A bare digit sets the number on the selected row, and screen jumps carry the modifier.** The digit's meaning follows the selection rather than the screen: @@ -2367,56 +2392,54 @@ or `5` pressed on a task, priority stopping at P3 where weight runs to 5. **Keys are advertised in two places, and the split between them is deliberate.** A contextual key line sits under every screen, listing the actions that apply to -the current screen and selection — and only those that change a task's state -or destiny, since a line the operator has to read twice has stopped being +the current screen and selection — and only those that change a task's state or +destiny, since a line the operator has to read twice has stopped being contextual. Where a lowercase key and its shifted sibling are two ways of doing *one* action, they take a single slot keyed on the pair and labelled with the base verb (`d/D dispatch`, `r/R refine`, `n/N new`, `a/A message`); keys that merely share a letter without sharing an action — the cockpit's `c` link -documents and `C` cancel a refine, the projects screen's `a` add and `A` -archive, the Config screen's `a` add viewer and `A` default agent — keep their -own slots, because pairing them would claim a kinship that is not there. What -each uppercase variant does differently is spelled out one level down, in the -**`?` key map**: a peek-style overlay, dismissed by any key but `tab`, listing -the current screen's *complete* bindings grouped into actions, navigation, and -screen switching. The overlay is sized to the terminal rather than to its -content, because the alternative is a map that quietly stops being complete. -The Actions column gives up width — its glosses ellipsised — before the -column beside it loses a character, and a map too tall for the terminal splits -into evenly filled pages that `tab` turns, which is the one key the overlay -keeps for itself. `tab` rather than a scroll key because paging is the -peek-sized gesture: the map is read, not navigated. A test renders every +documents and `C` cancel a refine, and the projects screen's `a` add and `A` +archive — keep their own slots, because pairing them would claim a kinship that +is not there. What each uppercase variant does differently is spelled out one +level down, in the **`?` key map**: a peek-style overlay, dismissed by any key +but `tab`, listing the current screen's *complete* bindings grouped into +actions, navigation, and screen switching. The overlay is sized to the terminal +rather than to its content, because the alternative is a map that quietly stops +being complete. The Actions column gives up width — its glosses ellipsised — +before the column beside it loses a character, and a map too tall for the +terminal splits into evenly filled pages that `tab` turns, which is the one key +the overlay keeps for itself. `tab` rather than a scroll key because paging is +the peek-sized gesture: the map is read, not navigated. A test renders every screen's map at 80x24 and pages through it asserting each entry appears whole, so the map outgrowing its overlay fails the build instead of silently shedding -its last rows. The map is what licenses the line's brevity — -navigation, display toggles like `x`/`h`, and browsing conveniences like `l` are -reachable and documented without ever crowding the line — so `?` itself is the -one key every screen's line always carries. The two review keys are the -deliberate exception to "state or destiny only": `o` (the local diff) and `g` -(the PR) change nothing, but on a task that has just come back for review, -looking at the diff *is* the operator's next action, and the line is where the -moment is announced. They earn the slots by being tightly gated on that moment, -and the moment is a state *plus* something to show: `o` on a `review` or -`running` task that carries a branch, since the diff it opens is built from that -branch, and `g` on a `review` task that carries a branch or a tracked PR, since -with neither there is no pull request to jump to and none that `pr` could open -— `plan_pr` refuses without a branch. The state alone is not enough, because a -task whose whole product is its summary — an investigation, a triage, an audit -— reaches `review` having never made a branch, and the line would then -advertise two keys that cannot act beside a card whose recommended verb, for -that very reason, is *accept* rather than *pr* (§3). So they are absent from -the line everywhere the argument for them does not hold, even though both keys -stay bound in every state (§8: `g` also jumps to a tracked PR, and links one). -The rule generalises in both directions: a read-only key belongs on the line -when the selection's state makes it the obvious next press, and a -state-changing one drops off it when that state has passed — `!` (deep) picks -the model of the *next* dispatch, so on a task under review, handed off, or -closed it toggles something the operator is not about to see, and the review -row reclaims the slot. Neither narrowing touches what the keys *do*; only the -advertisement follows the selection. The line's budget is eleven slots, and a -`review` row with a branch — `⏎ review`, `w wait`, `o open`, `g PR`, and the -unconditional rest — is the row that spends them all; the branchless one -hands two of them back. +its last rows. The map is what licenses the line's brevity — navigation, display +toggles like `x`/`h`, and browsing conveniences like `l` are reachable and +documented without ever crowding the line — so `?` itself is the one key every +screen's line always carries. The two review keys are the deliberate exception +to "state or destiny only": `o` (the local diff) and `g` (the PR) change +nothing, but on a task that has just come back for review, looking at the diff +*is* the operator's next action, and the line is where the moment is announced. +They earn the slots by being tightly gated on that moment, and the moment is a +state *plus* something to show: `o` on a `review` or `running` task that carries +a branch, since the diff it opens is built from that branch, and `g` on a +`review` task that carries a branch or a tracked PR, since with neither there is +no pull request to jump to and none that `pr` could open — `plan_pr` refuses +without a branch. The state alone is not enough, because a task whose whole +product is its summary — an investigation, a triage, an audit — reaches `review` +having never made a branch, and the line would then advertise two keys that +cannot act beside a card whose recommended verb, for that very reason, is +*accept* rather than *pr* (§3). So they are absent from the line everywhere the +argument for them does not hold, even though both keys stay bound in every state +(§8: `g` also jumps to a tracked PR, and links one). The rule generalises in +both directions: a read-only key belongs on the line when the selection's state +makes it the obvious next press, and a state-changing one drops off it when that +state has passed — `!` (deep) picks the model of the *next* dispatch, so on a +task under review, handed off, or closed it toggles something the operator is +not about to see, and the review row reclaims the slot. Neither narrowing +touches what the keys *do*; only the advertisement follows the selection. The +line's budget is eleven slots, and a `review` row with a branch — `⏎ review`, `w +wait`, `o open`, `g PR`, and the unconditional rest — is the row that spends +them all; the branchless one hands two of them back. **The same row is where Voro answers back, and it grows to fit what it has to say.** A status message — a refusal, or the summary of something that just @@ -2488,20 +2511,18 @@ and the shift is the operator saying they are willing to be taken somewhere. The rule binds *pairs*, and only pairs, which is the same line the key line already draws between a shifted sibling and a mere letter-sharer. It therefore has nothing to say about a key whose uppercase is a different action — the -cockpit's `c` link documents and `C` cancel a refine, the projects screen's `a` -add and `A` archive, the Config screen's `a` add viewer and `A` default agent -— nor about an uppercase key with no lowercase sibling at all: `J`/`K` and the -page keys scroll the pane the screen's selection cannot reach — the cockpit's -focus card, the Config screen's agents — which is one binding wearing one -meaning twice rather than two, and is why the second use took those letters -rather than fresh ones, and the Config screen's `V` picks the default viewer. -Those are the exceptions, named here so the convention is not read wider than it -is, and none is worth rebinding: the letters they share carry no kinship, and -moving a key the operator's fingers already know would buy a consistency nobody -reads. What the rule binds instead is the future — a heavier, interactive -variant of an existing action takes that action's shifted key rather than a -fresh letter, and a new uppercase binding that is neither of those needs a line -here saying why. +cockpit's `c` link documents and `C` cancel a refine, and the projects screen's +`a` add and `A` archive — nor about an uppercase key with no lowercase sibling +at all: `J`/`K` and the page keys scroll the pane the screen's selection cannot +reach — the cockpit's focus card, the Config screen's agents — which is one +binding wearing one meaning twice rather than two, and is why the second use +took those letters rather than fresh ones. Those are the exceptions, named here +so the convention is not read wider than it is, and none is worth rebinding: the +letters they share carry no kinship, and moving a key the operator's fingers +already know would buy a consistency nobody reads. What the rule binds instead +is the future — a heavier, interactive variant of an existing action takes that +action's shifted key rather than a fresh letter, and a new uppercase binding +that is neither of those needs a line here saying why. Core interactions: create a task by typing one line and letting a background agent expand it into a proposal (§8's quick propose, on the default