diff --git a/crates/rhema-claw/Cargo.toml b/crates/rhema-claw/Cargo.toml index ee3c5fd..002520f 100644 --- a/crates/rhema-claw/Cargo.toml +++ b/crates/rhema-claw/Cargo.toml @@ -86,3 +86,6 @@ bytes = "1" # feature without a feature of our own — tests are the only consumer rhema-core = { path = "../rhema-core", features = ["mock"] } tokio = { workspace = true, features = ["rt-multi-thread", "macros", "sync", "time"] } + +[lints] +workspace = true diff --git a/crates/rhema-claw/src/accounts_tools.rs b/crates/rhema-claw/src/accounts_tools.rs index f8c5e4e..7305f7f 100644 --- a/crates/rhema-claw/src/accounts_tools.rs +++ b/crates/rhema-claw/src/accounts_tools.rs @@ -29,7 +29,7 @@ use std::process::Stdio; use std::sync::Arc; use std::time::Duration; -use rhema_autonomous_agent::{ToolOp, ToolRegistry, async_trait}; +use rhema_autonomous_agent::{ToolError, ToolOp, ToolRegistry, async_trait}; use rhema_core::{ToolCall, ToolContext, ToolResolver, ToolResult, ToolSpec}; use rhema_identity::{Account, AccountDraft, IdentityStore, Secret, SecretKind}; use serde::Deserialize; @@ -166,7 +166,7 @@ pub fn forget_account( #[async_trait] impl ToolOp for IdentitySave { - async fn invoke(&self, args: Value) -> Result { + async fn invoke(&self, args: Value) -> Result { let args: SaveArgs = serde_json::from_value(args).map_err(|err| format!("bad arguments: {err}"))?; let kind = parse_kind(args.secret_kind.as_deref())?; @@ -204,7 +204,7 @@ struct IdentityList { #[async_trait] impl ToolOp for IdentityList { - async fn invoke(&self, _args: Value) -> Result { + async fn invoke(&self, _args: Value) -> Result { let accounts = self.accounts.list().map_err(|err| err.to_string())?; Ok(json!({ "accounts": accounts }).to_string()) } @@ -225,7 +225,7 @@ struct ForgetArgs { #[async_trait] impl ToolOp for IdentityForget { - async fn invoke(&self, args: Value) -> Result { + async fn invoke(&self, args: Value) -> Result { let args: ForgetArgs = serde_json::from_value(args).map_err(|err| format!("bad arguments: {err}"))?; let (forgot, watches_removed) = @@ -593,7 +593,7 @@ mod tests { ) } - fn call(tool: &dyn ToolOp, args: Value) -> Result { + fn call(tool: &dyn ToolOp, args: Value) -> Result { futures_block_on(tool.invoke(args)) } @@ -745,7 +745,7 @@ mod tests { }), ) .expect_err("rejected"); - assert!(err.contains("unknown secret_kind"), "{err}"); + assert!(err.message().contains("unknown secret_kind"), "{err}"); } /// The use half of the accounts: the command runs with the diff --git a/crates/rhema-claw/src/attachments.rs b/crates/rhema-claw/src/attachments.rs index a610c21..3fd8560 100644 --- a/crates/rhema-claw/src/attachments.rs +++ b/crates/rhema-claw/src/attachments.rs @@ -17,7 +17,7 @@ use std::sync::Arc; use std::sync::atomic::{AtomicU64, Ordering}; use std::time::{SystemTime, UNIX_EPOCH}; -use rhema_autonomous_agent::{ToolOp, ToolRegistry, async_trait}; +use rhema_autonomous_agent::{ToolError, ToolOp, ToolRegistry, async_trait}; use serde::Deserialize; use serde_json::{Value, json}; @@ -214,7 +214,7 @@ struct AttachArgs { #[async_trait] impl ToolOp for AttachFile { - async fn invoke(&self, args: Value) -> Result { + async fn invoke(&self, args: Value) -> Result { let args: AttachArgs = serde_json::from_value(args).map_err(|err| format!("bad arguments: {err}"))?; let relative = Path::new(&args.path); @@ -226,7 +226,8 @@ impl ToolOp for AttachFile { return Err(format!( "path must be relative to the workspace, without '..': {:?}", args.path - )); + ) + .into()); } let path = self.workspace.join(relative); let metadata = std::fs::metadata(&path) @@ -237,7 +238,8 @@ impl ToolOp for AttachFile { args.path, metadata.len(), MAX_ATTACHMENT_BYTES - )); + ) + .into()); } let bytes = std::fs::read(&path).map_err(|err| format!("cannot attach {:?}: {err}", args.path))?; diff --git a/crates/rhema-claw/src/backends.rs b/crates/rhema-claw/src/backends.rs index 648c56b..0b51c07 100644 --- a/crates/rhema-claw/src/backends.rs +++ b/crates/rhema-claw/src/backends.rs @@ -38,6 +38,31 @@ use rhema_provider_openrouter::OpenRouterProvider; use rhema_provider::ModelCard; use serde::{Deserialize, Serialize}; +/// Why a registry operation or a backend construction failed: an id or +/// kind that names nothing, a change that would leave the registry +/// empty, or a keyed provider constructed without its credential. +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +pub enum BackendError { + /// The id names no backend the registry holds. + #[error("unknown provider backend {0:?}")] + UnknownBackend(String), + /// Removing it would empty the registry, and handlers never face an + /// absent backend. + #[error("cannot remove the last provider backend")] + LastBackend, + /// No provider was constructible at boot: no credential resolved + /// for any keyed kind. + #[error("no provider backend is constructible: no key resolved for any provider")] + EmptyRegistry, + /// The kind names no adapter (`mock` is registry-injected, never + /// constructed). + #[error("unknown provider kind {0:?}")] + UnknownKind(String), + /// The kind is keyed and the params carry no usable `api_key`. + #[error("{0} needs an api_key")] + MissingKey(String), +} + /// The default model per backend kind, when neither the operator nor the /// settings store names one. pub fn default_model_for(kind: &str) -> &'static str { @@ -128,11 +153,9 @@ impl BackendRegistry { pub fn new( backends: Vec, seed: Option<(String, Option)>, - ) -> Result { + ) -> Result { if backends.is_empty() { - return Err( - "no provider backend is constructible: no key resolved for any provider".to_owned(), - ); + return Err(BackendError::EmptyRegistry); } let map: BTreeMap> = backends .into_iter() @@ -179,14 +202,14 @@ impl BackendRegistry { /// When the removed instance was active, the first remaining /// backend (ascending) takes over with its default model — the /// caller re-persists the pair it reads back from [`Self::active`]. - pub fn remove(&self, id: &str) -> Result<(), String> { + pub fn remove(&self, id: &str) -> Result<(), BackendError> { let mut backends = self.backends_lock(); if backends.len() == 1 && backends.contains_key(id) { - return Err("cannot remove the last provider backend".to_owned()); + return Err(BackendError::LastBackend); } let removed_was_active = self.active_lock().id == id; if backends.remove(id).is_none() { - return Err(format!("unknown provider backend {id:?}")); + return Err(BackendError::UnknownBackend(id.to_owned())); } if removed_was_active { let first = backends.values().next().expect("a backend remains"); @@ -213,12 +236,12 @@ impl BackendRegistry { /// Switches the active pair. An unknown instance id is an error; a /// model left unset falls back to that instance's default. - pub fn set_active(&self, id: &str, model: Option<&str>) -> Result<(), String> { + pub fn set_active(&self, id: &str, model: Option<&str>) -> Result<(), BackendError> { let default = { let backends = self.backends_lock(); let backend = backends .get(id) - .ok_or_else(|| format!("unknown provider backend {id:?}"))?; + .ok_or_else(|| BackendError::UnknownBackend(id.to_owned()))?; backend.default_model.clone() }; let model = model.map(str::to_owned).unwrap_or_else(|| default); @@ -297,9 +320,7 @@ pub enum AnyProvider { } impl AnyProvider { - /// The kind's live model listing. Llama servers list their own - /// `LlamaModel` rows, which carry ids only: mapped into the shared - /// card shape with nothing but the id filled. + /// The kind's live model listing, in the shared card shape. pub async fn list_models(&self) -> Result, ProviderError> { match self { AnyProvider::Google(provider) => provider.list_models().await, @@ -309,21 +330,7 @@ impl AnyProvider { AnyProvider::DeepSeek(provider) => provider.list_models().await, AnyProvider::Zai(provider) => provider.list_models().await, AnyProvider::Copilot(provider) => provider.list_models().await, - AnyProvider::LlamaCpp(provider) => { - let models = provider.list_models().await?; - Ok(models - .into_iter() - .map(|model| ModelCard { - display_name: model.id.clone(), - id: model.id, - description: String::new(), - input_token_limit: model.context_length, - output_token_limit: None, - generation_methods: vec!["generateContent".to_owned()], - pricing: None, - }) - .collect()) - } + AnyProvider::LlamaCpp(provider) => provider.list_models().await, } } @@ -472,7 +479,7 @@ pub fn param_fields_for(kind: &str) -> &'static [ParamField] { /// llama.cpp wants a `url` (the keyless default still applies when /// neither is given). Unknown kinds are an error — "mock" is /// registry-injected, never constructed. -pub fn construct(kind: &str, params: &ProviderParams) -> Result { +pub fn construct(kind: &str, params: &ProviderParams) -> Result { let provider = match kind { "google" => AnyProvider::Google(GoogleProvider::new(require_key(kind, params)?)), "anthropic" => AnyProvider::Anthropic(AnthropicProvider::new(require_key(kind, params)?)), @@ -505,7 +512,7 @@ pub fn construct(kind: &str, params: &ProviderParams) -> Result } AnyProvider::LlamaCpp(provider) } - _ => return Err(format!("unknown provider kind {kind:?}")), + _ => return Err(BackendError::UnknownKind(kind.to_owned())), }; let default_model = default_model_for(kind).to_owned(); let provider = Arc::new(provider); @@ -522,12 +529,12 @@ pub fn construct(kind: &str, params: &ProviderParams) -> Result } /// The kind's API key, or the error the credentials route answers with. -fn require_key(kind: &str, params: &ProviderParams) -> Result { +fn require_key(kind: &str, params: &ProviderParams) -> Result { params .api_key .clone() .filter(|key| !key.trim().is_empty()) - .ok_or_else(|| format!("{kind} needs an api_key")) + .ok_or_else(|| BackendError::MissingKey(kind.to_owned())) } /// Discovers every constructible backend instance from the claw's own diff --git a/crates/rhema-claw/src/bootstrap.rs b/crates/rhema-claw/src/bootstrap.rs index 438977d..92b4f1f 100644 --- a/crates/rhema-claw/src/bootstrap.rs +++ b/crates/rhema-claw/src/bootstrap.rs @@ -27,7 +27,7 @@ use std::path::{Path, PathBuf}; use std::sync::Arc; use rhema_agent_tools::render_template; -use rhema_autonomous_agent::{ToolOp, ToolRegistry, async_trait}; +use rhema_autonomous_agent::{ToolError, ToolOp, ToolRegistry, async_trait}; use rhema_core::ToolSpec; use serde_json::{Value, json}; @@ -253,7 +253,7 @@ pub struct BootstrapSave { #[async_trait] impl ToolOp for BootstrapSave { - async fn invoke(&self, args: Value) -> Result { + async fn invoke(&self, args: Value) -> Result { let args: SaveArgs = serde_json::from_value(args).map_err(|err| format!("bad arguments: {err}"))?; let kind = BootstrapKind::parse(&args.kind) diff --git a/crates/rhema-claw/src/consents.rs b/crates/rhema-claw/src/consents.rs index 058d1f8..cb79ddb 100644 --- a/crates/rhema-claw/src/consents.rs +++ b/crates/rhema-claw/src/consents.rs @@ -21,7 +21,7 @@ use std::collections::{BTreeMap, HashMap}; use std::sync::{Arc, Mutex, PoisonError}; use std::time::Duration; -use rhema_autonomous_agent::{ToolOp, ToolRegistry, async_trait}; +use rhema_autonomous_agent::{ToolError, ToolOp, ToolRegistry, async_trait}; use rusqlite::params; use serde::Deserialize; use serde_json::{Value, json}; @@ -399,12 +399,12 @@ struct AskSecretArgs { #[async_trait] impl ToolOp for AskSecret { - async fn invoke(&self, args: Value) -> Result { + async fn invoke(&self, args: Value) -> Result { let args: AskSecretArgs = serde_json::from_value(args).map_err(|err| format!("bad arguments: {err}"))?; let service = args.service.trim(); if service.is_empty() { - return Err("service must not be empty".to_owned()); + return Err("service must not be empty".to_owned().into()); } // Validate the kind spelling before the card exists: a typo'd // enum must fail the call, not the human's answer. diff --git a/crates/rhema-claw/src/handler.rs b/crates/rhema-claw/src/handler.rs index 2a3bdbd..94439b5 100644 --- a/crates/rhema-claw/src/handler.rs +++ b/crates/rhema-claw/src/handler.rs @@ -883,7 +883,10 @@ mod tests { _req: rhema_core::ModelRequest, _sink: &mut dyn EventSink, ) -> Result { - Err(rhema_core::ProviderError::Api("never called".into())) + Err(rhema_core::ProviderError::Api { + message: "never called".to_owned(), + status: None, + }) } } diff --git a/crates/rhema-claw/src/http.rs b/crates/rhema-claw/src/http.rs index 2e47ac1..90d16c9 100644 --- a/crates/rhema-claw/src/http.rs +++ b/crates/rhema-claw/src/http.rs @@ -114,7 +114,10 @@ impl ClawState { /// store, the consent table, the attachment store, and the /// configured bearers — the human `token` and the scoped /// `agent_token`. - #[allow(clippy::too_many_arguments)] // seventeen parts, each load-bearing + #[expect( + clippy::too_many_arguments, + reason = "seventeen parts, each load-bearing" + )] pub fn new( dispatcher: Arc, driver: Arc, @@ -746,7 +749,11 @@ async fn watches_add(State(state): State, Json(body): Json .into_response(); }; if let Err(reason) = validate_watch(kind, &body.repo, body.host.as_deref()) { - return (StatusCode::BAD_REQUEST, Json(json!({"error": reason}))).into_response(); + return ( + StatusCode::BAD_REQUEST, + Json(json!({"error": reason.to_string()})), + ) + .into_response(); } let added = match state.watches.add( kind.name(), @@ -1099,7 +1106,7 @@ async fn phone_command_result( Path(id): Path, Json(body): Json, ) -> Response { - let command = match state.phone.get_command(&id) { + let command = match state.phone.command_by_id(&id) { Ok(Some(command)) => command, Ok(None) => { return ( @@ -1760,7 +1767,10 @@ async fn providers_active( }; let model = body["model"].as_str(); if let Err(reason) = state.backends().set_active(provider, model) { - return Err((StatusCode::BAD_REQUEST, Json(json!({"error": reason})))); + return Err(( + StatusCode::BAD_REQUEST, + Json(json!({"error": reason.to_string()})), + )); } state .settings() diff --git a/crates/rhema-claw/src/identity.rs b/crates/rhema-claw/src/identity.rs index 7e82ba3..f92fa74 100644 --- a/crates/rhema-claw/src/identity.rs +++ b/crates/rhema-claw/src/identity.rs @@ -14,7 +14,7 @@ use std::collections::BTreeMap; use std::path::{Path, PathBuf}; use std::sync::Arc; -use rhema_autonomous_agent::{ToolOp, ToolRegistry, async_trait}; +use rhema_autonomous_agent::{ToolError, ToolOp, ToolRegistry, async_trait}; use serde::{Deserialize, Serialize}; use serde_json::{Value, json}; @@ -250,9 +250,9 @@ struct IdentitySet { #[async_trait] impl ToolOp for IdentitySet { - async fn invoke(&self, args: Value) -> Result { + async fn invoke(&self, args: Value) -> Result { if !args.is_object() { - return Err("bad arguments: expected an object".to_owned()); + return Err("bad arguments: expected an object".to_owned().into()); } // The texts save whole, so an argument the call omits must carry // the stored value forward — the same absent-means-keep rule the @@ -286,7 +286,7 @@ impl ToolOp for IdentitySet { clears.push(match name { "avatar" => ImageKind::Avatar, "logo" => ImageKind::Logo, - other => return Err(format!("unknown image {other:?}")), + other => return Err(format!("unknown image {other:?}").into()), }); } } diff --git a/crates/rhema-claw/src/interactive.rs b/crates/rhema-claw/src/interactive.rs index 3f6dc18..59cdd85 100644 --- a/crates/rhema-claw/src/interactive.rs +++ b/crates/rhema-claw/src/interactive.rs @@ -197,7 +197,7 @@ impl InteractiveHandler { /// shell (approval v1-permissive — the shell is the point of a /// resident coding agent — installed once, since resolution is /// synchronous within a turn), memory, and `bootstrap_save`. - #[allow(clippy::too_many_arguments)] // nine parts, each load-bearing + #[expect(clippy::too_many_arguments, reason = "nine parts, each load-bearing")] pub fn new( backends: Arc, workspace: impl Into, @@ -670,7 +670,10 @@ mod tests { /// The standard extension set over the tests' throwaway stores — /// the same chain the daemon assembles, built per test so each /// assertion runs against its own tables. - #[allow(clippy::too_many_arguments)] + #[expect( + clippy::too_many_arguments, + reason = "one store per table, per test: each assertion gets its own" + )] fn test_extensions( hub: Arc, watches: Arc, @@ -800,7 +803,11 @@ mod tests { ); let key = Key::from("interactive:web"); let agent = handler.agent(&key).expect("the agent builds"); - assert_eq!(agent.system().contains("zoom"), on, "on={on}"); + assert_eq!( + agent.system().is_some_and(|prompt| prompt.contains("zoom")), + on, + "on={on}" + ); assert_eq!(handler.transcript_mirror.is_some(), on, "on={on}"); } } diff --git a/crates/rhema-claw/src/job_tools.rs b/crates/rhema-claw/src/job_tools.rs index 39df94c..9594de3 100644 --- a/crates/rhema-claw/src/job_tools.rs +++ b/crates/rhema-claw/src/job_tools.rs @@ -17,7 +17,7 @@ use std::collections::BTreeMap; use std::sync::Arc; -use rhema_autonomous_agent::{ToolOp, ToolRegistry, async_trait}; +use rhema_autonomous_agent::{ToolError, ToolOp, ToolRegistry, async_trait}; use serde::Deserialize; use serde_json::{Value, json}; @@ -193,7 +193,7 @@ fn in_band(err: JobStoreError) -> String { #[async_trait] impl ToolOp for CodingAgent { - async fn invoke(&self, args: Value) -> Result { + async fn invoke(&self, args: Value) -> Result { let args: Args = serde_json::from_value(args).map_err(|err| format!("bad arguments: {err}"))?; match args.action.as_str() { @@ -213,6 +213,7 @@ impl ToolOp for CodingAgent { "unknown action {other:?}; one of add, drop, run, list, status" )), } + .map_err(ToolError::from) } } diff --git a/crates/rhema-claw/src/lib.rs b/crates/rhema-claw/src/lib.rs index 39b69f2..7183fc7 100644 --- a/crates/rhema-claw/src/lib.rs +++ b/crates/rhema-claw/src/lib.rs @@ -61,7 +61,6 @@ //! headless `--once` shape [`run_once`]) #![forbid(unsafe_code)] -#![deny(missing_docs)] pub mod accounts_tools; pub mod attachments; @@ -95,7 +94,7 @@ pub mod turn_store; pub mod watch_store; pub mod watch_tools; -pub use backends::{Backend, BackendRegistry}; +pub use backends::{Backend, BackendError, BackendRegistry}; pub use bootstrap::{ BOOTSTRAP_SAVE, Bootstrap, BootstrapError, BootstrapKind, BootstrapStatus, DEFAULT_SYSTEM_MD, render_system, @@ -117,4 +116,4 @@ pub use settings::SettingsStore; pub use task_source::{TASKS_SOURCE, TaskScheduler}; pub use task_store::{TaskKind, TaskRecord, TaskStore, TaskStoreError}; pub use turn_store::{TurnStore, TurnStoreError}; -pub use watch_store::{ClawStore, Watch, WatchRecord, WatchStoreError}; +pub use watch_store::{ClawStore, Watch, WatchRecord, WatchShapeError, WatchStoreError}; diff --git a/crates/rhema-claw/src/main.rs b/crates/rhema-claw/src/main.rs index 700e7fd..bc2cc60 100644 --- a/crates/rhema-claw/src/main.rs +++ b/crates/rhema-claw/src/main.rs @@ -145,7 +145,7 @@ fn resolve_backends(config: &ClawConfig) -> Result Result Result { + async fn invoke(&self, args: Value) -> Result { let args: NotifyArgs = serde_json::from_value(args).map_err(|err| format!("bad arguments: {err}"))?; let title = args.title.trim(); let body = args.body.trim(); if title.is_empty() { - return Err("title must not be empty".to_owned()); + return Err("title must not be empty".to_owned().into()); } if body.is_empty() { - return Err("body must not be empty".to_owned()); + return Err("body must not be empty".to_owned().into()); } let device = args .device diff --git a/crates/rhema-claw/src/phone.rs b/crates/rhema-claw/src/phone.rs index b1b6b3b..553a83a 100644 --- a/crates/rhema-claw/src/phone.rs +++ b/crates/rhema-claw/src/phone.rs @@ -49,7 +49,7 @@ use std::collections::BTreeMap; use std::sync::{Arc, Mutex, PoisonError}; use std::time::Duration; -use rhema_autonomous_agent::{ToolOp, ToolRegistry, async_trait}; +use rhema_autonomous_agent::{ToolError, ToolOp, ToolRegistry, async_trait}; use rusqlite::params; use serde::Deserialize; use serde_json::{Value, json}; @@ -412,7 +412,7 @@ impl PhoneStore { } /// The command with this id, when it exists. - pub fn get_command(&self, id: &str) -> Result, PhoneStoreError> { + pub fn command_by_id(&self, id: &str) -> Result, PhoneStoreError> { let conn = self.conn.lock().unwrap_or_else(PoisonError::into_inner); let mut stmt = conn .prepare( @@ -462,7 +462,7 @@ pub async fn wait_for_command_result( ) -> CommandOutcome { let deadline = tokio::time::Instant::now() + budget; loop { - if let Ok(Some(command)) = store.get_command(id) { + if let Ok(Some(command)) = store.command_by_id(id) { if let Some(ok) = command.result_ok { return CommandOutcome::Confirmed { ok, @@ -512,7 +512,7 @@ struct PhoneArgs { #[async_trait] impl ToolOp for PhoneTool { - async fn invoke(&self, args: Value) -> Result { + async fn invoke(&self, args: Value) -> Result { let args: PhoneArgs = serde_json::from_value(args).map_err(|err| format!("bad arguments: {err}"))?; let device = args @@ -551,6 +551,7 @@ impl ToolOp for PhoneTool { "unknown action {other:?}: status, devices, forget, or one of {CONTROL_ACTIONS:?}" )), } + .map_err(ToolError::from) } } @@ -980,7 +981,7 @@ mod tests { "a redelivery does not flip the verdict" ); let confirmed = store - .get_command(&command.id) + .command_by_id(&command.id) .expect("reads") .expect("exists"); assert_eq!(confirmed.result_ok, Some(true)); diff --git a/crates/rhema-claw/src/settings.rs b/crates/rhema-claw/src/settings.rs index eb3a931..f80dfec 100644 --- a/crates/rhema-claw/src/settings.rs +++ b/crates/rhema-claw/src/settings.rs @@ -73,7 +73,7 @@ impl SettingsStore { /// Every `(key, value)` whose key starts with `prefix`, ascending /// by key — discovery's view of the stored provider rows. - pub fn get_prefix(&self, prefix: &str) -> Result, SettingsError> { + pub fn rows_by_prefix(&self, prefix: &str) -> Result, SettingsError> { let conn = self.conn.lock().unwrap_or_else(PoisonError::into_inner); let mut statement = conn .prepare("SELECT key, value FROM settings WHERE key LIKE (?1 || '%') ORDER BY key") diff --git a/crates/rhema-claw/src/task_tools.rs b/crates/rhema-claw/src/task_tools.rs index b6562ec..fd382b8 100644 --- a/crates/rhema-claw/src/task_tools.rs +++ b/crates/rhema-claw/src/task_tools.rs @@ -15,7 +15,7 @@ use std::collections::BTreeMap; use std::sync::Arc; -use rhema_autonomous_agent::{ToolOp, ToolRegistry, async_trait}; +use rhema_autonomous_agent::{ToolError, ToolOp, ToolRegistry, async_trait}; use serde::Deserialize; use serde_json::{Value, json}; use time::OffsetDateTime; @@ -134,12 +134,12 @@ struct ScheduleTask { #[async_trait] impl ToolOp for ScheduleTask { - async fn invoke(&self, args: Value) -> Result { + async fn invoke(&self, args: Value) -> Result { let args: ScheduleArgs = serde_json::from_value(args).map_err(|err| format!("bad arguments: {err}"))?; let prompt = args.prompt.trim(); if prompt.is_empty() { - return Err("prompt must not be empty".to_owned()); + return Err("prompt must not be empty".to_owned().into()); } // A blank session reads as unset, and only a resident key can be // woken: the delivery target must be a session the resident @@ -153,7 +153,8 @@ impl ToolOp for ScheduleTask { if !is_resident_key(session) { return Err(format!( "session must be a resident key (interactive:, task:, or phone:), got {session:?}" - )); + ) + .into()); } } let record = match (args.at, args.every_secs) { @@ -165,7 +166,7 @@ impl ToolOp for ScheduleTask { } (None, Some(every_secs)) => { if every_secs == 0 { - return Err("every_secs must be at least 1".to_owned()); + return Err("every_secs must be at least 1".to_owned().into()); } let interval = every_secs .checked_mul(1000) @@ -176,10 +177,12 @@ impl ToolOp for ScheduleTask { .map_err(|err| err.to_string())? } (Some(_), Some(_)) => { - return Err("set exactly one of `at` or `every_secs`, not both".to_owned()); + return Err("set exactly one of `at` or `every_secs`, not both" + .to_owned() + .into()); } (None, None) => { - return Err("set one of `at` or `every_secs`".to_owned()); + return Err("set one of `at` or `every_secs`".to_owned().into()); } }; Ok(json!({ @@ -200,7 +203,7 @@ struct ListTasks { #[async_trait] impl ToolOp for ListTasks { - async fn invoke(&self, _args: Value) -> Result { + async fn invoke(&self, _args: Value) -> Result { let tasks = self.store.list().map_err(|err| err.to_string())?; Ok(json!({ "tasks": tasks }).to_string()) } @@ -213,7 +216,7 @@ struct CancelTask { #[async_trait] impl ToolOp for CancelTask { - async fn invoke(&self, args: Value) -> Result { + async fn invoke(&self, args: Value) -> Result { let args: CancelArgs = serde_json::from_value(args).map_err(|err| format!("bad arguments: {err}"))?; let removed = self.store.remove(args.id).map_err(|err| err.to_string())?; diff --git a/crates/rhema-claw/src/watch_store.rs b/crates/rhema-claw/src/watch_store.rs index 8355823..928fc1f 100644 --- a/crates/rhema-claw/src/watch_store.rs +++ b/crates/rhema-claw/src/watch_store.rs @@ -49,13 +49,17 @@ impl Watch { /// The repository coordinates, splitting `owner/name`. GitLab's owner /// keeps its full namespace path, which is why the split is lazy: the /// first `/` separates owner from name, never a deeper one. - pub fn repo_ref(&self) -> Result { - let (owner, name) = self - .repo - .split_once('/') - .ok_or_else(|| format!("repo must be owner/name, got {:?}", self.repo))?; + pub fn repo_ref(&self) -> Result { + let (owner, name) = + self.repo + .split_once('/') + .ok_or_else(|| WatchShapeError::RepoShape { + repo: self.repo.clone(), + })?; if owner.is_empty() || name.is_empty() { - return Err(format!("repo must be owner/name, got {:?}", self.repo)); + return Err(WatchShapeError::RepoShape { + repo: self.repo.clone(), + }); } Ok(RepoRef::new(owner, name)) } @@ -104,6 +108,25 @@ pub enum WatchStoreError { Backend(#[from] rusqlite::Error), } +/// Why a watch's coordinates failed their shape checks: a repo that is +/// not `owner/name`, or a gitea watch with no instance base URL. One +/// type for every entry path — the HTTP route, the agent tool, the +/// config watch — so the refusals read the same everywhere. +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +pub enum WatchShapeError { + /// The repo field is not `owner/name` (GitLab's owner may itself + /// carry slashes; the first `/` still separates). + #[error("repo must be owner/name, got {repo:?}")] + RepoShape { + /// The repo value as stored, for the error message. + repo: String, + }, + /// A gitea watch with no host: gitea is self-hosted only, so there + /// is nothing to poll. + #[error("gitea is self-hosted: set host to the instance base URL")] + MissingHost, +} + /// One watched repository, as stored. `token` never serializes: it /// reaches the forge clients, not the wire or the model. #[derive(Debug, Clone, PartialEq, Eq, Serialize)] @@ -129,13 +152,17 @@ impl WatchRecord { /// The repository coordinates, splitting `owner/name` — the same /// shape check a config watch passes. GitLab's owner keeps its full /// namespace path, which is why the split is lazy. - pub fn repo_ref(&self) -> Result { - let (owner, name) = self - .repo - .split_once('/') - .ok_or_else(|| format!("repo must be owner/name, got {:?}", self.repo))?; + pub fn repo_ref(&self) -> Result { + let (owner, name) = + self.repo + .split_once('/') + .ok_or_else(|| WatchShapeError::RepoShape { + repo: self.repo.clone(), + })?; if owner.is_empty() || name.is_empty() { - return Err(format!("repo must be owner/name, got {:?}", self.repo)); + return Err(WatchShapeError::RepoShape { + repo: self.repo.clone(), + }); } Ok(RepoRef::new(owner, name)) } @@ -352,7 +379,11 @@ fn now_millis() -> i64 { /// `owner/name` repo, and — gitea being self-hosted only — a host for /// gitea. Tokens are not required here: a blank one falls back to the /// forge's environment variable at poll time. -pub fn validate_watch(forge: ForgeKind, repo: &str, host: Option<&str>) -> Result<(), String> { +pub fn validate_watch( + forge: ForgeKind, + repo: &str, + host: Option<&str>, +) -> Result<(), WatchShapeError> { let watch = Watch { forge, repo: repo.trim().to_owned(), @@ -361,7 +392,7 @@ pub fn validate_watch(forge: ForgeKind, repo: &str, host: Option<&str>) -> Resul }; watch.repo_ref()?; if forge == ForgeKind::Gitea && watch.host.as_deref().filter(|h| !h.is_empty()).is_none() { - return Err("gitea is self-hosted: set host to the instance base URL".to_owned()); + return Err(WatchShapeError::MissingHost); } Ok(()) } @@ -372,16 +403,16 @@ mod tests { #[test] fn validation_fails_a_watch_loudly_and_specificly() { - assert!( - validate_watch(ForgeKind::GitHub, "acme", None) - .unwrap_err() - .contains("owner/name"), + assert_eq!( + validate_watch(ForgeKind::GitHub, "acme", None).unwrap_err(), + WatchShapeError::RepoShape { + repo: "acme".to_owned() + }, "an ownerless repo is loud" ); - assert!( - validate_watch(ForgeKind::Gitea, "acme/barn", None) - .unwrap_err() - .contains("self-hosted"), + assert_eq!( + validate_watch(ForgeKind::Gitea, "acme/barn", None).unwrap_err(), + WatchShapeError::MissingHost, "a hostless gitea watch is loud" ); validate_watch(ForgeKind::GitHub, "acme/widgets", None).expect("github validates"); diff --git a/crates/rhema-claw/src/watch_tools.rs b/crates/rhema-claw/src/watch_tools.rs index 19fc1b1..778062b 100644 --- a/crates/rhema-claw/src/watch_tools.rs +++ b/crates/rhema-claw/src/watch_tools.rs @@ -14,7 +14,7 @@ use std::collections::BTreeMap; use std::sync::Arc; -use rhema_autonomous_agent::{ToolOp, ToolRegistry, async_trait}; +use rhema_autonomous_agent::{ToolError, ToolOp, ToolRegistry, async_trait}; use serde::Deserialize; use serde_json::{Value, json}; @@ -98,13 +98,15 @@ struct WatchRepo { #[async_trait] impl ToolOp for WatchRepo { - async fn invoke(&self, args: Value) -> Result { + async fn invoke(&self, args: Value) -> Result { let args: WatchArgs = serde_json::from_value(args).map_err(|err| format!("bad arguments: {err}"))?; let kind = ForgeKind::parse(&args.forge) .ok_or_else(|| format!("unknown forge {:?}: github, gitea, or gitlab", args.forge))?; let host = args.host.as_deref().map(str::trim); - validate_watch(kind, &args.repo, host)?; + // The tool contract answers in band: the model sees the shape + // refusal as text it can correct. + validate_watch(kind, &args.repo, host).map_err(|err| err.to_string())?; let added = self .store .add( @@ -131,7 +133,7 @@ struct UnwatchRepo { #[async_trait] impl ToolOp for UnwatchRepo { - async fn invoke(&self, args: Value) -> Result { + async fn invoke(&self, args: Value) -> Result { let args: UnwatchArgs = serde_json::from_value(args).map_err(|err| format!("bad arguments: {err}"))?; let kind = ForgeKind::parse(&args.forge) @@ -157,7 +159,7 @@ struct ListWatches { #[async_trait] impl ToolOp for ListWatches { - async fn invoke(&self, _args: Value) -> Result { + async fn invoke(&self, _args: Value) -> Result { let watches = self.store.list().map_err(|err| err.to_string())?; let rows: Vec = watches .iter() diff --git a/crates/rhema-claw/tests/http.rs b/crates/rhema-claw/tests/http.rs index 4c8f1a2..ddf71b6 100644 --- a/crates/rhema-claw/tests/http.rs +++ b/crates/rhema-claw/tests/http.rs @@ -1643,7 +1643,7 @@ fn discovered_registry(dir: &tempfile::TempDir) -> Arc