refactor(claw): typed errors at the store and tool seams

BackendError and WatchShapeError replace the String registry and
watch seams, the tool operations report ToolError, the accessors
drop the get_ prefix, the too-many-arguments allows become
expects, and the llama catalog passes through unchanged.
This commit is contained in:
vhaudiquet committed 2026-10-25 07:02:48 +02:00
1 parent 90212d1e0f
commit 5eef154fcb
20 files changed
+180 -110

No files matched your search

+3
View File
@@ -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
+6 -6
View File
@@ -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<String, String> {
async fn invoke(&self, args: Value) -> Result<String, ToolError> {
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<String, String> {
async fn invoke(&self, _args: Value) -> Result<String, ToolError> {
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<String, String> {
async fn invoke(&self, args: Value) -> Result<String, ToolError> {
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<String, String> {
fn call(tool: &dyn ToolOp, args: Value) -> Result<String, ToolError> {
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
+6 -4
View File
@@ -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<String, String> {
async fn invoke(&self, args: Value) -> Result<String, ToolError> {
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))?;
+38 -31
View File
@@ -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<Backend>,
seed: Option<(String, Option<String>)>,
) -> Result<Self, String> {
) -> Result<Self, BackendError> {
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<String, Arc<Backend>> = 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<Vec<ModelCard>, 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<Backend, String> {
pub fn construct(kind: &str, params: &ProviderParams) -> Result<Backend, BackendError> {
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<Backend, String>
}
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<Backend, String>
}
/// The kind's API key, or the error the credentials route answers with.
fn require_key(kind: &str, params: &ProviderParams) -> Result<String, String> {
fn require_key(kind: &str, params: &ProviderParams) -> Result<String, BackendError> {
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
+2 -2
View File
@@ -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<String, String> {
async fn invoke(&self, args: Value) -> Result<String, ToolError> {
let args: SaveArgs =
serde_json::from_value(args).map_err(|err| format!("bad arguments: {err}"))?;
let kind = BootstrapKind::parse(&args.kind)
+3 -3
View File
@@ -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<String, String> {
async fn invoke(&self, args: Value) -> Result<String, ToolError> {
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.
+4 -1
View File
@@ -883,7 +883,10 @@ mod tests {
_req: rhema_core::ModelRequest,
_sink: &mut dyn EventSink,
) -> Result<rhema_core::ModelOutput, rhema_core::ProviderError> {
Err(rhema_core::ProviderError::Api("never called".into()))
Err(rhema_core::ProviderError::Api {
message: "never called".to_owned(),
status: None,
})
}
}
+14 -4
View File
@@ -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<Dispatcher>,
driver: Arc<Driver>,
@@ -746,7 +749,11 @@ async fn watches_add(State(state): State<ClawState>, Json(body): Json<WatchBody>
.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<String>,
Json(body): Json<PhoneCommandResult>,
) -> 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()
+4 -4
View File
@@ -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<String, String> {
async fn invoke(&self, args: Value) -> Result<String, ToolError> {
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()),
});
}
}
+10 -3
View File
@@ -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<BackendRegistry>,
workspace: impl Into<PathBuf>,
@@ -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<EventHub>,
watches: Arc<ClawStore>,
@@ -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}");
}
}
+3 -2
View File
@@ -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<String, String> {
async fn invoke(&self, args: Value) -> Result<String, ToolError> {
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)
}
}
+2 -3
View File
@@ -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};
+3 -2
View File
@@ -145,7 +145,7 @@ fn resolve_backends(config: &ClawConfig) -> Result<Arc<rhema_claw::BackendRegist
.as_ref()
.and_then(|store| {
store
.get_prefix(rhema_claw::backends::PROVIDER_KEY_PREFIX)
.rows_by_prefix(rhema_claw::backends::PROVIDER_KEY_PREFIX)
.ok()
})
.unwrap_or_default()
@@ -178,6 +178,7 @@ fn resolve_backends(config: &ClawConfig) -> Result<Arc<rhema_claw::BackendRegist
.map(|kind| (kind.to_owned(), config.model.clone()))
.or_else(|| rhema_claw::backends::credential_seed(&backends));
Ok(Arc::new(
rhema_claw::BackendRegistry::new(backends, seed).map_err(ClawError::Backend)?,
rhema_claw::BackendRegistry::new(backends, seed)
.map_err(|err| ClawError::Backend(err.to_string()))?,
))
}
+4 -4
View File
@@ -12,7 +12,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};
@@ -62,16 +62,16 @@ struct Notify {
#[async_trait]
impl ToolOp for Notify {
async fn invoke(&self, args: Value) -> Result<String, String> {
async fn invoke(&self, args: Value) -> Result<String, ToolError> {
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
+6 -5
View File
@@ -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<Option<Command>, PhoneStoreError> {
pub fn command_by_id(&self, id: &str) -> Result<Option<Command>, 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<String, String> {
async fn invoke(&self, args: Value) -> Result<String, ToolError> {
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));
+1 -1
View File
@@ -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<Vec<(String, String)>, SettingsError> {
pub fn rows_by_prefix(&self, prefix: &str) -> Result<Vec<(String, String)>, 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")
+12 -9
View File
@@ -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<String, String> {
async fn invoke(&self, args: Value) -> Result<String, ToolError> {
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<String, String> {
async fn invoke(&self, _args: Value) -> Result<String, ToolError> {
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<String, String> {
async fn invoke(&self, args: Value) -> Result<String, ToolError> {
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())?;
+51 -20
View File
@@ -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<RepoRef, String> {
let (owner, name) = self
.repo
pub fn repo_ref(&self) -> Result<RepoRef, WatchShapeError> {
let (owner, name) =
self.repo
.split_once('/')
.ok_or_else(|| format!("repo must be owner/name, got {:?}", self.repo))?;
.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<RepoRef, String> {
let (owner, name) = self
.repo
pub fn repo_ref(&self) -> Result<RepoRef, WatchShapeError> {
let (owner, name) =
self.repo
.split_once('/')
.ok_or_else(|| format!("repo must be owner/name, got {:?}", self.repo))?;
.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");
+7 -5
View File
@@ -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<String, String> {
async fn invoke(&self, args: Value) -> Result<String, ToolError> {
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<String, String> {
async fn invoke(&self, args: Value) -> Result<String, ToolError> {
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<String, String> {
async fn invoke(&self, _args: Value) -> Result<String, ToolError> {
let watches = self.store.list().map_err(|err| err.to_string())?;
let rows: Vec<Value> = watches
.iter()
+1 -1
View File
@@ -1643,7 +1643,7 @@ fn discovered_registry(dir: &tempfile::TempDir) -> Arc<rhema_claw::BackendRegist
.ok()
.and_then(|store| {
store
.get_prefix(rhema_claw::backends::PROVIDER_KEY_PREFIX)
.rows_by_prefix(rhema_claw::backends::PROVIDER_KEY_PREFIX)
.ok()
})
.unwrap_or_default()