diff --git a/docs/familiars.md b/docs/familiars.md index a4ce6762..e3fe11e7 100644 --- a/docs/familiars.md +++ b/docs/familiars.md @@ -161,6 +161,30 @@ access = "read-only" | `description` | | Full description used to build the persona system prompt. | | `pronouns` | | Appended to the persona prompt if present. | | `access` | | Tool-access tier: `"full"`, `"read-only"`, or `"search-only"`. Defaults to `"read-only"` when omitted. See [Tool access tiers](#tool-access-tiers) below. | +| `model` | | Optional model override for this familiar, e.g. `"claude-opus-4-8"`. When omitted the familiar inherits the session's default model. Lets you pin a persona to a model without shadowing it with a workspace agent. | + +--- + +## Managing the roster + +`~/.coven/familiars.toml` is **owned by the Coven daemon** when it is running. In that mode coven-code treats the roster as read-only and directs all edits to the daemon. + +**Standalone mode** (no daemon socket at `~/.coven/coven.sock`) is different: coven-code owns the file and can write it directly. + +- **First-run bootstrap.** Press **F2** with no familiars configured and coven-code writes a starter `~/.coven/familiars.toml` (a `read-only` guide and a `full`-access builder) and opens the switcher so you have something to pick and a template to edit. +- **Create / rename / remove** from the prompt: + + ```text + /familiar new [display name] # create a read-only familiar + /familiar rename # rename, preserving all fields + /familiar remove # delete (clears it if it was active) + ``` + + You can also remove a familiar from the visual `/familiar` menu. All of these refuse with a clear message when the daemon owns the file. +- **Switching:** `/familiar `, the **F2** quick switcher (type to filter, `— none —` clears the active familiar), or the `/familiar` menu. +- **Clearing vs wiping:** `/familiar clear` (alias `reset`) steps back to no active familiar. `/familiar wipe-roster` (alias `reset-roster`) is **destructive** — it deletes the roster and workspace agents — and requires `/familiar wipe-roster confirm`. + +If the roster file is malformed, coven-code surfaces the parse error at startup instead of silently dropping every familiar; unknown access tiers and duplicate/reserved ids are reported as warnings. --- diff --git a/src-rust/crates/api/src/error_handling.rs b/src-rust/crates/api/src/error_handling.rs index 2bd2643b..da031ecf 100644 --- a/src-rust/crates/api/src/error_handling.rs +++ b/src-rust/crates/api/src/error_handling.rs @@ -75,6 +75,26 @@ pub fn is_context_overflow(message: &str) -> bool { /// is classified as [`ProviderError::ContextOverflow`] rather than /// [`ProviderError::InvalidRequest`]. pub fn parse_error_response(status: u16, body: &str, provider: &ProviderId) -> ProviderError { + parse_error_response_with_retry_after(status, body, provider, None) +} + +/// Parse a `Retry-After` header value into whole seconds. +/// +/// Supports the delay-seconds form (`Retry-After: 30`). The HTTP-date form is +/// not parsed and yields `None` (callers fall back to their own back-off). +pub fn parse_retry_after_secs(value: &str) -> Option { + value.trim().parse::().ok() +} + +/// Like [`parse_error_response`], but threads a caller-extracted `Retry-After` +/// value (in seconds) into the [`ProviderError::RateLimited`] variant so the +/// server-provided back-off delay is preserved and surfaced to the user. +pub fn parse_error_response_with_retry_after( + status: u16, + body: &str, + provider: &ProviderId, + retry_after: Option, +) -> ProviderError { let json: Option = serde_json::from_str(body).ok(); let message = if let Some(ref j) = json { @@ -142,7 +162,7 @@ pub fn parse_error_response(status: u16, body: &str, provider: &ProviderId) -> P }, 429 => ProviderError::RateLimited { provider: provider.clone(), - retry_after: None, + retry_after, }, 413 => ProviderError::ContextOverflow { provider: provider.clone(), @@ -312,7 +332,38 @@ mod tests { fn test_parse_error_response_rate_limit() { let pid = ProviderId::new("openai"); let err = parse_error_response(429, "rate limited", &pid); - assert!(matches!(err, ProviderError::RateLimited { .. })); + assert!(matches!( + err, + ProviderError::RateLimited { + retry_after: None, + .. + } + )); + } + + #[test] + fn test_parse_error_response_threads_retry_after() { + let pid = ProviderId::new("openai"); + let err = parse_error_response_with_retry_after(429, "rate limited", &pid, Some(42)); + assert!(matches!( + err, + ProviderError::RateLimited { + retry_after: Some(42), + .. + } + )); + } + + #[test] + fn test_parse_retry_after_secs() { + assert_eq!(parse_retry_after_secs("30"), Some(30)); + assert_eq!(parse_retry_after_secs(" 15 "), Some(15)); + // HTTP-date form is unsupported → None (caller falls back to back-off). + assert_eq!( + parse_retry_after_secs("Wed, 21 Oct 2015 07:28:00 GMT"), + None + ); + assert_eq!(parse_retry_after_secs(""), None); } #[test] diff --git a/src-rust/crates/api/src/providers/codex.rs b/src-rust/crates/api/src/providers/codex.rs index e521e03c..e4a469cd 100644 --- a/src-rust/crates/api/src/providers/codex.rs +++ b/src-rust/crates/api/src/providers/codex.rs @@ -29,7 +29,7 @@ use futures::{Stream, StreamExt}; use serde_json::{json, Value}; use tracing::{debug, warn}; -use crate::error_handling::parse_error_response; +use crate::error_handling::{parse_error_response_with_retry_after, parse_retry_after_secs}; use crate::provider::{LlmProvider, ModelInfo}; use crate::provider_error::ProviderError; use crate::provider_types::{ @@ -39,6 +39,15 @@ use crate::provider_types::{ use crate::providers::responses_input::to_responses_input; +/// Extract a `Retry-After` delay (in seconds) from response headers, if the +/// server sent one in the delay-seconds form. +fn retry_after_from_headers(headers: &reqwest::header::HeaderMap) -> Option { + headers + .get(reqwest::header::RETRY_AFTER) + .and_then(|v| v.to_str().ok()) + .and_then(parse_retry_after_secs) +} + // --------------------------------------------------------------------------- // CodexProvider // --------------------------------------------------------------------------- @@ -333,6 +342,7 @@ impl CodexProvider { })?; let status = resp.status().as_u16(); + let retry_after = retry_after_from_headers(resp.headers()); let text = resp.text().await.map_err(|e| ProviderError::Other { provider: self.id.clone(), message: format!("Failed to read response body: {}", e), @@ -341,7 +351,12 @@ impl CodexProvider { })?; if !(200..300).contains(&(status as usize)) { - return Err(parse_error_response(status, &text, &self.id)); + return Err(parse_error_response_with_retry_after( + status, + &text, + &self.id, + retry_after, + )); } let json_val: Value = serde_json::from_str(&text).map_err(|e| ProviderError::Other { @@ -384,13 +399,19 @@ impl CodexProvider { let status = resp.status().as_u16(); if !(200..300).contains(&(status as usize)) { + let retry_after = retry_after_from_headers(resp.headers()); let text = resp.text().await.map_err(|e| ProviderError::Other { provider: self.id.clone(), message: format!("Failed to read response body: {}", e), status: Some(status), body: None, })?; - return Err(parse_error_response(status, &text, &self.id)); + return Err(parse_error_response_with_retry_after( + status, + &text, + &self.id, + retry_after, + )); } Ok(resp) diff --git a/src-rust/crates/commands/src/lib.rs b/src-rust/crates/commands/src/lib.rs index 82e75bc5..c6805500 100644 --- a/src-rust/crates/commands/src/lib.rs +++ b/src-rust/crates/commands/src/lib.rs @@ -8580,6 +8580,95 @@ fn infer_familiar_from_env() -> Option { }) } +/// Refusal message when the Coven daemon owns the roster file. +fn daemon_owned_refusal() -> CommandResult { + CommandResult::Error( + "Familiars are managed by the running Coven daemon — add, rename, or remove them \ + through the daemon. coven-code only writes ~/.coven/familiars.toml in standalone mode." + .to_string(), + ) +} + +/// `/familiar new [display name...]` — create a standalone familiar. +fn familiar_new(rest: &str) -> CommandResult { + let mut parts = rest.splitn(2, char::is_whitespace); + let id = parts.next().unwrap_or("").trim(); + if id.is_empty() { + return CommandResult::Error("Usage: /familiar new [display name]".to_string()); + } + if !claurst_core::coven_shared::can_write_familiars() { + return daemon_owned_refusal(); + } + let display = parts.next().map(str::trim).filter(|s| !s.is_empty()); + let fam = claurst_core::coven_shared::CovenFamiliar { + id: id.to_string(), + display_name: display.map(str::to_string), + emoji: None, + role: None, + description: None, + pronouns: None, + access: Some(claurst_core::coven_shared::DEFAULT_FAMILIAR_ACCESS.to_string()), + model: None, + }; + match claurst_core::coven_shared::create_familiar(fam) { + Ok(()) => CommandResult::Message(format!( + "Created familiar '{id}' (access: {}). Edit ~/.coven/familiars.toml to customize, \ + then switch with /familiar {id}.", + claurst_core::coven_shared::DEFAULT_FAMILIAR_ACCESS, + )), + Err(e) => CommandResult::Error(format!("Could not create familiar: {e}")), + } +} + +/// `/familiar remove ` — delete a standalone familiar. Clears the active +/// familiar too if it was the one removed. +fn familiar_remove(rest: &str, ctx: &CommandContext) -> CommandResult { + let id = rest.trim(); + if id.is_empty() { + return CommandResult::Error("Usage: /familiar remove ".to_string()); + } + if !claurst_core::coven_shared::can_write_familiars() { + return daemon_owned_refusal(); + } + match claurst_core::coven_shared::remove_familiar(id) { + Ok(removed) => { + let was_active = ctx + .config + .familiar + .as_deref() + .map(|a| a.eq_ignore_ascii_case(&removed.id)) + .unwrap_or(false); + if was_active { + let _ = save_settings_mutation(|s| s.config.familiar = None); + let mut new_config = ctx.config.clone(); + new_config.familiar = None; + return CommandResult::ConfigChangeMessage( + new_config, + format!("Removed familiar '{}' (was active — cleared).", removed.id), + ); + } + CommandResult::Message(format!("Removed familiar '{}'.", removed.id)) + } + Err(e) => CommandResult::Error(format!("Could not remove familiar: {e}")), + } +} + +/// `/familiar rename ` — rename a standalone familiar's id. +fn familiar_rename(rest: &str) -> CommandResult { + let mut parts = rest.split_whitespace(); + let (old_id, new_id) = (parts.next(), parts.next()); + let (Some(old_id), Some(new_id)) = (old_id, new_id) else { + return CommandResult::Error("Usage: /familiar rename ".to_string()); + }; + if !claurst_core::coven_shared::can_write_familiars() { + return daemon_owned_refusal(); + } + match claurst_core::coven_shared::rename_familiar(old_id, new_id) { + Ok(()) => CommandResult::Message(format!("Renamed familiar '{old_id}' → '{new_id}'.")), + Err(e) => CommandResult::Error(format!("Could not rename familiar: {e}")), + } +} + #[async_trait] impl SlashCommand for FamiliarCommand { fn name(&self) -> &str { @@ -8592,7 +8681,7 @@ impl SlashCommand for FamiliarCommand { vec!["familiars", "agent"] } fn help(&self) -> &str { - "Usage: /familiar [name | info | list|create|edit|delete [name] | managed ... | reset | reset-roster | auto]\n\n\ + "Usage: /familiar [name | info | list|create|edit|delete [name] | managed ... | clear | wipe-roster | auto]\n\n\ One surface for familiars and agents — both resolve through the same\n\ runtime agent map and access-tier security under the hood.\n\n\ /familiar Current familiar, roster, and agents (with access tiers)\n\ @@ -8601,9 +8690,13 @@ impl SlashCommand for FamiliarCommand { /familiar Details for a built-in or workspace agent\n\ /familiar info Details for any familiar or agent\n\ /familiar list|create|edit|delete [name] Manage workspace agents\n\ + /familiar new [name] Create a familiar (standalone; writes ~/.coven/familiars.toml)\n\ + /familiar rename Rename a familiar (standalone)\n\ + /familiar remove Delete a familiar (standalone)\n\ /familiar managed ... Manager-executor architecture (presets, budget)\n\ - /familiar reset Clear the active familiar\n\ - /familiar reset-roster Reset saved familiars and workspace agents\n\ + /familiar clear Step back to no active familiar (alias: reset)\n\ + /familiar wipe-roster DESTRUCTIVE — delete saved familiars + workspace\n\ + agents. Requires `wipe-roster confirm` (alias: reset-roster)\n\ /familiar auto Infer your familiar from the system username\n\n\ Roster lives in ~/.coven/familiars.toml (Coven daemon). In the TUI,\n\ bare /familiar opens the visual menu and F2 is the quick switcher." @@ -8623,10 +8716,26 @@ impl SlashCommand for FamiliarCommand { } // Manager-executor architecture (absorbed from /managed-agents). "managed" => return ManagedAgentsCommand.execute(rest, ctx).await, - // The old `/agents reset` roster wipe. `/familiar reset` clears - // the active familiar instead, so the destructive variant gets - // an explicit, unambiguous name. - "reset-roster" => { + // The old `/agents reset` roster wipe. `/familiar reset`/`clear` + // clears the active familiar instead, so the destructive variant + // gets an explicit, unambiguous name AND a confirmation gate — + // `reset` and `reset-roster` are one keystroke apart and the latter + // is irreversible. + "reset-roster" | "wipe-roster" => { + let confirmed = matches!( + rest.trim().to_lowercase().as_str(), + "confirm" | "yes" | "--yes" | "-y" + ); + if !confirmed { + return CommandResult::Message( + "\u{26a0} /familiar wipe-roster is destructive. It deletes \ + ~/.coven/familiars.toml, removes workspace agents \ + (.coven-code/agents/*.md), and clears your active familiar.\n\n\ + Re-run `/familiar wipe-roster confirm` to proceed.\n\ + To simply step back to no active familiar, use `/familiar clear`." + .to_string(), + ); + } return execute_named_command_from_slash("agents", "reset", ctx); } "info" => { @@ -8644,6 +8753,11 @@ impl SlashCommand for FamiliarCommand { )), }; } + // Standalone-only roster writes. When the Coven daemon is running it + // owns ~/.coven/familiars.toml, so these refuse and defer to it. + "new" => return familiar_new(rest), + "remove" => return familiar_remove(rest, ctx), + "rename" => return familiar_rename(rest), _ => {} } @@ -8654,8 +8768,9 @@ impl SlashCommand for FamiliarCommand { return CommandResult::Message(familiar_overview(ctx)); } - // Resolve switch target. - let target = if arg == "reset" { + // Resolve switch target. `clear` is the preferred, unambiguous name + // for stepping back to no active familiar; `reset` is kept as an alias. + let target = if arg == "reset" || arg == "clear" { None } else if arg == "auto" { match infer_familiar_from_env() { @@ -8715,7 +8830,7 @@ impl SlashCommand for FamiliarCommand { name, desc, access ) } - None => "Familiar reset to none.".to_string(), + None => "Familiar cleared — no active persona.".to_string(), }; CommandResult::ConfigChangeMessage(new_config, msg) diff --git a/src-rust/crates/core/src/coven_shared.rs b/src-rust/crates/core/src/coven_shared.rs index f5e4182d..00e02352 100644 --- a/src-rust/crates/core/src/coven_shared.rs +++ b/src-rust/crates/core/src/coven_shared.rs @@ -16,7 +16,7 @@ pub use crate::coven_daemon::{ DaemonReachability, DaemonSession, EventPage, EventRecord, FamiliarStatus, }; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; use std::path::PathBuf; /// Locate `~/.coven/` if it exists. @@ -123,23 +123,33 @@ pub fn is_disallowed_familiar_name(name: &str) -> bool { /// One entry in `~/.coven/familiars.toml`. /// /// Schema mirrors what the daemon serves at `GET /api/v1/familiars`. -#[derive(Debug, Clone, Deserialize)] +/// +/// `Serialize` is derived with `skip_serializing_if` on every optional field so +/// [`save_familiars`] round-trips a clean, minimal TOML file (no `field = ""` +/// noise) when coven-code owns the roster in standalone mode. +#[derive(Debug, Clone, Deserialize, Serialize)] pub struct CovenFamiliar { pub id: String, - #[serde(default)] + #[serde(default, skip_serializing_if = "Option::is_none")] pub display_name: Option, - #[serde(default)] + #[serde(default, skip_serializing_if = "Option::is_none")] pub emoji: Option, - #[serde(default)] + #[serde(default, skip_serializing_if = "Option::is_none")] pub role: Option, - #[serde(default)] + #[serde(default, skip_serializing_if = "Option::is_none")] pub description: Option, - #[serde(default)] + #[serde(default, skip_serializing_if = "Option::is_none")] pub pronouns: Option, /// Tool-access tier: `"full"`, `"read-only"`, or `"search-only"`. /// Absent → [`DEFAULT_FAMILIAR_ACCESS`] (`"read-only"`). - #[serde(default)] + #[serde(default, skip_serializing_if = "Option::is_none")] pub access: Option, + /// Optional model override for this familiar (e.g. `"claude-opus-4-8"`). + /// Absent → the familiar inherits the session's default model. This lets a + /// persona be pinned to a specific model without shadowing it with a + /// workspace agent `.md`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub model: Option, } impl CovenFamiliar { @@ -159,23 +169,355 @@ impl CovenFamiliar { } } -#[derive(Debug, Deserialize)] +#[derive(Debug, Deserialize, Serialize)] struct FamiliarsFile { #[serde(default)] familiar: Vec, } +/// A `~/.coven/familiars.toml` exists but could not be read or parsed. +/// +/// Carries the offending path and a human-readable message so the TUI can +/// surface it instead of the roster silently vanishing. +#[derive(Debug, Clone)] +pub struct FamiliarLoadError { + pub path: PathBuf, + pub message: String, +} + +impl std::fmt::Display for FamiliarLoadError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}: {}", self.path.display(), self.message) + } +} + +impl std::error::Error for FamiliarLoadError {} + +/// Load familiars, distinguishing "nothing to load" from "malformed file". +/// +/// - `Ok(None)` — no `~/.coven/` dir or no `familiars.toml` (normal standalone +/// operation; nothing to surface). +/// - `Ok(Some(vec))` — the file parsed. The vec may be empty if the file +/// declared no `[[familiar]]` entries, which is distinct from a missing file. +/// - `Err(_)` — the file exists but could not be read or parsed. Callers that +/// can show UI should surface this; a malformed roster otherwise disappears +/// with no signal to the user. +pub fn load_familiars_result() -> Result>, FamiliarLoadError> { + let Some(home) = coven_home() else { + return Ok(None); + }; + let path = home.join("familiars.toml"); + let raw = match std::fs::read_to_string(&path) { + Ok(raw) => raw, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(e) => { + return Err(FamiliarLoadError { + path, + message: e.to_string(), + }) + } + }; + match toml::from_str::(&raw) { + Ok(parsed) => Ok(Some(parsed.familiar)), + Err(e) => Err(FamiliarLoadError { + path, + message: e.to_string(), + }), + } +} + /// Load familiars from `~/.coven/familiars.toml`. /// Returns `None` if the daemon dir, the file, or the parse fails. +/// +/// This is the graceful-degradation entry point used by the security-critical +/// agent-merge paths: any failure collapses to `None` so a broken roster can +/// never widen the runtime agent map. UI surfaces that want to *report* a +/// malformed file should call [`load_familiars_result`] instead. pub fn load_familiars() -> Option> { - let path = coven_home()?.join("familiars.toml"); - let raw = std::fs::read_to_string(&path).ok()?; - let parsed: FamiliarsFile = toml::from_str(&raw).ok()?; - if parsed.familiar.is_empty() { - None + match load_familiars_result() { + Ok(Some(fams)) if !fams.is_empty() => Some(fams), + _ => None, + } +} + +/// Non-fatal warnings about a loaded roster, for surfacing in the UI. +/// +/// Covers the failure modes that otherwise happen silently: ids dropped for +/// using a reserved name, duplicate ids (only the first is used), and unknown +/// access tiers (which fail closed to [`DEFAULT_FAMILIAR_ACCESS`]). Returns an +/// empty vec for a clean roster. +pub fn familiar_roster_warnings(fams: &[CovenFamiliar]) -> Vec { + let mut warnings = Vec::new(); + let mut seen = std::collections::HashSet::new(); + for fam in fams { + let id = fam.id.trim().to_ascii_lowercase(); + if is_disallowed_familiar_name(&id) { + warnings.push(format!( + "familiar {:?} uses a reserved name and was skipped", + fam.id + )); + continue; + } + if !seen.insert(id) { + warnings.push(format!( + "duplicate familiar id {:?} — only the first is used", + fam.id + )); + } + if let Some(raw) = fam.access.as_deref() { + if canonicalize_access_tier(raw).is_none() { + warnings.push(format!( + "familiar {:?}: unknown access tier {:?} — using {:?}", + fam.id, raw, DEFAULT_FAMILIAR_ACCESS + )); + } + } + } + warnings +} + +// --------------------------------------------------------------------------- +// Writing the roster (standalone-only) +// --------------------------------------------------------------------------- + +/// Resolve the `~/.coven/` path even when the directory does not exist yet. +/// +/// Unlike [`coven_home`], this does not require the directory to be present — +/// it is the target for *writing* the roster in standalone mode. Respects +/// `COVEN_HOME`. +pub fn coven_home_path() -> Option { + if let Ok(override_path) = std::env::var("COVEN_HOME") { + if !override_path.is_empty() { + return Some(PathBuf::from(override_path)); + } + } + Some(dirs::home_dir()?.join(".coven")) +} + +/// Whether the Coven daemon appears to be running, detected by the presence of +/// its IPC socket at `~/.coven/coven.sock`. +/// +/// When the daemon is up it owns `familiars.toml`, so coven-code must treat the +/// roster as read-only. Standalone (no socket) is the only mode in which +/// coven-code writes the file itself. +pub fn daemon_socket_present() -> bool { + coven_home_path() + .map(|p| p.join("coven.sock").exists()) + .unwrap_or(false) +} + +/// Whether coven-code may write `~/.coven/familiars.toml` directly. +/// +/// True only in standalone mode (no daemon socket). When the daemon is running +/// the file is daemon-owned and writes must go through it instead. +pub fn can_write_familiars() -> bool { + !daemon_socket_present() +} + +/// Failure modes for the standalone roster-write API. +#[derive(Debug, Clone)] +pub enum FamiliarWriteError { + /// The Coven daemon is running and owns `familiars.toml`. + DaemonOwned, + /// `~/.coven/` could not be resolved (no home directory). + NoHome, + /// The existing roster file is malformed and must be fixed before writing. + ExistingUnreadable(String), + /// An id was empty or a reserved/disallowed name. + InvalidId(String), + /// A familiar with the target id already exists (create-only paths). + Duplicate(String), + /// The target id was not found (edit/remove/rename paths). + NotFound(String), + /// Serialization or filesystem I/O failed. + Io(String), +} + +impl std::fmt::Display for FamiliarWriteError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::DaemonOwned => write!( + f, + "the Coven daemon is running and owns ~/.coven/familiars.toml — \ + edit familiars through the daemon instead" + ), + Self::NoHome => write!(f, "could not resolve ~/.coven/ (no home directory)"), + Self::ExistingUnreadable(m) => { + write!(f, "existing familiars.toml is unreadable: {m}") + } + Self::InvalidId(id) => write!(f, "invalid familiar id {id:?}"), + Self::Duplicate(id) => write!(f, "a familiar with id {id:?} already exists"), + Self::NotFound(id) => write!(f, "no familiar with id {id:?}"), + Self::Io(m) => write!(f, "{m}"), + } + } +} + +impl std::error::Error for FamiliarWriteError {} + +/// Load the current roster for mutation. Missing file → empty vec; a malformed +/// file is a hard error so a write never silently discards existing entries. +fn load_roster_for_write() -> Result, FamiliarWriteError> { + match load_familiars_result() { + Ok(Some(fams)) => Ok(fams), + Ok(None) => Ok(Vec::new()), + Err(e) => Err(FamiliarWriteError::ExistingUnreadable(e.message)), + } +} + +/// Overwrite `~/.coven/familiars.toml` with `familiars` (standalone only). +/// +/// Refuses when the daemon is running ([`daemon_socket_present`]). Creates +/// `~/.coven/` if needed. This is the single write chokepoint the higher-level +/// upsert/remove/rename helpers funnel through. +pub fn save_familiars(familiars: &[CovenFamiliar]) -> Result<(), FamiliarWriteError> { + if daemon_socket_present() { + return Err(FamiliarWriteError::DaemonOwned); + } + let home = coven_home_path().ok_or(FamiliarWriteError::NoHome)?; + std::fs::create_dir_all(&home).map_err(|e| FamiliarWriteError::Io(e.to_string()))?; + let file = FamiliarsFile { + familiar: familiars.to_vec(), + }; + let toml = toml::to_string_pretty(&file) + .map_err(|e| FamiliarWriteError::Io(format!("serialize: {e}")))?; + std::fs::write(home.join("familiars.toml"), toml) + .map_err(|e| FamiliarWriteError::Io(e.to_string()))?; + Ok(()) +} + +/// Validate an id for a writable familiar: non-empty and not reserved. +fn validate_writable_id(id: &str) -> Result<(), FamiliarWriteError> { + let trimmed = id.trim(); + if trimmed.is_empty() { + return Err(FamiliarWriteError::InvalidId(id.to_string())); + } + if is_disallowed_familiar_name(trimmed) { + return Err(FamiliarWriteError::InvalidId(trimmed.to_string())); + } + Ok(()) +} + +/// Insert a familiar, or replace an existing one with the same (case-insensitive) +/// id. Standalone only. Returns whether an existing entry was replaced. +pub fn upsert_familiar(fam: CovenFamiliar) -> Result { + validate_writable_id(&fam.id)?; + let mut roster = load_roster_for_write()?; + let key = fam.id.trim().to_ascii_lowercase(); + let mut replaced = false; + if let Some(slot) = roster + .iter_mut() + .find(|f| f.id.trim().to_ascii_lowercase() == key) + { + *slot = fam; + replaced = true; } else { - Some(parsed.familiar) + roster.push(fam); } + save_familiars(&roster)?; + Ok(replaced) +} + +/// Create a new familiar, erroring if the id already exists. Standalone only. +pub fn create_familiar(fam: CovenFamiliar) -> Result<(), FamiliarWriteError> { + validate_writable_id(&fam.id)?; + let roster = load_roster_for_write()?; + let key = fam.id.trim().to_ascii_lowercase(); + if roster + .iter() + .any(|f| f.id.trim().to_ascii_lowercase() == key) + { + return Err(FamiliarWriteError::Duplicate(fam.id)); + } + let mut roster = roster; + roster.push(fam); + save_familiars(&roster) +} + +/// Remove a familiar by (case-insensitive) id. Standalone only. Returns the +/// removed entry. +pub fn remove_familiar(id: &str) -> Result { + let key = id.trim().to_ascii_lowercase(); + let mut roster = load_roster_for_write()?; + let Some(pos) = roster + .iter() + .position(|f| f.id.trim().to_ascii_lowercase() == key) + else { + return Err(FamiliarWriteError::NotFound(id.to_string())); + }; + let removed = roster.remove(pos); + save_familiars(&roster)?; + Ok(removed) +} + +/// Rename a familiar's id, preserving all other fields. Standalone only. +pub fn rename_familiar(old_id: &str, new_id: &str) -> Result<(), FamiliarWriteError> { + validate_writable_id(new_id)?; + let old_key = old_id.trim().to_ascii_lowercase(); + let new_key = new_id.trim().to_ascii_lowercase(); + let mut roster = load_roster_for_write()?; + if old_key != new_key + && roster + .iter() + .any(|f| f.id.trim().to_ascii_lowercase() == new_key) + { + return Err(FamiliarWriteError::Duplicate(new_id.to_string())); + } + let Some(slot) = roster + .iter_mut() + .find(|f| f.id.trim().to_ascii_lowercase() == old_key) + else { + return Err(FamiliarWriteError::NotFound(old_id.to_string())); + }; + slot.id = new_id.trim().to_string(); + save_familiars(&roster) +} + +/// A couple of example familiars written on first-run bootstrap so a brand-new +/// standalone user has a working roster to switch between and a concrete +/// template to edit. Access tiers are deliberately conservative. +pub fn starter_familiars() -> Vec { + vec![ + CovenFamiliar { + id: "sage".to_string(), + display_name: Some("Sage".to_string()), + emoji: Some("\u{1f989}".to_string()), // owl + role: Some("Guide".to_string()), + description: Some( + "A thoughtful pair-programmer who explains as they go.".to_string(), + ), + pronouns: Some("they/them".to_string()), + access: Some("read-only".to_string()), + model: None, + }, + CovenFamiliar { + id: "forge".to_string(), + display_name: Some("Forge".to_string()), + emoji: Some("\u{1f525}".to_string()), // fire + role: Some("Builder".to_string()), + description: Some("A hands-on familiar that writes and runs code.".to_string()), + pronouns: None, + access: Some("full".to_string()), + model: None, + }, + ] +} + +/// Write [`starter_familiars`] to `~/.coven/familiars.toml` for a first-run +/// standalone user. Refuses if the daemon owns the file or a roster already +/// exists (so it never clobbers real entries). Returns the written roster. +pub fn write_starter_roster() -> Result, FamiliarWriteError> { + if daemon_socket_present() { + return Err(FamiliarWriteError::DaemonOwned); + } + if !load_roster_for_write()?.is_empty() { + return Err(FamiliarWriteError::Duplicate( + "roster already exists".to_string(), + )); + } + let fams = starter_familiars(); + save_familiars(&fams)?; + Ok(fams) } /// Build a [`crate::config::AgentDefinition`] from a familiar so it can be @@ -208,7 +550,9 @@ pub fn familiar_to_agent_definition( let def = crate::config::AgentDefinition { description: Some(format!("{emoji} {role} — {desc_body}")), - model: None, + // An explicit per-familiar model pins the persona; otherwise `None` + // means "inherit the session default". + model: fam.model.clone().filter(|m| !m.trim().is_empty()), temperature: None, prompt: Some(prompt), access: fam.resolved_access().to_string(), @@ -436,6 +780,175 @@ role = "General Helper" assert!(load_familiars().is_none()); } + #[test] + fn load_familiars_result_distinguishes_absent_from_malformed() { + // Missing file → Ok(None), not an error. + let _g = with_coven_home(|_| {}); + assert!(matches!(load_familiars_result(), Ok(None))); + + // Malformed file → Err carrying the path + message. + let _g2 = with_coven_home(|home| { + fs::write(home.join("familiars.toml"), "this is = not [valid toml").unwrap(); + }); + let err = load_familiars_result().expect_err("malformed file should error"); + assert!(err.path.ends_with("familiars.toml")); + assert!(!err.message.is_empty()); + } + + #[test] + fn load_familiars_result_reports_empty_roster_as_some() { + // A file that parses but declares no familiars is distinct from a + // missing file: Ok(Some(empty)) vs Ok(None). + let _g = with_coven_home(|home| { + fs::write(home.join("familiars.toml"), "# no entries\n").unwrap(); + }); + match load_familiars_result() { + Ok(Some(v)) => assert!(v.is_empty()), + other => panic!("expected Ok(Some(empty)), got {other:?}"), + } + // The graceful wrapper still collapses empty to None. + assert!(load_familiars().is_none()); + } + + #[test] + fn familiar_roster_warnings_flags_unknown_tier_and_duplicates() { + let fams = vec![ + CovenFamiliar { + id: "willow".to_string(), + display_name: None, + emoji: None, + role: None, + description: None, + pronouns: None, + access: Some("readonly".to_string()), // typo → unknown tier + model: None, + }, + CovenFamiliar { + id: "Willow".to_string(), // duplicate id (case-insensitive) + display_name: None, + emoji: None, + role: None, + description: None, + pronouns: None, + access: Some("full".to_string()), + model: None, + }, + CovenFamiliar { + id: "val".to_string(), // reserved/disallowed name + display_name: None, + emoji: None, + role: None, + description: None, + pronouns: None, + access: None, + model: None, + }, + ]; + let warnings = familiar_roster_warnings(&fams); + assert!(warnings.iter().any(|w| w.contains("unknown access tier"))); + assert!(warnings.iter().any(|w| w.contains("duplicate familiar id"))); + assert!(warnings.iter().any(|w| w.contains("reserved name"))); + } + + #[test] + fn familiar_roster_warnings_empty_for_clean_roster() { + let fams = vec![CovenFamiliar { + id: "atlas".to_string(), + display_name: Some("Atlas".to_string()), + emoji: None, + role: None, + description: None, + pronouns: None, + access: Some("full".to_string()), + model: None, + }]; + assert!(familiar_roster_warnings(&fams).is_empty()); + } + + fn writable_familiar(id: &str) -> CovenFamiliar { + CovenFamiliar { + id: id.to_string(), + display_name: None, + emoji: None, + role: Some("Helper".to_string()), + description: None, + pronouns: None, + access: Some("read-only".to_string()), + model: None, + } + } + + #[test] + fn create_and_remove_familiar_round_trip() { + let _g = with_coven_home(|_| {}); + // No daemon socket in the temp home → writes are allowed. + assert!(can_write_familiars()); + + create_familiar(writable_familiar("nova")).expect("create"); + let roster = load_familiars().expect("roster after create"); + assert_eq!(roster.len(), 1); + assert_eq!(roster[0].id, "nova"); + + // Duplicate create is rejected. + let dup = create_familiar(writable_familiar("Nova")); + assert!(matches!(dup, Err(FamiliarWriteError::Duplicate(_)))); + + let removed = remove_familiar("nova").expect("remove"); + assert_eq!(removed.id, "nova"); + assert!(load_familiars().is_none(), "roster empty after remove"); + } + + #[test] + fn upsert_replaces_existing_and_rename_moves_id() { + let _g = with_coven_home(|_| {}); + create_familiar(writable_familiar("scout")).expect("create"); + + // Upsert with same id (different case) replaces rather than duplicates. + let mut updated = writable_familiar("Scout"); + updated.access = Some("full".to_string()); + let replaced = upsert_familiar(updated).expect("upsert"); + assert!(replaced); + let roster = load_familiars().expect("roster"); + assert_eq!(roster.len(), 1); + assert_eq!(roster[0].resolved_access(), "full"); + + rename_familiar("scout", "ranger").expect("rename"); + let roster = load_familiars().expect("roster"); + assert_eq!(roster[0].id, "ranger"); + } + + #[test] + fn write_rejects_reserved_and_missing_ids() { + let _g = with_coven_home(|_| {}); + // Reserved name is refused. + assert!(matches!( + create_familiar(writable_familiar("val")), + Err(FamiliarWriteError::InvalidId(_)) + )); + // Remove/rename of a missing id errors instead of silently succeeding. + assert!(matches!( + remove_familiar("ghost"), + Err(FamiliarWriteError::NotFound(_)) + )); + assert!(matches!( + rename_familiar("ghost", "wraith"), + Err(FamiliarWriteError::NotFound(_)) + )); + } + + #[test] + fn write_refused_when_daemon_socket_present() { + let _g = with_coven_home(|home| { + // Simulate a running daemon by creating its IPC socket path. + std::fs::write(home.join("coven.sock"), b"").unwrap(); + }); + assert!(!can_write_familiars()); + assert!(matches!( + create_familiar(writable_familiar("nova")), + Err(FamiliarWriteError::DaemonOwned) + )); + } + #[test] fn familiar_access_defaults_to_read_only_when_absent() { let _g = with_coven_home(|home| { @@ -493,10 +1006,13 @@ access = "search-only" description: Some("Builds and ships.".to_string()), pronouns: None, access: Some("full".to_string()), + model: Some("claude-opus-4-8".to_string()), }; let (id, def) = familiar_to_agent_definition(&fam); assert_eq!(id, "builder", "id should be lowercased for map keys"); assert_eq!(def.access, "full"); + // Explicit per-familiar model is threaded into the agent definition. + assert_eq!(def.model.as_deref(), Some("claude-opus-4-8")); assert!(def.visible); let prompt = def.prompt.as_deref().unwrap_or(""); assert!(prompt.contains("Builder")); @@ -513,9 +1029,12 @@ access = "search-only" description: None, pronouns: None, access: None, + model: None, }; let (_id, def) = familiar_to_agent_definition(&fam); assert_eq!(def.access, "read-only"); + // No model override → inherits session default. + assert_eq!(def.model, None); } #[test] @@ -844,6 +1363,7 @@ access = "read-only" description: None, pronouns: None, access: Some("READ-ONLY".into()), + model: None, }; assert_eq!(case_variant.resolved_access(), "read-only"); @@ -855,6 +1375,7 @@ access = "read-only" description: None, pronouns: None, access: Some("readonly".into()), + model: None, }; assert_eq!(typo.resolved_access(), DEFAULT_FAMILIAR_ACCESS); @@ -866,6 +1387,7 @@ access = "read-only" description: None, pronouns: None, access: Some("super-admin".into()), + model: None, }; assert_eq!(garbage.resolved_access(), DEFAULT_FAMILIAR_ACCESS); } diff --git a/src-rust/crates/core/src/keybindings.rs b/src-rust/crates/core/src/keybindings.rs index f0537339..aac0a45d 100644 --- a/src-rust/crates/core/src/keybindings.rs +++ b/src-rust/crates/core/src/keybindings.rs @@ -107,6 +107,69 @@ fn format_chord_string(chord: &Chord) -> String { .collect::>() .join(" ") } + +/// A pretty single-keystroke label for UI hints, e.g. `Ctrl+C`, `Enter`, +/// `Alt+H`, `↑`. +fn prettify_keystroke(ks: &ParsedKeystroke) -> String { + let mut parts: Vec = Vec::new(); + if ks.ctrl { + parts.push("Ctrl".to_string()); + } + if ks.alt { + parts.push("Alt".to_string()); + } + if ks.shift { + parts.push("Shift".to_string()); + } + if ks.meta { + parts.push("Meta".to_string()); + } + let key = match ks.key.as_str() { + "enter" => "Enter".to_string(), + "escape" => "Esc".to_string(), + "up" => "\u{2191}".to_string(), + "down" => "\u{2193}".to_string(), + "left" => "\u{2190}".to_string(), + "right" => "\u{2192}".to_string(), + "space" => "Space".to_string(), + "pageup" => "PgUp".to_string(), + "pagedown" => "PgDn".to_string(), + "tab" => "Tab".to_string(), + k if k.chars().count() == 1 => k.to_uppercase(), + k => { + let mut c = k.chars(); + c.next() + .map(|f| f.to_uppercase().collect::() + c.as_str()) + .unwrap_or_default() + } + }; + parts.push(key); + parts.join("+") +} + +/// The keystroke currently bound to `action`, as a display string — a user +/// override wins, otherwise the built-in default. `None` if nothing is bound. +/// +/// Matches on action name across contexts (action names like `submit` / +/// `openHelp` are unique enough for hint display), so first-run keybinding +/// hints stay in sync with a user's customized `keybindings.json`. +pub fn display_binding_for(user: &UserKeybindings, action: &str) -> Option { + // User override wins. + for b in &user.bindings { + if b.action.as_deref() == Some(action) { + let first = b.chord.split_whitespace().next().unwrap_or(&b.chord); + if let Some(ks) = parse_keystroke(first) { + return Some(prettify_keystroke(&ks)); + } + } + } + // Fall back to the built-in default. + default_bindings() + .iter() + .find(|pb| pb.action.as_deref() == Some(action)) + .and_then(|pb| pb.chord.first().map(prettify_keystroke)) +} + fn normalize_key(k: &str) -> String { match k { "esc" | "escape" => "escape".to_string(), diff --git a/src-rust/crates/tui/src/agents_view.rs b/src-rust/crates/tui/src/agents_view.rs index 0a5f0151..9ba06448 100644 --- a/src-rust/crates/tui/src/agents_view.rs +++ b/src-rust/crates/tui/src/agents_view.rs @@ -440,10 +440,37 @@ impl AgentsMenuState { .ok_or_else(|| "Selected agent is no longer available.".to_string())? .clone(); if def.source.starts_with("coven:familiar") { - return Err( - "Coven familiars are read-only in this menu. Remove them from ~/.coven/familiars.toml." - .to_string(), - ); + // In standalone mode coven-code owns the roster, so allow removal; + // when the daemon is running the file is daemon-owned and read-only. + if !claurst_core::coven_shared::can_write_familiars() { + return Err( + "Coven familiars are daemon-managed. Remove them via the daemon or from ~/.coven/familiars.toml." + .to_string(), + ); + } + let fam_id = def + .source + .strip_prefix("coven:familiar:") + .unwrap_or(&def.name) + .to_string(); + match claurst_core::coven_shared::remove_familiar(&fam_id) { + Ok(removed) => { + if let Some(root) = self.project_root.clone() { + self.definitions = load_agent_definitions(&root); + } else { + self.definitions.remove(idx); + } + self.route = AgentsRoute::List; + self.selected_row = if self.definitions.is_empty() { + 0 + } else { + (idx + 2).min(self.definitions.len() + 1) + }; + self.editor = AgentEditorState::new(); + return Ok(format!("Removed familiar {}.", removed.id)); + } + Err(e) => return Err(format!("Could not remove familiar: {e}")), + } } if def.source != "user" { return Err(format!( @@ -825,6 +852,7 @@ mod tests { description: Some("Builds and ships.".to_string()), pronouns: None, access: Some("full".to_string()), + model: None, }; let def = familiar_as_agent_def(&fam); let (id, core_def) = coven_shared::familiar_to_agent_definition(&fam); @@ -845,6 +873,7 @@ mod tests { description: None, pronouns: None, access: None, + model: None, }; let def = familiar_as_agent_def(&fam); assert_eq!(def.access.as_deref(), Some("read-only")); @@ -923,20 +952,52 @@ mod tests { } #[test] - fn delete_selected_definition_rejects_coven_familiar() { + fn delete_selected_definition_removes_familiar_in_standalone() { + let temp = tempfile::tempdir().expect("tempdir"); + let home = temp.path().join("home"); + let coven_home = temp.path().join("coven"); + std::fs::create_dir_all(&home).expect("home"); + std::fs::create_dir_all(&coven_home).expect("coven home"); + std::fs::write( + coven_home.join("familiars.toml"), + "[[familiar]]\nid = \"onyx\"\n", + ) + .expect("roster"); + let _guard = crate::app::test_env::EnvGuard::set(&home, &coven_home); + let mut state = AgentsMenuState::new(); state.definitions = vec![familiar_def("onyx", "Onyx")]; state.route = AgentsRoute::List; state.selected_row = 2; - let err = state + // Standalone (no daemon socket) → coven-code owns the roster and removes. + let msg = state .delete_selected_definition() - .expect_err("daemon familiar should not be removed from this menu"); + .expect("standalone familiar removal"); + assert_eq!(msg, "Removed familiar onyx."); + assert!(claurst_core::coven_shared::load_familiars().is_none()); + } - assert_eq!( - err, - "Coven familiars are read-only in this menu. Remove them from ~/.coven/familiars.toml." - ); + #[test] + fn delete_selected_definition_rejects_familiar_when_daemon_present() { + let temp = tempfile::tempdir().expect("tempdir"); + let home = temp.path().join("home"); + let coven_home = temp.path().join("coven"); + std::fs::create_dir_all(&home).expect("home"); + std::fs::create_dir_all(&coven_home).expect("coven home"); + // Simulate a running daemon: the roster is daemon-owned and read-only. + std::fs::write(coven_home.join("coven.sock"), b"").expect("socket"); + let _guard = crate::app::test_env::EnvGuard::set(&home, &coven_home); + + let mut state = AgentsMenuState::new(); + state.definitions = vec![familiar_def("onyx", "Onyx")]; + state.route = AgentsRoute::List; + state.selected_row = 2; + + let err = state + .delete_selected_definition() + .expect_err("daemon-owned familiar should not be removed from this menu"); + assert!(err.contains("daemon-managed"), "unexpected message: {err}"); } } diff --git a/src-rust/crates/tui/src/app.rs b/src-rust/crates/tui/src/app.rs index 70ca65e2..69a5d367 100644 --- a/src-rust/crates/tui/src/app.rs +++ b/src-rust/crates/tui/src/app.rs @@ -1112,6 +1112,19 @@ pub struct App { pub familiar_switcher_open: bool, pub familiar_switcher_list: Vec, pub familiar_switcher_idx: usize, + /// Incremental filter typed while the switcher is open. Empty = show all. + pub familiar_switcher_filter: String, +} + +/// One selectable row in the F2 familiar switcher. +/// +/// The synthetic [`FamiliarSwitcherEntry::Clear`] row always leads the list so +/// the user can step back to no active familiar without reaching for +/// `/familiar reset`; the rest map to roster ids. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum FamiliarSwitcherEntry { + Clear, + Familiar(String), } // Spinner verbs are now imported from claurst_core::spinner @@ -1285,6 +1298,7 @@ impl App { pub fn new(config: Config, cost_tracker: Arc) -> Self { // Auto-detect familiar from system username when none is configured. let mut config = config; + let familiar_explicit = config.familiar.is_some(); if config.familiar.is_none() { if let Some(inferred) = Self::infer_familiar_from_env() { config.familiar = Some(inferred); @@ -1292,7 +1306,7 @@ impl App { } let model_name = config.effective_model().to_string(); let user_keybindings = UserKeybindings::load(&Settings::config_dir()); - Self { + let mut app = Self { config, cost_tracker, messages: Vec::new(), @@ -1505,6 +1519,66 @@ impl App { familiar_switcher_open: false, familiar_switcher_list: Self::default_familiar_switcher_list(), familiar_switcher_idx: 0, + familiar_switcher_filter: String::new(), + }; + app.surface_familiar_roster_warnings(familiar_explicit); + app + } + + /// Surface a one-time status message when `~/.coven/familiars.toml` exists + /// but is malformed, or when the loaded roster has non-fatal problems + /// (unknown access tiers, or duplicate and reserved ids). Without this, a + /// broken roster silently vanishes with no signal to the user. + /// + /// `familiar_explicit` is true only when the active familiar came from + /// settings rather than username inference — stale-selection warnings fire + /// only in that case so an inferred default never nags a standalone user. + fn surface_familiar_roster_warnings(&mut self, familiar_explicit: bool) { + match claurst_core::coven_shared::load_familiars_result() { + Err(err) => { + self.status_message = Some(format!( + "familiars.toml could not be parsed ({}) — roster unavailable.", + err.message + )); + } + Ok(Some(fams)) if !fams.is_empty() => { + let warnings = claurst_core::coven_shared::familiar_roster_warnings(&fams); + if let Some(first) = warnings.first() { + let extra = warnings.len().saturating_sub(1); + self.status_message = Some(if extra > 0 { + format!("Familiar roster: {first} (+{extra} more)") + } else { + format!("Familiar roster: {first}") + }); + return; + } + // Roster is clean — warn only if an explicitly-selected familiar + // is missing from it. + if familiar_explicit { + if let Some(active) = self.config.familiar.as_deref() { + let active_lc = active.trim().to_ascii_lowercase(); + let present = fams + .iter() + .any(|f| f.id.trim().to_ascii_lowercase() == active_lc); + if !present { + self.status_message = Some(format!( + "Selected familiar '{active}' is not in the roster — using none.", + )); + } + } + } + } + // No roster file, or an empty one, while a familiar was explicitly + // configured: flag the stale selection so it isn't silently ignored. + _ => { + if familiar_explicit { + if let Some(active) = self.config.familiar.as_deref() { + self.status_message = Some(format!( + "Selected familiar '{active}' has no roster (~/.coven/familiars.toml) — using none.", + )); + } + } + } } } @@ -1520,6 +1594,66 @@ impl App { ids } + /// The switcher rows after applying the current filter. Leads with the + /// synthetic [`FamiliarSwitcherEntry::Clear`] row (unless the filter + /// excludes it) so the user can return to no active familiar, followed by + /// roster ids whose id contains the (case-insensitive) filter. + pub fn familiar_switcher_entries(&self) -> Vec { + let filter = self.familiar_switcher_filter.trim().to_ascii_lowercase(); + let mut entries = Vec::new(); + // The clear row shows for an empty filter or when the user is plainly + // typing toward "none" / "clear". + if filter.is_empty() || "none".contains(&filter) || "clear".contains(&filter) { + entries.push(FamiliarSwitcherEntry::Clear); + } + for id in &self.familiar_switcher_list { + if filter.is_empty() || id.to_ascii_lowercase().contains(&filter) { + entries.push(FamiliarSwitcherEntry::Familiar(id.clone())); + } + } + entries + } + + /// Close the F2 switcher and reset its transient filter/selection state. + fn close_familiar_switcher(&mut self) { + self.familiar_switcher_open = false; + self.familiar_switcher_filter.clear(); + self.familiar_switcher_idx = 0; + } + + /// First-run action when F2 is pressed with no familiars: in standalone + /// mode, write a starter `~/.coven/familiars.toml` and open the switcher so + /// the user has something to pick and a template to edit. When the Coven + /// daemon owns the roster, point the user at it instead of writing. + fn bootstrap_or_prompt_familiar_roster(&mut self) { + use claurst_core::coven_shared; + if !coven_shared::can_write_familiars() { + self.status_message = Some( + "Familiars are managed by the Coven daemon — add them there, then press F2." + .to_string(), + ); + return; + } + match coven_shared::write_starter_roster() { + Ok(fams) => { + self.familiar_switcher_list = fams.iter().map(|f| f.id.clone()).collect(); + self.familiar_switcher_open = true; + self.familiar_switcher_filter.clear(); + self.familiar_switcher_idx = 0; + self.push_notification( + crate::notifications::NotificationKind::Info, + "Created a starter roster at ~/.coven/familiars.toml — pick one or edit the file." + .to_string(), + Some(6), + ); + } + Err(err) => { + self.status_message = + Some(format!("Could not create a starter roster: {err}")); + } + } + } + /// Load token budget from environment or model defaults. /// Returns Some(max_tokens) if available, None otherwise. /// Only enabled when the `token_budget` feature flag is active. @@ -3147,40 +3281,74 @@ impl App { // ---- Familiar switcher (F2) ---------------------------------------- if self.familiar_switcher_open { + // Navigation runs over the filtered entry list (which leads with a + // synthetic "clear" row); typing narrows the roster incrementally. + let entries = self.familiar_switcher_entries(); + let len = entries.len(); + let ctrl = key.modifiers.contains(KeyModifiers::CONTROL); + let alt = key.modifiers.contains(KeyModifiers::ALT); match key.code { KeyCode::Esc | KeyCode::F(2) => { - self.familiar_switcher_open = false; + self.close_familiar_switcher(); + } + KeyCode::Down => { + if len > 0 { + self.familiar_switcher_idx = (self.familiar_switcher_idx + 1) % len; + } + } + KeyCode::Up => { + if len > 0 { + self.familiar_switcher_idx = (self.familiar_switcher_idx + len - 1) % len; + } } - KeyCode::Char('j') | KeyCode::Down => { - let len = self.familiar_switcher_list.len(); + KeyCode::Char('n') if ctrl => { if len > 0 { self.familiar_switcher_idx = (self.familiar_switcher_idx + 1) % len; } } - KeyCode::Char('k') | KeyCode::Up => { - let len = self.familiar_switcher_list.len(); + KeyCode::Char('p') if ctrl => { if len > 0 { self.familiar_switcher_idx = (self.familiar_switcher_idx + len - 1) % len; } } + KeyCode::Backspace => { + self.familiar_switcher_filter.pop(); + self.familiar_switcher_idx = 0; + } KeyCode::Enter => { - if let Some(id) = self - .familiar_switcher_list - .get(self.familiar_switcher_idx) - .cloned() - { - // Full activation, same as the menu and /familiar: - // mascot + persisted setting + agent mode/tool tier. - self.config.familiar = Some(id.clone()); - self.persist_familiar_setting(Some(id.clone())); - self.apply_familiar_agent_mode(Some(id.clone()), None); - self.push_notification( - crate::notifications::NotificationKind::Info, - format!("\u{2728} Familiar: {}", id), - None, - ); + match entries.get(self.familiar_switcher_idx) { + Some(FamiliarSwitcherEntry::Clear) => { + // Step back to no active familiar (same effect as + // `/familiar reset`) without leaving the switcher. + self.config.familiar = None; + self.persist_familiar_setting(None); + self.apply_familiar_agent_mode(None, None); + self.push_notification( + crate::notifications::NotificationKind::Info, + "Familiar cleared.".to_string(), + None, + ); + } + Some(FamiliarSwitcherEntry::Familiar(id)) => { + // Full activation, same as the menu and /familiar: + // mascot + persisted setting + agent mode/tool tier. + let id = id.clone(); + self.config.familiar = Some(id.clone()); + self.persist_familiar_setting(Some(id.clone())); + self.apply_familiar_agent_mode(Some(id.clone()), None); + self.push_notification( + crate::notifications::NotificationKind::Info, + format!("\u{2728} Familiar: {}", id), + None, + ); + } + None => {} } - self.familiar_switcher_open = false; + self.close_familiar_switcher(); + } + KeyCode::Char(c) if !ctrl && !alt => { + self.familiar_switcher_filter.push(c); + self.familiar_switcher_idx = 0; } _ => {} } @@ -3274,7 +3442,12 @@ impl App { if self.onboarding_dialog.is_provider_setup() { match key.code { KeyCode::Esc => { + // Esc = "skip for now". Persist completion so a brand-new + // user who dismisses isn't re-nagged with the same dialog + // every launch; the /connect hint in the welcome box keeps + // the affordance visible. self.onboarding_dialog.dismiss(); + let _ = Self::persist_onboarding_complete(); self.status_message = Some("Run /connect when you're ready to add a provider.".to_string()); } @@ -3289,7 +3462,10 @@ impl App { } match key.code { KeyCode::Esc => { + // Esc dismisses from any page and counts as completing + // onboarding, so the dialog doesn't reappear next launch. self.onboarding_dialog.dismiss(); + let _ = Self::persist_onboarding_complete(); } KeyCode::Enter | KeyCode::Right if self.onboarding_dialog.next_page() => { self.onboarding_dialog.dismiss(); @@ -4340,22 +4516,24 @@ impl App { } KeyCode::F(2) => { if self.familiar_switcher_open { - self.familiar_switcher_open = false; + self.close_familiar_switcher(); } else if self.familiar_switcher_list.is_empty() { - self.status_message = - Some("No saved familiars. Add ~/.coven/familiars.toml first.".to_string()); + self.bootstrap_or_prompt_familiar_roster(); } else { self.familiar_switcher_open = true; + self.familiar_switcher_filter.clear(); // Highlight the active familiar if one is set; otherwise - // start at the top. No built-in default is assumed. + // start at the top. Entries lead with the "clear" row, so + // account for that offset when locating the active id. + let entries = self.familiar_switcher_entries(); self.familiar_switcher_idx = self .config .familiar .as_deref() .and_then(|current| { - self.familiar_switcher_list - .iter() - .position(|id| id == current) + entries.iter().position(|e| { + matches!(e, FamiliarSwitcherEntry::Familiar(id) if id == current) + }) }) .unwrap_or(0); } @@ -4790,9 +4968,8 @@ impl App { self.config.permission_mode = claurst_core::config::PermissionMode::Default; } self.managed_agents_active = false; - self.familiar_switcher_open = false; + self.close_familiar_switcher(); self.familiar_switcher_list.clear(); - self.familiar_switcher_idx = 0; self.status_message = Some(summary.message()); } Err(err) => { @@ -7006,6 +7183,36 @@ mod tests { drop(guard); } + #[test] + fn familiar_switcher_entries_lead_with_clear_and_filter() { + let temp = tempfile::tempdir().expect("tempdir"); + let home = temp.path().join("home"); + let coven_home = temp.path().join("coven"); + std::fs::create_dir_all(&home).expect("home dir"); + std::fs::create_dir_all(&coven_home).expect("coven home dir"); + let _guard = EnvGuard::set(&home, &coven_home); + + let mut app = make_app(); + app.familiar_switcher_list = vec!["ember".to_string(), "onyx".to_string()]; + + // No filter → Clear leads, then all familiars in order. + app.familiar_switcher_filter.clear(); + let entries = app.familiar_switcher_entries(); + assert_eq!(entries[0], FamiliarSwitcherEntry::Clear); + assert_eq!(entries[1], FamiliarSwitcherEntry::Familiar("ember".to_string())); + assert_eq!(entries[2], FamiliarSwitcherEntry::Familiar("onyx".to_string())); + + // Filter narrows to matching ids and drops the Clear row. + app.familiar_switcher_filter = "ony".to_string(); + let entries = app.familiar_switcher_entries(); + assert_eq!(entries, vec![FamiliarSwitcherEntry::Familiar("onyx".to_string())]); + + // Typing toward "none" keeps the Clear row available. + app.familiar_switcher_filter = "non".to_string(); + let entries = app.familiar_switcher_entries(); + assert_eq!(entries, vec![FamiliarSwitcherEntry::Clear]); + } + #[test] fn startup_familiar_switcher_uses_saved_roster_only() { let temp = tempfile::tempdir().expect("tempdir"); @@ -7060,7 +7267,7 @@ role = "Research" } #[test] - fn f2_does_not_open_familiar_switcher_without_saved_roster() { + fn f2_bootstraps_starter_roster_when_standalone_and_empty() { let temp = tempfile::tempdir().expect("tempdir"); let home = temp.path().join("home"); let coven_home = temp.path().join("coven"); @@ -7069,13 +7276,38 @@ role = "Research" let _guard = EnvGuard::set(&home, &coven_home); let mut app = make_app(); + // No daemon socket → standalone → F2 writes a starter roster and opens. app.handle_key_event(press_key(KeyCode::F(2), KeyModifiers::NONE)); - assert!(!app.familiar_switcher_open); + assert!(app.familiar_switcher_open); assert_eq!( - app.status_message.as_deref(), - Some("No saved familiars. Add ~/.coven/familiars.toml first.") + app.familiar_switcher_list, + vec!["sage".to_string(), "forge".to_string()] ); + assert!(coven_home.join("familiars.toml").exists()); + } + + #[test] + fn f2_does_not_write_roster_when_daemon_present() { + let temp = tempfile::tempdir().expect("tempdir"); + let home = temp.path().join("home"); + let coven_home = temp.path().join("coven"); + std::fs::create_dir_all(&home).expect("home dir"); + std::fs::create_dir_all(&coven_home).expect("coven home dir"); + // Simulate a running daemon by creating its IPC socket. + std::fs::write(coven_home.join("coven.sock"), b"").expect("socket"); + let _guard = EnvGuard::set(&home, &coven_home); + + let mut app = make_app(); + app.handle_key_event(press_key(KeyCode::F(2), KeyModifiers::NONE)); + + assert!(!app.familiar_switcher_open); + assert!(!coven_home.join("familiars.toml").exists()); + assert!(app + .status_message + .as_deref() + .unwrap_or_default() + .contains("Coven daemon")); } #[test] diff --git a/src-rust/crates/tui/src/familiar_theme.rs b/src-rust/crates/tui/src/familiar_theme.rs index a8b39a6a..437ae074 100644 --- a/src-rust/crates/tui/src/familiar_theme.rs +++ b/src-rust/crates/tui/src/familiar_theme.rs @@ -134,6 +134,7 @@ pub fn resolve(id: &str, daemon_familiars: &[CovenFamiliar]) -> FamiliarTheme { description: None, pronouns: None, access: None, + model: None, }) } @@ -177,6 +178,7 @@ mod tests { description: None, pronouns: None, access: None, + model: None, } } diff --git a/src-rust/crates/tui/src/onboarding_dialog.rs b/src-rust/crates/tui/src/onboarding_dialog.rs index 3a0ec47b..fdd4808a 100644 --- a/src-rust/crates/tui/src/onboarding_dialog.rs +++ b/src-rust/crates/tui/src/onboarding_dialog.rs @@ -386,6 +386,18 @@ fn render_keybindings_page(frame: &mut Frame, state: &OnboardingDialogState, are ]) }; + // The input-editing actions are user-customizable via keybindings.json, so + // reflect the real binding rather than a hardcoded label that can drift. + // Fixed keys (Tab mode cycle, F1/F2, permission y/Y/n) live outside that + // system and stay static. + let user_kb = claurst_core::keybindings::UserKeybindings::load( + &claurst_core::config::Settings::config_dir(), + ); + let submit_key = claurst_core::keybindings::display_binding_for(&user_kb, "submit") + .unwrap_or_else(|| "Enter".to_string()); + let help_key = claurst_core::keybindings::display_binding_for(&user_kb, "openHelp") + .unwrap_or_else(|| "Alt+H".to_string()); + let (page_n, page_total) = state.page_progress(); let mut lines: Vec> = vec![ Line::from(vec![ @@ -409,7 +421,7 @@ fn render_keybindings_page(frame: &mut Frame, state: &OnboardingDialogState, are " Input", Style::default().fg(pink).add_modifier(Modifier::BOLD), )), - kb("Enter", "send message"), + kb(&submit_key, "send message"), kb("Shift+Enter", "newline"), kb("Ctrl+C", "interrupt / cancel"), kb("Tab", "cycle mode (build/plan/explore)"), @@ -424,7 +436,7 @@ fn render_keybindings_page(frame: &mut Frame, state: &OnboardingDialogState, are kb("Ctrl+Shift+A", "model picker"), kb("F1", "toggle help overlay"), kb("F2", "switch familiar"), - kb("Alt+H", "open help"), + kb(&help_key, "open help"), kb("Ctrl+B", "create / switch branch"), Line::from(""), Line::from(Span::styled( diff --git a/src-rust/crates/tui/src/render.rs b/src-rust/crates/tui/src/render.rs index 45c85d00..f920360f 100644 --- a/src-rust/crates/tui/src/render.rs +++ b/src-rust/crates/tui/src/render.rs @@ -1873,12 +1873,34 @@ fn render_welcome_box(frame: &mut Frame, app: &App, area: Rect) { .unwrap_or_else(|| "Edit AGENTS.md to add instructions for Coven Code".to_string()); let mut right_lines: Vec = Vec::new(); + // When unauthenticated, lead the right column with a prominent /connect + // call-to-action so a brand-new user has an obvious next step. The tips and + // what's-new sections still render below (kept for the startup chrome). + if !app.has_credentials { + let pink = Color::Rgb(139, 92, 246); + right_lines.push(Line::from(vec![ + Span::styled( + " /connect ", + Style::default() + .fg(readable_fg_on(pink)) + .bg(pink) + .add_modifier(Modifier::BOLD), + ), + Span::styled( + " connect a provider to begin", + Style::default().fg(pink).add_modifier(Modifier::BOLD), + ), + ])); + } right_lines.push(Line::from(Span::styled( "Tips for getting started", Style::default().fg(accent).add_modifier(Modifier::BOLD), ))); - // Word-wrap the tip on word boundaries so it stays readable when narrow. - for line in crate::dialogs::word_wrap(&tip_text, right_w_usize) { + // Word-wrap the tip on word boundaries; when the CTA is also shown, keep it + // to a single line so the fixed-height box doesn't clip What's new. + let tip_lines = crate::dialogs::word_wrap(&tip_text, right_w_usize); + let tip_take = if app.has_credentials { tip_lines.len() } else { 1 }; + for line in tip_lines.into_iter().take(tip_take) { right_lines.push(Line::from(Span::styled( line, Style::default().fg(Color::Gray), @@ -1893,7 +1915,10 @@ fn render_welcome_box(frame: &mut Frame, app: &App, area: Rect) { "What's new", Style::default().fg(accent).add_modifier(Modifier::BOLD), ))); - for item in claurst_core::constants::WHATS_NEW.iter().take(3) { + // Trim the changelog by one when the CTA line is present so the section + // stays inside the fixed box height. + let whats_new_take = if app.has_credentials { 3 } else { 2 }; + for item in claurst_core::constants::WHATS_NEW.iter().take(whats_new_take) { right_lines.push(Line::from(Span::styled( truncate_meta(item, right_w_usize), Style::default().fg(Color::Gray), @@ -3331,13 +3356,17 @@ pub fn render_teammate_header(teammate_id: &str, session_info: Option<&str>) -> // --------------------------------------------------------------------------- fn render_familiar_switcher(frame: &mut Frame, app: &App, area: Rect) { + use crate::app::FamiliarSwitcherEntry; use ratatui::widgets::{Block, Borders, Clear, List, ListItem, ListState}; - let list_len = app.familiar_switcher_list.len() as u16; + let entries = app.familiar_switcher_entries(); + // +2 for borders, +1 for the filter/hint footer line inside the block. + let list_len = entries.len() as u16; let popup_h = list_len - .saturating_add(2) - .min(area.height.saturating_sub(4)); - let popup_w = 40u16.min(area.width.saturating_sub(4)); + .saturating_add(3) + .min(area.height.saturating_sub(4)) + .max(4); + let popup_w = 44u16.min(area.width.saturating_sub(4)); let popup_x = area.x + area.width.saturating_sub(popup_w) / 2; let popup_y = area.y + area.height.saturating_sub(popup_h) / 2; let popup_area = Rect { @@ -3349,34 +3378,74 @@ fn render_familiar_switcher(frame: &mut Frame, app: &App, area: Rect) { frame.render_widget(Clear, popup_area); + let pink = Color::Rgb(139, 92, 246); let daemon_familiars = claurst_core::coven_shared::load_familiars().unwrap_or_default(); let interior_w = popup_w.saturating_sub(2); - let items: Vec = app - .familiar_switcher_list + // Footer shows the active filter (or a hint when empty) so incremental + // typing is discoverable. + let footer = if app.familiar_switcher_filter.is_empty() { + Line::from(Span::styled( + " type to filter · ↑↓ move · ⏎ select ", + Style::default().fg(Color::Rgb(120, 120, 130)), + )) + } else { + Line::from(vec![ + Span::styled(" filter: ", Style::default().fg(Color::Rgb(120, 120, 130))), + Span::styled( + app.familiar_switcher_filter.clone(), + Style::default().fg(Color::White).add_modifier(Modifier::BOLD), + ), + ]) + }; + + let items: Vec = entries .iter() .enumerate() - .map(|(i, id)| { - let theme = familiar_theme::resolve(id, &daemon_familiars); - let row = familiar_card::render_mini_row(&theme, interior_w); - let item = ListItem::new(row); - if i == app.familiar_switcher_idx { - item.style( - Style::default() - .bg(theme.palette.primary) - .fg(Color::Black) - .add_modifier(Modifier::BOLD), - ) - } else { - item + .map(|(i, entry)| { + let selected = i == app.familiar_switcher_idx; + match entry { + FamiliarSwitcherEntry::Clear => { + let line = Line::from(Span::styled( + " ✕ none (clear active familiar)", + Style::default().fg(Color::Rgb(170, 170, 180)), + )); + let item = ListItem::new(line); + if selected { + item.style( + Style::default() + .bg(Color::Rgb(70, 70, 80)) + .fg(Color::White) + .add_modifier(Modifier::BOLD), + ) + } else { + item + } + } + FamiliarSwitcherEntry::Familiar(id) => { + let theme = familiar_theme::resolve(id, &daemon_familiars); + let row = familiar_card::render_mini_row(&theme, interior_w); + let item = ListItem::new(row); + if selected { + item.style( + Style::default() + .bg(theme.palette.primary) + .fg(Color::Black) + .add_modifier(Modifier::BOLD), + ) + } else { + item + } + } } }) .collect(); let block = Block::default() .title(" \u{2728} Familiar (F2) ") + .title_bottom(footer) .borders(Borders::ALL) - .border_style(Style::default().fg(Color::Rgb(139, 92, 246))); + .border_style(Style::default().fg(pink)); let list = List::new(items).block(block); let mut state = ListState::default();