feat(hooks): add PreToolUseResult event and stable tool_call_id across tool lifecycle (#11120)

Signed-off-by: Tymofii Pidlisnyi <171286556+aeoess@users.noreply.github.com>
Signed-off-by: Douwe Osinga <douwe.osinga@gmail.com>
Co-authored-by: Tymofii Pidlisnyi <171286556+aeoess@users.noreply.github.com>
Co-authored-by: Douwe Osinga <douwe.osinga@gmail.com>
This commit is contained in:
Tymofii Pidlisnyi
2026-08-21 00:38:53 +00:00
committed by GitHub
parent 2f5169af35
commit d0fc739a4c
11 changed files with 2661 additions and 288 deletions
+427 -51
View File
@@ -666,16 +666,45 @@ impl Agent {
self.hook_manager.emit(event, ctx).await;
}
/// Observation-only record of what the `PreToolUse` chain decided. Carries
/// no veto: the decision has already been made by the time this runs.
async fn emit_pre_tool_use_result(
&self,
session: &Session,
tool_call_id: &str,
tool_name: &str,
tool_input: Option<&Value>,
outcome: &crate::hooks::HookChainOutcome,
) {
if !self
.hook_manager
.has_hooks(crate::hooks::HookEvent::PreToolUseResult)
{
return;
}
let ctx =
crate::hooks::HookContext::new(crate::hooks::HookEvent::PreToolUseResult, &session.id)
.with_tool(tool_name.to_string(), tool_input.cloned())
.with_tool_call_id(tool_call_id)
.with_working_dir(session.working_dir.to_string_lossy().to_string())
.with_pre_tool_use_outcome(outcome);
self.hook_manager
.emit(crate::hooks::HookEvent::PreToolUseResult, ctx)
.await;
}
fn with_post_tool_hook(
&self,
result: ToolCallResult,
tool_call: &CallToolRequestParams,
session: &Session,
tool_call_id: &str,
) -> ToolCallResult {
let hook_manager = self.hook_manager.clone();
let session_id = session.id.clone();
let working_dir = session.working_dir.to_string_lossy().to_string();
let tool_name = tool_call.name.to_string();
let tool_call_id = tool_call_id.to_string();
let tool_input = tool_call
.arguments
.as_ref()
@@ -706,6 +735,7 @@ impl Agent {
if hook_manager.has_hooks(event) {
let ctx = crate::hooks::HookContext::new(event, &session_id)
.with_tool(tool_name.clone(), tool_input.clone())
.with_tool_call_id(tool_call_id.as_str())
.with_working_dir(working_dir.clone());
hook_manager.emit(event, ctx).await;
}
@@ -1176,66 +1206,79 @@ impl Agent {
.await
.record_tool_arguments(&tool_call.arguments, &session.working_dir);
if self
let tool_input_for_hooks = tool_call
.arguments
.as_ref()
.map(|a| serde_json::Value::Object(a.clone()));
let pre_tool_outcome = if self
.hook_manager
.has_hooks(crate::hooks::HookEvent::PreToolUse)
{
let ctx =
crate::hooks::HookContext::new(crate::hooks::HookEvent::PreToolUse, &session.id)
.with_tool(
tool_call.name.to_string(),
tool_call
.arguments
.as_ref()
.map(|a| serde_json::Value::Object(a.clone())),
)
.with_tool(tool_call.name.to_string(), tool_input_for_hooks.clone())
.with_tool_call_id(request_id.as_str())
.with_working_dir(session.working_dir.to_string_lossy().to_string());
if let crate::hooks::HookDecision::Deny { reason, plugin } = self
.hook_manager
.emit_blocking(crate::hooks::HookEvent::PreToolUse, ctx)
self.hook_manager
.emit_blocking_with_outcome(crate::hooks::HookEvent::PreToolUse, ctx)
.await
{
return (
request_id,
Err(ErrorData::new(
ErrorCode::INTERNAL_ERROR,
format!(
"Tool call denied by policy hook `{plugin}`: {reason}. \
Do not retry; this is a policy denial, not a transient failure."
),
None,
)),
);
}
}
} else {
crate::hooks::HookChainOutcome::allow(false)
};
let tool_input_for_extended = tool_call
.arguments
.as_ref()
.map(|a| serde_json::Value::Object(a.clone()));
self.emit_pre_tool_extended_hooks(
&tool_call.name,
tool_input_for_extended.as_ref(),
// Emitted before the denial returns, so an observer sees the denial
// before the model receives the refusal. Best effort, like every other
// hook emission: a subscriber that fails or is absent changes nothing.
self.emit_pre_tool_use_result(
session,
request_id.as_str(),
&tool_call.name,
tool_input_for_hooks.as_ref(),
&pre_tool_outcome,
)
.await;
if let crate::hooks::HookDecision::Deny { reason, plugin } = pre_tool_outcome.decision {
return (
request_id,
Err(ErrorData::new(
ErrorCode::INTERNAL_ERROR,
format!(
"Tool call denied by policy hook `{plugin}`: {reason}. \
Do not retry; this is a policy denial, not a transient failure."
),
None,
)),
);
}
self.emit_pre_tool_extended_hooks(&tool_call.name, tool_input_for_hooks.as_ref(), session)
.await;
if tool_call.name == FINAL_OUTPUT_TOOL_NAME {
return if let Some(final_output_tool) = self.final_output_tool.lock().await.as_mut() {
let result = final_output_tool.execute_tool_call(tool_call.clone()).await;
(
request_id,
Ok(self.with_post_tool_hook(result, &tool_call, session)),
)
let result = self.with_post_tool_hook(result, &tool_call, session, &request_id);
(request_id, Ok(result))
} else {
(
request_id,
Err(ErrorData::new(
ErrorCode::INTERNAL_ERROR,
"Final output tool not defined".to_string(),
None,
)),
)
// This method has always reported a missing final-output tool as
// the outer error. Keep that contract and emit the failure
// observation directly, the same event the wrapper would emit.
let error = ErrorData::new(
ErrorCode::INTERNAL_ERROR,
"Final output tool not defined".to_string(),
None,
);
let failure = crate::hooks::HookEvent::PostToolUseFailure;
if self.hook_manager.has_hooks(failure) {
let ctx = crate::hooks::HookContext::new(failure, &session.id)
.with_tool(tool_call.name.to_string(), tool_input_for_hooks.clone())
.with_tool_call_id(request_id.as_str())
.with_working_dir(session.working_dir.to_string_lossy().to_string());
self.hook_manager.emit(failure, ctx).await;
}
(request_id, Err(error))
};
}
@@ -1273,10 +1316,8 @@ impl Agent {
debug!("WAITING_TOOL_END: {}", tool_call.name);
(
request_id,
Ok(self.with_post_tool_hook(result, &tool_call, session)),
)
let result = self.with_post_tool_hook(result, &tool_call, session, &request_id);
(request_id, Ok(result))
}
/// Save current extension state to session metadata
@@ -1696,14 +1737,14 @@ impl Agent {
)),
Arc::new(DoctorOperation),
Arc::new(ProjectOperation),
Arc::new(SkillOperation),
Arc::new(RecipeOperation),
Arc::new(SkillOperation::new(self.hook_manager.clone())),
Arc::new(RecipeOperation::new(self.hook_manager.clone())),
Arc::new(ToolExecutionOperation::new(
&self.current_goose_mode,
self.extension_manager.clone(),
self.hook_manager.clone(),
)),
Arc::new(UnknownToolOperation),
Arc::new(UnknownToolOperation::new(self.hook_manager.clone())),
Arc::new(RetryOperation::new(
&self.goal,
&self.grind,
@@ -4223,6 +4264,7 @@ mod tests {
)]))),
&tool_call,
&session,
"call-post-hook",
);
drop(entered);
drop(span);
@@ -5619,4 +5661,338 @@ echo start >> "$PLUGIN_ROOT/hook.log"
assert_eq!(stored.input_tokens, Some(1200));
assert_eq!(stored.output_tokens, Some(340));
}
/// Plugin fixture that can register several events at once, each with its
/// own matcher and script, and read back the JSON payloads a script recorded.
struct RecordingHookEnv {
_temp_dir: TempDir,
plugin_dir: PathBuf,
}
/// (event name, matcher or "" for none, script file name, script body)
type HookSpec<'a> = (&'a str, &'a str, &'a str, &'a str);
impl RecordingHookEnv {
fn new(specs: &[HookSpec<'_>]) -> Self {
let temp_dir = tempfile::tempdir().unwrap();
let plugin_dir = temp_dir.path().join("test-plugin");
std::fs::create_dir_all(plugin_dir.join("hooks")).unwrap();
let entries: Vec<String> = specs
.iter()
.map(|(event, matcher, script, _)| {
let matcher = if matcher.is_empty() {
String::new()
} else {
format!(r#""matcher": "{matcher}", "#)
};
format!(
r#""{event}": [{{{matcher}"hooks": [{{"type": "command", "command": "sh ${{PLUGIN_ROOT}}/{script}"}}]}}]"#
)
})
.collect();
std::fs::write(
plugin_dir.join("hooks/hooks.json"),
format!(r#"{{"hooks": {{{}}}}}"#, entries.join(", ")),
)
.unwrap();
for (_, _, script, script_body) in specs {
std::fs::write(plugin_dir.join(script), script_body).unwrap();
}
Self {
_temp_dir: temp_dir,
plugin_dir,
}
}
fn hook_manager(&self) -> crate::hooks::HookManager {
crate::hooks::HookManager::from_plugins_for_test(vec![DiscoveredPlugin {
name: "test-plugin".into(),
root: self.plugin_dir.clone(),
scope: PluginScope::Project,
}])
}
fn payloads(&self, log: &str) -> Vec<Value> {
std::fs::read_to_string(self.plugin_dir.join(log))
.unwrap_or_default()
.lines()
.filter(|line| !line.trim().is_empty())
.map(|line| serde_json::from_str(line).unwrap())
.collect()
}
}
const RECORD_PRE_SCRIPT: &str =
"#!/bin/sh\ncat >> \"$PLUGIN_ROOT/pre.log\"\nprintf '\\n' >> \"$PLUGIN_ROOT/pre.log\"\nexit 0\n";
const RECORD_RESULT_SCRIPT: &str =
"#!/bin/sh\ncat >> \"$PLUGIN_ROOT/result.log\"\nprintf '\\n' >> \"$PLUGIN_ROOT/result.log\"\nexit 0\n";
const RECORD_POST_SCRIPT: &str =
"#!/bin/sh\ncat >> \"$PLUGIN_ROOT/post.log\"\nprintf '\\n' >> \"$PLUGIN_ROOT/post.log\"\nexit 0\n";
const RECORD_POST_FAILURE_SCRIPT: &str =
"#!/bin/sh\ncat >> \"$PLUGIN_ROOT/postfail.log\"\nprintf '\\n' >> \"$PLUGIN_ROOT/postfail.log\"\nexit 0\n";
const DENY_AND_RECORD_SCRIPT: &str =
"#!/bin/sh\ncat >> \"$PLUGIN_ROOT/pre.log\"\nprintf '\\n' >> \"$PLUGIN_ROOT/pre.log\"\necho \"blocked by test policy\" >&2\nexit 2\n";
/// Logs its stdin like the others, writes nothing to stdout, and exits
/// non-zero. That is a hook that ran but never returned a decision.
const ABNORMAL_EXIT_AND_RECORD_SCRIPT: &str =
"#!/bin/sh\ncat >> \"$PLUGIN_ROOT/pre.log\"\nprintf '\\n' >> \"$PLUGIN_ROOT/pre.log\"\necho boom >&2\nexit 3\n";
async fn agent_with_hooks(
hook_manager: crate::hooks::HookManager,
) -> (Agent, Session, TempDir) {
let data_dir = TempDir::new().unwrap();
let data_path = data_dir.path().to_path_buf();
let session_manager = Arc::new(SessionManager::new(data_path.clone()));
let mut agent = Agent::with_config(AgentConfig::new(
Arc::clone(&session_manager),
Arc::new(PermissionManager::new(data_path)),
None,
GooseMode::default(),
false,
GoosePlatform::GooseCli,
));
agent.set_hook_manager_for_test(hook_manager);
let session = session_manager
.create_session(
std::env::current_dir().unwrap(),
"pre-tool-use-result".to_string(),
SessionType::Hidden,
GooseMode::default(),
)
.await
.unwrap();
(agent, session, data_dir)
}
fn shell_call() -> CallToolRequestParams {
use rmcp::object;
CallToolRequestParams::new("developer__shell")
.with_arguments(object!({ "command": "echo hi" }))
}
/// deny-invisible: the tool never dispatches, neither post event fires, and a
/// PreToolUseResult subscriber still sees the denial with blocked_by and reason.
#[tokio::test]
async fn pre_tool_use_result_observes_denial_that_post_hooks_never_see() {
let env = RecordingHookEnv::new(&[
("PreToolUse", "", "pre.sh", DENY_AND_RECORD_SCRIPT),
("PreToolUseResult", "", "result.sh", RECORD_RESULT_SCRIPT),
("PostToolUse", "", "post.sh", RECORD_POST_SCRIPT),
(
"PostToolUseFailure",
"",
"postfail.sh",
RECORD_POST_FAILURE_SCRIPT,
),
]);
let (agent, session, _data_dir) = agent_with_hooks(env.hook_manager()).await;
let (request_id, result) = agent
.dispatch_tool_call(shell_call(), "call-deny-1".to_string(), None, &session)
.await;
assert_eq!(request_id, "call-deny-1");
let Err(error) = result else {
panic!("a denied call must not dispatch");
};
assert!(error.message.contains("denied by policy hook"));
assert!(
env.payloads("post.log").is_empty(),
"PostToolUse must not fire for a denied call"
);
assert!(
env.payloads("postfail.log").is_empty(),
"PostToolUseFailure must not fire for a denied call"
);
let results = env.payloads("result.log");
assert_eq!(results.len(), 1);
assert_eq!(results[0]["event"], "PreToolUseResult");
assert_eq!(results[0]["decision"], "deny");
assert_eq!(results[0]["policy_evaluated"], true);
assert_eq!(results[0]["blocked_by"], "test-plugin");
assert_eq!(results[0]["reason"], "blocked by test policy");
assert_eq!(results[0]["tool_call_id"], "call-deny-1");
}
/// repeated identical calls: two calls with the same name and input in one
/// session correlate to their outcomes by tool_call_id, not by name plus input.
#[tokio::test]
async fn repeated_identical_calls_correlate_by_tool_call_id() {
let env = RecordingHookEnv::new(&[
("PreToolUse", "", "pre.sh", RECORD_PRE_SCRIPT),
("PreToolUseResult", "", "result.sh", RECORD_RESULT_SCRIPT),
("PostToolUse", "", "post.sh", RECORD_POST_SCRIPT),
(
"PostToolUseFailure",
"",
"postfail.sh",
RECORD_POST_FAILURE_SCRIPT,
),
]);
let (agent, session, _data_dir) = agent_with_hooks(env.hook_manager()).await;
for id in ["call-1", "call-2"] {
let (_, result) = agent
.dispatch_tool_call(shell_call(), id.to_string(), None, &session)
.await;
let Ok(handle) = result else {
panic!("dispatch must return a result handle");
};
let _ = handle.result.await;
}
let pres = env.payloads("pre.log");
let results = env.payloads("result.log");
let outcomes = env.payloads("postfail.log");
assert_eq!(pres.len(), 2);
assert_eq!(results.len(), 2);
assert_eq!(outcomes.len(), 2);
for payloads in [&pres, &results, &outcomes] {
assert_eq!(payloads[0]["tool_name"], payloads[1]["tool_name"]);
assert_eq!(payloads[0]["tool_input"], payloads[1]["tool_input"]);
}
let ids: Vec<&str> = results
.iter()
.map(|payload| payload["tool_call_id"].as_str().unwrap())
.collect();
assert_eq!(ids, vec!["call-1", "call-2"]);
assert_ne!(
ids[0], ids[1],
"identical name and input must still carry distinct ids"
);
for (index, id) in ids.iter().enumerate() {
assert_eq!(
pres[index]["tool_call_id"], results[index]["tool_call_id"],
"PreToolUse and PreToolUseResult must carry one id per call"
);
assert_eq!(
outcomes
.iter()
.filter(|payload| payload["tool_call_id"] == *id)
.count(),
1,
"each call must pair with exactly one outcome by id"
);
}
}
/// no matching hook: a PreToolUse rule is registered but its matcher does not
/// match, so nothing runs and the event reports allow with policy_evaluated false.
#[tokio::test]
async fn pre_tool_use_result_reports_allow_and_unevaluated_when_no_hook_matches() {
let env = RecordingHookEnv::new(&[
(
"PreToolUse",
"a_tool_name_that_never_matches",
"pre.sh",
DENY_AND_RECORD_SCRIPT,
),
("PreToolUseResult", "", "result.sh", RECORD_RESULT_SCRIPT),
]);
let (agent, session, _data_dir) = agent_with_hooks(env.hook_manager()).await;
let (_, result) = agent
.dispatch_tool_call(shell_call(), "call-allow-1".to_string(), None, &session)
.await;
let Ok(handle) = result else {
panic!("dispatch must return a result handle");
};
let _ = handle.result.await;
assert!(
env.payloads("pre.log").is_empty(),
"the non-matching rule must not run"
);
let results = env.payloads("result.log");
assert_eq!(results.len(), 1);
assert_eq!(results[0]["decision"], "allow");
assert_eq!(results[0]["policy_evaluated"], false);
assert!(results[0].get("blocked_by").is_none());
assert!(results[0].get("reason").is_none());
assert_eq!(results[0]["tool_call_id"], "call-allow-1");
}
/// sole abnormal hook: the only matching PreToolUse hook runs, writes nothing
/// to stdout and exits non-zero, so it never returned a decision. Execution
/// stays fail-open and the event reports allow with policy_evaluated false.
#[tokio::test]
async fn pre_tool_use_result_reports_unevaluated_when_the_only_hook_exits_without_a_decision() {
let env = RecordingHookEnv::new(&[
("PreToolUse", "", "pre.sh", ABNORMAL_EXIT_AND_RECORD_SCRIPT),
("PreToolUseResult", "", "result.sh", RECORD_RESULT_SCRIPT),
]);
let (agent, session, _data_dir) = agent_with_hooks(env.hook_manager()).await;
let (_, result) = agent
.dispatch_tool_call(shell_call(), "call-abnormal-1".to_string(), None, &session)
.await;
let Ok(handle) = result else {
panic!("dispatch must stay fail-open and return a result handle");
};
let _ = handle.result.await;
assert_eq!(
env.payloads("pre.log").len(),
1,
"the matching hook must still run",
);
let results = env.payloads("result.log");
assert_eq!(results.len(), 1);
assert_eq!(results[0]["decision"], "allow");
assert_eq!(results[0]["policy_evaluated"], false);
assert_eq!(results[0]["tool_call_id"], "call-abnormal-1");
}
/// inactive final output: the tool is not installed, so nothing executes. The
/// outer error stays the one this method has always returned, and the failure
/// is still observed exactly once, carrying the request id.
#[tokio::test]
async fn inactive_final_output_keeps_the_outer_error_and_emits_one_failure_event() {
use rmcp::object;
let env = RecordingHookEnv::new(&[
("PreToolUseResult", "", "result.sh", RECORD_RESULT_SCRIPT),
("PostToolUse", "", "post.sh", RECORD_POST_SCRIPT),
(
"PostToolUseFailure",
"",
"postfail.sh",
RECORD_POST_FAILURE_SCRIPT,
),
]);
// agent_with_hooks builds the agent through Agent::with_config, which
// leaves final_output_tool as None, so the tool is inactive here without
// any extra setup.
let (agent, session, _data_dir) = agent_with_hooks(env.hook_manager()).await;
let call = CallToolRequestParams::new(FINAL_OUTPUT_TOOL_NAME)
.with_arguments(object!({ "answer": "unused" }));
let (_, result) = agent
.dispatch_tool_call(call, "call-inactive-1".to_string(), None, &session)
.await;
let Err(error) = result else {
panic!("an inactive final-output tool must report the outer error");
};
assert_eq!(error.message, "Final output tool not defined");
assert_eq!(error.code, ErrorCode::INTERNAL_ERROR);
let failures = env.payloads("postfail.log");
assert_eq!(
failures.len(),
1,
"the failure must be observed exactly once",
);
assert_eq!(failures[0]["tool_call_id"], "call-inactive-1");
assert_eq!(failures[0]["tool_name"], FINAL_OUTPUT_TOOL_NAME);
assert!(
env.payloads("post.log").is_empty(),
"PostToolUse must not fire for a tool that never ran",
);
}
}
+2 -1
View File
@@ -8,6 +8,7 @@ use serde_json::Value;
use std::borrow::Cow;
pub const FINAL_OUTPUT_TOOL_NAME: &str = "recipe__final_output";
pub const FINAL_OUTPUT_SUCCESS_MESSAGE: &str = "Final output successfully collected.";
pub const FINAL_OUTPUT_CONTINUATION_MESSAGE: &str =
"You MUST call the `final_output` tool NOW with the final output for the user.";
@@ -131,7 +132,7 @@ impl FinalOutputTool {
Ok(parsed_value) => {
self.final_output = Some(Self::parsed_final_output_string(parsed_value));
ToolCallResult::from(Ok(rmcp::model::CallToolResult::success(vec![
ContentBlock::text("Final output successfully collected.".to_string()),
ContentBlock::text(FINAL_OUTPUT_SUCCESS_MESSAGE.to_string()),
])))
}
Err(error) => ToolCallResult::from(Err(ErrorData {
@@ -4,26 +4,36 @@ use std::collections::HashSet;
use anyhow::{anyhow, Result};
use async_trait::async_trait;
use rmcp::model::Tool;
use rmcp::model::{CallToolResult, ContentBlock, Tool};
use tracing_futures::Instrument;
use crate::agents::final_output_tool::{
FinalOutputTool, FINAL_OUTPUT_CONTINUATION_MESSAGE, FINAL_OUTPUT_TOOL_NAME,
FinalOutputTool, FINAL_OUTPUT_CONTINUATION_MESSAGE, FINAL_OUTPUT_SUCCESS_MESSAGE,
FINAL_OUTPUT_TOOL_NAME,
};
use crate::agents::state_machine::ops_toolcalling::{
pending_tool_requests, tool_span, ToolDisposition,
emit_post_tool_use, pending_tool_requests, run_pre_tool_hooks, tool_span, ToolDisposition,
};
use crate::agents::state_machine::{
applied, ends_turn, last_effective_role, messages_since_kickoff, not_applicable, yielded_with,
ConversationEffect, Emitter, GooseEffect, Operation, OperationResult, SlashCommand,
};
use crate::agents::tool_execution::CHAT_MODE_TOOL_SKIPPED_RESPONSE;
use crate::config::GooseMode;
use crate::conversation::message::{Message, MessageContent};
use crate::conversation::{Conversation, EffectiveRole};
use crate::hooks::HookManager;
use crate::session::Session;
pub struct RecipeOperation;
pub struct RecipeOperation {
hook_manager: HookManager,
}
impl RecipeOperation {
pub fn new(hook_manager: HookManager) -> Self {
Self { hook_manager }
}
fn final_output(session: &Session) -> Result<Option<FinalOutputTool>> {
session
.recipe
@@ -34,16 +44,82 @@ impl RecipeOperation {
.map_err(|error| anyhow!(error))
}
fn assistant_block_bounds(messages: &[Message], message_index: usize) -> (usize, usize) {
let start = (0..message_index)
.rev()
.take_while(|index| messages[*index].role == rmcp::model::Role::Assistant)
.last()
.unwrap_or(message_index);
let end = (message_index + 1..messages.len())
.take_while(|index| messages[*index].role == rmcp::model::Role::Assistant)
.last()
.map_or(message_index + 1, |index| index + 1);
(start, end)
}
fn has_unanswered_siblings(messages: &[Message], request_id: &str) -> bool {
let answered: HashSet<&str> = messages
.iter()
.flat_map(|message| &message.content)
.filter_map(|content| match content {
MessageContent::ToolResponse(response) => Some(response.id.as_str()),
_ => None,
})
.collect();
let Some(message_index) = messages.iter().position(|message| {
message.content.iter().any(|content| {
matches!(
content,
MessageContent::ToolRequest(request) if request.id == request_id
)
})
}) else {
return false;
};
let (start, end) = Self::assistant_block_bounds(messages, message_index);
messages[start..end]
.iter()
.flat_map(|message| &message.content)
.any(|content| match content {
// Another unanswered final-output call is not a reason to wait.
// This operation drains them one per pass, so treating a sibling
// final-output call as unfinished work would deadlock the pair:
// each would wait for the other and neither would be answered.
// Ordinary tool calls still have to finish first.
MessageContent::ToolRequest(request) => {
request.id != request_id
&& !answered.contains(request.id.as_str())
&& !request
.tool_call
.as_ref()
.is_ok_and(|tool_call| tool_call.name == FINAL_OUTPUT_TOOL_NAME)
}
_ => false,
})
}
fn successful_final_output(messages: &[Message]) -> Option<String> {
let answered_responses: HashSet<&str> = messages
.iter()
.flat_map(|message| &message.content)
.filter_map(|content| match content {
MessageContent::ToolResponse(response) => Some(response.id.as_str()),
_ => None,
})
.collect();
let successful_responses: HashSet<&str> = messages
.iter()
.flat_map(|message| &message.content)
.filter_map(|content| match content {
MessageContent::ToolResponse(response)
if response
.tool_result
.as_ref()
.is_ok_and(|result| result.is_error != Some(true)) =>
if response.tool_result.as_ref().is_ok_and(|result| {
result.is_error != Some(true)
&& result.content.iter().any(|content| {
content
.as_text()
.is_some_and(|text| text.text == FINAL_OUTPUT_SUCCESS_MESSAGE)
})
}) =>
{
Some(response.id.as_str())
}
@@ -51,25 +127,42 @@ impl RecipeOperation {
})
.collect();
messages
.iter()
.rev()
.flat_map(|message| message.content.iter().rev())
.find_map(|content| match content {
MessageContent::ToolRequest(request)
if successful_responses.contains(request.id.as_str()) =>
{
request.tool_call.as_ref().ok().and_then(|tool_call| {
(tool_call.name == FINAL_OUTPUT_TOOL_NAME).then(|| {
serde_json::Value::Object(
tool_call.arguments.clone().unwrap_or_default(),
)
.to_string()
for (message_index, message) in messages.iter().enumerate().rev() {
let output = message
.content
.iter()
.rev()
.find_map(|content| match content {
MessageContent::ToolRequest(request)
if successful_responses.contains(request.id.as_str()) =>
{
request.tool_call.as_ref().ok().and_then(|tool_call| {
(tool_call.name == FINAL_OUTPUT_TOOL_NAME).then(|| {
serde_json::Value::Object(
tool_call.arguments.clone().unwrap_or_default(),
)
.to_string()
})
})
})
}
_ => None,
})
}
_ => None,
});
if output.is_some() {
let (block_start, block_end) =
Self::assistant_block_bounds(messages, message_index);
let siblings_answered = messages[block_start..block_end]
.iter()
.flat_map(|message| &message.content)
.all(|content| match content {
MessageContent::ToolRequest(request) => {
answered_responses.contains(request.id.as_str())
}
_ => true,
});
return siblings_answered.then_some(output).flatten();
}
}
None
}
async fn command_error(
@@ -200,24 +293,81 @@ impl Operation<Session, GooseEffect> for RecipeOperation {
.is_ok_and(|tool_call| tool_call.name == FINAL_OUTPUT_TOOL_NAME)
});
if let Some((request, _)) = pending {
if session.goose_mode == GooseMode::Chat {
let mut response = Message::user();
response.add_tool_response_with_metadata(
request.id,
Ok(CallToolResult::success(vec![ContentBlock::text(
CHAT_MODE_TOOL_SKIPPED_RESPONSE,
)])),
request.metadata.as_ref(),
);
let response = emit.message(response).await;
return applied([response.into()]);
}
if Self::has_unanswered_siblings(messages, &request.id) {
return not_applicable();
}
let tool_call = request
.tool_call
.map_err(|error| anyhow!("final output tool call could not be parsed: {error}"))?;
let span = tool_span(&tool_call.name, &request.id, &session.id);
let result = final_output
.execute_tool_call(tool_call)
.instrument(span.clone())
.await;
let output = result.result.instrument(span.clone()).await;
match &output {
Ok(result) if result.is_error == Some(true) => {
span.record("error.type", "tool_error");
// `recipe__final_output` is executed here rather than by
// ToolExecutionOperation, which is registered after this one. Run the
// same hook lifecycle it would have run, so the state machine and the
// legacy loop agree on what a final-output call emits.
let tool_input = tool_call
.arguments
.as_ref()
.map(|arguments| serde_json::Value::Object(arguments.clone()));
let output = match run_pre_tool_hooks(
&self.hook_manager,
session,
&request.id,
&tool_call.name,
tool_input.as_ref(),
)
.instrument(span.clone())
.await
{
// A denial returns before execution and emits no post event, the
// same shape ToolExecutionOperation has: its dispatch returns the
// denial before the post-hook wrapper is ever applied.
Err(denial) => Err(denial),
Ok(()) => {
let result = final_output
.execute_tool_call(tool_call.clone())
.instrument(span.clone())
.await;
let output = result.result.instrument(span.clone()).await;
match &output {
Ok(result) if result.is_error == Some(true) => {
span.record("error.type", "tool_error");
}
Err(_) => {
span.record("error.type", "tool_execution_error");
}
_ => {}
}
// Post event carries the same tool_call_id as the pre events.
// The large-response rewrite ToolExecutionOperation applies is
// deliberately not reused: the recipe's structured output is
// the deliverable, not a payload to offload to a temp file.
emit_post_tool_use(
&self.hook_manager,
&session.id,
&session.working_dir.to_string_lossy(),
&tool_call.name,
&request.id,
tool_input.as_ref(),
&output,
)
.instrument(span.clone())
.await;
output
}
Err(_) => {
span.record("error.type", "tool_execution_error");
}
_ => {}
}
};
let mut response = Message::user();
response.add_tool_response_with_metadata(request.id, output, request.metadata.as_ref());
let response = emit.message(response).await;
@@ -5,13 +5,14 @@ use std::path::{Path, PathBuf};
use anyhow::{anyhow, Result};
use async_trait::async_trait;
use goose_sdk_types::custom_requests::{SourceEntry, SourceType};
use rmcp::model::{CallToolResult, ContentBlock, JsonObject, Tool};
use rmcp::model::{CallToolResult, ContentBlock, ErrorData, JsonObject, Tool};
use schemars::{schema_for, JsonSchema};
use serde::Deserialize;
use serde_json::Value;
use tracing_futures::Instrument;
use crate::agents::state_machine::ops_toolcalling::{
pending_tool_requests, tool_span, ToolDisposition,
emit_post_tool_use, pending_tool_requests, run_pre_tool_hooks, tool_span, ToolDisposition,
};
use crate::agents::state_machine::{
applied, messages_since_kickoff, not_applicable, yielded_with, ConversationEffect, Emitter,
@@ -21,11 +22,14 @@ use crate::agents::tool_execution::{CHAT_MODE_TOOL_SKIPPED_RESPONSE, DECLINED_RE
use crate::config::GooseMode;
use crate::conversation::message::Message;
use crate::conversation::Conversation;
use crate::hooks::HookManager;
use crate::session::Session;
const LOAD_SKILL_TOOL_NAME: &str = "load_skill";
pub struct SkillOperation;
pub struct SkillOperation {
hook_manager: HookManager,
}
#[derive(Deserialize, JsonSchema)]
struct LoadSkillParams {
@@ -187,6 +191,10 @@ fn load_supporting_file(
}
impl SkillOperation {
pub fn new(hook_manager: HookManager) -> Self {
Self { hook_manager }
}
async fn command_response(
conversation: &Conversation,
message: String,
@@ -308,38 +316,80 @@ impl Operation<Session, GooseEffect> for SkillOperation {
let mut response = Message::user();
for (request, disposition) in pending {
let result = match disposition {
let result: std::result::Result<CallToolResult, ErrorData> = match disposition {
ToolDisposition::Execute if session.goose_mode == GooseMode::Chat => {
CallToolResult::success(vec![ContentBlock::text(CHAT_MODE_TOOL_SKIPPED_RESPONSE)])
// Nothing executes in chat mode, so no tool lifecycle runs.
Ok(CallToolResult::success(vec![ContentBlock::text(
CHAT_MODE_TOOL_SKIPPED_RESPONSE,
)]))
}
ToolDisposition::Execute => {
let tool_call = request.tool_call.as_ref().map_err(|error| {
anyhow!("load_skill tool call could not be parsed: {error}")
})?;
let span = tool_span(&tool_call.name, &request.id, &session.id);
let result = {
let _entered = span.enter();
execute_skill(&session.working_dir, tool_call.arguments.clone())
};
if result.is_error == Some(true) {
span.record("error.type", "tool_error");
// `load_skill` is executed here rather than by
// ToolExecutionOperation, which is registered after this one.
// Run the same hook lifecycle it would have run, so the state
// machine and the legacy loop agree on what a skill load emits.
let tool_input = tool_call
.arguments
.as_ref()
.map(|arguments| Value::Object(arguments.clone()));
match run_pre_tool_hooks(
&self.hook_manager,
session,
&request.id,
&tool_call.name,
tool_input.as_ref(),
)
.instrument(span.clone())
.await
{
// A denial returns before execution and emits no post
// event, the same shape ToolExecutionOperation has: its
// dispatch returns the denial before the post-hook wrapper
// is ever applied.
Err(denial) => Err(denial),
Ok(()) => {
let result = {
let _entered = span.enter();
execute_skill(&session.working_dir, tool_call.arguments.clone())
};
if result.is_error == Some(true) {
span.record("error.type", "tool_error");
}
let output = Ok(result);
// Post event carries the same tool_call_id as the pre
// events. The large-response rewrite
// ToolExecutionOperation applies is deliberately not
// reused: a skill body is content the model is meant to
// read, not a payload to offload to a temp file.
emit_post_tool_use(
&self.hook_manager,
&session.id,
&session.working_dir.to_string_lossy(),
&tool_call.name,
&request.id,
tool_input.as_ref(),
&output,
)
.instrument(span.clone())
.await;
output
}
}
result
}
ToolDisposition::Decline => {
CallToolResult::error(vec![ContentBlock::text(DECLINED_RESPONSE)])
}
ToolDisposition::Decline => Ok(CallToolResult::error(vec![ContentBlock::text(
DECLINED_RESPONSE,
)])),
ToolDisposition::ParseError(error) => {
CallToolResult::error(vec![ContentBlock::text(format!(
Ok(CallToolResult::error(vec![ContentBlock::text(format!(
"The tool call could not be parsed: {error}. Correct the arguments and try again."
))])
))]))
}
};
response.add_tool_response_with_metadata(
request.id,
Ok(result),
request.metadata.as_ref(),
);
response.add_tool_response_with_metadata(request.id, result, request.metadata.as_ref());
}
let response = emit.message(response).await;
applied([response.into()])
@@ -22,7 +22,7 @@ use crate::config::GooseMode;
use crate::conversation::message::{ActionRequiredData, Message, MessageContent, ToolRequest};
use crate::conversation::Conversation;
use crate::hints::load_hints::SubdirectoryHintTracker;
use crate::hooks::{HookContext, HookDecision, HookEvent, HookManager};
use crate::hooks::{HookChainOutcome, HookContext, HookDecision, HookEvent, HookManager};
use crate::session::{EnabledExtensionsState, ExtensionState, Session};
use std::sync::Arc;
use tokio::sync::Mutex;
@@ -80,6 +80,239 @@ pub(super) fn tool_span(tool_name: &str, tool_call_id: &str, session_id: &str) -
)
}
/// Observation-only record of what the `PreToolUse` chain decided. Carries
/// no veto: the decision has already been made by the time this runs.
async fn emit_pre_tool_use_result(
hook_manager: &HookManager,
session: &Session,
tool_call_id: &str,
tool_name: &str,
tool_input: Option<&serde_json::Value>,
outcome: &HookChainOutcome,
) {
if !hook_manager.has_hooks(HookEvent::PreToolUseResult) {
return;
}
let context = HookContext::new(HookEvent::PreToolUseResult, &session.id)
.with_tool(tool_name.to_string(), tool_input.cloned())
.with_tool_call_id(tool_call_id)
.with_working_dir(session.working_dir.to_string_lossy().to_string())
.with_pre_tool_use_outcome(outcome);
hook_manager
.emit(HookEvent::PreToolUseResult, context)
.await;
}
/// Runs the `PreToolUse` chain, emits `PreToolUseResult`, and reports a denial
/// as the error the caller must return instead of executing.
///
/// Shared rather than duplicated because it carries policy: which decisions
/// block, what `policy_evaluated` means, and that the result event is emitted
/// on both the allow and the deny path. `RecipeOperation` executes
/// `recipe__final_output` without going through [`ToolExecutionOperation`], so
/// it calls this to get the identical lifecycle rather than its own copy.
pub(super) async fn run_pre_tool_hooks(
hook_manager: &HookManager,
session: &Session,
tool_call_id: &str,
tool_name: &str,
tool_input: Option<&serde_json::Value>,
) -> std::result::Result<(), ErrorData> {
let outcome = if hook_manager.has_hooks(HookEvent::PreToolUse) {
let context = HookContext::new(HookEvent::PreToolUse, &session.id)
.with_tool(tool_name.to_string(), tool_input.cloned())
.with_tool_call_id(tool_call_id)
.with_working_dir(session.working_dir.to_string_lossy().to_string());
hook_manager
.emit_blocking_with_outcome(HookEvent::PreToolUse, context)
.await
} else {
HookChainOutcome::allow(false)
};
// Emitted before the denial returns, so an observer sees the denial
// before the model receives the refusal. Best effort, like every other
// hook emission: a subscriber that fails or is absent changes nothing.
emit_pre_tool_use_result(
hook_manager,
session,
tool_call_id,
tool_name,
tool_input,
&outcome,
)
.await;
if let HookDecision::Deny { reason, plugin } = outcome.decision {
tracing::Span::current().record("error.type", "hook_denied");
return Err(ErrorData::new(
rmcp::model::ErrorCode::INTERNAL_ERROR,
format!(
"Tool call denied by policy hook `{plugin}`: {reason}. \
Do not retry; this is a policy denial, not a transient failure."
),
None,
));
}
Ok(())
}
async fn emit_with_matcher(
hook_manager: &HookManager,
event: HookEvent,
session: &Session,
matcher: String,
tool_name: &str,
tool_input: Option<serde_json::Value>,
) {
if !hook_manager.has_hooks(event) {
return;
}
let mut context = HookContext::new(event, &session.id)
.with_tool(tool_name.to_string(), tool_input)
.with_working_dir(session.working_dir.to_string_lossy().to_string());
context.matcher_context = Some(matcher);
hook_manager.emit(event, context).await;
}
pub(super) async fn emit_extended_pre_hooks(
hook_manager: &HookManager,
tool_name: &str,
tool_input: Option<&serde_json::Value>,
session: &Session,
) {
let (event, matcher) = match categorize_tool(tool_name) {
ToolCategory::Shell => (
HookEvent::BeforeShellExecution,
tool_input.and_then(|input| string_argument(input, &["command"])),
),
ToolCategory::Read => (
HookEvent::BeforeReadFile,
tool_input.and_then(|input| string_argument(input, &["path", "file", "file_path"])),
),
ToolCategory::Write | ToolCategory::Other => return,
};
if let Some(matcher) = matcher {
emit_with_matcher(
hook_manager,
event,
session,
matcher,
tool_name,
tool_input.cloned(),
)
.await;
}
}
/// Emits the post-tool event for a finished call and reports which one fired.
///
/// Which event fires is policy: a result flagged `is_error` counts as a failure,
/// as does a transport error. Shared so every execution path classifies the
/// outcome the same way. Takes the session id and working dir as strings because
/// the caller inside [`with_post_tool_hooks`] holds owned copies, not a session.
pub(super) async fn emit_post_tool_use(
hook_manager: &HookManager,
session_id: &str,
working_dir: &str,
tool_name: &str,
tool_call_id: &str,
tool_input: Option<&serde_json::Value>,
result: &std::result::Result<CallToolResult, ErrorData>,
) -> HookEvent {
let event = match result {
Ok(result) if result.is_error != Some(true) => HookEvent::PostToolUse,
_ => HookEvent::PostToolUseFailure,
};
if hook_manager.has_hooks(event) {
let context = HookContext::new(event, session_id)
.with_tool(tool_name.to_string(), tool_input.cloned())
.with_tool_call_id(tool_call_id)
.with_working_dir(working_dir.to_string());
hook_manager.emit(event, context).await;
}
event
}
/// Wraps a tool result so the post-tool event fires once the call completes,
/// carrying the same `tool_call_id` the pre events carried.
pub(super) fn with_post_tool_hooks(
hook_manager: &HookManager,
result: ToolCallResult,
tool_call: &CallToolRequestParams,
session: &Session,
span: tracing::Span,
tool_call_id: &str,
) -> ToolCallResult {
let hook_manager = hook_manager.clone();
let session_id = session.id.clone();
let working_dir = session.working_dir.to_string_lossy().to_string();
let tool_name = tool_call.name.to_string();
let tool_call_id = tool_call_id.to_string();
let tool_input = tool_call
.arguments
.as_ref()
.map(|arguments| serde_json::Value::Object(arguments.clone()));
let category = categorize_tool(&tool_name);
let future = async move {
let result =
crate::agents::large_response_handler::process_tool_response(result.result.await);
match &result {
Ok(result) if result.is_error == Some(true) => {
tracing::Span::current().record("error.type", "tool_error");
}
Err(_) => {
tracing::Span::current().record("error.type", "tool_execution_error");
}
_ => {}
}
let event = emit_post_tool_use(
&hook_manager,
&session_id,
&working_dir,
&tool_name,
&tool_call_id,
tool_input.as_ref(),
&result,
)
.await;
if event == HookEvent::PostToolUse {
let extended = match category {
ToolCategory::Shell => Some((
HookEvent::AfterShellExecution,
tool_input
.as_ref()
.and_then(|input| string_argument(input, &["command"])),
)),
ToolCategory::Write => Some((
HookEvent::AfterFileEdit,
tool_input
.as_ref()
.and_then(|input| string_argument(input, &["path", "file", "file_path"])),
)),
ToolCategory::Read | ToolCategory::Other => None,
};
if let Some((event, Some(matcher))) = extended {
if hook_manager.has_hooks(event) {
let mut context = HookContext::new(event, &session_id)
.with_tool(tool_name, tool_input)
.with_working_dir(working_dir);
context.matcher_context = Some(matcher);
hook_manager.emit(event, context).await;
}
}
}
result
}
.instrument(span);
ToolCallResult {
notification_stream: result.notification_stream,
action_required_stream: result.action_required_stream,
result: Box::new(future.boxed()),
}
}
pub struct ToolExecutionOperation<'a> {
goose_mode: &'a Mutex<GooseMode>,
extension_manager: Arc<ExtensionManager>,
@@ -99,122 +332,6 @@ impl<'a> ToolExecutionOperation<'a> {
}
}
async fn emit_with_matcher(
&self,
event: HookEvent,
session: &Session,
matcher: String,
tool_name: &str,
tool_input: Option<serde_json::Value>,
) {
if !self.hook_manager.has_hooks(event) {
return;
}
let mut context = HookContext::new(event, &session.id)
.with_tool(tool_name.to_string(), tool_input)
.with_working_dir(session.working_dir.to_string_lossy().to_string());
context.matcher_context = Some(matcher);
self.hook_manager.emit(event, context).await;
}
async fn emit_extended_pre_hooks(
&self,
tool_name: &str,
tool_input: Option<&serde_json::Value>,
session: &Session,
) {
let (event, matcher) = match categorize_tool(tool_name) {
ToolCategory::Shell => (
HookEvent::BeforeShellExecution,
tool_input.and_then(|input| string_argument(input, &["command"])),
),
ToolCategory::Read => (
HookEvent::BeforeReadFile,
tool_input.and_then(|input| string_argument(input, &["path", "file", "file_path"])),
),
ToolCategory::Write | ToolCategory::Other => return,
};
if let Some(matcher) = matcher {
self.emit_with_matcher(event, session, matcher, tool_name, tool_input.cloned())
.await;
}
}
fn with_post_hooks(
&self,
result: ToolCallResult,
tool_call: &CallToolRequestParams,
session: &Session,
span: tracing::Span,
) -> ToolCallResult {
let hook_manager = self.hook_manager.clone();
let session_id = session.id.clone();
let working_dir = session.working_dir.to_string_lossy().to_string();
let tool_name = tool_call.name.to_string();
let tool_input = tool_call
.arguments
.as_ref()
.map(|arguments| serde_json::Value::Object(arguments.clone()));
let category = categorize_tool(&tool_name);
let future = async move {
let result =
crate::agents::large_response_handler::process_tool_response(result.result.await);
match &result {
Ok(result) if result.is_error == Some(true) => {
tracing::Span::current().record("error.type", "tool_error");
}
Err(_) => {
tracing::Span::current().record("error.type", "tool_execution_error");
}
_ => {}
}
let event = match &result {
Ok(result) if result.is_error != Some(true) => HookEvent::PostToolUse,
_ => HookEvent::PostToolUseFailure,
};
if hook_manager.has_hooks(event) {
let context = HookContext::new(event, &session_id)
.with_tool(tool_name.clone(), tool_input.clone())
.with_working_dir(working_dir.clone());
hook_manager.emit(event, context).await;
}
if event == HookEvent::PostToolUse {
let extended = match category {
ToolCategory::Shell => Some((
HookEvent::AfterShellExecution,
tool_input
.as_ref()
.and_then(|input| string_argument(input, &["command"])),
)),
ToolCategory::Write => Some((
HookEvent::AfterFileEdit,
tool_input.as_ref().and_then(|input| {
string_argument(input, &["path", "file", "file_path"])
}),
)),
ToolCategory::Read | ToolCategory::Other => None,
};
if let Some((event, Some(matcher))) = extended {
if hook_manager.has_hooks(event) {
let mut context = HookContext::new(event, &session_id)
.with_tool(tool_name, tool_input)
.with_working_dir(working_dir);
context.matcher_context = Some(matcher);
hook_manager.emit(event, context).await;
}
}
}
result
}
.instrument(span);
ToolCallResult {
notification_stream: result.notification_stream,
action_required_stream: result.action_required_stream,
result: Box::new(future.boxed()),
}
}
async fn dispatch_tool_call(
&self,
tool_call: CallToolRequestParams,
@@ -230,33 +347,27 @@ impl<'a> ToolExecutionOperation<'a> {
.arguments
.as_ref()
.map(|arguments| serde_json::Value::Object(arguments.clone()));
if self.hook_manager.has_hooks(HookEvent::PreToolUse) {
let context = HookContext::new(HookEvent::PreToolUse, &session.id)
.with_tool(tool_call.name.to_string(), tool_input.clone())
.with_working_dir(session.working_dir.to_string_lossy().to_string());
if let HookDecision::Deny { reason, plugin } = self
.hook_manager
.emit_blocking(HookEvent::PreToolUse, context)
.await
{
tracing::Span::current().record("error.type", "hook_denied");
return Err(ErrorData::new(
rmcp::model::ErrorCode::INTERNAL_ERROR,
format!(
"Tool call denied by policy hook `{plugin}`: {reason}. \
Do not retry; this is a policy denial, not a transient failure."
),
None,
));
}
}
self.emit_extended_pre_hooks(&tool_call.name, tool_input.as_ref(), session)
.await;
run_pre_tool_hooks(
&self.hook_manager,
session,
request_id.as_str(),
&tool_call.name,
tool_input.as_ref(),
)
.await?;
emit_extended_pre_hooks(
&self.hook_manager,
&tool_call.name,
tool_input.as_ref(),
session,
)
.await;
let context = crate::agents::tool_execution::ToolCallContext::new(
session.id.clone(),
Some(session.working_dir.clone()),
Some(request_id),
Some(request_id.clone()),
);
let result = self
.extension_manager
@@ -270,7 +381,14 @@ impl<'a> ToolExecutionOperation<'a> {
);
ToolCallResult::from(Err(error))
});
Ok(self.with_post_hooks(result, &tool_call, session, result_span))
Ok(with_post_tool_hooks(
&self.hook_manager,
result,
&tool_call,
session,
result_span,
&request_id,
))
}
.instrument(span)
.await
@@ -2,22 +2,36 @@
use anyhow::Result;
use async_trait::async_trait;
use rmcp::model::{CallToolResult, ContentBlock};
use rmcp::model::{CallToolResult, ContentBlock, ErrorCode, ErrorData};
use tracing_futures::Instrument;
use crate::agents::final_output_tool::FINAL_OUTPUT_TOOL_NAME;
use crate::agents::state_machine::effects::GooseEffect;
use crate::agents::state_machine::ops_toolcalling::{
pending_tool_requests, tool_span, ToolDisposition,
emit_extended_pre_hooks, emit_post_tool_use, pending_tool_requests, run_pre_tool_hooks,
tool_span, ToolDisposition,
};
use crate::agents::state_machine::{
applied, messages_since_kickoff, not_applicable, Emitter, Operation, OperationResult,
};
use crate::agents::tool_execution::{CHAT_MODE_TOOL_SKIPPED_RESPONSE, DECLINED_RESPONSE};
use crate::config::GooseMode;
use crate::conversation::message::Message;
use crate::conversation::Conversation;
use crate::hooks::HookManager;
use crate::session::Session;
pub(super) const UNCLAIMED_TOOL_ERROR: &str = "goose.unclaimed_tool";
pub struct UnknownToolOperation;
pub struct UnknownToolOperation {
hook_manager: HookManager,
}
impl UnknownToolOperation {
pub fn new(hook_manager: HookManager) -> Self {
Self { hook_manager }
}
}
#[async_trait]
impl Operation<Session, GooseEffect> for UnknownToolOperation {
@@ -31,7 +45,26 @@ impl Operation<Session, GooseEffect> for UnknownToolOperation {
conversation: &Conversation,
emit: &Emitter,
) -> Result<OperationResult<GooseEffect>> {
let pending = pending_tool_requests(messages_since_kickoff(conversation)?);
let active_final_output = session
.recipe
.as_ref()
.is_some_and(|recipe| recipe.response.is_some());
let pending: Vec<_> = pending_tool_requests(messages_since_kickoff(conversation)?)
.into_iter()
.filter(|(request, disposition)| {
// Reserve for RecipeOperation only the final-output calls it will
// actually execute. A declined one is left here so it still gets a
// response; RecipeOperation matches on Execute alone and would
// otherwise leave the request unanswered, which strict providers can
// reject on the next request.
!(active_final_output
&& *disposition == ToolDisposition::Execute
&& request
.tool_call
.as_ref()
.is_ok_and(|tool_call| tool_call.name == FINAL_OUTPUT_TOOL_NAME))
})
.collect();
if pending.is_empty() {
return not_applicable();
}
@@ -44,7 +77,6 @@ impl Operation<Session, GooseEffect> for UnknownToolOperation {
.map(|tool_call| tool_call.name.as_ref())
.unwrap_or("unknown");
let span = tool_span(tool_name, &request.id, &session.id);
span.record("error.type", "tool_not_available");
let (result, unclaimed) = match disposition {
ToolDisposition::ParseError(error) => (
Ok(CallToolResult::error(vec![ContentBlock::text(format!(
@@ -52,26 +84,91 @@ impl Operation<Session, GooseEffect> for UnknownToolOperation {
))])),
false,
),
ToolDisposition::Execute | ToolDisposition::Decline => request
.tool_call
.as_ref()
.map(|tool_call| {
(
Ok(CallToolResult::error(vec![ContentBlock::text(format!(
"Tool '{}' is not available.",
tool_call.name
))])),
true,
)
})
.unwrap_or_else(|error| {
(
ToolDisposition::Execute if session.goose_mode == GooseMode::Chat => (
Ok(CallToolResult::success(vec![ContentBlock::text(
CHAT_MODE_TOOL_SKIPPED_RESPONSE,
)])),
false,
),
ToolDisposition::Execute => {
match request.tool_call.as_ref() {
Ok(tool_call) => {
let tool_input = tool_call
.arguments
.as_ref()
.map(|arguments| serde_json::Value::Object(arguments.clone()));
match run_pre_tool_hooks(
&self.hook_manager,
session,
&request.id,
&tool_call.name,
tool_input.as_ref(),
)
.instrument(span.clone())
.await
{
Err(denial) => (Err(denial), false),
Ok(()) => {
emit_extended_pre_hooks(
&self.hook_manager,
&tool_call.name,
tool_input.as_ref(),
session,
)
.instrument(span.clone())
.await;
let (output, unclaimed) =
if tool_call.name == FINAL_OUTPUT_TOOL_NAME {
span.record("error.type", "final_output_not_defined");
(
Err(ErrorData::new(
ErrorCode::INTERNAL_ERROR,
"Final output tool not defined".to_string(),
None,
)),
false,
)
} else {
span.record("error.type", "tool_not_available");
(
Ok(CallToolResult::error(vec![
ContentBlock::text(format!(
"Tool '{}' is not available.",
tool_call.name
)),
])),
true,
)
};
emit_post_tool_use(
&self.hook_manager,
&session.id,
&session.working_dir.to_string_lossy(),
&tool_call.name,
&request.id,
tool_input.as_ref(),
&output,
)
.instrument(span.clone())
.await;
(output, unclaimed)
}
}
}
Err(error) => (
Ok(CallToolResult::error(vec![ContentBlock::text(format!(
"The tool call could not be parsed: {error}."
))])),
false,
)
}),
),
}
}
ToolDisposition::Decline => (
Ok(CallToolResult::error(vec![ContentBlock::text(
DECLINED_RESPONSE,
)])),
false,
),
};
let mut metadata = request.metadata.clone();
if unclaimed {
File diff suppressed because it is too large Load Diff
@@ -143,14 +143,14 @@ impl TestPipeline {
)),
Arc::new(DoctorOperation),
Arc::new(ProjectOperation),
Arc::new(SkillOperation),
Arc::new(RecipeOperation),
Arc::new(SkillOperation::new(self.hook_manager.clone())),
Arc::new(RecipeOperation::new(self.hook_manager.clone())),
Arc::new(ToolExecutionOperation::new(
&self.goose_mode,
self.extension_manager.clone(),
self.hook_manager.clone(),
)),
Arc::new(UnknownToolOperation),
Arc::new(UnknownToolOperation::new(self.hook_manager.clone())),
Arc::new(RetryOperation::new(
&self.goal,
&self.grind,
@@ -10,6 +10,7 @@ use crate::agents::final_output_tool::FINAL_OUTPUT_TOOL_NAME;
use crate::agents::platform_extensions::MANAGE_EXTENSIONS_TOOL_NAME_COMPLETE;
use crate::agents::tool_execution::CHAT_MODE_TOOL_SKIPPED_RESPONSE;
use crate::config::GooseMode;
use crate::conversation::message::MessageContent;
use crate::recipe::{Recipe, Response};
use goose_providers::model::ModelConfig;
@@ -81,7 +82,7 @@ async fn reconstruction_and_session_isolation() -> Result<()> {
.with_goose_mode(GooseMode::Chat)
.await;
let pipeline = pipeline.reconstruct().await?;
let pipeline = pipeline.reconstruct().await?.with_max_turns(2);
let restored = pipeline.session().await?;
assert_eq!(restored.provider_name.as_deref(), Some("openai"));
assert_eq!(
@@ -102,14 +103,50 @@ async fn reconstruction_and_session_isolation() -> Result<()> {
FINAL_OUTPUT_TOOL_NAME,
json!({ "answer": "state restored" }),
);
api.on(CHAT_MODE_TOOL_SKIPPED_RESPONSE)
.reply("structured output stayed disabled in chat mode");
let restored_call_index = api.call_count();
let restored_result = pipeline.run(["check restored state"]).await?;
restored_result.assert_message(-1, Agent, r#"{"answer":"state restored"}"#);
let restored_messages = restored_result.conversation().messages();
assert!(
restored_messages
.iter()
.flat_map(|message| &message.content)
.any(|content| match content {
MessageContent::ToolResponse(response) =>
response
.tool_result
.as_ref()
.is_ok_and(|result| result.content.iter().any(|content| {
content
.as_text()
.is_some_and(|text| text.text == CHAT_MODE_TOOL_SKIPPED_RESPONSE)
})),
_ => false,
}),
"a restored Chat-mode recipe must skip its final-output tool"
);
assert!(
restored_messages
.iter()
.all(|message| message.as_concat_text() != r#"{"answer":"state restored"}"#),
"a skipped final-output call must not be collected after reconstruction"
);
let restored_call = api.calls()[restored_call_index].clone();
assert!(restored_call.uses_model("gpt-4o"));
assert!(restored_call.advertises_tool("analyze"));
assert!(restored_call.advertises_tool(FINAL_OUTPUT_TOOL_NAME));
let pipeline = pipeline.with_goose_mode(GooseMode::Auto).await;
let pipeline = pipeline.reconstruct().await?;
api.on("collect restored structured state").call(
FINAL_OUTPUT_TOOL_NAME,
json!({ "answer": "state restored" }),
);
let restored_auto = pipeline.run(["collect restored structured state"]).await?;
restored_auto.assert_message(-1, Agent, r#"{"answer":"state restored"}"#);
let pipeline = pipeline.with_goose_mode(GooseMode::Chat).await;
pipeline
.session_manager
.update(&pipeline.session_id)
@@ -118,7 +155,8 @@ async fn reconstruction_and_session_isolation() -> Result<()> {
.await?;
let pipeline = pipeline.reconstruct().await?;
api.on("try the restored calculator").call(ADD, value(1));
api.on(CHAT_MODE_TOOL_SKIPPED_RESPONSE)
let next_tool_call_id = format!("dummy-tool-call-{}", api.call_count() + 1);
api.on(next_tool_call_id)
.reply("chat mode kept the tool idle");
let chat = pipeline.run(["try the restored calculator"]).await?;
chat.assert_message(-2, ToolResponse, CHAT_MODE_TOOL_SKIPPED_RESPONSE);
+262 -7
View File
@@ -50,6 +50,7 @@ const DEFAULT_HOOK_TIMEOUT_SECS: u64 = 30;
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub enum HookEvent {
PreToolUse,
PreToolUseResult,
PostToolUse,
PostToolUseFailure,
SessionStart,
@@ -66,6 +67,7 @@ impl HookEvent {
fn name(&self) -> &'static str {
match self {
HookEvent::PreToolUse => "PreToolUse",
HookEvent::PreToolUseResult => "PreToolUseResult",
HookEvent::PostToolUse => "PostToolUse",
HookEvent::PostToolUseFailure => "PostToolUseFailure",
HookEvent::SessionStart => "SessionStart",
@@ -82,6 +84,7 @@ impl HookEvent {
fn from_name(name: &str) -> Option<Self> {
Some(match name {
"PreToolUse" => HookEvent::PreToolUse,
"PreToolUseResult" => HookEvent::PreToolUseResult,
"PostToolUse" => HookEvent::PostToolUse,
"PostToolUseFailure" => HookEvent::PostToolUseFailure,
"SessionStart" => HookEvent::SessionStart,
@@ -158,6 +161,11 @@ pub struct HookContext {
pub event: String,
pub session_id: String,
pub matcher_context: Option<String>,
/// Stable identifier for one tool call, the same value goose records as
/// `gen_ai.tool.call.id`. Correlates the pre and post events of a single
/// call, which tool name plus input cannot do when a call repeats.
#[serde(skip_serializing_if = "Option::is_none")]
pub tool_call_id: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub tool_name: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
@@ -170,6 +178,19 @@ pub struct HookContext {
pub last_assistant_message: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub working_dir: Option<String>,
/// `PreToolUseResult` only: "allow" or "deny". There is no third value.
#[serde(skip_serializing_if = "Option::is_none")]
pub decision: Option<String>,
/// `PreToolUseResult` only: true when at least one matching `PreToolUse`
/// hook ran to completion for this call.
#[serde(skip_serializing_if = "Option::is_none")]
pub policy_evaluated: Option<bool>,
/// `PreToolUseResult` on deny only: the plugin that denied.
#[serde(skip_serializing_if = "Option::is_none")]
pub blocked_by: Option<String>,
/// `PreToolUseResult` on deny only: the reason the plugin gave.
#[serde(skip_serializing_if = "Option::is_none")]
pub reason: Option<String>,
}
impl HookContext {
@@ -178,12 +199,17 @@ impl HookContext {
event: event.to_string(),
session_id: session_id.into(),
matcher_context: None,
tool_call_id: None,
tool_name: None,
tool_input: None,
tool_output: None,
message: None,
last_assistant_message: None,
working_dir: None,
decision: None,
policy_evaluated: None,
blocked_by: None,
reason: None,
}
}
@@ -219,6 +245,26 @@ impl HookContext {
self.working_dir = Some(dir.into());
self
}
pub fn with_tool_call_id(mut self, tool_call_id: impl Into<String>) -> Self {
self.tool_call_id = Some(tool_call_id.into());
self
}
/// Populate the `PreToolUseResult` outcome fields. `blocked_by` and `reason`
/// are set only on deny, so an allow payload omits them entirely.
pub(crate) fn with_pre_tool_use_outcome(mut self, outcome: &HookChainOutcome) -> Self {
self.policy_evaluated = Some(outcome.policy_evaluated);
match &outcome.decision {
HookDecision::Allow => self.decision = Some("allow".to_string()),
HookDecision::Deny { reason, plugin } => {
self.decision = Some("deny".to_string());
self.blocked_by = Some(plugin.clone());
self.reason = Some(reason.clone());
}
}
self
}
}
#[derive(Debug, Clone, PartialEq, Eq)]
@@ -227,6 +273,29 @@ pub enum HookDecision {
Deny { reason: String, plugin: String },
}
/// Result of running a blocking hook chain: the decision, plus whether any
/// matching hook actually ran to completion for this event. A hook counts as
/// evaluated when it exited 0 or returned a decision. A hook that exited
/// non-zero without a decision, failed to spawn, timed out, or was never
/// reached does not count.
///
/// Crate-internal: the public [`HookManager::emit_blocking`] contract is
/// unchanged and still returns a [`HookDecision`].
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) struct HookChainOutcome {
pub decision: HookDecision,
pub policy_evaluated: bool,
}
impl HookChainOutcome {
pub(crate) fn allow(policy_evaluated: bool) -> Self {
Self {
decision: HookDecision::Allow,
policy_evaluated,
}
}
}
/// Loads and executes plugin hooks.
#[derive(Debug, Default, Clone)]
pub struct HookManager {
@@ -477,18 +546,30 @@ impl HookManager {
/// to stdout. All other failures (spawn, timeout, other non-zero exits)
/// are logged and treated as Allow — a misbehaving hook MUST NOT block.
pub async fn emit_blocking(&self, event: HookEvent, ctx: HookContext) -> HookDecision {
self.emit_blocking_with_outcome(event, ctx).await.decision
}
/// Like [`Self::emit_blocking`], but also reports whether any matching hook
/// ran to completion, which `PreToolUseResult` needs for `policy_evaluated`.
pub(crate) async fn emit_blocking_with_outcome(
&self,
event: HookEvent,
ctx: HookContext,
) -> HookChainOutcome {
let Some(rules) = self.rules.get(&event) else {
return HookDecision::Allow;
return HookChainOutcome::allow(false);
};
let payload = match serde_json::to_string(&ctx) {
Ok(s) => s,
Err(err) => {
warn!(event = %event, error = %err, "Failed to serialize hook context");
return HookDecision::Allow;
return HookChainOutcome::allow(false);
}
};
let mut policy_evaluated = false;
for rule in rules {
if let Some(matcher) = &rule.matcher {
let target = ctx.matcher_context.as_deref().unwrap_or("");
@@ -503,7 +584,15 @@ impl HookManager {
.run_action(event, &ctx.session_id, rule, command, &payload, *timeout)
.await
{
Ok(o) => o,
Ok(output) => {
// Exiting 0 or returning a decision is what makes a hook
// an evaluation. A non-zero exit with no decision means
// the hook did not answer, so it does not count.
if output.status.success() || deny_reason(&output).is_some() {
policy_evaluated = true;
}
output
}
Err(err) => {
warn!(
plugin = %rule.plugin_name,
@@ -524,15 +613,18 @@ impl HookManager {
reason = %reason,
"Plugin hook denied tool call",
);
return HookDecision::Deny {
reason,
plugin: rule.plugin_name.clone(),
return HookChainOutcome {
decision: HookDecision::Deny {
reason,
plugin: rule.plugin_name.clone(),
},
policy_evaluated,
};
}
}
}
HookDecision::Allow
HookChainOutcome::allow(policy_evaluated)
}
}
@@ -797,6 +889,169 @@ mod tests {
HookManager::from_plugins(plugins, false)
}
/// `decision` is "allow" or "deny" and nothing else, and `blocked_by` and
/// `reason` appear only on the deny arm — absent from an allow payload
/// rather than present as null or an empty string.
#[test]
fn pre_tool_use_result_payload_reports_decision_and_denies_alone_carry_the_plugin() {
let payload = |outcome: &HookChainOutcome| -> Value {
let ctx = HookContext::new(HookEvent::PreToolUseResult, "session-1")
.with_tool("developer__shell", None)
.with_tool_call_id("call-1")
.with_pre_tool_use_outcome(outcome);
serde_json::from_str(&serde_json::to_string(&ctx).unwrap()).unwrap()
};
let allow = payload(&HookChainOutcome::allow(true));
assert_eq!(allow["decision"], "allow");
assert_eq!(allow["policy_evaluated"], true);
assert_eq!(allow["tool_call_id"], "call-1");
assert!(
allow.get("blocked_by").is_none(),
"allow payload must omit blocked_by entirely, got {:?}",
allow.get("blocked_by")
);
assert!(
allow.get("reason").is_none(),
"allow payload must omit reason entirely, got {:?}",
allow.get("reason")
);
let deny = payload(&HookChainOutcome {
decision: HookDecision::Deny {
reason: "blocked by test policy".to_string(),
plugin: "test-plugin".to_string(),
},
policy_evaluated: true,
});
assert_eq!(deny["decision"], "deny");
assert_eq!(deny["blocked_by"], "test-plugin");
assert_eq!(deny["reason"], "blocked by test policy");
for value in [&allow, &deny] {
let decision = value["decision"].as_str().unwrap();
assert!(
matches!(decision, "allow" | "deny"),
"decision must be allow or deny, got {decision}"
);
}
let unevaluated = payload(&HookChainOutcome::allow(false));
assert_eq!(unevaluated["decision"], "allow");
assert_eq!(unevaluated["policy_evaluated"], false);
}
/// A hook is an evaluation only if it exited 0 or returned a decision. A
/// non-zero exit carrying no decision means the hook never answered, and an
/// earlier hook that did answer keeps the aggregate true.
#[tokio::test]
async fn policy_evaluated_counts_clean_exits_and_decisions_only() {
let plugin = |root: &Path, name: &str, command: &str| -> DiscoveredPlugin {
let hooks = format!(
r#"{{"hooks":{{"PreToolUse":[{{"hooks":[{{"type":"command","command":"{command}"}}]}}]}}}}"#
);
DiscoveredPlugin {
name: name.into(),
root: write_plugin(root, name, &hooks),
scope: PluginScope::User,
}
};
let ctx =
|| HookContext::new(HookEvent::PreToolUse, "s").with_tool("developer__shell", None);
// Case 1: the only hook exits non-zero with nothing on stdout. It gave no
// decision, so the call is allowed and nothing was evaluated.
let tmp = tempfile::tempdir().unwrap();
let mgr = make_manager(vec![plugin(tmp.path(), "abnormal", "exit 3")]);
let outcome = mgr
.emit_blocking_with_outcome(HookEvent::PreToolUse, ctx())
.await;
assert_eq!(outcome.decision, HookDecision::Allow);
assert!(
!outcome.policy_evaluated,
"a sole hook exiting 3 with no decision must not count as evaluated",
);
// Case 2: one hook exits 0 and another exits non-zero. policy_evaluated is
// an at-least-one aggregate, so the clean exit keeps it true.
let tmp = tempfile::tempdir().unwrap();
let mgr = make_manager(vec![
plugin(tmp.path(), "a", "exit 0"),
plugin(tmp.path(), "b", "exit 3"),
]);
let outcome = mgr
.emit_blocking_with_outcome(HookEvent::PreToolUse, ctx())
.await;
assert_eq!(outcome.decision, HookDecision::Allow);
assert!(
outcome.policy_evaluated,
"a hook that exited 0 must keep policy_evaluated true when a later hook fails",
);
// Case 3: the only hook exits 2 with a reason. That is a decision, so it
// both denies and counts as evaluated.
let tmp = tempfile::tempdir().unwrap();
let mgr = make_manager(vec![plugin(
tmp.path(),
"denier",
"echo refused by policy >&2; exit 2",
)]);
let outcome = mgr
.emit_blocking_with_outcome(HookEvent::PreToolUse, ctx())
.await;
assert_eq!(
outcome.decision,
HookDecision::Deny {
reason: "refused by policy".to_string(),
plugin: "denier".to_string(),
}
);
assert!(
outcome.policy_evaluated,
"an exit 2 decision must count as evaluated",
);
}
/// PreToolUseResult honours its matcher against the tool name like every
/// other tool-scoped event, so a subscriber can watch one tool rather than
/// every call.
#[tokio::test]
async fn pre_tool_use_result_matcher_targets_the_tool_name() {
let tmp = tempfile::tempdir().unwrap();
let root = write_plugin(
tmp.path(),
"p",
r#"{"hooks":{"PreToolUseResult":[{"matcher":"^developer__shell$","hooks":[{"type":"command","command":"echo ran >> \"$PLUGIN_ROOT/marker.log\""}]}]}}"#,
);
let marker = root.join("marker.log");
let mgr = make_manager(vec![DiscoveredPlugin {
name: "p".into(),
root,
scope: PluginScope::User,
}]);
let lines = || {
std::fs::read_to_string(&marker)
.unwrap_or_default()
.lines()
.filter(|line| !line.trim().is_empty())
.count()
};
mgr.emit(
HookEvent::PreToolUseResult,
HookContext::new(HookEvent::PreToolUseResult, "s").with_tool("developer__shell", None),
)
.await;
assert_eq!(lines(), 1, "the matching tool must run the hook");
mgr.emit(
HookEvent::PreToolUseResult,
HookContext::new(HookEvent::PreToolUseResult, "s").with_tool("other__tool", None),
)
.await;
assert_eq!(lines(), 1, "a non-matching tool must not run the hook");
}
#[test]
fn ignores_unknown_events() {
let tmp = tempfile::tempdir().unwrap();
@@ -136,6 +136,7 @@ Use `${PLUGIN_ROOT}` in a command to reference the plugin directory. goose also
| `Stop` | goose finishes a turn or receives a stop event | None |
| `UserPromptSubmit` | The user submits a prompt | Prompt text |
| `PreToolUse` | Before goose runs a tool | Tool name |
| `PreToolUseResult` | After the `PreToolUse` chain resolves, for allowed and denied calls alike, before the tool runs or the denial is returned. Observation only | Tool name |
| `PostToolUse` | After a tool succeeds | Tool name |
| `PostToolUseFailure` | After a tool fails | Tool name |
| `BeforeReadFile` | Before goose reads a file | File path |
@@ -153,6 +154,8 @@ The matcher is a regular expression, not a glob. A bare `"*"` is an invalid rege
`AfterFileEdit` and `AfterShellExecution` only run after successful tool calls. To react to failed edits, failed shell commands, or other failed tool calls, use `PostToolUseFailure`.
:::
`PreToolUseResult` is observation only in authority, not asynchronous in delivery. Matching hooks are run and awaited before goose continues to the tool or returns the denial, so a slow subscriber adds its runtime, up to its timeout, to the tool call. Delivery is best effort and not durable: a subscriber that fails or is absent changes nothing about the decision, and no record is kept if the hook does not run.
## Hook Payload
When a hook runs, goose writes a JSON payload to the command's stdin. Every payload includes the event name and session ID. The remaining fields are only present when they apply to the event, so a hook should treat them as optional.
@@ -167,6 +170,11 @@ When a hook runs, goose writes a JSON payload to the command's stdin. Every payl
| `message` | Prompt text the user submitted, on `UserPromptSubmit`. |
| `last_assistant_message` | Final assistant text for the turn, on `Stop` when there is assistant output. |
| `working_dir` | Working directory of the session, on tool events. |
| `tool_call_id` | Stable identifier for one tool call, on `PreToolUse`, `PreToolUseResult`, `PostToolUse`, and `PostToolUseFailure`. Correlates the events of a single call, which tool name plus input cannot do when the same call repeats. |
| `decision` | `allow` or `deny`, on `PreToolUseResult`. There is no third value. |
| `policy_evaluated` | `true` when at least one matching `PreToolUse` hook exited 0 or returned a decision (exit `2`, or `{"decision":"block"}` on stdout), on `PreToolUseResult`. A hook that exits non-zero without a decision, fails to spawn, or times out does not count, and neither does the absence of a matching hook. It is an at-least-one value: a hook that evaluated keeps it `true` even if a later hook fails. |
| `blocked_by` | Plugin whose hook denied the call, on `PreToolUseResult` when `decision` is `deny`. |
| `reason` | Reason the denying hook gave, on `PreToolUseResult` when `decision` is `deny`. |
Example payload for a tool event: