From 62383deb562383a640f953bdfdc052d07ceb9d09 Mon Sep 17 00:00:00 2001 From: CodeWhale Agent Date: Mon, 29 Jun 2026 16:42:22 -0700 Subject: [PATCH] feat(tui): ship the Hotbar hidden by default until setup opt-in (#3807) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A fresh install (no `hotbar` key) showed the eight default Hotbar slots, because `resolve_hotbar_bindings` mapped an absent key to the built-in defaults. The Hotbar is now an opt-in personalization feature, so a fresh config must render no panel. - config: `resolve_hotbar_bindings(None, ...)` now resolves to no bindings instead of the defaults, so the panel, the Alt+1-8 dispatch, and the setup wizard are all empty until the user opts in. An explicit `[[hotbar]]` config (configured users) and `hotbar = []` (already disabled) are byte-for-byte unaffected — only the meaning of an absent key changes, so no settings migration is needed. - Add `default_hotbar_bindings_toml()` so `/hotbar on` can persist the recommended slots explicitly, keeping `DEFAULT_HOTBAR_ACTIONS` the single source of truth. - `/hotbar on` (`restore_hotbar_defaults`) now writes the explicit default bindings instead of deleting the key (which would now hide the Hotbar). `/hotbar off` still writes `hotbar = []`; `/hotbar` still opens the wizard. Removed the now-unused `remove_hotbar_from_config` and its tests. Updated the tests that asserted the old default-visible behaviour to the opt-in contract and added round-trip coverage for `/hotbar on`. --- crates/config/src/lib.rs | 22 ++++- crates/config/src/tests.rs | 35 ++++---- crates/tui/src/config_persistence.rs | 123 ++++++--------------------- crates/tui/src/tui/hotbar/setup.rs | 4 +- crates/tui/src/tui/sidebar.rs | 43 +++++----- crates/tui/src/tui/ui.rs | 19 +++-- crates/tui/src/tui/ui/tests.rs | 7 +- 7 files changed, 110 insertions(+), 143 deletions(-) diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index e0bd5981c..b5e5cc4ce 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -780,6 +780,23 @@ pub fn default_hotbar_bindings() -> Vec { .collect() } +/// The default hotbar slots in on-disk (`[[hotbar]]`) form. Since #3807 an +/// absent `hotbar` key means "hidden", so `/hotbar on` persists these explicit +/// bindings rather than deleting the key. Kept in terms of +/// [`default_hotbar_bindings`] so `DEFAULT_HOTBAR_ACTIONS` stays the single +/// source of truth. +#[must_use] +pub fn default_hotbar_bindings_toml() -> Vec { + default_hotbar_bindings() + .into_iter() + .map(|binding| HotbarBindingToml { + slot: binding.slot, + action: binding.action, + label: binding.label, + }) + .collect() +} + #[must_use] pub fn resolve_hotbar_bindings( configured: Option<&[HotbarBindingToml]>, @@ -797,7 +814,10 @@ pub fn resolve_hotbar_bindings( label: binding.label.clone(), }) .collect::>(), - None => default_hotbar_bindings(), + // #3807: an absent `hotbar` key means the Hotbar is hidden until the + // user opts in (via the setup wizard or `/hotbar on`). Only an explicit + // `[[hotbar]]` config produces bindings. `Some([])` stays "disabled". + None => Vec::new(), }; let mut by_slot: BTreeMap = BTreeMap::new(); diff --git a/crates/config/src/tests.rs b/crates/config/src/tests.rs index 08aa8165f..3941c72c2 100644 --- a/crates/config/src/tests.rs +++ b/crates/config/src/tests.rs @@ -335,29 +335,30 @@ fn provider_auth_source_rejects_empty_command() { } #[test] -fn hotbar_defaults_when_config_is_absent() { +fn hotbar_hidden_when_config_is_absent() { + // #3807: an absent `hotbar` key resolves to no bindings, so the Hotbar is + // hidden until the user opts in. The default slots are still available + // explicitly via `default_hotbar_bindings_toml()` (what `/hotbar on` writes). let config = ConfigToml::default(); let resolved = config.resolve_hotbar_bindings(&DEFAULT_HOTBAR_ACTIONS); assert_eq!(resolved.warnings, Vec::new()); - assert_eq!(resolved.bindings, default_hotbar_bindings()); + assert!( + resolved.bindings.is_empty(), + "fresh config must resolve to no hotbar bindings: {:?}", + resolved.bindings + ); + + // The explicit default set still expands to the eight recommended slots. + let mut explicit = ConfigToml::default(); + explicit.hotbar = Some(default_hotbar_bindings_toml()); assert_eq!( - resolved - .bindings - .iter() - .map(|binding| (binding.slot, binding.action.as_str())) - .collect::>(), - vec![ - (1, "voice.toggle"), - (2, "session.compact"), - (3, "mode.plan"), - (4, "mode.agent"), - (5, "mode.yolo"), - (6, "palette.open"), - (7, "sidebar.toggle"), - (8, "trust.toggle"), - ] + explicit + .resolve_hotbar_bindings(&DEFAULT_HOTBAR_ACTIONS) + .bindings, + default_hotbar_bindings(), + "an explicit default-bindings config still shows all eight slots" ); } diff --git a/crates/tui/src/config_persistence.rs b/crates/tui/src/config_persistence.rs index 33135cfcd..3216c2376 100644 --- a/crates/tui/src/config_persistence.rs +++ b/crates/tui/src/config_persistence.rs @@ -381,41 +381,6 @@ pub(crate) fn persist_hotbar_bindings( Ok(path) } -/// Remove the `hotbar` key from the resolved config so the resolver falls back -/// to the built-in default slots (`/hotbar on` / `/hotbar reset`). Unlike -/// `persist_hotbar_bindings(&[])` — which writes an explicit `hotbar = []` and -/// therefore *disables* the hotbar — this deletes the key entirely, restoring -/// defaults. If the file or key is absent there is nothing to restore; we still -/// return the resolved path. Surrounding keys are preserved via `toml_edit`; -/// a comment used as the removed key's own leading decor is removed with it -/// (standard `toml_edit` key-prefix behavior). -pub(crate) fn remove_hotbar_from_config(config_path: Option<&Path>) -> anyhow::Result { - use anyhow::Context; - use std::fs; - - let path = config_toml_path(config_path)?; - if !path.exists() { - // No config file ⇒ no `hotbar` key ⇒ defaults already apply. - return Ok(path); - } - let raw = fs::read_to_string(&path) - .with_context(|| format!("failed to read config at {}", path.display()))?; - if raw.trim().is_empty() { - return Ok(path); - } - let mut document = raw - .parse::() - .with_context(|| format!("failed to edit config at {}", path.display()))?; - let table = document.as_table_mut(); - if table.remove("hotbar").is_none() { - // Key wasn't present ⇒ defaults already apply; avoid a needless rewrite. - return Ok(path); - } - fs::write(&path, document.to_string()) - .with_context(|| format!("failed to write config at {}", path.display()))?; - Ok(path) -} - pub(crate) fn config_toml_path(config_path: Option<&Path>) -> anyhow::Result { use anyhow::Context; @@ -726,6 +691,33 @@ mod tests { assert_eq!(parsed.hotbar, Some(bindings)); } + #[test] + fn persist_default_hotbar_bindings_round_trips_for_hotbar_on() { + // #3807: `/hotbar on` persists the explicit default slots (an absent key + // now means hidden), and they read back as the eight recommended slots. + let temp_root = temp_root("codewhale-hotbar-on-defaults"); + fs::create_dir_all(&temp_root).unwrap(); + let _guard = EnvGuard::new(&temp_root); + + let defaults = codewhale_config::default_hotbar_bindings_toml(); + assert_eq!(defaults.len(), codewhale_config::HOTBAR_SLOT_COUNT as usize); + + let path = persist_hotbar_bindings(None, &defaults).expect("persist should succeed"); + let body = fs::read_to_string(&path).expect("written file should be readable"); + assert!(body.contains("[[hotbar]]"), "hotbar table missing: {body}"); + + let parsed: codewhale_config::ConfigToml = + toml::from_str(&body).expect("written hotbar config should parse"); + assert_eq!(parsed.hotbar, Some(defaults)); + + // The persisted defaults resolve back to all eight recommended slots. + let resolved = parsed.resolve_hotbar_bindings(&codewhale_config::DEFAULT_HOTBAR_ACTIONS); + assert_eq!( + resolved.bindings, + codewhale_config::default_hotbar_bindings() + ); + } + #[test] fn persist_hotbar_bindings_preserves_comments_and_replaces_existing_tables() { let temp_root = temp_root("codewhale-hotbar-persist-comments"); @@ -796,65 +788,4 @@ enabled = true toml::from_str(&body).expect("written hotbar config should parse"); assert_eq!(parsed.hotbar, Some(Vec::new())); } - - #[test] - fn remove_hotbar_from_config_deletes_key_to_restore_defaults() { - let temp_root = temp_root("codewhale-hotbar-remove"); - fs::create_dir_all(&temp_root).unwrap(); - let _guard = EnvGuard::new(&temp_root); - - let path = temp_root.join(".codewhale").join("config.toml"); - fs::create_dir_all(path.parent().unwrap()).unwrap(); - // Seed a config with a hotbar binding and an unrelated key whose own - // trailing comment is NOT the hotbar key's decor (so it must survive). - fs::write( - &path, - "hotbar = [{ slot = 1, action = \"mode.plan\" }]\nother = true # keep me\n", - ) - .unwrap(); - - let returned = remove_hotbar_from_config(Some(&path)).expect("remove should succeed"); - assert_eq!(returned, path); - - let body = fs::read_to_string(&path).expect("written file should be readable"); - assert!( - !body.contains("hotbar"), - "hotbar key should be gone: {body}" - ); - // An unrelated key and its own trailing comment must survive the rewrite. - assert!( - body.contains("other = true"), - "unrelated key should survive: {body}" - ); - assert!( - body.contains("keep me"), - "comment that is not the hotbar key's decor should survive: {body}" - ); - - let parsed: codewhale_config::ConfigToml = - toml::from_str(&body).expect("restored config should parse"); - assert_eq!( - parsed.hotbar, None, - "removing the key reads back as None so the resolver falls back to defaults" - ); - } - - #[test] - fn remove_hotbar_from_config_is_a_noop_when_key_absent() { - let temp_root = temp_root("codewhale-hotbar-remove-absent"); - fs::create_dir_all(&temp_root).unwrap(); - let _guard = EnvGuard::new(&temp_root); - - let path = temp_root.join(".codewhale").join("config.toml"); - fs::create_dir_all(path.parent().unwrap()).unwrap(); - fs::write(&path, "other = true\n").unwrap(); - - let before = fs::read_to_string(&path).unwrap(); - remove_hotbar_from_config(Some(&path)).expect("noop should succeed"); - let after = fs::read_to_string(&path).unwrap(); - assert_eq!( - before, after, - "an absent hotbar key should not trigger a rewrite" - ); - } } diff --git a/crates/tui/src/tui/hotbar/setup.rs b/crates/tui/src/tui/hotbar/setup.rs index 255af1883..e3087577e 100644 --- a/crates/tui/src/tui/hotbar/setup.rs +++ b/crates/tui/src/tui/hotbar/setup.rs @@ -625,7 +625,9 @@ mod tests { ); assert_eq!(view.selected_source(), Some(HotbarActionCategory::App)); assert!(view.recommended_action_ids().contains("mode.agent")); - assert!(view.checked_action_ids().contains("mode.plan")); + // #3807: a fresh config seeds no bindings, so the wizard opens with + // nothing checked until the user opts in. + assert!(view.checked_action_ids().is_empty()); } #[test] diff --git a/crates/tui/src/tui/sidebar.rs b/crates/tui/src/tui/sidebar.rs index 4c2659013..72bd8b287 100644 --- a/crates/tui/src/tui/sidebar.rs +++ b/crates/tui/src/tui/sidebar.rs @@ -95,10 +95,12 @@ fn split_sidebar_hotbar_area(area: Rect, show_hotbar: bool) -> (Rect, Option