diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a419d423..5d77a462b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,31 @@ during the 2.x migration are no longer part of the TinyAgents workspace. The unreleased entries below describe the changes accumulated before this major release. +## Unreleased: tool hook denials answer the model + +**Breaking (behaviour).** A `tinyagents-hooks` tool-hook `Deny` no longer +aborts the run. `ToolHooksMiddleware` now returns +`TinyAgentsError::ToolFailed`, so the agent loop answers the call with a failed +tool result carrying `tool hook '' denied : ` and the run +continues; the tool never runs. This matches `HookPermission::Deny`'s +contract ("The agent is told why") and covers blank or invented tool names a +host denies, which used to end the turn before the unknown-tool policy ran. + +Hosts that relied on a deny ending the run must set `continue_: Some(false)` +on the decision. That form stops the run with the new +`TinyAgentsError::ToolRefused`, classified as `TerminalReason::ToolRefused` and, +for hosted callers, `HostedErrorKind::ToolRefused` with the message +`hosted agent invocation stopped: host policy refused a tool call` instead of +the generic `hosted agent invocation failed`. The refusal text never carries +the call's arguments. `Ask` is unchanged and still fails the run with +`TinyAgentsError::Tool`. + +An answered deny releases its tool-call slot like every other `before_tool` +refusal, so a model that keeps calling a denied tool is bounded by +`RunLimits::max_model_calls`. `TinyAgentsError`, `TerminalReason` and +`HostedErrorKind` are `#[non_exhaustive]`, so the new variants are additive. +See `docs/modules/hooks/README.md`. + ## Unreleased: model observations report who served a call and its first token **Breaking.** `TurnObservation::Model` and `TurnObservation::ModelDispatch` diff --git a/crates/tinyagents-harness/src/error.rs b/crates/tinyagents-harness/src/error.rs index 4a7f5a442..6dd780078 100644 --- a/crates/tinyagents-harness/src/error.rs +++ b/crates/tinyagents-harness/src/error.rs @@ -170,6 +170,20 @@ pub enum TinyAgentsError { #[error("permanent tool failure: {0}")] ToolFailed(String), + /// Host policy refused a tool call and asked for the **run** to stop, + /// rather than for the model to be told and carry on. + /// + /// A `before_tool` refusal answered to the model is + /// [`Self::ToolFailed`]; this variant is the fatal form, raised for + /// example by a `tinyagents-hooks` deny carrying `continue: false`. The + /// payload names the refusing hook, the tool and the reason, never the + /// call's arguments. The run ends with + /// [`crate::terminal::TerminalReason::ToolRefused`], and a hosted caller + /// sees [`crate::runtime::HostedErrorKind::ToolRefused`]. Never retried by + /// [`crate::retry::is_retryable`]. + #[error("tool call refused: {0}")] + ToolRefused(String), + /// A tool (from [`tinytools::Tool::execute`]) or a `before_tool` /// middleware asked for **human approval** before this call runs (A2). /// diff --git a/crates/tinyagents-harness/src/runtime/agent.rs b/crates/tinyagents-harness/src/runtime/agent.rs index 194e4bb3e..f4980d918 100644 --- a/crates/tinyagents-harness/src/runtime/agent.rs +++ b/crates/tinyagents-harness/src/runtime/agent.rs @@ -96,6 +96,10 @@ pub enum HostedErrorKind { Policy, /// The model provider failed the call. Provider, + /// Host tool policy refused a call and stopped the run + /// ([`TinyAgentsError::ToolRefused`]). A refusal answered to the model + /// does not end the run and never surfaces here. + ToolRefused, /// Any other internal failure not covered by a more specific kind. Internal, } @@ -167,6 +171,7 @@ fn classify_hosted_error(error: &TinyAgentsError) -> HostedErrorKind { | TinyAgentsError::PhysicalDispatchAdmission(_) => HostedErrorKind::LimitExceeded, TinyAgentsError::Validation(_) | TinyAgentsError::Steering(_) => HostedErrorKind::Policy, TinyAgentsError::Provider(_) | TinyAgentsError::Model(_) => HostedErrorKind::Provider, + TinyAgentsError::ToolRefused(_) => HostedErrorKind::ToolRefused, _ => HostedErrorKind::Internal, } } @@ -191,7 +196,25 @@ fn hosted_error_message(kind: HostedErrorKind) -> &'static str { HostedErrorKind::LimitExceeded => "hosted agent invocation exceeded a configured limit", HostedErrorKind::Policy => "hosted agent invocation was rejected by policy", HostedErrorKind::Provider => "hosted agent invocation failed at the model provider", - HostedErrorKind::Internal => "hosted agent invocation failed", + HostedErrorKind::ToolRefused => { + "hosted agent invocation stopped: host policy refused a tool call" + } + HostedErrorKind::Internal => GENERIC_HOSTED_FAILURE, + } +} + +const GENERIC_HOSTED_FAILURE: &str = "hosted agent invocation failed"; + +/// The fixed message for a failed hosted stream, chosen from the run's typed +/// terminal outcome because the stream carries only the raw error text. +fn hosted_stream_failure_message( + outcome: Option<&crate::terminal::TerminalOutcome>, +) -> &'static str { + match outcome.map(|outcome| outcome.reason) { + Some(crate::terminal::TerminalReason::ToolRefused) => { + hosted_error_message(HostedErrorKind::ToolRefused) + } + _ => GENERIC_HOSTED_FAILURE, } } @@ -243,6 +266,7 @@ impl From for TinyAgentsError { HostedErrorKind::StreamIdleTimeout => TinyAgentsError::StreamIdleTimeout(error.message), HostedErrorKind::LimitExceeded => TinyAgentsError::LimitExceeded(error.message), HostedErrorKind::Policy => TinyAgentsError::Validation(error.message), + HostedErrorKind::ToolRefused => TinyAgentsError::ToolRefused(error.message), HostedErrorKind::Provider | HostedErrorKind::Internal => { TinyAgentsError::Model(error.message) } @@ -254,6 +278,10 @@ impl From for TinyAgentsError { #[path = "agent_dispatch_refusal_tests.rs"] mod dispatch_refusal_tests; +#[cfg(test)] +#[path = "agent_tool_refusal_tests.rs"] +mod tool_refusal_tests; + /// A host-owned turn request. /// /// `agent_id` is opaque to the harness. It is resolved only through the host @@ -454,7 +482,7 @@ impl Stream for AgentStream<'_, fn sanitize_hosted_stream_item(mut item: AgentStreamItem) -> AgentStreamItem { match &mut item { AgentStreamItem::Failed { error, run } => { - *error = "hosted agent invocation failed".to_string(); + *error = hosted_stream_failure_message(run.terminal.as_ref()).to_string(); // The partial run carries its typed outcome; keep the // classification, drop the raw failure text. if let Some(outcome) = run.terminal.as_mut() { @@ -497,7 +525,7 @@ fn sanitize_hosted_event(record: &mut EventRecord) { *error = "hosted middleware failed".to_string(); } AgentEvent::RunFailed { error, outcome, .. } => { - *error = "hosted agent invocation failed".to_string(); + *error = hosted_stream_failure_message(outcome.as_ref()).to_string(); // The typed outcome mirrors the raw error text; keep its // classification (reason, class, phase) and drop the detail. if let Some(outcome) = outcome { diff --git a/crates/tinyagents-harness/src/runtime/agent_tool_refusal_tests.rs b/crates/tinyagents-harness/src/runtime/agent_tool_refusal_tests.rs new file mode 100644 index 000000000..1522122cd --- /dev/null +++ b/crates/tinyagents-harness/src/runtime/agent_tool_refusal_tests.rs @@ -0,0 +1,126 @@ +use super::{ + HostedError, HostedErrorKind, hosted_error, sanitize_hosted_event, sanitize_hosted_stream_item, +}; +use crate::agent_loop::AgentStreamItem; +use crate::error::TinyAgentsError; +use crate::events::{AgentEvent, EventRecord}; +use crate::ids::{EventId, RunId}; +use crate::middleware::AgentRun; +use crate::terminal::{TerminalOutcome, TerminalReason, TimeoutPhase}; + +const REFUSED: &str = "hosted agent invocation stopped: host policy refused a tool call"; + +fn refusal() -> TinyAgentsError { + TinyAgentsError::ToolRefused("tool hook 'allow_list' denied secret: forbidden".into()) +} + +fn refused_run() -> AgentRun { + let mut run = AgentRun::new(); + run.terminal = Some(TerminalOutcome::from_error( + &refusal(), + TimeoutPhase::AfterTurn, + )); + run +} + +#[test] +fn tool_refusal_is_classified_as_its_own_terminal_reason() { + let outcome = TerminalOutcome::from_error(&refusal(), TimeoutPhase::AfterTurn); + assert_eq!(outcome.reason, TerminalReason::ToolRefused); + assert_eq!(outcome.class, crate::terminal::TerminalClass::Failure); +} + +#[test] +fn hosted_error_names_a_tool_refusal_without_its_detail() { + let hosted = hosted_error(&refusal(), refused_run()); + + assert_eq!(hosted.kind, HostedErrorKind::ToolRefused); + assert_eq!(hosted.message, REFUSED); + let outcome = hosted.run.as_ref().and_then(|run| run.terminal.as_ref()); + assert_eq!( + outcome.map(|outcome| outcome.message.as_str()), + Some(REFUSED) + ); + assert_eq!( + outcome.map(|outcome| outcome.reason), + Some(TerminalReason::ToolRefused) + ); +} + +#[test] +fn hosted_tool_refusal_round_trips_to_its_own_variant() { + let hosted = HostedError { + kind: HostedErrorKind::ToolRefused, + message: REFUSED.to_owned(), + timeout_bound: None, + dispatch_refusal: None, + run: None, + }; + assert!(matches!( + TinyAgentsError::from(hosted), + TinyAgentsError::ToolRefused(message) if message == REFUSED + )); +} + +#[test] +fn tool_refusal_is_not_retryable() { + assert!(!crate::retry::is_retryable(&refusal())); +} + +#[test] +fn hosted_stream_failure_names_a_tool_refusal() { + let item = sanitize_hosted_stream_item(AgentStreamItem::Failed { + error: refusal().to_string(), + run: Box::new(refused_run()), + }); + let AgentStreamItem::Failed { error, run } = item else { + panic!("still a failure"); + }; + assert_eq!(error, REFUSED); + assert_eq!( + run.terminal + .as_ref() + .map(|outcome| outcome.message.as_str()), + Some(REFUSED) + ); +} + +#[test] +fn hosted_stream_failure_stays_generic_for_other_errors() { + let mut run = AgentRun::new(); + run.terminal = Some(TerminalOutcome::from_error( + &TinyAgentsError::Tool("boom".into()), + TimeoutPhase::AfterTurn, + )); + let item = sanitize_hosted_stream_item(AgentStreamItem::Failed { + error: "tool error: boom".into(), + run: Box::new(run), + }); + assert!(matches!( + item, + AgentStreamItem::Failed { error, .. } if error == "hosted agent invocation failed" + )); +} + +#[test] +fn hosted_run_failed_event_names_a_tool_refusal() { + let error = refusal(); + let mut record = EventRecord { + id: EventId::new("evt-0"), + offset: 0, + event: AgentEvent::RunFailed { + run_id: RunId::new("run"), + error: error.to_string(), + outcome: Some(TerminalOutcome::from_error(&error, TimeoutPhase::AfterTurn)), + }, + }; + sanitize_hosted_event(&mut record); + let AgentEvent::RunFailed { error, outcome, .. } = &record.event else { + panic!("still RunFailed"); + }; + assert_eq!(error, REFUSED); + assert_eq!( + outcome.as_ref().map(|outcome| outcome.message.as_str()), + Some(REFUSED) + ); +} diff --git a/crates/tinyagents-harness/src/terminal.rs b/crates/tinyagents-harness/src/terminal.rs index 05b116f69..4cff6ad5c 100644 --- a/crates/tinyagents-harness/src/terminal.rs +++ b/crates/tinyagents-harness/src/terminal.rs @@ -25,6 +25,7 @@ //! | `TinyAgentsError::StreamIdleTimeout` (breaker threshold) | `ProviderFailed(Some(StreamIdleTimeout))` | `Timeout` | //! | provider / model / overflow / empty-response errors | `ProviderFailed(reason)` | `Failure` | //! | tool errors | `ToolFailed` | `Failure` | +//! | `TinyAgentsError::ToolRefused` (host policy stopped the run) | `ToolRefused` | `Failure` | //! | run caps, depth caps | `LimitReached(..)` | `Failure` | //! | steering pause | `Paused` | `Suspended` | //! | deferred tool calls | `Deferred` | `Suspended` | @@ -42,7 +43,7 @@ //! 3. idle / provider-call timeout (`ProviderFailed(Some(Timeout))`) //! 4. limits (`LimitReached`) //! 5. no-progress / repeat guard (`Halted`) -//! 6. other failures (`ProviderFailed`, `ToolFailed`, `Internal`) +//! 6. other failures (`ProviderFailed`, `ToolFailed`, `ToolRefused`, `Internal`) //! 7. suspension (`Paused`, `Deferred`) //! 8. success (`Completed`) //! @@ -118,6 +119,9 @@ pub enum TerminalReason { ProviderFailed(Option), /// A tool failed in a way that ended the run. ToolFailed, + /// Host policy refused a tool call and stopped the run + /// ([`TinyAgentsError::ToolRefused`]). + ToolRefused, /// Anything the other reasons do not name. Internal, } @@ -137,6 +141,7 @@ impl TerminalReason { | Self::Halted | Self::ProviderFailed(_) | Self::ToolFailed + | Self::ToolRefused | Self::Internal => TerminalClass::Failure, } } @@ -151,7 +156,7 @@ impl TerminalReason { )) => 2, Self::LimitReached(_) => 3, Self::Halted => 4, - Self::ProviderFailed(_) | Self::ToolFailed | Self::Internal => 5, + Self::ProviderFailed(_) | Self::ToolFailed | Self::ToolRefused | Self::Internal => 5, Self::Paused | Self::Deferred => 6, Self::Completed => 7, } @@ -285,6 +290,7 @@ impl TerminalOutcome { E::Tool(_) | E::ToolFailed(_) | E::ToolNotFound(_) | E::ModelRetry(_) => { Self::new(TerminalReason::ToolFailed, message) } + E::ToolRefused(_) => Self::new(TerminalReason::ToolRefused, message), E::LimitExceeded(_) | E::RecursionLimit(_) | E::SubAgentDepth(_) diff --git a/crates/tinyagents-hooks/src/command/exec.rs b/crates/tinyagents-hooks/src/command/exec.rs index b795b18c3..e57ab6e72 100644 --- a/crates/tinyagents-hooks/src/command/exec.rs +++ b/crates/tinyagents-hooks/src/command/exec.rs @@ -129,7 +129,7 @@ fn finish( Err(error) if definition.fail_closed => { tracing::warn!("[hooks] {label} failed and is fail-closed; denying: {error}"); HookRun { - output: HookOutput::deny(format!("hook '{label}' could not run: {error}")), + output: HookOutput::deny(format!("hook '{label}' could not run")), label, duration, error: Some(error), diff --git a/crates/tinyagents-hooks/src/command/exec_tests.rs b/crates/tinyagents-hooks/src/command/exec_tests.rs index cac057a64..bedcba376 100644 --- a/crates/tinyagents-hooks/src/command/exec_tests.rs +++ b/crates/tinyagents-hooks/src/command/exec_tests.rs @@ -148,6 +148,9 @@ mod spawner { assert!(open.error.unwrap().contains("no isolation available")); let closed = run_with(refuse(), &hook(true)).await; assert!(closed.output.is_deny()); + let reason = closed.output.agent_message.unwrap(); + assert!(!reason.contains("no isolation available"), "{reason}"); + assert!(closed.error.unwrap().contains("no isolation available")); } #[tokio::test] diff --git a/crates/tinyagents-hooks/src/command/types.rs b/crates/tinyagents-hooks/src/command/types.rs index f301ffa54..a00f176cd 100644 --- a/crates/tinyagents-hooks/src/command/types.rs +++ b/crates/tinyagents-hooks/src/command/types.rs @@ -508,7 +508,8 @@ pub struct HookOutput { /// Text appended to a tool result or to the session's system context. #[serde(default, skip_serializing_if = "Option::is_none")] pub additional_context: Option, - /// `false` aborts prompt submission on `beforeSubmitPrompt`. + /// `false` aborts prompt submission on `beforeSubmitPrompt`. On a + /// tool-hook deny, `false` stops the run instead of answering the model. #[serde(default, rename = "continue", skip_serializing_if = "Option::is_none")] pub continue_: Option, /// Injects another user turn on `stop` / `subagentStop`. diff --git a/crates/tinyagents-hooks/src/tool/mod.rs b/crates/tinyagents-hooks/src/tool/mod.rs index 3e1c77567..46d44fbd3 100644 --- a/crates/tinyagents-hooks/src/tool/mod.rs +++ b/crates/tinyagents-hooks/src/tool/mod.rs @@ -1,7 +1,11 @@ //! Typed in-process hooks around tool execution. //! -//! Hooks run in registration order. A denial or unresolved approval request -//! refuses a call, while argument rewrites flow to subsequent hooks. +//! Hooks run in registration order, and argument rewrites flow to subsequent +//! hooks. A denial refuses the call without running it and answers the model +//! with a failed tool result carrying the reason, so the run carries on. A +//! denial with `continue_: Some(false)` stops the run with +//! [`TinyAgentsError::ToolRefused`](tinyagents_harness::error::TinyAgentsError::ToolRefused). +//! An unresolved approval request (`Ask`) fails the run. use async_trait::async_trait; use serde::{Deserialize, Serialize}; @@ -187,6 +191,7 @@ where let decision = hook.before_tool_decision(&ctx.data, &event).await; if decision.is_deny() || decision.is_ask() { let ask = decision.is_ask(); + let stop_run = decision.continue_ == Some(false); let reason = decision .agent_message .or(decision.user_message) @@ -196,11 +201,26 @@ where } else { "denied" }; - return Err(TinyAgentsError::Tool(format!( + let message = format!( "tool hook '{}' {kind} {}: {reason}", hook.name(), event.tool_name - ))); + ); + tracing::debug!( + hook = hook.name(), + tool = %event.tool_name, + call_id = %event.call_id, + ask, + stop_run, + "[tool_hooks] refused tool call" + ); + return Err(if ask { + TinyAgentsError::Tool(message) + } else if stop_run { + TinyAgentsError::ToolRefused(message) + } else { + TinyAgentsError::ToolFailed(message) + }); } if let Some(input) = decision.updated_input { call.arguments = input.clone(); @@ -334,6 +354,10 @@ where #[path = "tool_tests.rs"] mod tests; +#[cfg(test)] +#[path = "tool_deny_tests.rs"] +mod deny_tests; + #[async_trait] impl ToolHook for crate::command::HookEngine { fn name(&self) -> &str { diff --git a/crates/tinyagents-hooks/src/tool/tool_deny_tests.rs b/crates/tinyagents-hooks/src/tool/tool_deny_tests.rs new file mode 100644 index 000000000..9cf6c8f90 --- /dev/null +++ b/crates/tinyagents-hooks/src/tool/tool_deny_tests.rs @@ -0,0 +1,240 @@ +use super::*; +use crate::command::{HookOutput, HookPermission}; +use std::sync::Arc; +use tinyagents_harness::context::{RunConfig, RunContext}; +use tinyagents_harness::error::TinyAgentsError; +use tinyagents_harness::host::FixedModelResolver; +use tinyagents_harness::limits::RunLimits; +use tinyagents_harness::middleware::AgentRun; +use tinyagents_harness::runtime::{AgentHarness, RunPolicy}; +use tinyagents_harness::testkit::{FakeTool, ScriptedModel, text_response, tool_call_response}; +use tinyinference_llm::message::Message; +use tinyinference_llm::model::ModelResponse; +use tinyinference_llm::tool::ToolCall; + +struct Host; +impl HookIdentity for Host { + fn session_id(&self) -> Option { + Some("session".into()) + } + fn agent_id(&self) -> Option { + Some("agent".into()) + } + fn cwd(&self) -> Option { + None + } +} + +struct AllowListHook { + allowed: &'static [&'static str], + refusal: HookOutput, +} + +#[async_trait] +impl ToolHook for AllowListHook { + fn name(&self) -> &str { + "allow_list" + } + async fn before_tool_decision( + &self, + _host: &Host, + context: &ToolHookContext, + ) -> ToolHookDecision { + if self.allowed.contains(&context.tool_name.as_str()) { + ToolHookDecision::default() + } else { + self.refusal.clone() + } + } +} + +struct Fixture { + harness: AgentHarness<(), Host>, + model: Arc, + secret: Arc, + lookup: Arc, +} + +fn fixture(refusal: HookOutput, responses: Vec) -> Fixture { + let model = Arc::new(ScriptedModel::new(responses)); + let secret = Arc::new(FakeTool::returning("secret", "secret ran")); + let lookup = Arc::new(FakeTool::returning("lookup", "found")); + let mut harness: AgentHarness<(), Host> = AgentHarness::new(); + harness.with_model_resolver(Arc::new(FixedModelResolver::named( + "scripted", + model.clone(), + ))); + harness.register_tool(secret.clone()); + harness.register_tool(lookup.clone()); + harness.push_middleware(Arc::new(ToolHooksMiddleware::new(vec![Arc::new( + AllowListHook { + allowed: &["lookup"], + refusal, + }, + )]))); + Fixture { + harness, + model, + secret, + lookup, + } +} + +async fn run(fixture: &Fixture) -> tinyagents_harness::error::Result { + fixture + .harness + .invoke_in_context( + &(), + RunContext::new(RunConfig::new("deny"), Host), + vec![Message::user("go")], + ) + .await +} + +fn secret_call(id: &str) -> ModelResponse { + tool_call_response(ToolCall::new( + id, + "secret", + serde_json::json!({"token": "arg-value-must-not-leak"}), + )) +} + +fn tool_result_text(messages: &[Message], call_id: &str) -> Option { + messages.iter().find_map(|message| match message { + Message::Tool(tool) if tool.tool_call_id == call_id => Some(message.text()), + _ => None, + }) +} + +#[tokio::test] +async fn denied_call_is_answered_to_the_model_and_the_run_finishes() { + let fixture = fixture( + HookOutput::deny("secret is not available to this agent"), + vec![secret_call("call-1"), text_response("done without it")], + ); + + let run = run(&fixture) + .await + .expect("a denial must not abort the run"); + + assert!( + fixture.secret.calls().is_empty(), + "a denied tool never runs" + ); + assert_eq!( + run.final_response + .as_ref() + .map(ModelResponse::text) + .as_deref(), + Some("done without it") + ); + let answer = tool_result_text(&run.messages, "call-1").expect("the call is answered"); + assert!( + answer.contains("secret is not available to this agent"), + "the model sees the deny reason: {answer}" + ); + let second_request = &fixture.model.requests()[1]; + assert!(second_request.messages.iter().any(|message| matches!( + message, + Message::Tool(tool) if tool.tool_call_id == "call-1" + ))); +} + +#[tokio::test] +async fn denied_call_with_continue_false_aborts_with_a_refusal_error() { + let refusal = HookOutput { + continue_: Some(false), + ..HookOutput::deny("secret is forbidden") + }; + let fixture = fixture(refusal, vec![secret_call("call-1"), text_response("never")]); + + let error = run(&fixture) + .await + .expect_err("continue_ = false stops the run"); + + assert!( + fixture.secret.calls().is_empty(), + "a denied tool never runs" + ); + let TinyAgentsError::ToolRefused(message) = &error else { + panic!("expected ToolRefused, got {error:?}"); + }; + assert!(message.contains("secret is forbidden"), "{message}"); + assert!(message.contains("secret"), "{message}"); + assert!( + !error.to_string().contains("arg-value-must-not-leak"), + "the refusal never carries tool arguments: {error}" + ); + assert_eq!(fixture.model.requests().len(), 1); +} + +#[tokio::test] +async fn ask_still_refuses_the_run() { + let ask = HookOutput { + permission: Some(HookPermission::Ask), + agent_message: Some("needs a human".into()), + ..Default::default() + }; + let fixture = fixture(ask, vec![secret_call("call-1"), text_response("never")]); + + let error = run(&fixture).await.expect_err("ask is unchanged"); + + assert!(fixture.secret.calls().is_empty()); + assert!( + matches!(&error, TinyAgentsError::Tool(message) if message.contains("requires approval for secret")), + "{error:?}" + ); +} + +#[tokio::test] +async fn blank_tool_name_denied_by_a_hook_is_answered_not_fatal() { + let fixture = fixture( + HookOutput::deny("that tool is not on the allow list"), + vec![ + tool_call_response(ToolCall::new("call-blank", "", serde_json::json!({}))), + tool_call_response(ToolCall::new( + "call-lookup", + "lookup", + serde_json::json!({}), + )), + text_response("recovered"), + ], + ); + + let run = run(&fixture) + .await + .expect("a blank denied name is answered"); + + let answer = tool_result_text(&run.messages, "call-blank").expect("answered"); + assert!(answer.contains("not on the allow list"), "{answer}"); + assert_eq!(fixture.lookup.calls().len(), 1); + assert_eq!( + run.final_response + .as_ref() + .map(ModelResponse::text) + .as_deref(), + Some("recovered") + ); +} + +#[tokio::test] +async fn a_model_that_keeps_calling_a_denied_tool_stops_at_the_model_call_cap() { + let responses = (0..20).map(|i| secret_call(&format!("call-{i}"))).collect(); + let mut fixture = fixture(HookOutput::deny("never allowed"), responses); + fixture.harness.with_policy(RunPolicy { + limits: RunLimits::default().with_max_model_calls(3), + ..RunPolicy::default() + }); + + let error = run(&fixture).await.expect_err("the loop must terminate"); + + assert!( + matches!(error, TinyAgentsError::LimitExceeded(_)), + "{error:?}" + ); + assert_eq!(fixture.model.requests().len(), 3); + assert!( + fixture.secret.calls().is_empty(), + "a denied tool never runs" + ); +} diff --git a/crates/tinyagents-hooks/src/tool/tool_tests.rs b/crates/tinyagents-hooks/src/tool/tool_tests.rs index 3e3c0dec1..cae328106 100644 --- a/crates/tinyagents-hooks/src/tool/tool_tests.rs +++ b/crates/tinyagents-hooks/src/tool/tool_tests.rs @@ -444,23 +444,65 @@ async fn before_hook_can_rewrite_input_and_append_context_together() { } #[tokio::test] -async fn deny_and_ask_both_refuse_calls() { - for decision in [ - HookOutput::deny("blocked"), - HookOutput { - permission: Some(crate::command::HookPermission::Ask), - agent_message: Some("approval".into()), - ..Default::default() - }, - ] { - let middleware = middleware(decision, Arc::default()); - let mut context = run("one"); - assert!( - Middleware::before_tool(&middleware, &mut context, &(), &mut call("id")) - .await - .is_err() - ); - } +async fn deny_answers_the_model_and_ask_fails_the_call() { + let middleware = middleware(HookOutput::deny("blocked"), Arc::default()); + let mut context = run("one"); + let error = Middleware::before_tool(&middleware, &mut context, &(), &mut call("id")) + .await + .unwrap_err(); + assert!( + matches!(&error, tinyagents_harness::error::TinyAgentsError::ToolFailed(message) + if message == "tool hook 'test' denied read: blocked"), + "{error:?}" + ); + + let ask = HookOutput { + permission: Some(crate::command::HookPermission::Ask), + agent_message: Some("approval".into()), + ..Default::default() + }; + let middleware = middleware_for(ask); + let error = Middleware::before_tool(&middleware, &mut run("one"), &(), &mut call("id")) + .await + .unwrap_err(); + assert!( + matches!(&error, tinyagents_harness::error::TinyAgentsError::Tool(message) + if message == "tool hook 'test' requires approval for read: approval"), + "{error:?}" + ); +} + +#[tokio::test] +async fn deny_with_continue_false_stops_the_run() { + let decision = HookOutput { + continue_: Some(false), + ..HookOutput::deny("blocked") + }; + let error = Middleware::before_tool( + &middleware_for(decision), + &mut run("one"), + &(), + &mut call("id"), + ) + .await + .unwrap_err(); + assert!( + matches!(&error, tinyagents_harness::error::TinyAgentsError::ToolRefused(message) + if message == "tool hook 'test' denied read: blocked"), + "{error:?}" + ); +} + +#[tokio::test] +async fn denied_call_leaves_no_pending_post_hook_state() { + let middleware = middleware_for(HookOutput::deny("blocked")); + let mut context = run("one"); + let _ = Middleware::before_tool(&middleware, &mut context, &(), &mut call("id")).await; + assert!(middleware.arguments.lock().unwrap().is_empty()); +} + +fn middleware_for(decision: HookOutput) -> ToolHooksMiddleware { + middleware(decision, Arc::default()) } #[tokio::test] diff --git a/docs/modules/harness/hosting.md b/docs/modules/harness/hosting.md index 819bb85ea..fe455c9d7 100644 --- a/docs/modules/harness/hosting.md +++ b/docs/modules/harness/hosting.md @@ -72,6 +72,13 @@ fn should_retry(error: &HostedError) -> bool { } ``` +`HostedErrorKind::ToolRefused` means host tool policy refused a call and asked +for the run to stop (`TinyAgentsError::ToolRefused`). Its message is +`hosted agent invocation stopped: host policy refused a tool call`, and the +hosted stream's `Failed` item and `RunFailed` event carry the same message when +the run's `TerminalOutcome::reason` is `ToolRefused`. It is not retryable as-is; +the host's policy decides whether the turn can run again. + `TimeoutBound` is `#[non_exhaustive]`; treat an unknown bound like `Run`. `HostedError` gained the public `timeout_bound` field, so code that builds it with a struct literal must set it. Recursive hosted children convert back to diff --git a/docs/modules/harness/terminal-outcome.md b/docs/modules/harness/terminal-outcome.md index ad656b992..07fb77ae9 100644 --- a/docs/modules/harness/terminal-outcome.md +++ b/docs/modules/harness/terminal-outcome.md @@ -6,6 +6,12 @@ The `reason` identifies the specific cause, while `class` provides a stable coarse-grained category for hosts that need only success, timeout, cancellation, failure, or suspension. +`TerminalReason::ToolRefused` (class `Failure`) marks a run that host tool +policy stopped (`TinyAgentsError::ToolRefused`, such as a hook deny with +`continue_: Some(false)`). A refusal answered to the model does not end the +run. The full error-to-reason mapping and merge precedence live in the +`terminal` module docs. + Outcomes preserve the legacy human-readable message. Timeout outcomes also record whether the deadline landed before a provider call, during one, or after a call. Provider-start state is tracked separately so a host can tell a diff --git a/docs/modules/hooks/README.md b/docs/modules/hooks/README.md index 5f9a8c236..0476227aa 100644 --- a/docs/modules/hooks/README.md +++ b/docs/modules/hooks/README.md @@ -16,6 +16,28 @@ the model. `HookIdentity` lets a host provide session, agent, and working-path identity. `sanitize_tool_output` produces a content-free summary for learning records. +A deny never runs the tool. By default it answers the model: the call gets a +failed tool result (`TinyAgentsError::ToolFailed`, folded into +`ToolResult::failed`) reading `tool hook '' denied : `, and +the run continues so the model can pick another tool or answer without it. This +covers a blank or invented tool name too, since hooks run before the +unknown-tool policy. A deny that also sets `continue_: Some(false)` stops the +run with `TinyAgentsError::ToolRefused`; the run ends with +`TerminalReason::ToolRefused` and a hosted caller sees +`HostedErrorKind::ToolRefused` instead of the generic internal failure. The +refusal text names the hook, tool and reason, never the call's arguments. + +| Decision | Tool runs | Run | +| --- | --- | --- | +| `Deny` | no | continues; the model sees the reason | +| `Deny` with `continue_: Some(false)` | no | stops with `ToolRefused` | +| `Ask` | no | fails with `TinyAgentsError::Tool` | + +An answered deny releases its tool-call slot, like every other `before_tool` +refusal, so a model that keeps calling a denied tool is bounded by the +model-call cap (`RunLimits::max_model_calls`), not the tool-call cap. Hosts +that relied on a deny ending the run must set `continue_: Some(false)`. + The command engine is available at `tinyagents_hooks::command`. The runtime re-exports it from `tinyagents_runtime::command_hooks` for one release.